|
[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 4/28/26 11:06, Roger Pau Monné wrote:
> 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.
Sure.
>
>>
>> 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.
Will do.
>
>>
>> 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.
Since the changes are not that big I can indeed use #ifdef within
dstd.asl directly and avoid the split. There would be specific files
declaring the machine type and I can keep the current concatenation
mecanism.
>
> 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.
Yes, I'll define that in dsdt_i440.asl and dsdt_q35.asl along with the
isa device sbdf.
Regards,
--
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 |