Message-ID: <db551fce-632e-c84f-3a3e-2733af9dcd46@amazon.com>
Date:   Mon, 28 Feb 2022 23:00:55 +0100
From:   Alexander Graf <graf@...zon.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Ard Biesheuvel <ardb@...nel.org>
CC:     "Jason A. Donenfeld" <Jason@...c4.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-crypto <linux-crypto@...r.kernel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        "Len Brown" <lenb@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: Re: [PATCH 2/3 v6] ACPI: allow longer device IDs

Hi Andy,

On 28.02.22 22:27, Andy Shevchenko wrote:
> On Mon, Feb 28, 2022 at 10:02:43PM +0100, Ard Biesheuvel wrote:
>> On Mon, 28 Feb 2022 at 21:47, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>>> On Mon, Feb 28, 2022 at 9:28 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
>>>> From: Alexander Graf <graf@...zon.com>
>>>>
>>>> We create a list of ACPI "PNP" IDs which contains _HID, _CID, and CLS
>>>> entries of the respective devices. However, when making structs for
>>>> matching, we squeeze those IDs into acpi_device_id, which only has 9
>>>> bytes space to store the identifier. The subsystem actually captures the
>>>> full length of the IDs, and the modalias has the full length, but this
>>>> struct we use for matching is limited. It originally had 16 bytes, but
>>>> was changed to only have 9 in 6543becf26ff ("mod/file2alias: make
>>>> modalias generation safe for cross compiling"), presumably on the theory
>>>> that it would match the ACPI spec so it didn't matter.
>>>> Unfortunately, while most people adhere to the ACPI specs, Microsoft
>>>> decided that its VM Generation Counter device [1] should only be
>>>> identifiable by _CID with a value of "VM_Gen_Counter", which is longer
>>>> than 9 characters.
>>> Then why do we not see the ECR from somebody to update the spec or to
>>> fix MS' abuse of it?
>>> I believe _this_ should be the prerequisite to the proposed change.
>> What exactly are you suggesting here? That the contributor of this
>> patch joins the UEFI forum as an individual adopter in order to get
>> the ACPI spec updated before we can advance with this patch? Or that
>> he works with Microsoft to get them to refrain from violating it?
>>
>> I don't think that is reasonable or realistic. The kernel is already
>> riddled with UEFI and ACPI quirks that are only there because some
>> teams at MS don't take the ACPI spec too literally (which is why they
>> have their own AML compiler, for one), and PC vendors only care about
>> the Windows sticker, so they don't care about the ACPI spec either.
>>
>> So I don't think this is the right time to get pedantic about this.
>> Our ACPI subsystem already deals with CIDs that are longer than 8
>> characters (which are btw permitted by the ACPI spec for bus topology
>> related metadata), the only thing being changed here is the ability to
>> actually match against such identifiers.
> My point is that this is clear abuse of the spec and:
> 1) we have to enable the broken, because it is already in the wild with
>     the comment that this is an issue
>
> AND
>
> 2) issue an ECR / work with MS to make sure they understand the problem.
>
> This can be done in parallel. What I meant as a prerequisite is to start doing
> 2) while we have 1) on table.


While trying to revalidate whether this really is breaking the spec, 
I've tried to reread the respective section in it and I'm afraid that it 
may be valid use of the _CID identifier:


"""

6.1.2 _CID (Compatible ID)

This optional object is used to supply OSPM with a device’s Plug and 
Play-Compatible Device ID. Use _CID objects when a device has no other 
defined hardware standard method to report its compatible IDs. The _CID 
object is valid only within a Full Device Descriptor. An _HID object 
must also be present.

Arguments:

None

Return Value:
An Integer or String containing a single CID or a Package containing a 
list of CIDs A _CID object evaluates to either:

  *

    A single Compatible Device ID

  *

    A package of Compatible Device IDs for the device – in the order of
    preference, highest preference first.

Each Compatible Device ID must be either:

  *

    A valid HID value (a 32-bit compressed EISA type ID or a string such
    as “ACPI0004”).

  *

    A string that uses a bus-specific nomenclature. For example, _CID
    can be used to specify the PCI ID. The format of a PCI ID string is
    one of the following:

"PCI\CC_ccss" "PCI\CC_ccsspp" 
"PCI\VEN_vvvv&DEV_dddd&SUBSYS_ssssssss&REV_rr" 
"PCI\VEN_vvvv&DEV_dddd&SUBSYS_ssssssss" "PCI\VEN_vvvv&DEV_dddd&REV_rr" 
"PCI\VEN_vvvv&DEV_dddd"

"""

In this case, you could interpret things as looking at "bus-specific 
nomenclature" case which even in the examples mentioned in the spec 
exceeds the 8 character limit we impose on the matching logic today.

There still is spec violation in Hyper-V's VMGenID device's _HID value 
which doesn't follow the PNP format, but that's not relevant here. _CID 
doesn't seem to have the same restrictions?


Alex





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Powered by blists - more mailing lists