Re: [PATCH v1] acpi: increase maximum size for "etc/table-loader" blob

[Michael S. Tsirkin]

On Tue, Mar 02, 2021 at 10:43:55AM +0100, David Hildenbrand wrote:
> On 02.03.21 10:06, Igor Mammedov wrote:
> > On Mon,  1 Mar 2021 11:48:33 +0100
> > David Hildenbrand <david@redhat.com> wrote:
> > 
> > > The resizeable memory region that is created for the cmd blob has a 
> > > maximum
> > > size of ACPI_BUILD_ALIGN_SIZE - 4k. This used to be sufficient, however,
> > > as we try fitting in additional data (e.g., vmgenid, nvdimm, intel-iommu),
> > > we require more than 4k and can crash QEMU when trying to resize the
> > > resizeable memory region beyond its maximum size:
> > >    $ build/qemu-system-x86_64 --enable-kvm \
> > >        -machine q35,nvdimm=on \
> > >        -smp 1 \
> > >        -cpu host \
> > >        -m size=2G,slots=8,maxmem=4G \
> > >        -object memory-backend-file,id=mem0,mem-path=/tmp/nvdimm,size=256M 
> > > \
> > >        -device nvdimm,label-size=131072,memdev=mem0,id=nvdimm0,slot=1 \
> > >        -nodefaults \
> > >        -device vmgenid \
> > >        -device intel-iommu
> > 
> > I don't see what's here that would make cmd_blob go above 4k.
> > can you try identify what actually fills it up (perhaps we have a hidden 
> > bug elsewhere)?
> 
> VM initialization:
> 
> bios_linker_loader_alloc: allocating memory for 'etc/acpi/tables'
>  -> new table size: 128
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 64 - 9659
>  -> new table size: 256
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 384
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 512
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 640
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 9659 - 9903
>  -> new table size: 768
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 9903 - 10023
>  -> new table size: 896
> bios_linker_loader_alloc: allocating memory for 'etc/vmgenid_guid'
>  -> new table size: 1024
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/vmgenid_guid'
>  -> new table size: 1280
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10023 - 10225
>  -> new table size: 1408
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10225 - 10281
>  -> new table size: 1536
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10281 - 10505
>  -> new table size: 1664
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10505 - 10577
>  -> new table size: 1792
> bios_linker_loader_alloc: allocating memory for 'etc/acpi/nvdimm-mem'
>  -> new table size: 1920
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/nvdimm-mem'
>  -> new table size: 2048
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10577 - 11471
>  -> new table size: 2176
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 11471 - 11695
>  -> new table size: 2304
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 11695 - 11735
>  -> new table size: 2432
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2560
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2688
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2816
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2944
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3072
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3200
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3328
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3456
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3584
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 11735 - 11807
>  -> new table size: 3712
> bios_linker_loader_alloc: allocating memory for 'etc/acpi/rsdp'
>  -> new table size: 3840
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/rsdp' to 
> 'etc/acpi/tables'
>  -> new table size: 3968
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/rsdp' for 
> range 0 - 20
>  -> new table size: 4096
> 
> 
> When the bios/guest boots up:
> 
> bios_linker_loader_alloc: allocating memory for 'etc/acpi/tables'
>  -> new table size: 128
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 64 - 9769
>  -> new table size: 256
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 384
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 512
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 640
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 9769 - 10013
>  -> new table size: 768
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10013 - 10133
>  -> new table size: 896
> bios_linker_loader_alloc: allocating memory for 'etc/vmgenid_guid'
>  -> new table size: 1024
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/vmgenid_guid'
>  -> new table size: 1280
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10133 - 10335
>  -> new table size: 1408
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10335 - 10391
>  -> new table size: 1536
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10391 - 10615
>  -> new table size: 1664
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10615 - 10675
>  -> new table size: 1792
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10675 - 10747
>  -> new table size: 1920
> bios_linker_loader_alloc: allocating memory for 'etc/acpi/nvdimm-mem'
>  -> new table size: 2048
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/nvdimm-mem'
>  -> new table size: 2176
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 10747 - 11641
>  -> new table size: 2304
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 11641 - 11865
>  -> new table size: 2432
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 11865 - 11905
>  -> new table size: 2560
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2688
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2816
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 2944
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3072
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3200
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3328
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3456
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3584
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3712
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
> 'etc/acpi/tables'
>  -> new table size: 3840
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
> range 11905 - 11981
>  -> new table size: 3968
> bios_linker_loader_alloc: allocating memory for 'etc/acpi/rsdp'
>  -> new table size: 4096
> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/rsdp' to 
> 'etc/acpi/tables'
>  -> new table size: 4224
> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/rsdp' for 
> range 0 - 20
>  -> new table size: 4352

yea it's because each command has space for 2 file names, so it's size
is 128 bytes. So just 32 commands is 4k.

Question is what is the extra table and why isn't it there before boot?

-> bios_linker_loader_alloc: allocating memory for 'etc/acpi/rsdp'
+> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/tables' to 
'etc/acpi/tables'
 >  -> new table size: 3840
-> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/rsdp' to 
'etc/acpi/tables'
+> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/tables' for 
range 11905 - 11981
 >  -> new table size: 3968
-> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/rsdp' for 
range 0 - 20
+> bios_linker_loader_alloc: allocating memory for 'etc/acpi/rsdp'
 >  -> new table size: 4096
+> bios_linker_loader_add_pointer: adding pointer in 'etc/acpi/rsdp' to 
'etc/acpi/tables'
+>  -> new table size: 4224
+> bios_linker_loader_add_checksum: Adding checksum for 'etc/acpi/rsdp' for 
range 0 - 20
+>  -> new table size: 4352

simply put I would expect the command blob size to be the same before
and after migration.

> 
> 
> -- 
> Thanks,
> 
> David / dhildenb