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: <20240305203231.GA538524@bhelgaas>
Date: Tue, 5 Mar 2024 14:32:31 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Alexey Kardashevskiy <aik@....com>
Cc: Lukas Wunner <lukas@...ner.de>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH kernel v2] pci/doe: Support discovery version

On Tue, Mar 05, 2024 at 05:02:27PM +1100, Alexey Kardashevskiy wrote:
> On 28/2/24 07:41, Lukas Wunner wrote:
> > On Mon, Feb 26, 2024 at 02:31:14PM +1100, Alexey Kardashevskiy wrote:
> > > Does PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER need to be in pci-regs.h?
> > 
> > Yes that's fine.
> > 
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -1144,6 +1144,7 @@
> > >   #define PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH		0x0003ffff
> > >   #define PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX		0x000000ff
> > > +#define PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER	0x0000ff00
> > >   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID		0x0000ffff
> > >   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL		0x00ff0000
> > >   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX	0xff000000
> > 
> > "DISCOVER" duplicates the preceding "DISC", maybe just
> > "PCI_DOE_DATA_OBJECT_DISC_REQ_3_VERSION" for simplicity?
> 
> Well, mostly because the PCIe spec specifically says "discovery" in the
> field description, not just "version", but ok, I'll drop it.
> 
> btw "DISC" is just confusing, it has nothing to do with discs. _PROTOCOL is
> not even correct anymore, now, in PCIe r6.1 it is called "type", lovely :)
> s/PCI_DOE_DATA_OBJECT_DISC_/PCI_DOE_DISCOVERY_/ (because DO==DATA_OBJECT)
> imho would do better but may be some other day.

Agreed, and there are only a couple uses so not hard to change,
although it is already in uapi/.  Maybe nobody really uses it yet
though?

> Less ugly since we want to keep it 80 chars long (do we, still?). Like this
> looks meh:
> 
>         u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
>                                     *index) |
> FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_DISCOVER_VER,
>                                     (capver >= 2) ? 2 : 0);
> 
>         __le32 request_pl_le = cpu_to_le32(request_pl);
> 
> If we did 100 chars, I could do:
> 
>         u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
> *index) |
>                          FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_VER,
> (capver >= 2) ? 2 : 0);
>         __le32 request_pl_le = cpu_to_le32(request_pl);

Personally I prefer 80 columns because all the rest of drivers/pci is
that, and it's a hassle when browsing to have to resize the window to
either accommodate wider lines or avoid wasting screen space when all
the lines are shorter.

But there are exceptions.  Strings don't need to be broken because
grepping for error messages works better when they're continued.  This
situation might be an exception, too.  We don't need to slavishly
adhere to 80 if the result looks terrible.

Bjorn

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ