| Message ID | 1473274769-19129-10-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 07.09.16 at 20:59, <boris.ostrovsky@oracle.com> wrote: > In prepearation to moving acpi sources into generally available > libacpi: > > 1. Pass IOAPIC/LAPIC/PCI mask values via struct acpi_config > 2. Modify include files search paths to point to acpi directory > 3. Macro-ise include file for build.c that defines various > utilities used by that file. Users of libacpi will be expected > to define this macro when compiling build.c > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > --- > Changes in v3: > * Instead of adding x86.h pass APIC/IOAPIC info via acpi_config parameter. > * Use <> instead of "" for include directive > > > tools/firmware/hvmloader/Makefile | 3 ++- > tools/firmware/hvmloader/acpi/README | 16 ++++++++++++---- > tools/firmware/hvmloader/acpi/build.c | 19 ++++++++++--------- > tools/firmware/hvmloader/acpi/libacpi.h | 7 +++++++ > tools/firmware/hvmloader/hvmloader.c | 2 +- > tools/firmware/hvmloader/rombios.c | 2 +- > tools/firmware/hvmloader/seabios.c | 5 +++-- > tools/firmware/hvmloader/util.c | 15 +++++++++++++-- > tools/firmware/rombios/32bit/Makefile | 2 +- > tools/firmware/rombios/32bit/tcgbios/Makefile | 2 +- > tools/firmware/rombios/32bit/util.h | 2 +- > 11 files changed, 52 insertions(+), 23 deletions(-) > > diff --git a/tools/firmware/hvmloader/Makefile > b/tools/firmware/hvmloader/Makefile > index b6c5b83..77e95f1 100644 > --- a/tools/firmware/hvmloader/Makefile > +++ b/tools/firmware/hvmloader/Makefile > @@ -76,7 +76,8 @@ smbios.o: CFLAGS += > -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\"" > ACPI_PATH = acpi > ACPI_FILES = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c > ACPI_OBJS = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o > -$(ACPI_OBJS): CFLAGS += -I$(ACPI_PATH) -I. > +$(ACPI_OBJS): CFLAGS += -I. -DLIBACPI_STDUTILS=\"../util.h\" > +CFLAGS += -I$(ACPI_PATH) > vpath build.c $(ACPI_PATH) > vpath static_tables.c $(ACPI_PATH) > OBJS += $(ACPI_OBJS) > diff --git a/tools/firmware/hvmloader/acpi/README > b/tools/firmware/hvmloader/acpi/README > index 210d5ba..2b9d6e1 100644 > --- a/tools/firmware/hvmloader/acpi/README > +++ b/tools/firmware/hvmloader/acpi/README > @@ -1,11 +1,19 @@ > -ACPI Table for domain firmware > +ACPI builder for domain firmware > > > -INSTALL > +BUILDING ACPI > ----------------- > -Simply make is OK. > -# make > +Users of ACPI builder are expected to provide an include file that makes > available > +the following: > +* strncpy > +* printf > +* NULL > +* test_bit > +* offsetof > > +When compiling build.c, the name of this include file should be given to > +compiler as -DLIBACPI_STDUTILS=\"<filename>\". See > tools/firmware/hvmloader/Makefile > +for an example. > > Note on DSDT Table > ------------------ > diff --git a/tools/firmware/hvmloader/acpi/build.c > b/tools/firmware/hvmloader/acpi/build.c > index 2098920..1cd640d 100644 > --- a/tools/firmware/hvmloader/acpi/build.c > +++ b/tools/firmware/hvmloader/acpi/build.c > @@ -13,15 +13,13 @@ > * GNU Lesser General Public License for more details. > */ > > +#include LIBACPI_STDUTILS > #include "acpi2_0.h" > #include "libacpi.h" > #include "ssdt_s3.h" > #include "ssdt_s4.h" > #include "ssdt_tpm.h" > #include "ssdt_pm.h" > -#include "../config.h" > -#include "../util.h" > -#include "../vnuma.h" > #include <xen/hvm/hvm_xs_strings.h> > #include <xen/hvm/params.h> > > @@ -81,6 +79,9 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt > *ctxt, > struct hvm_info_table *hvminfo = config->hvminfo; > int i, sz; > > + if ( config->lapic_id == NULL ) > + return NULL; > + > sz = sizeof(struct acpi_20_madt); > sz += sizeof(struct acpi_20_madt_intsrcovr) * 16; > sz += sizeof(struct acpi_20_madt_ioapic); > @@ -97,7 +98,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt > *ctxt, > madt->header.oem_revision = ACPI_OEM_REVISION; > madt->header.creator_id = ACPI_CREATOR_ID; > madt->header.creator_revision = ACPI_CREATOR_REVISION; > - madt->lapic_addr = LAPIC_BASE_ADDRESS; > + madt->lapic_addr = config->lapic_base_address; > madt->flags = ACPI_PCAT_COMPAT; > > if ( config->table_flags & ACPI_HAS_IOAPIC ) > @@ -116,7 +117,7 @@ static struct acpi_20_madt *construct_madt(struct > acpi_ctxt *ctxt, > intsrcovr->gsi = 2; > intsrcovr->flags = 0x0; > } > - else if ( PCI_ISA_IRQ_MASK & (1U << i) ) > + else if ( config->pci_isa_irq_mask & (1U << i) ) > { > /* PCI: active-low level-triggered. */ > intsrcovr->gsi = i; > @@ -135,8 +136,8 @@ static struct acpi_20_madt *construct_madt(struct > acpi_ctxt *ctxt, > memset(io_apic, 0, sizeof(*io_apic)); > io_apic->type = ACPI_IO_APIC; > io_apic->length = sizeof(*io_apic); > - io_apic->ioapic_id = IOAPIC_ID; > - io_apic->ioapic_addr = ioapic_base_address; > + io_apic->ioapic_id = config->ioapic_id; > + io_apic->ioapic_addr = config->ioapic_base_address; > > lapic = (struct acpi_20_madt_lapic *)(io_apic + 1); > } > @@ -152,7 +153,7 @@ static struct acpi_20_madt *construct_madt(struct > acpi_ctxt *ctxt, > lapic->length = sizeof(*lapic); > /* Processor ID must match processor-object IDs in the DSDT. */ > lapic->acpi_processor_id = i; > - lapic->apic_id = LAPIC_ID(i); > + lapic->apic_id = config->lapic_id(i); > lapic->flags = (test_bit(i, hvminfo->vcpu_online) > ? ACPI_LOCAL_APIC_ENABLED : 0); > lapic++; > @@ -240,7 +241,7 @@ static struct acpi_20_srat *construct_srat(struct > acpi_ctxt *ctxt, > processor->type = ACPI_PROCESSOR_AFFINITY; > processor->length = sizeof(*processor); > processor->domain = config->numa.vcpu_to_vnode[i]; > - processor->apic_id = LAPIC_ID(i); > + processor->apic_id = config->lapic_id(i); > processor->flags = ACPI_LOCAL_APIC_AFFIN_ENABLED; > processor++; > } > diff --git a/tools/firmware/hvmloader/acpi/libacpi.h > b/tools/firmware/hvmloader/acpi/libacpi.h > index 3bcd226..d803139 100644 > --- a/tools/firmware/hvmloader/acpi/libacpi.h > +++ b/tools/firmware/hvmloader/acpi/libacpi.h > @@ -84,6 +84,13 @@ struct acpi_config { > > /* RSDP address */ > unsigned int rsdp; > + > + /* x86-specific parameters */ > + uint16_t (*lapic_id)(unsigned cpu); Why uint16_t? Legacy APIC IDs in MADT are in an 8-bit field, and x2APIC IDs require 32 bits. Depending on whether we want to be able to re-use the same function for x2APIC support (once that gets added) the return type should change accordingly. (But if we were to re-use it, I guess a second parameter would then also be needed.) And then - down the road you're not planning to build a shared library using this interface? Otherwise we'd need to consider some compatibility aspects. Jan
On 09/08/2016 10:05 AM, Jan Beulich wrote: >> >> diff --git a/tools/firmware/hvmloader/acpi/libacpi.h >> b/tools/firmware/hvmloader/acpi/libacpi.h >> index 3bcd226..d803139 100644 >> --- a/tools/firmware/hvmloader/acpi/libacpi.h >> +++ b/tools/firmware/hvmloader/acpi/libacpi.h >> @@ -84,6 +84,13 @@ struct acpi_config { >> >> /* RSDP address */ >> unsigned int rsdp; >> + >> + /* x86-specific parameters */ >> + uint16_t (*lapic_id)(unsigned cpu); > Why uint16_t? Legacy APIC IDs in MADT are in an 8-bit field, and > x2APIC IDs require 32 bits. Depending on whether we want to be > able to re-use the same function for x2APIC support (once that > gets added) the return type should change accordingly. (But if we > were to re-use it, I guess a second parameter would then also be > needed.) No good reason for uint16_t, should be uint8_t. I think deciding on how interface will look for x2APIC should be deferred until when we support it. > > And then - down the road you're not planning to build a shared > library using this interface? Otherwise we'd need to consider some > compatibility aspects. No specific plans for now, the objects are expected to be built and linked directly to whoever wants to use this (so libacpi is somewhat of a misnomer, similar to libelf). -boris
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile index b6c5b83..77e95f1 100644 --- a/tools/firmware/hvmloader/Makefile +++ b/tools/firmware/hvmloader/Makefile @@ -76,7 +76,8 @@ smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\"" ACPI_PATH = acpi ACPI_FILES = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c ACPI_OBJS = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o -$(ACPI_OBJS): CFLAGS += -I$(ACPI_PATH) -I. +$(ACPI_OBJS): CFLAGS += -I. -DLIBACPI_STDUTILS=\"../util.h\" +CFLAGS += -I$(ACPI_PATH) vpath build.c $(ACPI_PATH) vpath static_tables.c $(ACPI_PATH) OBJS += $(ACPI_OBJS) diff --git a/tools/firmware/hvmloader/acpi/README b/tools/firmware/hvmloader/acpi/README index 210d5ba..2b9d6e1 100644 --- a/tools/firmware/hvmloader/acpi/README +++ b/tools/firmware/hvmloader/acpi/README @@ -1,11 +1,19 @@ -ACPI Table for domain firmware +ACPI builder for domain firmware -INSTALL +BUILDING ACPI ----------------- -Simply make is OK. -# make +Users of ACPI builder are expected to provide an include file that makes available +the following: +* strncpy +* printf +* NULL +* test_bit +* offsetof +When compiling build.c, the name of this include file should be given to +compiler as -DLIBACPI_STDUTILS=\"<filename>\". See tools/firmware/hvmloader/Makefile +for an example. Note on DSDT Table ------------------ diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c index 2098920..1cd640d 100644 --- a/tools/firmware/hvmloader/acpi/build.c +++ b/tools/firmware/hvmloader/acpi/build.c @@ -13,15 +13,13 @@ * GNU Lesser General Public License for more details. */ +#include LIBACPI_STDUTILS #include "acpi2_0.h" #include "libacpi.h" #include "ssdt_s3.h" #include "ssdt_s4.h" #include "ssdt_tpm.h" #include "ssdt_pm.h" -#include "../config.h" -#include "../util.h" -#include "../vnuma.h" #include <xen/hvm/hvm_xs_strings.h> #include <xen/hvm/params.h> @@ -81,6 +79,9 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt, struct hvm_info_table *hvminfo = config->hvminfo; int i, sz; + if ( config->lapic_id == NULL ) + return NULL; + sz = sizeof(struct acpi_20_madt); sz += sizeof(struct acpi_20_madt_intsrcovr) * 16; sz += sizeof(struct acpi_20_madt_ioapic); @@ -97,7 +98,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt, madt->header.oem_revision = ACPI_OEM_REVISION; madt->header.creator_id = ACPI_CREATOR_ID; madt->header.creator_revision = ACPI_CREATOR_REVISION; - madt->lapic_addr = LAPIC_BASE_ADDRESS; + madt->lapic_addr = config->lapic_base_address; madt->flags = ACPI_PCAT_COMPAT; if ( config->table_flags & ACPI_HAS_IOAPIC ) @@ -116,7 +117,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt, intsrcovr->gsi = 2; intsrcovr->flags = 0x0; } - else if ( PCI_ISA_IRQ_MASK & (1U << i) ) + else if ( config->pci_isa_irq_mask & (1U << i) ) { /* PCI: active-low level-triggered. */ intsrcovr->gsi = i; @@ -135,8 +136,8 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt, memset(io_apic, 0, sizeof(*io_apic)); io_apic->type = ACPI_IO_APIC; io_apic->length = sizeof(*io_apic); - io_apic->ioapic_id = IOAPIC_ID; - io_apic->ioapic_addr = ioapic_base_address; + io_apic->ioapic_id = config->ioapic_id; + io_apic->ioapic_addr = config->ioapic_base_address; lapic = (struct acpi_20_madt_lapic *)(io_apic + 1); } @@ -152,7 +153,7 @@ static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt, lapic->length = sizeof(*lapic); /* Processor ID must match processor-object IDs in the DSDT. */ lapic->acpi_processor_id = i; - lapic->apic_id = LAPIC_ID(i); + lapic->apic_id = config->lapic_id(i); lapic->flags = (test_bit(i, hvminfo->vcpu_online) ? ACPI_LOCAL_APIC_ENABLED : 0); lapic++; @@ -240,7 +241,7 @@ static struct acpi_20_srat *construct_srat(struct acpi_ctxt *ctxt, processor->type = ACPI_PROCESSOR_AFFINITY; processor->length = sizeof(*processor); processor->domain = config->numa.vcpu_to_vnode[i]; - processor->apic_id = LAPIC_ID(i); + processor->apic_id = config->lapic_id(i); processor->flags = ACPI_LOCAL_APIC_AFFIN_ENABLED; processor++; } diff --git a/tools/firmware/hvmloader/acpi/libacpi.h b/tools/firmware/hvmloader/acpi/libacpi.h index 3bcd226..d803139 100644 --- a/tools/firmware/hvmloader/acpi/libacpi.h +++ b/tools/firmware/hvmloader/acpi/libacpi.h @@ -84,6 +84,13 @@ struct acpi_config { /* RSDP address */ unsigned int rsdp; + + /* x86-specific parameters */ + uint16_t (*lapic_id)(unsigned cpu); + uint32_t lapic_base_address; + uint32_t ioapic_base_address; + uint16_t pci_isa_irq_mask; + uint8_t ioapic_id; }; void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config); diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c index bbd4e34..77b9536 100644 --- a/tools/firmware/hvmloader/hvmloader.c +++ b/tools/firmware/hvmloader/hvmloader.c @@ -24,8 +24,8 @@ #include "config.h" #include "pci_regs.h" #include "apic_regs.h" -#include "acpi/acpi2_0.h" #include "vnuma.h" +#include <acpi2_0.h> #include <xen/version.h> #include <xen/hvm/params.h> #include <xen/arch-x86/hvm/start_info.h> diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c index 9acf03f..56b39b7 100644 --- a/tools/firmware/hvmloader/rombios.c +++ b/tools/firmware/hvmloader/rombios.c @@ -24,12 +24,12 @@ #include "../rombios/config.h" #include "smbios_types.h" -#include "acpi/libacpi.h" #include "pci_regs.h" #include "util.h" #include "hypercall.h" #include "option_rom.h" +#include <libacpi.h> #include <xen/hvm/params.h> #define ROM_INCLUDE_ROMBIOS diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c index 44ff0d7..870576a 100644 --- a/tools/firmware/hvmloader/seabios.c +++ b/tools/firmware/hvmloader/seabios.c @@ -25,8 +25,9 @@ #include "util.h" #include "smbios_types.h" -#include "acpi/acpi2_0.h" -#include "acpi/libacpi.h" + +#include <acpi2_0.h> +#include <libacpi.h> extern unsigned char dsdt_anycpu_qemu_xen[]; extern int dsdt_anycpu_qemu_xen_len; diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c index 8773e69..6d636bf 100644 --- a/tools/firmware/hvmloader/util.c +++ b/tools/firmware/hvmloader/util.c @@ -22,8 +22,8 @@ #include "hypercall.h" #include "ctype.h" #include "vnuma.h" -#include "acpi/acpi2_0.h" -#include "acpi/libacpi.h" +#include <acpi2_0.h> +#include <libacpi.h> #include <stdint.h> #include <xen/xen.h> #include <xen/memory.h> @@ -882,6 +882,11 @@ static void acpi_mem_free(struct acpi_ctxt *ctxt, { } +static uint16_t acpi_lapic_id(unsigned cpu) +{ + return LAPIC_ID(cpu); +} + void hvmloader_acpi_build_tables(struct acpi_config *config, unsigned int physical) { @@ -891,6 +896,12 @@ void hvmloader_acpi_build_tables(struct acpi_config *config, /* Allocate and initialise the acpi info area. */ mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1); + config->lapic_base_address = LAPIC_BASE_ADDRESS; + config->lapic_id = acpi_lapic_id; + config->ioapic_base_address = ioapic_base_address; + config->ioapic_id = IOAPIC_ID; + config->pci_isa_irq_mask = PCI_ISA_IRQ_MASK; + if ( uart_exists(0x3f8) ) config->table_flags |= ACPI_HAS_COM1; if ( uart_exists(0x2f8) ) diff --git a/tools/firmware/rombios/32bit/Makefile b/tools/firmware/rombios/32bit/Makefile index 396906c..71399d2 100644 --- a/tools/firmware/rombios/32bit/Makefile +++ b/tools/firmware/rombios/32bit/Makefile @@ -3,7 +3,7 @@ include $(XEN_ROOT)/tools/firmware/Rules.mk TARGET = 32bitbios_flat.h -CFLAGS += $(CFLAGS_xeninclude) -I.. +CFLAGS += $(CFLAGS_xeninclude) -I.. -I../../hvmloader/acpi SUBDIRS = tcgbios diff --git a/tools/firmware/rombios/32bit/tcgbios/Makefile b/tools/firmware/rombios/32bit/tcgbios/Makefile index f6f2649..6822027 100644 --- a/tools/firmware/rombios/32bit/tcgbios/Makefile +++ b/tools/firmware/rombios/32bit/tcgbios/Makefile @@ -3,7 +3,7 @@ include $(XEN_ROOT)/tools/firmware/Rules.mk TARGET = tcgbiosext.o -CFLAGS += $(CFLAGS_xeninclude) -I.. -I../.. +CFLAGS += $(CFLAGS_xeninclude) -I.. -I../.. -I../../../hvmloader/acpi .PHONY: all all: $(TARGET) diff --git a/tools/firmware/rombios/32bit/util.h b/tools/firmware/rombios/32bit/util.h index e245be6..6b90c43 100644 --- a/tools/firmware/rombios/32bit/util.h +++ b/tools/firmware/rombios/32bit/util.h @@ -1,7 +1,7 @@ #ifndef UTIL_H #define UTIL_H -#include "../hvmloader/acpi/acpi2_0.h" +#include <acpi2_0.h> void outb(uint16_t addr, uint8_t val); void outw(uint16_t addr, uint16_t val);
In prepearation to moving acpi sources into generally available libacpi: 1. Pass IOAPIC/LAPIC/PCI mask values via struct acpi_config 2. Modify include files search paths to point to acpi directory 3. Macro-ise include file for build.c that defines various utilities used by that file. Users of libacpi will be expected to define this macro when compiling build.c Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- Changes in v3: * Instead of adding x86.h pass APIC/IOAPIC info via acpi_config parameter. * Use <> instead of "" for include directive tools/firmware/hvmloader/Makefile | 3 ++- tools/firmware/hvmloader/acpi/README | 16 ++++++++++++---- tools/firmware/hvmloader/acpi/build.c | 19 ++++++++++--------- tools/firmware/hvmloader/acpi/libacpi.h | 7 +++++++ tools/firmware/hvmloader/hvmloader.c | 2 +- tools/firmware/hvmloader/rombios.c | 2 +- tools/firmware/hvmloader/seabios.c | 5 +++-- tools/firmware/hvmloader/util.c | 15 +++++++++++++-- tools/firmware/rombios/32bit/Makefile | 2 +- tools/firmware/rombios/32bit/tcgbios/Makefile | 2 +- tools/firmware/rombios/32bit/util.h | 2 +- 11 files changed, 52 insertions(+), 23 deletions(-)