Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 12 Aug 2023 10:05:09 +0200
From:   Lukas Wunner <lukas@wunner.de>
To:     Alistair Francis <alistair23@gmail.com>
Cc:     Damien Le Moal <dlemoal@kernel.org>, bhelgaas@google.com,
        linux-pci@vger.kernel.org, Jonathan.Cameron@huawei.com,
        alex.williamson@redhat.com, christian.koenig@amd.com,
        kch@nvidia.com, gregkh@linuxfoundation.org, logang@deltatee.com,
        linux-kernel@vger.kernel.org, chaitanyak@nvidia.com,
        rdunlap@infradead.org, Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [PATCH v4] PCI/DOE: Expose the DOE protocols via sysfs
Message-ID: <20230812080509.GA9469@wunner.de>
References: <20230810163342.1059509-1-alistair.francis@wdc.com>
 <b3d437f5-fe33-4677-e336-a67ac9b8d477@kernel.org>
 <CAKmqyKOpgTUOzPMhe3Dr1H6BiFZYHrBHFpiESyXitRHbdH0+LA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAKmqyKOpgTUOzPMhe3Dr1H6BiFZYHrBHFpiESyXitRHbdH0+LA@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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@kernel.org> 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