diff mbox series

hw/arm/virt: use pflash image real size when mapping

Message ID 790EEEF3-0799-4507-BF30-DA85440E766F@tencent.com (mailing list archive)
State New, archived
Headers show
Series hw/arm/virt: use pflash image real size when mapping | expand

Commit Message

haibinzhang(张海斌) Sept. 18, 2020, 12:26 p.m. UTC
Default size of arm-virt pflash image is 64MB which
will cost extra 128MB(64MBx2) memory per qemu process
and 12.5GB for 100 qemu processes. Host memory is
precious and it is valuable to reduce pflash image size.
For compatibility arm-virt uses real size when mapping.

Signed-off-by: Haibin Zhang <haibinzhang@tencent.com>
---
 hw/arm/virt.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

--
2.23.0

Comments

haibinzhang(张海斌) Sept. 19, 2020, 10:03 a.m. UTC | #1
On Sep 18, 2020, at 9:41 PM, Philippe Mathieu-Daudé <philmd@redhat.com<mailto:philmd@redhat.com>> wrote:

Cc'ing firmware experts.

On 9/18/20 2:26 PM, haibinzhang(寮犳捣鏂? wrote:
Default size of arm-virt pflash image is 64MB which
will cost extra 128MB(64MBx2) memory per qemu process
and 12.5GB for 100 qemu processes. Host memory is
precious and it is valuable to reduce pflash image size.
For compatibility arm-virt uses real size when mapping.

Flash#0 is a device because eventually its model will handle
sector/block protection, so firmware can do CapsuleUpdate
(updating itself). Meanwhile you could treat flash#0 as a pure
ROM device... But this wouldn't be the 'arm-virt' machine anymore.


Example:
1. Get QEMU_EFI.fd and QEMU_VARS.fd from tianocore/edk2

2. Make pflash images
  dd if=/dev/zero bs=1M count=64 of=flash0.img
  dd if=/dev/zero bs=1M count=64 of=flash1.img
  dd if=QEMU_EFI.fd bs=1M of=flash0.img conv=notrunc
  dd if=QEMU_VARS.fd bs=1M of=flash1.img conv=notrunc

3. Start VM
  qemu-system-aarch64 -machine virt,accel=kvm,gic-version=3 \
  -drive file=flash0.img,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=flash1.img,if=pflash,format=raw,unit=1 \
  …

In virt machine, VIRT_FLASH size is fixed 128MB (fixed 64MB for Flash0, fixed 64MB for Flash1).
Flash0 and Flash1 are two Block Backend devices and will be copied into two memory storages alloced
in virt_flash_map, not file mapping. Size of memory storage is fixed 64MB.

Since size checking, virt machine currently doesn’t support <64MB pflash images. So, get real
size of pflash image first, and use real size to alloc memory storage.

I always use 8MB Flash0 and Flash1, using following commands
  dd if=/dev/zero bs=1M count=8 of=flash0.img
  dd if=/dev/zero bs=1M count=8 of=flash1.img
  dd if=QEMU_EFI.fd bs=1M of=flash0.img conv=notrunc
  dd if=QEMU_VARS.fd bs=1M of=flash1.img conv=notrunc


Signed-off-by: Haibin Zhang <haibinzhang@tencent.com<mailto:haibinzhang@tencent.com>>
---
hw/arm/virt.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index acf9bfbece..3545e12865 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -50,6 +50,7 @@
#include "sysemu/sysemu.h"
#include "sysemu/tpm.h"
#include "sysemu/kvm.h"
+#include "sysemu/block-backend.h"
#include "hw/loader.h"
#include "exec/address-spaces.h"
#include "qemu/bitops.h"
@@ -1001,10 +1002,27 @@ static void virt_flash_map(VirtMachineState *vms,
     */
    hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2;
    hwaddr flashbase = vms->memmap[VIRT_FLASH].base;
+    int64_t realsize;
+    BlockBackend *blk;

-    virt_flash_map1(vms->flash[0], flashbase, flashsize,
+    realsize = flashsize;
+    blk = pflash_cfi01_get_blk(vms->flash[0]);
+    if (blk) {
+        realsize = blk_getlength(blk);
+        realsize = realsize < flashsize ? realsize : flashsize;
+    }

Stefan recently posted "nvdimm: read-only file support" which
might be a better way to achieve what you want:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg741137.html


Flash0 is read-only and shared by all VMs, but Flash1 is not and private for each VM.

Btw., how to create a nvdimm device mapping to address range 0~128MB?

+
+    virt_flash_map1(vms->flash[0], flashbase, realsize,
                    secure_sysmem);
-    virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize,
+
+    realsize = flashsize;
+    blk = pflash_cfi01_get_blk(vms->flash[1]);
+    if (blk) {
+        realsize = blk_getlength(blk);
+        realsize = realsize < flashsize ? realsize : flashsize;
+    }
+
+    virt_flash_map1(vms->flash[1], flashbase + flashsize, realsize,
                    sysmem);
}

--
2.23.0
haibinzhang(张海斌) Sept. 21, 2020, 5:34 a.m. UTC | #2
Resend the mail using plain text format

> On Sep 18, 2020, at 9:41 PM, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
> Cc'ing firmware experts.
> 
> On 9/18/20 2:26 PM, haibinzhang(寮犳捣鏂? wrote:
>> Default size of arm-virt pflash image is 64MB which
>> will cost extra 128MB(64MBx2) memory per qemu process
>> and 12.5GB for 100 qemu processes. Host memory is
>> precious and it is valuable to reduce pflash image size.
>> For compatibility arm-virt uses real size when mapping.
> 
> Flash#0 is a device because eventually its model will handle
> sector/block protection, so firmware can do CapsuleUpdate
> (updating itself). Meanwhile you could treat flash#0 as a pure
> ROM device... But this wouldn't be the 'arm-virt' machine anymore.
> 

But Flash#0/#1 are corresponding to two rom MemoryRegions related to memory cost.

Example:
1. Get QEMU_EFI.fd and QEMU_VARS.fd from tianocore/edk2

2. Make pflash images
  dd if=/dev/zero bs=1M count=64 of=flash0.img
  dd if=/dev/zero bs=1M count=64 of=flash1.img
  dd if=QEMU_EFI.fd bs=1M of=flash0.img conv=notrunc
  dd if=QEMU_VARS.fd bs=1M of=flash1.img conv=notrunc

3. Start VM
  qemu-system-aarch64 -machine virt,accel=kvm,gic-version=3 \
  -drive file=flash0.img,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=flash1.img,if=pflash,format=raw,unit=1 \
  …

In virt machine, VIRT_FLASH size is fixed 128MB (fixed 64MB for Flash0, fixed 64MB for Flash1).
Flash0 and Flash1 are two Block Backend devices and will be copied into two memory storages alloced 
in virt_flash_map, not file mapping. Size of memory storage is fixed 64MB.

Since size checking, virt machine currently doesn’t support <64MB pflash images. So, get real
size of pflash image first, and use real size to alloc memory storage.

I always use 8MB Flash0 and Flash1, using following commands
  dd if=/dev/zero bs=1M count=8 of=flash0.img
  dd if=/dev/zero bs=1M count=8 of=flash1.img
  dd if=QEMU_EFI.fd bs=1M of=flash0.img conv=notrunc
  dd if=QEMU_VARS.fd bs=1M of=flash1.img conv=notrunc

>> 
>> Signed-off-by: Haibin Zhang <haibinzhang@tencent.com>
>> ---
>> hw/arm/virt.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index acf9bfbece..3545e12865 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -50,6 +50,7 @@
>> #include "sysemu/sysemu.h"
>> #include "sysemu/tpm.h"
>> #include "sysemu/kvm.h"
>> +#include "sysemu/block-backend.h"
>> #include "hw/loader.h"
>> #include "exec/address-spaces.h"
>> #include "qemu/bitops.h"
>> @@ -1001,10 +1002,27 @@ static void virt_flash_map(VirtMachineState *vms,
>>      */
>>     hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2;
>>     hwaddr flashbase = vms->memmap[VIRT_FLASH].base;
>> +    int64_t realsize;
>> +    BlockBackend *blk;
>> 
>> -    virt_flash_map1(vms->flash[0], flashbase, flashsize,
>> +    realsize = flashsize;
>> +    blk = pflash_cfi01_get_blk(vms->flash[0]);
>> +    if (blk) {
>> +        realsize = blk_getlength(blk);
>> +        realsize = realsize < flashsize ? realsize : flashsize;
>> +    }
> 
> Stefan recently posted "nvdimm: read-only file support" which
> might be a better way to achieve what you want:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg741137.html

Flash0 is read-only and shared by all VMs, but Flash1 is not and private for each VM.

Btw., how to create a nvdimm device mapping to address range 0~128MB?

> 
>> +
>> +    virt_flash_map1(vms->flash[0], flashbase, realsize,
>>                     secure_sysmem);
>> -    virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize,
>> +
>> +    realsize = flashsize;
>> +    blk = pflash_cfi01_get_blk(vms->flash[1]);
>> +    if (blk) {
>> +        realsize = blk_getlength(blk);
>> +        realsize = realsize < flashsize ? realsize : flashsize;
>> +    }
>> +
>> +    virt_flash_map1(vms->flash[1], flashbase + flashsize, realsize,
>>                     sysmem);
>> }
>> 
>> --
>> 2.23.0
Laszlo Ersek Sept. 22, 2020, 6:57 a.m. UTC | #3
On 09/18/20 15:41, Philippe Mathieu-Daudé wrote:
> Cc'ing firmware experts.
> 
> On 9/18/20 2:26 PM, haibinzhang(寮犳捣鏂�) wrote:
>> Default size of arm-virt pflash image is 64MB which
>> will cost extra 128MB(64MBx2) memory per qemu process
>> and 12.5GB for 100 qemu processes. Host memory is
>> precious and it is valuable to reduce pflash image size.
>> For compatibility arm-virt uses real size when mapping.
> 
> Flash#0 is a device because eventually its model will handle
> sector/block protection, so firmware can do CapsuleUpdate
> (updating itself). Meanwhile you could treat flash#0 as a pure
> ROM device... But this wouldn't be the 'arm-virt' machine anymore.
> 
>>
>> Signed-off-by: Haibin Zhang <haibinzhang@tencent.com>
>> ---
>>  hw/arm/virt.c | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index acf9bfbece..3545e12865 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -50,6 +50,7 @@
>>  #include "sysemu/sysemu.h"
>>  #include "sysemu/tpm.h"
>>  #include "sysemu/kvm.h"
>> +#include "sysemu/block-backend.h"
>>  #include "hw/loader.h"
>>  #include "exec/address-spaces.h"
>>  #include "qemu/bitops.h"
>> @@ -1001,10 +1002,27 @@ static void virt_flash_map(VirtMachineState *vms,
>>       */
>>      hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2;
>>      hwaddr flashbase = vms->memmap[VIRT_FLASH].base;
>> +    int64_t realsize;
>> +    BlockBackend *blk;
>>
>> -    virt_flash_map1(vms->flash[0], flashbase, flashsize,
>> +    realsize = flashsize;
>> +    blk = pflash_cfi01_get_blk(vms->flash[0]);
>> +    if (blk) {
>> +        realsize = blk_getlength(blk);
>> +        realsize = realsize < flashsize ? realsize : flashsize;
>> +    }
> 
> Stefan recently posted "nvdimm: read-only file support" which
> might be a better way to achieve what you want:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg741137.html
> 
>> +
>> +    virt_flash_map1(vms->flash[0], flashbase, realsize,
>>                      secure_sysmem);
>> -    virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize,
>> +
>> +    realsize = flashsize;
>> +    blk = pflash_cfi01_get_blk(vms->flash[1]);
>> +    if (blk) {
>> +        realsize = blk_getlength(blk);
>> +        realsize = realsize < flashsize ? realsize : flashsize;
>> +    }
>> +
>> +    virt_flash_map1(vms->flash[1], flashbase + flashsize, realsize,
>>                      sysmem);
>>  }
>>
>> --
>> 2.23.0
>>
> 

We've been here before.

  [Qemu-devel] [RFC PATCH] hw/arm/virt: use variable size of flash device to save memory

  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06773.html
  http://mid.mail-archive.com/20190325125142.11628-1-zhengxiang9@huawei.com

I don't have anything to add.

Laszlo
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index acf9bfbece..3545e12865 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -50,6 +50,7 @@ 
 #include "sysemu/sysemu.h"
 #include "sysemu/tpm.h"
 #include "sysemu/kvm.h"
+#include "sysemu/block-backend.h"
 #include "hw/loader.h"
 #include "exec/address-spaces.h"
 #include "qemu/bitops.h"
@@ -1001,10 +1002,27 @@  static void virt_flash_map(VirtMachineState *vms,
      */
     hwaddr flashsize = vms->memmap[VIRT_FLASH].size / 2;
     hwaddr flashbase = vms->memmap[VIRT_FLASH].base;
+    int64_t realsize;
+    BlockBackend *blk;

-    virt_flash_map1(vms->flash[0], flashbase, flashsize,
+    realsize = flashsize;
+    blk = pflash_cfi01_get_blk(vms->flash[0]);
+    if (blk) {
+        realsize = blk_getlength(blk);
+        realsize = realsize < flashsize ? realsize : flashsize;
+    }
+
+    virt_flash_map1(vms->flash[0], flashbase, realsize,
                     secure_sysmem);
-    virt_flash_map1(vms->flash[1], flashbase + flashsize, flashsize,
+
+    realsize = flashsize;
+    blk = pflash_cfi01_get_blk(vms->flash[1]);
+    if (blk) {
+        realsize = blk_getlength(blk);
+        realsize = realsize < flashsize ? realsize : flashsize;
+    }
+
+    virt_flash_map1(vms->flash[1], flashbase + flashsize, realsize,
                     sysmem);
 }