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

[Alistair Francis]

On Mon, Jul 8, 2024 at 10:55 AM Krzysztof Wilczyński <kw@xxxxxxxxx> 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@xxxxxxxxx
>
> Would this help with DOE features?

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

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