[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend ima
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image |
Date: |
Tue, 7 May 2019 21:03:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
Hi Markus,
On 05/07/19 20:01, Markus Armbruster wrote:
> The subject is slightly misleading. Holes read as zero. So do
> non-holes full of zeroes. The patch avoids reading the former, but
> still reads the latter.
>
> Xiang Zheng <address@hidden> writes:
>
>> Currently we fill the memory space with two 64MB NOR images when
>> using persistent UEFI variables on virt board. Actually we only use
>> a very small(non-zero) part of the memory while the rest significant
>> large(zero) part of memory is wasted.
>
> Neglects to mention that the "virt board" is ARM.
>
>> So this patch checks the block status and only writes the non-zero part
>> into memory. This requires pflash devices to use sparse files for
>> backends.
>
> I started to draft an improved commit message, but then I realized this
> patch can't work.
>
> The pflash_cfi01 device allocates its device memory like this:
>
> memory_region_init_rom_device(
> &pfl->mem, OBJECT(dev),
> &pflash_cfi01_ops,
> pfl,
> pfl->name, total_len, &local_err);
>
> pflash_cfi02 is similar.
>
> memory_region_init_rom_device() calls
> memory_region_init_rom_device_nomigrate() calls qemu_ram_alloc() calls
> qemu_ram_alloc_internal() calls g_malloc0(). Thus, all the device
> memory gets written to even with this patch.
As far as I can see, qemu_ram_alloc_internal() calls g_malloc0() only to
allocate the the new RAMBlock object called "new_block". The actual
guest RAM allocation occurs inside ram_block_add(), which is also called
by qemu_ram_alloc_internal().
One frame outwards the stack, qemu_ram_alloc() passes NULL to
qemu_ram_alloc_internal(), for the 4th ("host") parameter. Therefore, in
qemu_ram_alloc_internal(), we set "new_block->host" to NULL as well.
Then in ram_block_add(), we take the (!new_block->host) branch, and call
phys_mem_alloc().
Unfortunately, "phys_mem_alloc" is a function pointer, set with
phys_mem_set_alloc(). The phys_mem_set_alloc() function is called from
"target/s390x/kvm.c" (setting the function pointer to
legacy_s390_alloc()), so it doesn't apply in this case. Therefore we end
up calling the default qemu_anon_ram_alloc() function, through the
funcptr. (I think anyway.)
And qemu_anon_ram_alloc() boils down to mmap() + MAP_ANONYMOUS, in
qemu_ram_mmap(). (Even on PPC64 hosts, because qemu_anon_ram_alloc()
passes (-1) for "fd".)
I may have missed something, of course -- I obviously didn't test it,
just speculated from the source.
Thanks
Laszlo
>
> I'm afraid you neglected to test.
>
> I still believe this approach can be made to work. Need a replacement
> for memory_region_init_rom_device() that uses mmap() with MAP_ANONYMOUS.
>
- [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Xiang Zheng, 2019/05/05
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Peter Maydell, 2019/05/05
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Markus Armbruster, 2019/05/07
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image,
Laszlo Ersek <=
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Markus Armbruster, 2019/05/08
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Xiang Zheng, 2019/05/09
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Markus Armbruster, 2019/05/09
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Xiang Zheng, 2019/05/10
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Markus Armbruster, 2019/05/10
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Xiang Zheng, 2019/05/11
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Markus Armbruster, 2019/05/13
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Kevin Wolf, 2019/05/13
- Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image, Markus Armbruster, 2019/05/13