Re: [Qemu-devel] [PATCH 6/8] hw/acpi/Kconfig: Introduce the ACPI_PIIX4 config

[Philippe Mathieu-Daudé]

Hi Igor,

Le jeu. 21 mars 2019 14:36, Igor Mammedov <address@hidden> a écrit :

> On Tue, 19 Mar 2019 23:47:22 +0100 (CET)
> BALATON Zoltan <address@hidden> wrote:
>
> > On Tue, 19 Mar 2019, Igor Mammedov wrote:
> > > On Sun, 17 Mar 2019 01:22:57 +0100
> > > Philippe Mathieu-Daudé <address@hidden> wrote:
> > >
> > >> The PIIX4 chipset is a generic southbridge and can be used by
> > >> non-X86 hardware. Introduce the ACPI_PIIX4 Kconfig.
> > >> Hardware that requires ACPI but doesn't need the PIIX4 chipset
> > >> won't compile it.
> > > I know that mips somewhat depends on piix but does it actually
> > > implements, uses or plans to use ACPI in practice?


Linux PIIX driver is generic. But Linux Malta works without configuring it
too. We loose power saving features.


> >
> > I think it does not compile without due to PIIX depends on it. Likewise
> yep, looks like for piix_pm we need to pull in pci/cpu/mem and nvdimm
> stuff even if it's not going to be used
>
> > for VT82C686. I get errors about undefined functions when linking a
> > ppc board using it (not yet upstream) without this patch:
> > http://patchwork.ozlabs.org/patch/1054018/
> >
> > ../hw/isa/vt82c686.o: In function `vt82c686b_pm_realize':
> > hw/isa/vt82c686.c:375: undefined reference to `apm_init'
> > hw/isa/vt82c686.c:381: undefined reference to `acpi_pm_tmr_init'
> > hw/isa/vt82c686.c:382: undefined reference to `acpi_pm1_evt_init'
> > ../hw/isa/vt82c686.o: In function `pm_update_sci':
> > hw/isa/vt82c686.c:195: undefined reference to `acpi_pm1_evt_get_sts'
> > ../hw/isa/vt82c686.o: In function `vt82c686b_pm_realize':
> > hw/isa/vt82c686.c:383: undefined reference to `acpi_pm1_cnt_init'
> > ../hw/isa/vt82c686.o: In function `pm_update_sci':
> > hw/isa/vt82c686.c:203: undefined reference to `acpi_pm_tmr_update'
>
> If you a going to reuse vt82c686 then following patch should help
>
>
> diff --git a/default-configs/mips-softmmu-common.mak
> b/default-configs/mips-softmmu-common.mak
> index 0795d52..9c2cf15 100644
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -16,10 +16,11 @@ CONFIG_PCSPK=y
>  CONFIG_PCKBD=y
>  CONFIG_FDC=y
>  CONFIG_ACPI=y
> -CONFIG_ACPI_X86=y
> -CONFIG_ACPI_MEMORY_HOTPLUG=y
> -CONFIG_ACPI_NVDIMM=y
> -CONFIG_ACPI_CPU_HOTPLUG=y
> +CONFIG_ACPI_GENERIC_HW=y
> +#CONFIG_ACPI_X86=y
> +#CONFIG_ACPI_MEMORY_HOTPLUG=y
> +#CONFIG_ACPI_NVDIMM=y
> +#CONFIG_ACPI_CPU_HOTPLUG=y
>  CONFIG_APM=y
>  CONFIG_I8257=y
>  CONFIG_PIIX4=y
> diff --git a/dtc b/dtc
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 88f18909db731a627456f26d779445f84e449536
> +Subproject commit 88f18909db731a627456f26d779445f84e449536-dirty
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index eca3bee..319707f 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -4,10 +4,14 @@ config ACPI
>  config ACPI_X86
>      bool
>      select ACPI
> +    select ACPI_GENERIC_HW
>      select ACPI_NVDIMM
>      select ACPI_CPU_HOTPLUG
>      select ACPI_MEMORY_HOTPLUG
>
> +config ACPI_GENERIC_HW
> +    bool
> +
>  config ACPI_X86_ICH
>      bool
>      select ACPI_X86
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index ba93c5b..2b524bd 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,12 +1,13 @@
>  ifeq ($(CONFIG_ACPI),y)
> -common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> +common-obj-$(CONFIG_ACPI_GENERIC_HW) += core.o
> +common-obj-$(CONFIG_ACPI_X86) += piix4.o pcihp.o
>  common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>  common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>  common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>  common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
> -common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> +common-obj-$(call lnot,$(CONFIG_ACPI_GENERIC_HW)) += acpi-stub.o
>
>  common-obj-y += acpi_interface.o
>  common-obj-y += bios-linker-loader.o
>
>
> > ../hw/isa/vt82c686.o:(.data.rel+0x370): undefined reference to
> `vmstate_apm'
> > collect2: error: ld returned 1 exit status
>
> for that you need to include CONFIG_APM
>
> >
> > Regards,
> > BALATON Zoltan
> >
> > >> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > >> ---
> > >>  default-configs/mips-softmmu-common.mak | 2 +-
> > >>  hw/acpi/Kconfig                         | 7 +++++++
> > >>  hw/acpi/Makefile.objs                   | 3 ++-
> > >>  hw/i386/Kconfig                         | 2 ++
> > >>  4 files changed, 12 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/default-configs/mips-softmmu-common.mak
> b/default-configs/mips-softmmu-common.mak
> > >> index 6f49a3a8c7..21c6e3cbe3 100644
> > >> --- a/default-configs/mips-softmmu-common.mak
> > >> +++ b/default-configs/mips-softmmu-common.mak
> > >> @@ -16,7 +16,7 @@ CONFIG_PCSPK=y
> > >>  CONFIG_I8042=y
> > >>  CONFIG_FDC=y
> > >>  CONFIG_ACPI=y
> > >> -CONFIG_ACPI_X86=y
> > >> +CONFIG_ACPI_PIIX4=y
> > >>  CONFIG_ACPI_MEMORY_HOTPLUG=y
> > >>  CONFIG_ACPI_NVDIMM=y
> > >>  CONFIG_ACPI_CPU_HOTPLUG=y
> > >> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> > >> index b18ddb06da..10f24f3b41 100644
> > >> --- a/hw/acpi/Kconfig
> > >> +++ b/hw/acpi/Kconfig
> > >> @@ -8,6 +8,13 @@ config ACPI_X86
> > >>      select ACPI_CPU_HOTPLUG
> > >>      select ACPI_MEMORY_HOTPLUG
> > >>
> > >> +config ACPI_PIIX4
> > >> +    bool
> > >> +    select ACPI
> > >> +    select ACPI_NVDIMM
> > >> +    select ACPI_CPU_HOTPLUG
> > >> +    select ACPI_MEMORY_HOTPLUG
> > >> +
> > >>  config ACPI_ICH9
> > >>      bool
> > >>      select ACPI
> > >> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> > >> index dff94d35e7..c5c2c1cf03 100644
> > >> --- a/hw/acpi/Makefile.objs
> > >> +++ b/hw/acpi/Makefile.objs
> > >> @@ -1,4 +1,5 @@
> > >> -common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> > >> +common-obj-$(CONFIG_ACPI) += core.o
>
> this will break linking on for arm target at least
>

Are you sure? I'm pretty sure I tested it there. I don't think ACPI is
different that
ACPI_GENERIC_HW, I'd rather avoid it.
Anyway I don't have computer access so I'll check / rework during next
development cycle.


>
> > >> +common-obj-$(CONFIG_ACPI_PIIX4) += piix4.o pcihp.o
> > >>  common-obj-$(CONFIG_ACPI_ICH9) += ich9.o tco.o
> > >>  common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> > >>  common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> > >> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> > >> index 7fa5395894..64f9957047 100644
> > >> --- a/hw/i386/Kconfig
> > >> +++ b/hw/i386/Kconfig
> > >> @@ -69,6 +69,8 @@ config ISAPC
> > >>      # FIXME: it is in the same file as i440fx, and does not compile
> > >>      # if separated
> > >>      depends on I440FX
> > >> +    # QEMU isapc weirdness: it uses PIIX3 with ACPI features from
> PIIX4
> > >> +    select ACPI_PIIX4
> > >>
> > >>  config Q35
> > >>      bool
> > >
> > >
>
>
>