Re: [PATCH v4] PCI/DOE: Expose the DOE protocols via sysfs

[Lukas Wunner]

On Fri, Aug 11, 2023 at 02:40:45PM -0400, Alistair Francis wrote:
> On Thu, Aug 10, 2023 at 9:04???PM Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
> > > This uses pci_sysfs_init() instead of the ->is_visible() function as
> > > is_visible only applies to the attributes under the group. Which
> > > means that every PCIe device will see a `doe_protos` directory, no
> > > matter if DOE is supported at all on the device.
> > >
> > > On top of that ->is_visible() is only called
> > > (fs/sysfs/group.c:create_files()) if there are sub attrs, which we
> > > don't necessary have. There are no static attrs, instead they are
> > > all generated dynamically.
> >
> > You said that the kernel caches the protocols supported. So it should
> > not be hard to allocate one attribute for each of the supported protocols
> > when these are discovered, no ?
> 
> I couldn't figure out a way to get this to work. You end up with a
> race between the sysfs group being created and the attributes being
> created. The DOE features are probed before the sysfs init creates the
> group.

If you look at device_add_attrs() in drivers/base/core.c, you'll notice
it calls device_add_groups() for the class, type and dev->groups.

pci_dev_attr_groups[] is assigned through the type.

What you want to do is amend pci_alloc_dev() to allocate enough space
for a struct attribute_group, in addition to struct pci_dev, then
assign it to dev->groups in that same function.  Define a macro
for the size.  Initially you'll need two struct attribute_group
elements, one for your DOE element plus one for the terminating
zero element.

If there are DOE mailboxes, let ->is_visible of the DOE group
return true and use "doe" as its ->name to make attributes appear
in a "doe" subdirectory.  Finally allocate and fill a struct
struct attribute[] array with all the protocols found in all the
mailboxes.

Thanks,

Lukas