linux-kernel - Re: [PATCH V14 3/7] PCI/DOE: Add DOE mailbox support functions

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] [day] [month] [year] [list]

Message-ID: <20220720122423.00004ea6@Huawei.com>
Date:   Wed, 20 Jul 2022 12:24:23 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Ira Weiny <ira.weiny@...el.com>
CC:     Dan Williams <dan.j.williams@...el.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        "Li, Ming" <ming4.li@...el.com>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Lukas Wunner <lukas@...ner.de>,
        Alison Schofield <alison.schofield@...el.com>,
        "Vishal Verma" <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        "Ben Widawsky" <bwidawsk@...nel.org>,
        <linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>,
        <linux-pci@...r.kernel.org>
Subject: Re: [PATCH V14 3/7] PCI/DOE: Add DOE mailbox support functions

On Tue, 19 Jul 2022 12:16:06 -0700
Ira Weiny <ira.weiny@...el.com> wrote:

> On Tue, Jul 19, 2022 at 05:35:53PM +0100, Jonathan Cameron wrote:
> > On Thu, 14 Jul 2022 20:04:20 -0700
> > ira.weiny@...el.com wrote:
> >   
> > > From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > > 
> > > Introduced in a PCIe r6.0, sec 6.30, DOE provides a config space based
> > > mailbox with standard protocol discovery.  Each mailbox is accessed
> > > through a DOE Extended Capability.
> > > 
> > > Each DOE mailbox must support the DOE discovery protocol in addition to
> > > any number of additional protocols.
> > > 
> > > Define core PCIe functionality to manage a single PCIe DOE mailbox at a
> > > defined config space offset.  Functionality includes iterating,
> > > creating, query of supported protocol, and task submission.  Destruction
> > > of the mailboxes is device managed.
> > > 
> > > Cc: "Li, Ming" <ming4.li@...el.com>
> > > Cc: Bjorn Helgaas <helgaas@...nel.org>
> > > Cc: Matthew Wilcox <willy@...radead.org>
> > > Acked-by: Bjorn Helgaas <helgaas@...nel.org>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > > Co-developed-by: Ira Weiny <ira.weiny@...el.com>
> > > Signed-off-by: Ira Weiny <ira.weiny@...el.com>  
> > Hi Ira,
> > 
> > Thanks for persisting with this!
> > 
> > So, I think this works, but there is at least one 'sleep' I can't
> > see a purpose for.  I think it's just a left over from refactoring.
> > 
> > A few other more trivial things inline.
> > 
> > Thanks,
> > 
> > Jonathan
> > 
> >   
> > >   
> > >>  # Endpoint library must be initialized before its users  
> > >  obj-$(CONFIG_PCI_ENDPOINT)	+= endpoint/
> > > diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> > > new file mode 100644
> > > index 000000000000..12c3762be22f
> > > --- /dev/null
> > > +++ b/drivers/pci/doe.c
> > > @@ -0,0 +1,546 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Data Object Exchange
> > > + *	PCIe r6.0, sec 6.30 DOE
> > > + *
> > > + * Copyright (C) 2021 Huawei
> > > + *	Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > > + *
> > > + * Copyright (C) 2022 Intel Corporation
> > > + *	Ira Weiny <ira.weiny@...el.com>
> > > + */
> > > +
> > > +#define dev_fmt(fmt) "DOE: " fmt
> > > +
> > > +#include <linux/bitfield.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/jiffies.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/pci-doe.h>
> > > +#include <linux/workqueue.h>
> > > +
> > > +#define PCI_DOE_PROTOCOL_DISCOVERY 0
> > > +
> > > +#define PCI_DOE_BUSY_MAX_RETRIES 16  
> > Left over from removed code.  
> 
> I think Dan may have taken these.  If so I'll send a clean up.  If not I can
> spin.  Let me check.
> 
Absolutely. All tiny improvements, so fine to go in next cycle given late timing.

> > > +/**
> > > + * pci_doe_supports_prot() - Return if the DOE instance supports the given
> > > + *			     protocol
> > > + * @doe_mb: DOE mailbox capability to query
> > > + * @vid: Protocol Vendor ID
> > > + * @type: Protocol type
> > > + *
> > > + * RETURNS: True if the DOE mailbox supports the protocol specified
> > > + */
> > > +bool pci_doe_supports_prot(struct pci_doe_mb *doe_mb, u16 vid, u8 type)
> > > +{
> > > +	unsigned long index;
> > > +	void *entry;
> > > +
> > > +	/* The discovery protocol must always be supported */
> > > +	if (vid == PCI_VENDOR_ID_PCI_SIG && type == PCI_DOE_PROTOCOL_DISCOVERY)
> > > +		return true;  
> > 
> > Given how cheap this look up is now it's all in xarray, we could drop this
> > 'optimization'.  I'm fairly sure the discovery protocol will always be
> > discovered (spec says it must be returned when calling itself as the fist
> > protocol).  
> 
> No we can't because this is called before the xarray is populated with the
> discovery protocol.  This was actually added not as an optimization but to
> allow the discovery protocol to run through the common query path.
> 

Ah.  I was too lazy to check if that was the case :)

Jonathan