[PATCH 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Rafael J. Wysocki]

On Tue, Apr 8, 2025 at 10:48 PM Bjorn Helgaas <helgaas at kernel.org> wrote:
>
> On Wed, Apr 02, 2025 at 09:36:01PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Apr 2, 2025 at 8:48 PM Bjorn Helgaas <helgaas at kernel.org> wrote:
>
> > > I don't *expect* rev 5 to be different.  But as a user of it, why
> > > would I change working, tested code that is not broken?
> > >
> > > The PCI DPC function 0xc is an example where rev 5 (per ECN) and rev 6
> > > (per r3.3) are not compatible.
> > >
> > > If the OS implemented rev 5, then learns via function 0 that function
> > > 0xc is also supported at rev 6, and starts using the same OS code with
> > > rev 6, the OS is broken.
> >
> > Yes, in this case the backward compatibility language in the _DSM
> > definition is obviously not followed.
>
> Rev 5 in the ECN isn't compatible with rev 6 in the PCI FW r3.3 spec,
> so it doesn't follow the ACPI compatibility requirement.  And this is
> documented in PCI FW, which says "Fn 0xC was added with rev 5 (see ECN
> for rev 5 details); here is how rev 6 works."
>
> An OS implemented to the ECN doesn't know that rev 6 is different from
> rev 5; it assumes they're the same because ACPI says we can assume
> that, and PCI FW r3.3 even says the OS should use the same rev for all
> functions.

OK (and this is important because PCI FW r3.3 is the spec defining the
interface)

> If OS adds support for rev 6 of a some other function, it is supposed to use
> rev 6 of Fn 0xC, which doesn't work as the OS expects.

IMV with respect to _DSM, the spec that has defined the interface (PCI
FW r3.3) takes precedence over the ACPI spec regardless of what the
latter is saying.  In this case ACPI provides a framework the
interface can be based on, but the actual interface is not defined by
it.

Also, I think that the OS should use rev 6 if it is supported by the
firmware (for all functions) and it should literally follow the
definition of rev 6.

If rev 6 is not supported (or some functions needed by the OS are not
implemented by the firmware), it should fall back to rev 5 and in this
case it should literally follow the definition of rev 5.

And so on.

> I guess one could argue that "OS didn't add rev 6 support for anything
> until PCI FW r3.3 added a function at rev 6, r3.3 did mention the
> difference between Fn 0xC rev 5 and 6, and OS should have looked at
> all the already-implemented unrelated functions for possible changes
> between rev 5 and rev 6."

Yes, it should.

> I think that causes unnecessary changes in unrelated code.  The OS can
> avoid the problem by *always* using Fn 0xC rev 5, regardless of what
> rev it uses for other functions.

What if the functions on the firmware side depend on each other
interfally and the firmware gets confused if revisions are mixed up on
the OS side?

Since PCI FW r3.3 says that the OS should use the same rev for all
functions, that's what it should do because the firmware may
reasonably assume that it will.

> Of course, the OS can include support for multiple revs, e.g., it
> could add support for Fn 0xC rev 6 and use that when available.  And
> it could retain support for Fn 0xC rev 5 and use that if rev 6 isn't
> available.

That's what I mean basically.

In practice, the OS needs to assume that the firmware may not
implement a certain rev and so it needs to support other revisions
anyway.