Re: [PATCH v4] PCI: hotplug: Add 'uevent' sysfs attribute to trigger slot events

Re: [PATCH v4] PCI: hotplug: Add 'uevent' sysfs attribute to trigger slot events

[Bjorn Helgaas]

[+cc Leon, driver-core]

(Ramesh, when you post new versions of a series, please cc anybody who
responded to earlier versions.  Also, v2, v3, and v4 are identical, so
there's no need to post them as new "versions"; you can just ping the
original thread or label them as "RESEND")

On Wed, May 20, 2026 at 05:13:20PM -0500, Ramesh Errabolu wrote:
> Add a write-only 'uevent' sysfs attribute for synthesizing
> uevents for a PCI slot. This extends the existing uevent
> support which emits a KOBJ_ADD uevent in pci_hp_add() with
> the ability to replay such uevents for cold plugged devices.
> As such events are only emitted by hotplug capable PCI slots
> so is the support for synthesizing them.
> 
> The change was validated by manually triggering 'add' uevent
> for a specific hotplug PCI slot:
> 
>     $ echo "add $(uuidgen)" | sudo tee   \
>                 /sys/bus/pci/slots/<slot-id>/uevent
> 
> Signed-off-by: Ramesh Errabolu <ramesh@linux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/pci/hotplug/pci_hotplug_core.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
> index fadcf98a8a66..c3634b1cc7a8 100644
> --- a/drivers/pci/hotplug/pci_hotplug_core.c
> +++ b/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -173,12 +173,27 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
>  
>  static struct pci_slot_attribute hotplug_slot_attr_presence = {
>  	.attr = {.name = "adapter", .mode = S_IFREG | S_IRUGO},
>  	.show = presence_read_file,
>  };
>  
> +static ssize_t uevent_write_file(struct pci_slot *slot,
> +				 const char *buf, size_t len)
> +{
> +	int rc;
> +
> +	rc = kobject_synth_uevent(&slot->kobj, buf, len);

I haven't followed the discussion closely, but I'm skeptical because
this would be the only use of kobject_synth_uevent() outside the
driver core.  That means a change like this should include a
description of something unique about this PCI slot situation that is
different from all other buses.

For driver-core, the preceding discussion is here:
https://lore.kernel.org/linux-pci/20260225150815.81268-1-ramesh@linux.ibm.com/t/#m57bf51ce1c073b685b391867d4a9932e5f9dccc9

> +	return rc ? rc : len;
> +}
> +
> +static struct pci_slot_attribute hotplug_slot_attr_uevent = {
> +	.attr = {.name = "uevent", .mode = S_IFREG | 0200},
> +	.show = NULL,

Unnecessary to set ".show = NULL" since that's the default.

> +	.store = uevent_write_file
> +};
> +
>  static ssize_t test_write_file(struct pci_slot *pci_slot, const char *buf,
>  			       size_t count)
>  {
>  	struct hotplug_slot *slot = pci_slot->hotplug;
>  	unsigned long ltest;
>  	u32 test;
> @@ -251,12 +266,17 @@ static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
>  		if (retval)
>  			dev_err(&pci_slot->bus->dev,
>  				"Error creating sysfs link (%d)\n", retval);
>  		kobject_put(kobj);
>  	}
>  
> +	retval = sysfs_create_file(&pci_slot->kobj,
> +				   &hotplug_slot_attr_uevent.attr);

Krzysztof, any sysfs race issues here?  I assume this (and the
existing uses for power, attention, latch, etc) are not candidates for
static attributes?

> +	if (retval)
> +		goto exit_uevent;
> +
>  	if (has_power_file(slot)) {
>  		retval = sysfs_create_file(&pci_slot->kobj,
>  					   &hotplug_slot_attr_power.attr);
>  		if (retval)
>  			goto exit_power;
>  	}
> @@ -303,19 +323,24 @@ static int fs_add_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
>  		sysfs_remove_file(&pci_slot->kobj,
>  				  &hotplug_slot_attr_attention.attr);
>  exit_attention:
>  	if (has_power_file(slot))
>  		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
>  exit_power:
> +	sysfs_remove_file(&pci_slot->kobj,
> +			  &hotplug_slot_attr_uevent.attr);
> +exit_uevent:
>  	sysfs_remove_link(&pci_slot->kobj, "module");
>  exit:
>  	return retval;
>  }
>  
>  static void fs_remove_slot(struct hotplug_slot *slot, struct pci_slot *pci_slot)
>  {
> +	sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_uevent.attr);
> +
>  	if (has_power_file(slot))
>  		sysfs_remove_file(&pci_slot->kobj, &hotplug_slot_attr_power.attr);
>  
>  	if (has_attention_file(slot))
>  		sysfs_remove_file(&pci_slot->kobj,
>  				  &hotplug_slot_attr_attention.attr);
> -- 
> 2.43.0
> 

Re: [PATCH v4] PCI: hotplug: Add 'uevent' sysfs attribute to trigger slot events

[Niklas Schnelle]

On Thu, 2026-05-21 at 09:48 -0500, Bjorn Helgaas wrote:
> [+cc Leon, driver-core]
> 
> (Ramesh, when you post new versions of a series, please cc anybody who
> responded to earlier versions.  Also, v2, v3, and v4 are identical, so
> there's no need to post them as new "versions"; you can just ping the
> original thread or label them as "RESEND")
> 
> On Wed, May 20, 2026 at 05:13:20PM -0500, Ramesh Errabolu wrote:
> > Add a write-only 'uevent' sysfs attribute for synthesizing
> > uevents for a PCI slot. This extends the existing uevent
> > support which emits a KOBJ_ADD uevent in pci_hp_add() with
> > the ability to replay such uevents for cold plugged devices.
> > As such events are only emitted by hotplug capable PCI slots
> > so is the support for synthesizing them.
> > 
> > The change was validated by manually triggering 'add' uevent
> > for a specific hotplug PCI slot:
> > 
> >     $ echo "add $(uuidgen)" | sudo tee   \
> >                 /sys/bus/pci/slots/<slot-id>/uevent
> > 
> > Signed-off-by: Ramesh Errabolu <ramesh@linux.ibm.com>
> > Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> >  drivers/pci/hotplug/pci_hotplug_core.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
> > index fadcf98a8a66..c3634b1cc7a8 100644
> > --- a/drivers/pci/hotplug/pci_hotplug_core.c
> > +++ b/drivers/pci/hotplug/pci_hotplug_core.c
> > @@ -173,12 +173,27 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
> >  
> >  static struct pci_slot_attribute hotplug_slot_attr_presence = {
> >  	.attr = {.name = "adapter", .mode = S_IFREG | S_IRUGO},
> >  	.show = presence_read_file,
> >  };
> >  
> > +static ssize_t uevent_write_file(struct pci_slot *slot,
> > +				 const char *buf, size_t len)
> > +{
> > +	int rc;
> > +
> > +	rc = kobject_synth_uevent(&slot->kobj, buf, len);
> 
> I haven't followed the discussion closely, but I'm skeptical because
> this would be the only use of kobject_synth_uevent() outside the
> driver core.  That means a change like this should include a
> description of something unique about this PCI slot situation that is
> different from all other buses.
> 
> For driver-core, the preceding discussion is here:
> https://lore.kernel.org/linux-pci/20260225150815.81268-1-ramesh@linux.ibm.com/t/#m57bf51ce1c073b685b391867d4a9932e5f9dccc9

Sorry for the late reply. In the previous discussion we decided to move
this here because this is the same file where the existing add uevent,
which we're trying to synthesize for cold plug, is handled in
pci_hp_add(). So from our point of view this is really a missing part
to that existing uevent support that is causing real issues for us. In
particular we need to be able to create udev rules that react on
plugging of an s390 hotplug slot. On s390 a hotplug slot being plugged
is how the (machine) hypervisor makes a PCI function available to the
system where the function may first appear as powered off to multiple
Linux instances and only gets actually attached once one instances
powers it on. For example such a rule would be used to automatically
power on the function in cases where we know that we're the only
instance that will see it. Of course we'd like to have the same rule
handle hotplug (works already) and cold plug during boot (needs
synthesized uevents).

As for what's special, I think it is that as far as I understand the
PCI hotplug slot "drivers" don't seem to be well integrated with the
driver model. For example neither our s390_pci_hpc nor the acpiphp
hotplug driver seem to actually have an associated struct device_driver
nor do these hotplug slots appear on a bus which I guess would allow
using the bus's uevent. Fixing that does seem like a much bigger effort
though.

There is also the call to kobject_synth_uevent() in
kernel/module/main.c which looks at least somewhat similar. But yeah I
do see your point that basically all other drivers and devices get
their uevent syfs file from the driver core.

Thanks,
Niklas