Re: [PULL 06/35] hw/acpi: refactor acpi hp modules so that targets can just use what they need

[BB]

On July 21, 2022 12:13:06 AM GMT+02:00, Ani Sinha <ani@anisinha.ca> wrote:
>
>
>On Wed, 20 Jul 2022, Peter Maydell wrote:
>
>> On Wed, 20 Jul 2022 at 19:37, Ani Sinha <ani@anisinha.ca> wrote:
>> >
>> >
>> >
>> > On Tue, 19 Jul 2022, Peter Maydell wrote:
>> >
>> > > On Sat, 4 Sept 2021 at 22:36, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > > How is this intended to work? The obvious fix from my point
>> > > of view would just be to say "piix4.c requires pcihp.c"
>> > > and cause CONFIG_ACPI_PIIX4 to pull in CONFIG_ACPI_PCIHP,
>> > > but that seems like it would be rather undoing the point
>> > > of this change.
>> >
>> > Yes. From the commit log and the vague recollection I have about this
>> > change :
>> >
>> > > For example, mips only needs support for PIIX4 and does not
>> > > need acpi pci hotplug support or cpu hotplug support or memory hotplug
>> > support
>> > > etc
>> >
>> > So does malta really need acpi hotplug? If not, then the stubbing out of
>> > the vmstate struct is correct.
>>
>> It's not, because the vmstate struct is actually used when you
>> savevm/loadvm a malta machine. If the malta shouldn't have
>> acpi hotplug then we need to arrange for the hotplug code
>> to be avoided at an earlier point, not just stub in the
>> vmstate struct field.
>
>yes I think that would be more appropriate fix, I agree. Since mst added
>that vmstate, maybe he can comment on this.
>

Despite its name, ACPI_PIIX4 is also used by PIIX3, which is required by the 
"pc" machines. These machines support migration etc. which may explain why 
ACPI_PIIX4 has the vmstate struct field. To me it just looks like an oversight 
that the Malta board doesn't support all ACPI_PIIX4 features.

Best regards,
Bernhard