[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH-for-5.1? v3 1/2] memory: Allow monkey-patching MemoryRegi
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [RFC PATCH-for-5.1? v3 1/2] memory: Allow monkey-patching MemoryRegion access sizes |
Date: |
Tue, 21 Jul 2020 16:23:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 7/21/20 2:49 PM, Peter Maydell wrote:
> On Tue, 21 Jul 2020 at 13:39, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 7/21/20 2:33 PM, Peter Maydell wrote:
>>> On Tue, 21 Jul 2020 at 13:31, Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> wrote:
>>> Don't you now need to g_memfree() mr->ops somewhere? Otherwise
>>> you've leaked it if the device which owned this MemoryRegion
>>> is hot-unplugged, I think.
>>
>> I haven't thinking of hot-unplug. I went with the simplest fix
>> considering we are in freeze, and fixing the bug was more
>> important that a leak at this point.
>> I'll have a look at freeing this memory, hoping it is still less
>> disruptive than a proper architectural change to fix this problem.
>
> Instead of g_memdup()ing the ops struct here, you could maybe
> do it in isa_register_ioport() instead. Then you don't need to
> worry about leaks because we know all ISA devices are not
> hotpluggable, and the ugliness is also a bit more constrained
> to the ISA code. (Coverity probably still thinks it's a leak though.)
I tried that first but got:
memory.c: In function ‘memory_region_initfn’:
memory.c:1221:13: error: assignment discards ‘const’ qualifier from
pointer target type [-Werror=discarded-qualifiers]
1221 | mr->ops = &unassigned_mem_ops;
| ^
memory.c: In function ‘memory_region_init_io’:
memory.c:1488:13: error: assignment discards ‘const’ qualifier from
pointer target type [-Werror=discarded-qualifiers]
1488 | mr->ops = ops ? ops : &unassigned_mem_ops;
| ^
memory.c: In function ‘memory_region_init_ram_device_ptr’:
memory.c:1625:13: error: assignment discards ‘const’ qualifier from
pointer target type [-Werror=discarded-qualifiers]
1625 | mr->ops = &ram_device_mem_ops;
| ^
memory.c: In function ‘memory_region_init_rom_device_nomigrate’:
memory.c:1667:13: error: assignment discards ‘const’ qualifier from
pointer target type [-Werror=discarded-qualifiers]
1667 | mr->ops = ops;
| ^
Since this whole series is a kludge, I'm tempted to cast that to
non-const but it starts to get really ugly...
>
> thanks
> -- PMM
>
[RFC PATCH-for-5.1? v3 2/2] hw/isa/isa-bus: Ensure ISA I/O regions are 8/16/32-bit accessible, Philippe Mathieu-Daudé, 2020/07/21