On 3/1/21 2:38 PM, David Edmondson wrote:
> On Monday, 2021-03-01 at 12:50:33 +01, Philippe Mathieu-Daudé wrote:
>
>> 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 <stefa...@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <phi...@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.
>
> This is pretty much where I got to when I tried using mmap() and gave up
> (mostly because I figured that adding layer violating checks to the
> pflash driver would not be well received, but also because we don't
> share the same underlying file between multiple VMs and I wasn't sure
> that it would eventually work well for writable devices).
Kevin suggested on IRC (#qemu-block, you are welcome to join) to
introduce a new blk_*() interface to mmap an image (or possibly
part of it), and have it work with non-zero raw offsets.
