Re: [RFC PATCH 3/3] hw/block/pflash: use memory_region_init_rom_device_from_file()

[Philippe Mathieu-Daudé]

On 2/26/21 9:23 AM, David Edmondson wrote:
> On Friday, 2021-02-26 at 00:02:38 +01, Philippe Mathieu-Daudé wrote:
> 
>> If the block drive is read-only we will model a "protected" flash
>> device. We can thus use memory_region_init_rom_device_from_file()
>> which mmap the backing file when creating the MemoryRegion.
>> If the same backing file is used by multiple QEMU instances, this
>> reduces the memory footprint (this is often the case with the
>> CODE flash image from OVMF and AAVMF).
>>
>> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/block/pflash_cfi01.c | 20 ++++++++++++++------
>>  hw/block/pflash_cfi02.c | 18 ++++++++++++++----
>>  2 files changed, 28 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>> index a5fa8d8b74a..5757391df1c 100644
>> --- a/hw/block/pflash_cfi01.c
>> +++ b/hw/block/pflash_cfi01.c
>> @@ -743,11 +743,19 @@ static void pflash_cfi01_realize(DeviceState *dev, 
>> Error **errp)
>>          pfl->ro = 0;
>>      }
>>  
>> -    memory_region_init_rom_device(
>> -        &pfl->mem, OBJECT(dev),
>> -        &pflash_cfi01_ops,
>> -        pfl,
>> -        pfl->name, total_len, errp);
>> +    if (pfl->blk && pfl->ro) {
>> +        memory_region_init_rom_device_from_file(&pfl->mem, OBJECT(dev),
>> +                                                &pflash_cfi01_ops, pfl,
>> +                                                pfl->name, total_len,
>> +                                                qemu_real_host_page_size,
>> +                                                RAM_SHARED,
>> +                                                blk_bs(pfl->blk)->filename,
> 
> How will this behave if someone does:
> 
>     -drive file=OVMF_CODE.fd.qcow2,index=0,if=pflash,format=qcow2,readonly=on
> 
> Honestly, I'm not sure why they would, but it works today.

OK I can add a check for "raw" driver, but I don't know to check for
offset == 0.