[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 3/3] hw/block/pflash: use memory_region_init_rom_device_f
From: |
David Edmondson |
Subject: |
Re: [RFC PATCH 3/3] hw/block/pflash: use memory_region_init_rom_device_from_file() |
Date: |
Mon, 01 Mar 2021 13:38:41 +0000 |
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 <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.
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).
dme.
--
Driving at 90 down those country lanes, singing to "Tiny Dancer".