linux-kernel - Re: [RFC PATCH v3 02/16] cxl/acpi: Add an acpi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dfc13eb2a2a9a3297100bbda34c00f28554a83e2.camel@intel.com>
Date:   Wed, 20 Jan 2021 19:21:08 +0000
From:   "Verma, Vishal L" <vishal.l.verma@...el.com>
To:     "Widawsky, Ben" <ben.widawsky@...el.com>,
        "rafael@...nel.org" <rafael@...nel.org>
CC:     "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "hch@...radead.org" <hch@...radead.org>,
        "Kelley, Sean V" <sean.v.kelley@...el.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "jcm@...masters.org" <jcm@...masters.org>,
        "linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
        "Jonathan.Cameron@...wei.com" <Jonathan.Cameron@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "helgaas@...nel.org" <helgaas@...nel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "daniel.lll@...baba-inc.com" <daniel.lll@...baba-inc.com>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "cbrowy@...ry-design.com" <cbrowy@...ry-design.com>,
        "Weiny, Ira" <ira.weiny@...el.com>
Subject: Re: [RFC PATCH v3 02/16] cxl/acpi: Add an acpi_cxl module for the CXL
 interconnect

On Wed, 2021-01-13 at 13:40 +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 12, 2021 at 1:29 AM Ben Widawsky <ben.widawsky@...el.com> wrote:
> > 
> > From: Vishal Verma <vishal.l.verma@...el.com>
> > 
> > Add an acpi_cxl module to coordinate the ACPI portions of the CXL
> > (Compute eXpress Link) interconnect. This driver binds to ACPI0017
> > objects in the ACPI tree, and coordinates access to the resources
> > provided by the ACPI CEDT (CXL Early Discovery Table).
> > 
> > It also coordinates operations of the root port _OSC object to notify
> > platform firmware that the OS has native support for the CXL
> > capabilities of endpoints.
> 
> This doesn't happen here, but in the next patch.
> 
> > Note: the actbl1.h changes are speculative. The expectation is that they
> > will arrive through the ACPICA tree in due time.
> 
> So why don't you put them into a separate patch and drop it from the
> series when not necessary any more?

[snip]

> > +/*
> > + * If/when CXL support is defined by other platform firmware the kernel
> > + * will need a mechanism to select between the platform specific version
> > + * of this routine, until then, hard-code ACPI assumptions
> > + */
> > +int cxl_bus_acquire(struct pci_dev *pdev)
> > +{
> > +       struct acpi_device *adev;
> > +       struct pci_dev *root_port;
> > +       struct device *root;
> > +
> > +       root_port = pcie_find_root_port(pdev);
> > +       if (!root_port)
> > +               return -ENXIO;
> > +
> > +       root = root_port->dev.parent;
> > +       if (!root)
> > +               return -ENXIO;
> > +
> > +       adev = ACPI_COMPANION(root);
> > +       if (!adev)
> > +               return -ENXIO;
> > +
> > +       /* TODO: OSC enabling */
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(cxl_bus_acquire);
> 
> I would move the addition of cxl_bus_acquire() entirely to the next
> patch, it looks quite confusing to me as is.

Makes sense - and also agreed with all of your other comments. I've
cleaned this up for the next revision. Thanks Rafael!
>