linux-kernel - Re: [PATCH v13 3/4] PCI/DOE: Expose the DOE features via sysfs

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: <CAKmqyKOa2nf1yRuZ_zpkH422JRkoHi2cC0Yq8RnNap6Meu80Uw@mail.gmail.com>
Date: Tue, 9 Jul 2024 10:50:24 +1000
From: Alistair Francis <alistair23@...il.com>
To: Krzysztof Wilczyński <kw@...ux.com>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>, bhelgaas@...gle.com, 
	linux-pci@...r.kernel.org, lukas@...ner.de, alex.williamson@...hat.com, 
	christian.koenig@....com, kch@...dia.com, gregkh@...uxfoundation.org, 
	logang@...tatee.com, linux-kernel@...r.kernel.org, chaitanyak@...dia.com, 
	rdunlap@...radead.org, Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v13 3/4] PCI/DOE: Expose the DOE features via sysfs

On Mon, Jul 8, 2024 at 10:55 AM Krzysztof Wilczyński <kw@...ux.com> wrote:
>
> Hello,
>
> > > Any input from a PCI maintainer here?
>
> Something that I am curious about: can we make this a single file with a
> bitmask inside that denotes what DOE features are enabled?  Would this be
> approach be even feasible here?

In theory there can be any vendor ID (16-bits but not 0xFFFF) and any
feature (8-bits). So there is a huge possibility of values here.

>
> Thoughts?  Or is it too late to think about this now?

It's just too many possible options to use a bitmask. I guess we could
use a feature bit mask per vendor if people feel strongly

>
> > > There are basically two approaches.
> > >
> > >  1. We can have a pci_doe_sysfs_init() function that is called where
> > > we dynamically add the entries, like in v12
> > >  2. We can go down the dev->groups and device_add() path, like this
> > > patch and discussed at
> > > https://lore.kernel.org/all/20231019165829.GA1381099@bhelgaas/
> > >
> > > For the second we will have to create a global pci_doe_sysfs_group
> > > that contains all possible DOE entries on the system and then have the
> > > show functions determine if they should be displayed for that device.
> > >
> > > Everytime we call pci_doe_init() we can check for any missing entries
> > > in pci_doe_sysfs_group.attrs and then realloc
> > > pci_doe_sysfs_group.attrs to add them.
> > > Untested, but that should work
> > > even for hot-plugged devices. pci_doe_sysfs_group.attrs would just
> > > grow forever though as I don't think we have an easy way to deallocate
> > > anything as we aren't sure if we are the only entry.
> >
> > I think this needs to be per device, not global and you'll have to manually
> > do the group visibility magic rather than using the macros.
>
> Lukas proposes a very interesting feature of kernfs recently per:
>
>   https://lore.kernel.org/linux-pci/16490618cbde91b5aac04873c39c8fb7666ff686.1719771133.git.lukas@wunner.de
>
> Would this help with DOE features?

That was the previous approach used here:
https://lore.kernel.org/linux-pci/20240626045926.680380-3-alistair.francis@wdc.com/

Bjorn wanted to try and avoid using a function pci_doe_sysfs_init()
[1], which is what I tried here. It sounds like the v12 approach is
the way to go then. I'll send a v14 based on v12 with the comments
addressed

1: https://lore.kernel.org/all/20231019165829.GA1381099@bhelgaas/

Alistair

>
>         Krzysztof