lkml.org 
[lkml]   [2022]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 09/15] cxl/acpi: Determine PCI host bridge's ACPI UID
    On 31.08.22 12:00:27, Jonathan Cameron wrote:
    > On Wed, 31 Aug 2022 10:15:57 +0200
    > Robert Richter <rrichter@amd.com> wrote:
    >
    > > The UID is needed to read the RCH's CEDT entry with the RCRB base
    > > address. Determine the host's UID from its ACPI fw node.
    > >
    > > Signed-off-by: Robert Richter <rrichter@amd.com>
    > > ---
    > > drivers/cxl/acpi.c | 12 ++++++++++++
    > > 1 file changed, 12 insertions(+)
    > >
    > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
    > > index f9cdf23a91a8..b3146b7ae922 100644
    > > --- a/drivers/cxl/acpi.c
    > > +++ b/drivers/cxl/acpi.c
    > > @@ -368,8 +368,20 @@ struct pci_host_bridge *cxl_find_next_rch(struct pci_host_bridge *host)
    > > static int __init cxl_restricted_host_probe(struct platform_device *pdev)
    > > {
    > > struct pci_host_bridge *host = NULL;
    > > + struct acpi_device *adev;
    > > + unsigned long long uid = ~0;
    > >
    > > while ((host = cxl_find_next_rch(host)) != NULL) {
    > > + adev = ACPI_COMPANION(&host->dev);
    > > + if (!adev || !adev->pnp.unique_id ||
    > > + (kstrtoull(adev->pnp.unique_id, 10, &uid) < 0))
    >
    > There is an acpi_device_uid() accessor function that should probably be
    > used here.

    That accessor actually does not help really, there is no null pointer
    check for adev. Using it actually adds more complexity since another
    variable is introduced plus you need to look at the function's
    implementation anyway.

    The adev->pnp.unique_id access pattern is widely used in the kernel, I
    don't expect changes in the data struct here.

    > Also, should a fialure to convert to an integer (or one within
    > limits) be something we paper over? Feels like we should fail
    > hard if that happens.

    This is a real corner case and close to a broken firmware
    implementation. I think current dbg messages are good to find where
    the detection stops.

    > Admittedly I can't immediately find any spec that states that
    > the _UID should be either integer or under 32 bits...
    > ACPI allows a string and CXL just says it's 4 bytes long.

    IIRC the UID can be implemented as string or 8 bytes, there is no
    limitation then. That's why the range check below.

    -Robert

    >
    > > + continue;
    > > +
    > > + dev_dbg(&adev->dev, "host uid: %llu\n", uid);
    > > +
    > > + if (uid > U32_MAX)
    > > + continue;
    > > +
    > > dev_info(&host->dev, "host supports CXL\n");
    > > }
    > >
    >

    \
     
     \ /
      Last update: 2022-09-01 08:54    [W:2.529 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site