|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 01/17] libacpi: Split dsdt.asl file and extract i440 specific parts
On Fri, Mar 13, 2026 at 04:35:01PM +0000, Thierry Escande wrote:
> In order to factorize the common parts between i440 and q35 dsdt files,
> this patch splits dsdt.asl and put the i440 specific parts into
> dsdt_i400.asl.
>
> This also makes use of #include directives instead of file
> concatenations to build the asl files.
>
> Also, the anycpu asl files generation makes use of makefile pattern
> rules to avoid duplication for i440 and q35.
>
> Becuase the LPC controller BDF (which differs between i440 and q35) must
s/Becuase/Because/
> be set at device declaration, it is still set in dsdt.asl by checking
> for a MACHINE_TYPE_I440 macro defined in dsdt_i400.asl.
s/i400/i440/
> Signed-off-by: Thierry Escande <thierry.escande@xxxxxxxxxx>
> ---
> tools/firmware/hvmloader/Makefile | 2 +-
> tools/firmware/hvmloader/ovmf.c | 4 ++--
> tools/firmware/hvmloader/seabios.c | 4 ++--
> tools/firmware/hvmloader/util.c | 4 ++--
> tools/firmware/hvmloader/util.h | 4 ++--
> tools/libacpi/Makefile | 10 ++++-----
> tools/libacpi/dsdt.asl | 25 ++++-----------------
> tools/libacpi/dsdt_i440.asl | 36 ++++++++++++++++++++++++++++++
> 8 files changed, 53 insertions(+), 36 deletions(-)
> create mode 100644 tools/libacpi/dsdt_i440.asl
>
> diff --git a/tools/firmware/hvmloader/Makefile
> b/tools/firmware/hvmloader/Makefile
> index 21de72187d..bdc33a877f 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -78,7 +78,7 @@ rombios.o: roms.inc
> smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
>
> ACPI_PATH = ../../libacpi
> -DSDT_FILES += dsdt_anycpu_qemu_xen.c
> +DSDT_FILES += dsdt_i440_anycpu_qemu_xen.c
> ACPI_OBJS = $(patsubst %.c,%.o,$(DSDT_FILES)) build.o static_tables.o
> $(ACPI_OBJS): CFLAGS += -iquote . -DLIBACPI_STDUTILS=\"$(CURDIR)/util.h\"
> CFLAGS += -I$(ACPI_PATH)
> diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> index 23610a0717..d264a50c73 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -119,8 +119,8 @@ static void ovmf_load(const struct bios_config *config,
> static void ovmf_acpi_build_tables(void)
> {
> struct acpi_config config = {
> - .dsdt_anycpu = dsdt_anycpu_qemu_xen,
> - .dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len,
> + .dsdt_anycpu = dsdt_i440_anycpu_qemu_xen,
> + .dsdt_anycpu_len = dsdt_i440_anycpu_qemu_xen_len,
> .dsdt_15cpu = NULL,
> .dsdt_15cpu_len = 0
> };
> diff --git a/tools/firmware/hvmloader/seabios.c
> b/tools/firmware/hvmloader/seabios.c
> index 444d118ddb..74b0406b5a 100644
> --- a/tools/firmware/hvmloader/seabios.c
> +++ b/tools/firmware/hvmloader/seabios.c
> @@ -90,8 +90,8 @@ static void seabios_acpi_build_tables(void)
> {
> uint32_t rsdp = (uint32_t)scratch_alloc(sizeof(struct acpi_20_rsdp), 0);
> struct acpi_config config = {
> - .dsdt_anycpu = dsdt_anycpu_qemu_xen,
> - .dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len,
> + .dsdt_anycpu = dsdt_i440_anycpu_qemu_xen,
> + .dsdt_anycpu_len = dsdt_i440_anycpu_qemu_xen_len,
> .dsdt_15cpu = NULL,
> .dsdt_15cpu_len = 0,
> };
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index e651342681..f1ed1eb48d 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -843,8 +843,8 @@ void hvmloader_acpi_build_tables(struct acpi_config
> *config,
> s = xenstore_read("platform/device-model", "");
> if ( !strncmp(s, "qemu_xen", 9) )
> {
> - config->dsdt_anycpu = dsdt_anycpu_qemu_xen;
> - config->dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len;
> + config->dsdt_anycpu = dsdt_i440_anycpu_qemu_xen;
> + config->dsdt_anycpu_len = dsdt_i440_anycpu_qemu_xen_len;
> config->dsdt_15cpu = NULL;
> config->dsdt_15cpu_len = 0;
> }
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 765a013ddd..3c5eeff5e7 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -381,8 +381,8 @@ extern struct e820map memory_map;
> bool check_overlap(uint64_t start, uint64_t size,
> uint64_t reserved_start, uint64_t reserved_size);
>
> -extern const unsigned char dsdt_anycpu_qemu_xen[], dsdt_anycpu[],
> dsdt_15cpu[];
> -extern const int dsdt_anycpu_qemu_xen_len, dsdt_anycpu_len, dsdt_15cpu_len;
> +extern const unsigned char dsdt_i440_anycpu_qemu_xen[], dsdt_anycpu[],
> dsdt_15cpu[];
> +extern const int dsdt_i440_anycpu_qemu_xen_len, dsdt_anycpu_len,
> dsdt_15cpu_len;
While there you can make this unsigned int, like the dsdt_anycpu_len
field.
>
> unsigned long acpi_pages_allocated(void);
>
> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
> index b21a64c6b4..d3d4bc9543 100644
> --- a/tools/libacpi/Makefile
> +++ b/tools/libacpi/Makefile
> @@ -11,7 +11,7 @@ endif
>
> MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>
> -C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_anycpu_qemu_xen.c
> dsdt_pvh.c
> +C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c dsdt_i440_anycpu_qemu_xen.c
> dsdt_pvh.c
> C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c
> DSDT_FILES ?= $(C_SRC-y)
> C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES))
> @@ -39,18 +39,16 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl
> $(MK_DSDT): mk_dsdt.c
> $(HOSTCC) $(HOSTCFLAGS) $(MKDSDT_CFLAGS-y) $(CFLAGS_xeninclude)
> -D__XEN_TOOLS__ -o $@ mk_dsdt.c
>
> -$(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl
> $(MK_DSDT)
> +$(ACPI_BUILD_DIR)/dsdt_%_anycpu_qemu_xen.asl: dsdt_%.asl dsdt.asl
> dsdt_acpi_info.asl $(MK_DSDT)
> # Remove last bracket
> awk 'NR > 1 {print s} {s=$$0}' $< > $@.$(TMP_SUFFIX)
> - cat dsdt_acpi_info.asl >> $@.$(TMP_SUFFIX)
> $(MK_DSDT) --debug=$(debug) --dm-version qemu-xen >> $@.$(TMP_SUFFIX)
> mv -f $@.$(TMP_SUFFIX) $@
>
> # NB. awk invocation is a portable alternative to 'head -n -1'
> -$(ACPI_BUILD_DIR)/dsdt_%cpu.asl: dsdt.asl dsdt_acpi_info.asl $(MK_DSDT)
> +$(ACPI_BUILD_DIR)/dsdt_%cpu.asl: dsdt_i440.asl dsdt.asl dsdt_acpi_info.asl
> $(MK_DSDT)
> # Remove last bracket
> awk 'NR > 1 {print s} {s=$$0}' $< > $@.$(TMP_SUFFIX)
> - cat dsdt_acpi_info.asl >> $@.$(TMP_SUFFIX)
> $(MK_DSDT) --debug=$(debug) --maxcpu $* >> $@.$(TMP_SUFFIX)
> mv -f $@.$(TMP_SUFFIX) $@
>
> @@ -65,7 +63,7 @@ $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl: $(MK_DSDT)
> mv -f $@.$(TMP_SUFFIX) $@
>
> $(C_SRC): $(ACPI_BUILD_DIR)/%.c: $(ACPI_BUILD_DIR)/%.asl
> - $(IASL) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $<
> + $(IASL) -vs -I $(CURDIR) -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $<
> sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >
> $@.$(TMP_SUFFIX)
> echo "int $*_len=sizeof($*);" >> $@.$(TMP_SUFFIX)
> mv -f $@.$(TMP_SUFFIX) $@
> diff --git a/tools/libacpi/dsdt.asl b/tools/libacpi/dsdt.asl
> index 32b42f85ae..130826fdcc 100644
> --- a/tools/libacpi/dsdt.asl
> +++ b/tools/libacpi/dsdt.asl
> @@ -5,8 +5,6 @@
> * Copyright (c) 2004, Intel Corporation.
> */
>
> -DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
> -{
> Name (\PMBS, 0x0C00)
> Name (\PMLN, 0x08)
> Name (\IOB1, 0x00)
> @@ -199,7 +197,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
>
> Device (ISA)
> {
> - Name (_ADR, 0x00010000) /* device 1, fn 0 */
> + /* Error will be raised if the machine type is not defined */
> + #ifdef MACHINE_TYPE_I440
> + Name (_ADR, 0x00010000) /* device 1, fn 0 */
> + #endif
I think we want the preprocessor directives not indented the same as
asl code, like we do in C, iow:
Device (ISA)
{
/* Error will be raised if the machine type is not defined */
#ifdef MACHINE_TYPE_I440
Name (_ADR, 0x00010000) /* device 1, fn 0 */
#endif
Same with the includes and defines below.
>
> OperationRegion(PIRQ, PCI_Config, 0x60, 0x4)
> Scope(\) {
> @@ -329,23 +330,6 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
> })
> }
>
> - Device (FDC0)
> - {
> - Name (_HID, EisaId ("PNP0700"))
> - Method (_STA, 0, NotSerialized)
> - {
> - Return (0x0F)
> - }
> -
> - Name (_CRS, ResourceTemplate ()
> - {
> - IO (Decode16, 0x03F0, 0x03F0, 0x01, 0x06)
> - IO (Decode16, 0x03F7, 0x03F7, 0x01, 0x01)
> - IRQNoFlags () {6}
> - DMA (Compatibility, NotBusMaster, Transfer8) {2}
> - })
> - }
I need to look at the Q35 changes, but ff the only differences are the
Name() method at the top and this, we might as well use an #ifdef
here, and there's no need to split into so many separate files? I
need to look at further patches.
The ACPI table generation stuff could certainly do with an overall
rework.
> Device (UAR1)
> {
> Name (_HID, EisaId ("PNP0501"))
> @@ -444,4 +428,3 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
> Method(_PIC, 1) {
> Store(Arg0, PICD)
> }
> -}
> diff --git a/tools/libacpi/dsdt_i440.asl b/tools/libacpi/dsdt_i440.asl
> new file mode 100644
> index 0000000000..e80c454ad9
> --- /dev/null
> +++ b/tools/libacpi/dsdt_i440.asl
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: LGPL-2.1-only */
> +/******************************************************************************
> + * DSDT for Xen with Qemu device model (for i440 machine)
> + *
> + * Copyright (c) 2004, Intel Corporation.
> + */
> +
> +DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
> +{
> + #define MACHINE_TYPE_I440
IMO we probably want to define this outside of the DefinitionBlock
section, just after the copyright header.
> +
> + #include "dsdt.asl"
> +
> + Scope (\_SB.PCI0.ISA)
> + {
> + Device (FDC0)
> + {
> + Name (_HID, EisaId ("PNP0700"))
> +
> + Method (_STA, 0, NotSerialized)
> + {
> + Return (0x0F)
> + }
> +
> + Name (_CRS, ResourceTemplate ()
> + {
> + IO (Decode16, 0x03F0, 0x03F0, 0x01, 0x06)
> + IO (Decode16, 0x03F7, 0x03F7, 0x01, 0x01)
> + IRQNoFlags () {6}
> + DMA (Compatibility, NotBusMaster, Transfer8) {2}
> + })
> + }
> + }
> +
> + #include "dsdt_acpi_info.asl"
> +}
> --
> 2.51.0
>
>
>
> --
> Thierry Escande | Vates XCP-ng Developer
>
> XCP-ng & Xen Orchestra - Vates solutions
>
> web: https://vates.tech
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |