Re: [PATCH 0/6] Add various undefined MMIO r/w functions

[Paolo Bonzini]

On 17/06/20 15:20, Philippe Mathieu-Daudé wrote:
> On 6/17/20 3:06 PM, Alex Williamson wrote:
>> On Wed, 17 Jun 2020 16:39:56 +1000
>> David Gibson <david@gibson.dropbear.id.au> wrote:
>>
>>> On Wed, Jun 17, 2020 at 11:09:27AM +0530, P J P wrote:
>>>> From: Prasad J Pandit <pjp@fedoraproject.org>
>>>>
>>>> Hello,
>>>>
>>>> This series adds various undefined MMIO read/write functions
>>>> to avoid potential guest crash via a NULL pointer dereference.  
>>>
>>> Hrm.  If this is such a common problem, maybe we should just add a
>>> NULL check in the common paths.
>>
>> +1, clearly the behavior is already expected.  Thanks,
> 
> 20 months ago Peter suggested:
> 
> "assert that every MemoryRegionOps has pointers to callbacks
>  in it, when it is registered in memory_region_init_io() and
>  memory_region_init_rom_device_nomigrate()."
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg573310.html
> 
> Li Qiang refers to this post from Paolo:
> 
>>  static const MemoryRegionOps notdirty_mem_ops = {
>> +    .read = notdirty_mem_read,
>>      .write = notdirty_mem_write,
>>      .valid.accepts = notdirty_mem_accepts,
>>      .endianness = DEVICE_NATIVE_ENDIAN,
> 
> "This cannot happen, since TLB_NOTDIRTY is only added
>  to the addr_write member (see accel/tcg/cputlb.c)."

I'm now okay with asserting it, as long as notdirty_mem_read abort()s.

Paolo