Re: [PATCH] hw/vfio: let readonly flag take effect for mmaped regions

[Paolo Bonzini <pbonzini@redhat.com>]

On 27/03/20 11:51, Philippe Mathieu-Daudé wrote:
>>  diff --git a/memory.c b/memory.c
>> index 601b749906..4b1071dc74 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1313,6 +1313,9 @@ static void memory_region_ram_device_write(void
>> *opaque, hwaddr addr,
>>       MemoryRegion *mr = opaque;
>>         trace_memory_region_ram_device_write(get_cpu_index(), mr,
>> addr, data, size);
>> +    if (mr->readonly) {
>> +        return;
>> +    }
> 
> Shouldn't this be in memory_region_dispatch_write()?

No, in general you want memory regions to get writes, so that they
become for example a machine-check exception of some sorts.  However,
memory_region_ram_device_write should probably be changed to a
.write_with_attrs operation, so that it can return MEMTX_ERROR.

> Please split this patch in 2, this (generic) hunk as first patch, then
> the VFIO more specific change.
> 
>>         switch (size) {
>>       case 1:
>>
> 

No need, I can just add my Acked-by for Alex to pick up the patch.

Paolo