On 2015/5/8 0:09, Peter Maydell wrote: > On 7 May 2015 at 10:29, Shannon Zhao <zhaoshengl...@huawei.com> wrote: >> From: Shannon Zhao <shannon.z...@linaro.org> >> >> Expose the needed device information to the table generation >> insfrastructure and register a machine_init_done notify to > > "infrastructure". > >> call virt_acpi_build(). >> >> Add CONFIG_ACPI to arm-softmmu.mak. >> >> Signed-off-by: Shannon Zhao <zhaoshengl...@huawei.com> >> Signed-off-by: Shannon Zhao <shannon.z...@linaro.org> >> --- >> default-configs/arm-softmmu.mak | 1 + >> default-configs/i386-softmmu.mak | 3 ++ >> default-configs/mips-softmmu.mak | 3 ++ >> default-configs/mips64-softmmu.mak | 3 ++ >> default-configs/mips64el-softmmu.mak | 3 ++ >> default-configs/mipsel-softmmu.mak | 3 ++ >> default-configs/x86_64-softmmu.mak | 3 ++ >> hw/acpi/Makefile.objs | 5 ++- >> hw/arm/virt.c | 78 >> ++++++++++++++++++++++++++++++++---- >> hw/i2c/Makefile.objs | 2 +- >> 10 files changed, 94 insertions(+), 10 deletions(-) >> >> diff --git a/default-configs/arm-softmmu.mak >> b/default-configs/arm-softmmu.mak >> index a767e4b..74f1db3 100644 >> --- a/default-configs/arm-softmmu.mak >> +++ b/default-configs/arm-softmmu.mak >> @@ -101,3 +101,4 @@ CONFIG_ALLWINNER_A10=y >> CONFIG_XIO3130=y >> CONFIG_IOH3420=y >> CONFIG_I82801B11=y >> +CONFIG_ACPI=y >> diff --git a/default-configs/i386-softmmu.mak >> b/default-configs/i386-softmmu.mak >> index 6a74e00..d2de500 100644 >> --- a/default-configs/i386-softmmu.mak >> +++ b/default-configs/i386-softmmu.mak >> @@ -15,6 +15,9 @@ CONFIG_PCSPK=y >> CONFIG_PCKBD=y >> CONFIG_FDC=y >> CONFIG_ACPI=y >> +CONFIG_ACPI_CORE=y >> +CONFIG_ACPI_MEMORY_HOTPLUG=y >> +CONFIG_ACPI_CPU_HOTPLUG=y > > This is splitting the basic CONFIG_ACPI into several pieces, right? > I think that deserves its own patch. >
Ok, will split this patch. > What's the difference now between "CONFIG_ACPI" and "CONFIG_ACPI_CORE" ? > I didn't find a proper name. Maybe we should name it "CONFIG_ACPI_x86" for "core.o piix4.o ich9.o pcihp.o" as these files are for x86. >> CONFIG_APM=y >> CONFIG_I8257=y >> CONFIG_IDE_ISA=y >> diff --git a/default-configs/mips-softmmu.mak >> b/default-configs/mips-softmmu.mak >> index cce2c81..c96d42d 100644 >> --- a/default-configs/mips-softmmu.mak >> +++ b/default-configs/mips-softmmu.mak >> @@ -15,6 +15,9 @@ CONFIG_PCSPK=y >> CONFIG_PCKBD=y >> CONFIG_FDC=y >> CONFIG_ACPI=y >> +CONFIG_ACPI_CORE=y >> +CONFIG_ACPI_MEMORY_HOTPLUG=y >> +CONFIG_ACPI_CPU_HOTPLUG=y >> CONFIG_APM=y >> CONFIG_I8257=y >> CONFIG_PIIX4=y >> diff --git a/default-configs/mips64-softmmu.mak >> b/default-configs/mips64-softmmu.mak >> index 7a88a08..d229f9e 100644 >> --- a/default-configs/mips64-softmmu.mak >> +++ b/default-configs/mips64-softmmu.mak >> @@ -15,6 +15,9 @@ CONFIG_PCSPK=y >> CONFIG_PCKBD=y >> CONFIG_FDC=y >> CONFIG_ACPI=y >> +CONFIG_ACPI_CORE=y >> +CONFIG_ACPI_MEMORY_HOTPLUG=y >> +CONFIG_ACPI_CPU_HOTPLUG=y >> CONFIG_APM=y >> CONFIG_I8257=y >> CONFIG_PIIX4=y >> diff --git a/default-configs/mips64el-softmmu.mak >> b/default-configs/mips64el-softmmu.mak >> index 095de43..ea31b8b 100644 >> --- a/default-configs/mips64el-softmmu.mak >> +++ b/default-configs/mips64el-softmmu.mak >> @@ -15,6 +15,9 @@ CONFIG_PCSPK=y >> CONFIG_PCKBD=y >> CONFIG_FDC=y >> CONFIG_ACPI=y >> +CONFIG_ACPI_CORE=y >> +CONFIG_ACPI_MEMORY_HOTPLUG=y >> +CONFIG_ACPI_CPU_HOTPLUG=y >> CONFIG_APM=y >> CONFIG_I8257=y >> CONFIG_PIIX4=y >> diff --git a/default-configs/mipsel-softmmu.mak >> b/default-configs/mipsel-softmmu.mak >> index 0e25108..9a4462e 100644 >> --- a/default-configs/mipsel-softmmu.mak >> +++ b/default-configs/mipsel-softmmu.mak >> @@ -15,6 +15,9 @@ CONFIG_PCSPK=y >> CONFIG_PCKBD=y >> CONFIG_FDC=y >> CONFIG_ACPI=y >> +CONFIG_ACPI_CORE=y >> +CONFIG_ACPI_MEMORY_HOTPLUG=y >> +CONFIG_ACPI_CPU_HOTPLUG=y >> CONFIG_APM=y >> CONFIG_I8257=y >> CONFIG_PIIX4=y >> diff --git a/default-configs/x86_64-softmmu.mak >> b/default-configs/x86_64-softmmu.mak >> index 46b87dd..11019b6 100644 >> --- a/default-configs/x86_64-softmmu.mak >> +++ b/default-configs/x86_64-softmmu.mak >> @@ -15,6 +15,9 @@ CONFIG_PCSPK=y >> CONFIG_PCKBD=y >> CONFIG_FDC=y >> CONFIG_ACPI=y >> +CONFIG_ACPI_CORE=y >> +CONFIG_ACPI_MEMORY_HOTPLUG=y >> +CONFIG_ACPI_CPU_HOTPLUG=y >> CONFIG_APM=y >> CONFIG_I8257=y >> CONFIG_IDE_ISA=y >> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs >> index b9fefa7..511771a 100644 >> --- a/hw/acpi/Makefile.objs >> +++ b/hw/acpi/Makefile.objs >> @@ -1,5 +1,6 @@ >> -common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o >> -common-obj-$(CONFIG_ACPI) += memory_hotplug.o >> +common-obj-$(CONFIG_ACPI_CORE) += core.o piix4.o ich9.o pcihp.o > > Why is x86 specific stuff like piix4 in the CONFIG_ACPI_CORE set? > >> +common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o >> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o >> common-obj-$(CONFIG_ACPI) += acpi_interface.o >> common-obj-$(CONFIG_ACPI) += bios-linker-loader.o >> common-obj-$(CONFIG_ACPI) += aml-build.o >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 565f573..9291045 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -43,6 +43,7 @@ >> #include "qemu/bitops.h" >> #include "qemu/error-report.h" >> #include "hw/pci-host/gpex.h" >> +#include "hw/arm/virt-acpi-build.h" >> >> #define NUM_VIRTIO_TRANSPORTS 32 >> >> @@ -60,6 +61,11 @@ >> #define GIC_FDT_IRQ_PPI_CPU_START 8 >> #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 >> >> +#define ARCH_TIMER_VIRT_IRQ 11 >> +#define ARCH_TIMER_S_EL1_IRQ 13 >> +#define ARCH_TIMER_NS_EL1_IRQ 14 >> +#define ARCH_TIMER_NS_EL2_IRQ 10 >> + >> enum { >> VIRT_FLASH, >> VIRT_MEM, >> @@ -149,6 +155,29 @@ static const int a15irqmap[] = { >> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ >> }; >> >> +static AcpiMadtInfo madt_info = { >> + (MemMap *)&a15memmap[VIRT_GIC_CPU], >> + (MemMap *)&a15memmap[VIRT_GIC_DIST] > > These casts are really bad. You should just make sure the > types agree properly so we don't need a cast at all, rather > than having two different types which we implicitly require > to have identical structure. > Ok, will fix this. >> +}; >> + >> +static AcpiDsdtInfo dsdt_info = { > > "AcpiDsdtInfo" sounds like it ought to be a generic > ACPI structure, but in fact it's been defined in > include/hw/arm/virt-acpi-build.h. Is it actually > generic but in the wrong place? Or if it's not > generic, It's not generic. This structure is used to get the virt borad info. > why can't the ACPI table building code use > our existing MemMapEntry[] and irqmap[] arrays to > get the information it wants, rather than having > its own data structures that we have to copy everything > across to? If there's missing info or unhelpful layout > in the current virt data structures we can always > improve them. > Ok, will reuse MemMapEntry[] and irqmap[]. >> + (MemMap *)&a15memmap[VIRT_UART], >> + .uart_irq = &a15irqmap[VIRT_UART], > > Please don't mix named-initializer and non-named-initializer > syntax like this. > >> + (MemMap *)&a15memmap[VIRT_MMIO], >> + .virtio_mmio_irq = &a15irqmap[VIRT_MMIO], >> + .virtio_mmio_num = NUM_VIRTIO_TRANSPORTS, >> + (MemMap *)&a15memmap[VIRT_RTC], >> + .rtc_irq = &a15irqmap[VIRT_RTC], >> + (MemMap *)&a15memmap[VIRT_FLASH], >> +}; > > thanks > -- PMM > > > . > -- Shannon