| Message ID | 20241222161444.1558599-2-Ariel.Otilibili-Anieli@eurecom.fr (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | libacpi/Makefile: clear ASL warning about PCI0 | expand |
On 22.12.2024 17:10, Ariel Otilibili wrote: > iasl has one ID per warning [6]; subsequent commits will address other ASL warnings. > > ``` > $ awk 'NR>533 && NR<556 {print NR ":" $0}' source/compiler/aslmethod.c > 534: case PARSEOP_DEVICE: > 535: > 536: /* Check usage of _HID and _ADR objects */ > 537: > 538: HidExists = ApFindNameInDeviceTree (METHOD_NAME__HID, Op); > 539: AdrExists = ApFindNameInDeviceTree (METHOD_NAME__ADR, Op); > 540: > 541: if (!HidExists && !AdrExists) > 542: { > 543: AslError (ASL_ERROR, ASL_MSG_MISSING_DEPENDENCY, Op, > 544: "Device object requires a _HID or _ADR"); > 545: } > 546: else if (HidExists && AdrExists) > 547: { > 548: /* > 549: * According to the ACPI spec, "A device object must contain > 550: * either an _HID object or an _ADR object, but should not contain > 551: * both". > 552: */ > 553: AslError (ASL_WARNING, ASL_MSG_MULTIPLE_TYPES, Op, > 554: "Device object requires either a _HID or _ADR, but not both"); > 555: } > > $ awk 'NR>188 && NR<206 || NR==432 || /ASL_MSG_MULTIPLE_TYPES/ {print NR ":" $0}' source/compiler/aslmessages.h > 189:/* > 190: * Values (message IDs) for all compiler messages. There are currently > 191: * three distinct blocks of error messages (so that they can be expanded > 192: * individually): > 193: * Main ASL compiler > 194: * Data Table compiler > 195: * Preprocessor > 196: * > 197: * NOTE1: This list must match the tables of message strings in the file > 198: * aslmessages.c exactly. > 199: * > 200: * NOTE2: With the introduction of the -vw option to disable specific > 201: * messages, new messages should only be added to the end of these > 202: * lists, so that values for existing messages are not disturbed. > 203: */ > 204:typedef enum > 205:{ > 280: ASL_MSG_MULTIPLE_TYPES, > 432:} ASL_MESSAGE_IDS; From this I can't conclude that the same message ID (ASL_MSG_MULTIPLE_TYPES here) can't (in principle) be used in multiple places, for similar purposes. Pretty certainly we want to avoid disabling unrelated warnings elsewhere (including ones only to be surfaced by future versions of iasl). > --- a/tools/libacpi/Makefile > +++ b/tools/libacpi/Makefile > @@ -21,6 +21,8 @@ H_SRC += $(addprefix $(ACPI_BUILD_DIR)/, ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slat > MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64 > MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86 > > +IASL_WARNS=3073 If we were to go this route, the variable name better would make clear that this is a list of warnings to be disabled. And then instead of ... > @@ -32,7 +34,7 @@ TMP_SUFFIX = tmp > all: $(C_SRC) $(H_SRC) > > $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl > - $(IASL) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $< > + $(IASL) $(IASL_WARNS:%=-vw%) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $< > sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@ > rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex) > > @@ -65,7 +67,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) $(IASL_WARNS:%=-vw%) -vs -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) $@ ... continuing to need to alter two places, I guess we'd be better off introducing IASL_FLAGS or some such, where the -vs then could also go. Jan
On Friday, December 27, 2024 15:27 CET, Jan Beulich <jbeulich@suse.com> wrote: > On 22.12.2024 17:10, Ariel Otilibili wrote: > > iasl has one ID per warning [6]; subsequent commits will address other ASL warnings. > > > > ``` > > $ awk 'NR>533 && NR<556 {print NR ":" $0}' source/compiler/aslmethod.c > > 534: case PARSEOP_DEVICE: > > 535: > > 536: /* Check usage of _HID and _ADR objects */ > > 537: > > 538: HidExists = ApFindNameInDeviceTree (METHOD_NAME__HID, Op); > > 539: AdrExists = ApFindNameInDeviceTree (METHOD_NAME__ADR, Op); > > 540: > > 541: if (!HidExists && !AdrExists) > > 542: { > > 543: AslError (ASL_ERROR, ASL_MSG_MISSING_DEPENDENCY, Op, > > 544: "Device object requires a _HID or _ADR"); > > 545: } > > 546: else if (HidExists && AdrExists) > > 547: { > > 548: /* > > 549: * According to the ACPI spec, "A device object must contain > > 550: * either an _HID object or an _ADR object, but should not contain > > 551: * both". > > 552: */ > > 553: AslError (ASL_WARNING, ASL_MSG_MULTIPLE_TYPES, Op, > > 554: "Device object requires either a _HID or _ADR, but not both"); > > 555: } > > > > $ awk 'NR>188 && NR<206 || NR==432 || /ASL_MSG_MULTIPLE_TYPES/ {print NR ":" $0}' source/compiler/aslmessages.h > > 189:/* > > 190: * Values (message IDs) for all compiler messages. There are currently > > 191: * three distinct blocks of error messages (so that they can be expanded > > 192: * individually): > > 193: * Main ASL compiler > > 194: * Data Table compiler > > 195: * Preprocessor > > 196: * > > 197: * NOTE1: This list must match the tables of message strings in the file > > 198: * aslmessages.c exactly. > > 199: * > > 200: * NOTE2: With the introduction of the -vw option to disable specific > > 201: * messages, new messages should only be added to the end of these > > 202: * lists, so that values for existing messages are not disturbed. > > 203: */ > > 204:typedef enum > > 205:{ > > 280: ASL_MSG_MULTIPLE_TYPES, > > 432:} ASL_MESSAGE_IDS; > > From this I can't conclude that the same message ID (ASL_MSG_MULTIPLE_TYPES > here) can't (in principle) be used in multiple places, for similar purposes. > Pretty certainly we want to avoid disabling unrelated warnings elsewhere > (including ones only to be surfaced by future versions of iasl). > In principle, the same ID won’t be used twice. Though I will push a PR to the project sometime next year. So far ASL_MSG_MULTIPLE_TYPES has been used once, I will ask the ID be made specific about _HID and _ADR. I did open a PR that clarifies, from which has the MUST been enforced, ACPI 6.3 Errata A [1]. [1] https://github.com/acpica/acpica/pull/992 > > --- a/tools/libacpi/Makefile > > +++ b/tools/libacpi/Makefile > > @@ -21,6 +21,8 @@ H_SRC += $(addprefix $(ACPI_BUILD_DIR)/, ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slat > > MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64 > > MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86 > > > > +IASL_WARNS=3073 > > If we were to go this route, the variable name better would make clear that > this is a list of warnings to be disabled. Sure, Jan. > > And then instead of ... > > > @@ -32,7 +34,7 @@ TMP_SUFFIX = tmp > > all: $(C_SRC) $(H_SRC) > > > > $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl > > - $(IASL) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $< > > + $(IASL) $(IASL_WARNS:%=-vw%) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $< > > sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@ > > rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex) > > > > @@ -65,7 +67,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) $(IASL_WARNS:%=-vw%) -vs -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) $@ > > ... continuing to need to alter two places, I guess we'd be better off > introducing IASL_FLAGS or some such, where the -vs then could also go. > Thanks for the feedback, Jan. Sometime next year I will push a new version. My best regards to you and your dear ones. > Jan
diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile index b21a64c6b4..4668ecb365 100644 --- a/tools/libacpi/Makefile +++ b/tools/libacpi/Makefile @@ -21,6 +21,8 @@ H_SRC += $(addprefix $(ACPI_BUILD_DIR)/, ssdt_tpm.h ssdt_tpm2.h ssdt_laptop_slat MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64 MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86 +IASL_WARNS=3073 + # Suffix for temporary files. # # We will also use this suffix to workaround a bug in older iasl @@ -32,7 +34,7 @@ TMP_SUFFIX = tmp all: $(C_SRC) $(H_SRC) $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl - $(IASL) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $< + $(IASL) $(IASL_WARNS:%=-vw%) -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $< sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@ rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex) @@ -65,7 +67,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) $(IASL_WARNS:%=-vw%) -vs -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) $@
iasl complains _HID and _ADR cannot be used at the same time: ``` iasl -vs -p tools/firmware/hvmloader/dsdt_anycpu.tmp -tc tools/firmware/hvmloader/dsdt_anycpu.asl 2>&1 | grep -B1 HID tools/firmware/hvmloader/dsdt_anycpu.asl 40: Device (PCI0) Warning 3073 - Multiple types ^ (Device object requires either a _HID or _ADR, but not both) ``` The usage of both _HID and _ADR has changed [1,2]: From ACPI 2.0 (Jul. 27, 2000; Section 6.1, page 146): "A device object must contain either an _HID object or an _ADR object, but can contain both." [3] To ACPI 6.0 (April 2015; Section 6.1, page 278), "A device object must contain either an _HID object or an _ADR object, but should not contain both." [4] And from ACPI 6.0 to ACPI 6.5 (Aug. 2022): "A device object must contain either an _HID object or an _ADR object, but must not contain both." [5] Using its ID, the warning is now filtered. ``` $ iasl -vw3073 -vs -p ../firmware/hvmloader/dsdt_anycpu.tmp -tc ../firmware/hvmloader/dsdt_anycpu.asl 2>&1 | grep HID; echo $? 1 ``` iasl has one ID per warning [6]; subsequent commits will address other ASL warnings. ``` $ awk 'NR>533 && NR<556 {print NR ":" $0}' source/compiler/aslmethod.c 534: case PARSEOP_DEVICE: 535: 536: /* Check usage of _HID and _ADR objects */ 537: 538: HidExists = ApFindNameInDeviceTree (METHOD_NAME__HID, Op); 539: AdrExists = ApFindNameInDeviceTree (METHOD_NAME__ADR, Op); 540: 541: if (!HidExists && !AdrExists) 542: { 543: AslError (ASL_ERROR, ASL_MSG_MISSING_DEPENDENCY, Op, 544: "Device object requires a _HID or _ADR"); 545: } 546: else if (HidExists && AdrExists) 547: { 548: /* 549: * According to the ACPI spec, "A device object must contain 550: * either an _HID object or an _ADR object, but should not contain 551: * both". 552: */ 553: AslError (ASL_WARNING, ASL_MSG_MULTIPLE_TYPES, Op, 554: "Device object requires either a _HID or _ADR, but not both"); 555: } $ awk 'NR>188 && NR<206 || NR==432 || /ASL_MSG_MULTIPLE_TYPES/ {print NR ":" $0}' source/compiler/aslmessages.h 189:/* 190: * Values (message IDs) for all compiler messages. There are currently 191: * three distinct blocks of error messages (so that they can be expanded 192: * individually): 193: * Main ASL compiler 194: * Data Table compiler 195: * Preprocessor 196: * 197: * NOTE1: This list must match the tables of message strings in the file 198: * aslmessages.c exactly. 199: * 200: * NOTE2: With the introduction of the -vw option to disable specific 201: * messages, new messages should only be added to the end of these 202: * lists, so that values for existing messages are not disturbed. 203: */ 204:typedef enum 205:{ 280: ASL_MSG_MULTIPLE_TYPES, 432:} ASL_MESSAGE_IDS; $ git remote -v origin git@github.com:acpica/acpica.git (fetch) origin git@github.com:acpica/acpica.git (push) $ git log --pretty='%h ("%s")' -n1 7dae72155 ("Logfile: Changes for version 20241212") ``` [1] https://uefi.org/acpi/specs [2] https://uefi.org/specifications [3] https://uefi.org/sites/default/files/resources/ACPI_2.pdf [4] https://uefi.org/sites/default/files/resources/ACPI_6.0.pdf [5] https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html?highlight=_hid#device-identification-objects [6] https://github.com/acpica/acpica Fixes: 5a8b28bfd4 ("tools/libacpi: cleanup Makefile, don't check for iasl binary") Signed-off-by: Ariel Otilibili <Ariel.Otilibili-Anieli@eurecom.fr> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Anthony PERARD <anthony.perard@vates.tech> --- tools/libacpi/Makefile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)