Re: [PATCH 3/3] tests/acpi: Update virt/SSDT.memhp

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 19 Jan 2024 at 17:19, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 1/19/24 15:29, Peter Maydell wrote:
> > On Mon, 15 Jan 2024 at 04:35, Bin Meng <bin.meng@windriver.com> wrote:
> >>
> >> The Arm dtb changes caused an address change:
> >>
> >>  DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x00000001)
> >>  {
> >>      [ ... ]
> >> -    Name (MEMA, 0x43C80000)
> >> +    Name (MEMA, 0x43D80000)
> >>  }
> >>
> >> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >>
> >> ---
> >
> > You should follow up (with Laszlo?) to make sure we understand
> > why reducing the size of the generated dtb has caused this
> > change in the ACPI tables. In particular, if we made the
> > dtb *smaller* why has the allocated address here got *larger*?
>
> As a very roughly stated trait (i.e., I'm not claiming this is an exact,
> hard rule), the UEFI memory allocator hands out chunks top-down. An
> earlier allocation (such as the DTB's) shrinking is consistent with
> further allocations being serviced at higher addresses.
>
> >
> > This particular bit of the ACPI tables does seem to be
> > annoyingly unstable, though -- for instance commit 55abfc1ffbe54c0
> > we had to change this figure when we updated to a newer EDK2
> > version, and similarly commit 5f88dd43d0 for the same reason.
> > I wonder if we can or should make our data-check be more
> > loose about the address reported here, given what Laszlo
> > says about how we're basically looking at the address of some
> > memory the guest allocated. (cc'd the bios-tables-test
> > maintainers for their opinion.)
>
> Right, the allocation address is generally unpredictable. (That's why
> the ACPI linker/loader "language" had to be extended with an extra
> command, for the sake of the vmgenid device -- so that the firmware
> could send the allocation GPA back to QEMU in an "architected" way.)
>
> >
> > I'm also a little concerned that if the ACPI generated
> > tables care about the dtb size then we're now going to
> > have a situation where any patch we make to the virt board
> > that changes the generated dtb at all will result in the
> > ACPI tables changing. That would be annoying.
>
> This is generally inevitable, it's just how the ACPI linker/loader
> works. The guest allocator can only work with the memory map it gets
> from QEMU. The same effect is triggered BTW if you don't change the DTB
> but change (on the QEMU command line) the guest RAM size. The ACPI
> tables will be allocated at different addresses than before, and so the
> pointer fields in other tables, to those tables, will also change.

Mmm, but previously we weren't packing the dtb we created,
so it would always be the same 1MB regardless of what and
how much we put into it. After this patchset it will be packed
down to its "real" size, so the size will be much more variable.

thanks
-- PMM