| Message ID | 20181107041305.26905-1-alex.hung@canonical.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | lib: add PL011 to _CID whitelist | expand |
On 07/11/2018 04:13, Alex Hung wrote: > When attending UEFI Plugfest, a vendor suggested _CID PL011 is used in > the field, and this ID is added to a whitelist to avoid false positive. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/lib/src/fwts_acpi_object_eval.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c > index 0bc0f134..95f7df76 100644 > --- a/src/lib/src/fwts_acpi_object_eval.c > +++ b/src/lib/src/fwts_acpi_object_eval.c > @@ -968,6 +968,10 @@ bool fwts_method_valid_HID_string(char *str) > return true; > } > > + /* PL011 is used out there, so whitelist it */ > + if (strlen(str) == 5 && !strncmp(str, "PL011", 5)) > + return true; > + > return false; > } > > Acked-by: Colin Ian King <colin.king@canonical.com>
I was in UEFI plugfest two, at first I think we can whitelist this ID, but
ACPI specification do define the type of the IDs,
A valid PNP ID must be of the form "AAA####" where A is an uppercase
letter and # is a hex
digit. A valid ACPI ID must be of the form "NNNN####" where N is an
uppercase letter or a
digit ('0'-'9') and # is a hex digit.
"PL011" indeed not follow the ACPI specification.
They know it is not following the ACPI specification, they cannot change
it because the legacy machine use it.
I think it maybe good idea let FWTS to spot all failures which not
following the ACPI specification, even if it is in used.
Any thoughts?
Cheers,
Ivan
On 11/7/18 12:13 PM, Alex Hung wrote:
> When attending UEFI Plugfest, a vendor suggested _CID PL011 is used in
> the field, and this ID is added to a whitelist to avoid false positive.
>
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
> src/lib/src/fwts_acpi_object_eval.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
> index 0bc0f134..95f7df76 100644
> --- a/src/lib/src/fwts_acpi_object_eval.c
> +++ b/src/lib/src/fwts_acpi_object_eval.c
> @@ -968,6 +968,10 @@ bool fwts_method_valid_HID_string(char *str)
> return true;
> }
>
> + /* PL011 is used out there, so whitelist it */
> + if (strlen(str) == 5 && !strncmp(str, "PL011", 5))
> + return true;
> +
> return false;
> }
>
On Wed, Nov 7, 2018 at 4:52 PM ivanhu <ivan.hu@canonical.com> wrote: > > I was in UEFI plugfest two, at first I think we can whitelist this ID, but > > ACPI specification do define the type of the IDs, > > A valid PNP ID must be of the form "AAA####" where A is an uppercase > letter and # is a hex > digit. A valid ACPI ID must be of the form "NNNN####" where N is an > uppercase letter or a > digit ('0'-'9') and # is a hex digit. > > "PL011" indeed not follow the ACPI specification. > > They know it is not following the ACPI specification, they cannot change > it because the legacy machine use it. > > I think it maybe good idea let FWTS to spot all failures which not > following the ACPI specification, even if it is in used. > > Any thoughts? It is unlikely to undo the used CID. The failures will always be there, even if the CID is supported by firmware and OS. This failure will make little sense as it may never get fixed. > > > Cheers, > > Ivan > > On 11/7/18 12:13 PM, Alex Hung wrote: > > When attending UEFI Plugfest, a vendor suggested _CID PL011 is used in > > the field, and this ID is added to a whitelist to avoid false positive. > > > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > > --- > > src/lib/src/fwts_acpi_object_eval.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c > > index 0bc0f134..95f7df76 100644 > > --- a/src/lib/src/fwts_acpi_object_eval.c > > +++ b/src/lib/src/fwts_acpi_object_eval.c > > @@ -968,6 +968,10 @@ bool fwts_method_valid_HID_string(char *str) > > return true; > > } > > > > + /* PL011 is used out there, so whitelist it */ > > + if (strlen(str) == 5 && !strncmp(str, "PL011", 5)) > > + return true; > > + > > return false; > > } > > > > -- > fwts-devel mailing list > fwts-devel@lists.ubuntu.com > Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
On 11/7/18 1:51 AM, ivanhu wrote: > I was in UEFI plugfest two, at first I think we can whitelist this ID, but > > ACPI specification do define the type of the IDs, > > A valid PNP ID must be of the form "AAA####" where A is an uppercase > letter and # is a hex > digit. A valid ACPI ID must be of the form "NNNN####" where N is an > uppercase letter or a > digit ('0'-'9') and # is a hex digit. > > "PL011" indeed not follow the ACPI specification. > > They know it is not following the ACPI specification, they cannot change > it because the legacy machine use it. > > I think it maybe good idea let FWTS to spot all failures which not > following the ACPI specification, even if it is in used. > > Any thoughts? Section 6.1.2 of the spec is pretty clear on what is allowed for _CID; 'PL011' is not allowed. It is not in the PCI or ACPI ID registries, either. I have seen this used in some very, very old firmware, and the vendor that made the firmware was forced to correct it as soon as I saw it. So, that firmware has since been corrected, and there is no excuse for it to still be in use, even on a "legacy system." The value should be 'ARMH0011', so this is broken firmware, and if the kernel is accepting 'PL011', that's wrong, too. Vendors should not be using this at all. The test should fail. > Cheers, > > Ivan > > On 11/7/18 12:13 PM, Alex Hung wrote: >> When attending UEFI Plugfest, a vendor suggested _CID PL011 is used in >> the field, and this ID is added to a whitelist to avoid false positive. >> >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> src/lib/src/fwts_acpi_object_eval.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c >> index 0bc0f134..95f7df76 100644 >> --- a/src/lib/src/fwts_acpi_object_eval.c >> +++ b/src/lib/src/fwts_acpi_object_eval.c >> @@ -968,6 +968,10 @@ bool fwts_method_valid_HID_string(char *str) >> return true; >> } >> >> + /* PL011 is used out there, so whitelist it */ >> + if (strlen(str) == 5 && !strncmp(str, "PL011", 5)) >> + return true; >> + >> return false; >> } >> > >
On Thu, Nov 8, 2018 at 8:18 AM Al Stone <al.stone@linaro.org> wrote: > > On 11/7/18 1:51 AM, ivanhu wrote: > > I was in UEFI plugfest two, at first I think we can whitelist this ID, but > > > > ACPI specification do define the type of the IDs, > > > > A valid PNP ID must be of the form "AAA####" where A is an uppercase > > letter and # is a hex > > digit. A valid ACPI ID must be of the form "NNNN####" where N is an > > uppercase letter or a > > digit ('0'-'9') and # is a hex digit. > > > > "PL011" indeed not follow the ACPI specification. > > > > They know it is not following the ACPI specification, they cannot change > > it because the legacy machine use it. > > > > I think it maybe good idea let FWTS to spot all failures which not > > following the ACPI specification, even if it is in used. > > > > Any thoughts? > > Section 6.1.2 of the spec is pretty clear on what is allowed for _CID; 'PL011' > is not allowed. It is not in the PCI or ACPI ID registries, either. > > I have seen this used in some very, very old firmware, and the vendor that made > the firmware was forced to correct it as soon as I saw it. So, that firmware > has since been corrected, and there is no excuse for it to still be in use, even > on a "legacy system." > > The value should be 'ARMH0011', so this is broken firmware, and if the kernel is > accepting 'PL011', that's wrong, too. > > Vendors should not be using this at all. The test should fail. Thanks Al for the clarification. Now I know what to ask vendors to change to. > > > Cheers, > > > > Ivan > > > > On 11/7/18 12:13 PM, Alex Hung wrote: > >> When attending UEFI Plugfest, a vendor suggested _CID PL011 is used in > >> the field, and this ID is added to a whitelist to avoid false positive. > >> > >> Signed-off-by: Alex Hung <alex.hung@canonical.com> > >> --- > >> src/lib/src/fwts_acpi_object_eval.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c > >> index 0bc0f134..95f7df76 100644 > >> --- a/src/lib/src/fwts_acpi_object_eval.c > >> +++ b/src/lib/src/fwts_acpi_object_eval.c > >> @@ -968,6 +968,10 @@ bool fwts_method_valid_HID_string(char *str) > >> return true; > >> } > >> > >> + /* PL011 is used out there, so whitelist it */ > >> + if (strlen(str) == 5 && !strncmp(str, "PL011", 5)) > >> + return true; > >> + > >> return false; > >> } > >> > > > > > > > -- > ciao, > al > ----------------------------------- > Al Stone > Software Engineer > Linaro Enterprise Group > al.stone@linaro.org > ----------------------------------- > > -- > fwts-devel mailing list > fwts-devel@lists.ubuntu.com > Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c index 0bc0f134..95f7df76 100644 --- a/src/lib/src/fwts_acpi_object_eval.c +++ b/src/lib/src/fwts_acpi_object_eval.c @@ -968,6 +968,10 @@ bool fwts_method_valid_HID_string(char *str) return true; } + /* PL011 is used out there, so whitelist it */ + if (strlen(str) == 5 && !strncmp(str, "PL011", 5)) + return true; + return false; }
When attending UEFI Plugfest, a vendor suggested _CID PL011 is used in the field, and this ID is added to a whitelist to avoid false positive. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/lib/src/fwts_acpi_object_eval.c | 4 ++++ 1 file changed, 4 insertions(+)