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,
+ true, errp);
+ } else {
+ memory_region_init_rom_device(&pfl->mem, OBJECT(dev),
+ &pflash_cfi01_ops, pfl,
+ pfl->name, total_len, errp);
+ }
if (*errp) {
return;
}
@@ -755,7 +763,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
- if (pfl->blk) {
+ if (pfl->blk && !pfl->ro) {
if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
errp)) {
vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 4f62ce8917d..d57f64d7732 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -803,16 +803,26 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
pfl->ro = 0;
}
- memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
- &pflash_cfi02_ops, pfl, pfl->name,
- pfl->chip_len, errp);
+ if (pfl->blk && pfl->ro) {
+ memory_region_init_rom_device_from_file(&pfl->orig_mem, OBJECT(pfl),
+ &pflash_cfi02_ops, pfl,
+ pfl->name, pfl->chip_len,
+ qemu_real_host_page_size,
+ RAM_SHARED,
+ blk_bs(pfl->blk)->filename,
+ true, errp);
+ } else {
+ memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
+ &pflash_cfi02_ops, pfl, pfl->name,
+ pfl->chip_len, errp);
+ }
if (*errp) {
return;
}
pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
- if (pfl->blk) {
+ if (pfl->blk && !pfl->ro) {
if (!blk_check_size_and_read_all(pfl->blk, pfl->storage,
pfl->chip_len, errp)) {
vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
--
2.26.2
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.
> + true, errp);
> + } else {
> + memory_region_init_rom_device(&pfl->mem, OBJECT(dev),
> + &pflash_cfi01_ops, pfl,
> + pfl->name, total_len, errp);
> + }
> if (*errp) {
> return;
> }
> @@ -755,7 +763,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
> pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
>
> - if (pfl->blk) {
> + if (pfl->blk && !pfl->ro) {
> if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
> errp)) {
> vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 4f62ce8917d..d57f64d7732 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -803,16 +803,26 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
> pfl->ro = 0;
> }
>
> - memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
> - &pflash_cfi02_ops, pfl, pfl->name,
> - pfl->chip_len, errp);
> + if (pfl->blk && pfl->ro) {
> + memory_region_init_rom_device_from_file(&pfl->orig_mem, OBJECT(pfl),
> + &pflash_cfi02_ops, pfl,
> + pfl->name, pfl->chip_len,
> + qemu_real_host_page_size,
> + RAM_SHARED,
> + blk_bs(pfl->blk)->filename,
> + true, errp);
> + } else {
> + memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
> + &pflash_cfi02_ops, pfl, pfl->name,
> + pfl->chip_len, errp);
> + }
> if (*errp) {
> return;
> }
>
> pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
>
> - if (pfl->blk) {
> + if (pfl->blk && !pfl->ro) {
> if (!blk_check_size_and_read_all(pfl->blk, pfl->storage,
> pfl->chip_len, errp)) {
> vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
> --
> 2.26.2
dme.
--
And you're standing here beside me, I love the passing of time.
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.
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".
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 <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).
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.
© 2016 - 2025 Red Hat, Inc.