Re: [PATCH v3 1/1] libacpi/Makefile: clear ASL warning about PCI0

["Ariel Otilibili-Anieli" <Ariel.Otilibili-Anieli@xxxxxxxxxx>]

On Friday, December 27, 2024 15:27 CET, Jan Beulich <jbeulich@xxxxxxxx> 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