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

[David Hildenbrand]

On 02.03.21 11:07, Michael S. Tsirkin wrote:
> 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.

Seems to be the "MCFG" table. I only see that pop up when the guest boots.

It depends on acpi_get_mcfg(). When the VM is created, it is not mapped 
(mcfg->base == PCIE_BASE_ADDR_UNMAPPED), thus we don't create the table. 
Once the bios configured/mapped it (wild guess), we create the table.

Anyhow, we seem to end up > 4k in the end.

--
Thanks,

David / dhildenb