| 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
| ||
|
Message-ID: <16183432-f46d-4f3e-af0e-524678445136@nvidia.com>
Date: Fri, 3 May 2024 09:18:26 +0300
From: Shay Drori <shayd@...dia.com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: <netdev@...r.kernel.org>, <pabeni@...hat.com>, <davem@...emloft.net>,
<kuba@...nel.org>, <edumazet@...gle.com>, <david.m.ertman@...el.com>,
<rafael@...nel.org>, <ira.weiny@...el.com>, <linux-rdma@...r.kernel.org>,
<leon@...nel.org>, <tariqt@...dia.com>, Parav Pandit <parav@...dia.com>
Subject: Re: [PATCH 1/2] driver core: auxiliary bus: show auxiliary device
IRQs
On 03/05/2024 8:10, Greg KH wrote:
> External email: Use caution opening links or attachments
>
>
> On Fri, May 03, 2024 at 07:31:03AM +0300, Shay Drory wrote:
>> PCI subfunctions (SF) are anchored on the auxiliary bus. PCI physical
>> and virtual functions are anchored on the PCI bus; the irq information
>> of each such function is visible to users via sysfs directory "msi_irqs"
>> containing file for each irq entry. However, for PCI SFs such information
>> is unavailable. Due to this users have no visibility on IRQs used by the
>> SFs.
>> Secondly, an SF is a multi function device supporting rdma, netdevice
>> and more. Without irq information at the bus level, the user is unable
>> to view or use the affinity of the SF IRQs.
>>
>> Hence to match to the equivalent PCI PFs and VFs, add "irqs" directory,
>> for supporting auxiliary devices, containing file for each irq entry.
>>
>> Additionally, the PCI SFs sometimes share the IRQs with peer SFs. This
>> information is also not available to the users. To overcome this
>> limitation, each irq sysfs entry shows if irq is exclusive or shared.
>>
>> For example:
>> $ ls /sys/bus/auxiliary/devices/mlx5_core.sf.1/irqs/
>> 50 51 52 53 54 55 56 57 58
>> $ cat /sys/bus/auxiliary/devices/mlx5_core.sf.1/irqs/52
>> exclusive
>
> Not all the world (i.e. not all auxbus users) are PCI devices, right?
Right. auxiliary users are PCI sfs and other devices.
> So what happens for non-PCI devices?
The sysfs extension is optional, i.e. only for PCI SFs the sysfs irq
information is optionally exposed.
Rest of the other users just continue as is without sysfs functionality.
>
>>
>> Reviewed-by: Parav Pandit <parav@...dia.com>
>> Signed-off-by: Shay Drory <shayd@...dia.com>
>> ---
>> Documentation/ABI/testing/sysfs-bus-auxiliary | 14 ++
>> drivers/base/auxiliary.c | 170 +++++++++++++++++-
>> include/linux/auxiliary_bus.h | 15 +-
>> 3 files changed, 196 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-auxiliary
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-auxiliary b/Documentation/ABI/testing/sysfs-bus-auxiliary
>> new file mode 100644
>> index 000000000000..3b8299d49d9e
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-auxiliary
>> @@ -0,0 +1,14 @@
>> +What: /sys/bus/auxiliary/devices/.../irqs/
>> +Date: April, 2024
>> +Contact: Shay Drory <shayd@...dia.com>
>> +Description:
>> + The /sys/devices/.../irqs directory contains a variable set of
>> + files, with each file is named as irq number similar to PCI PF
>> + or VF's irq number located in msi_irqs directory.
>> +
>> +What: /sys/bus/auxiliary/devices/.../irqs/<N>
>> +Date: April, 2024
>> +Contact: Shay Drory <shayd@...dia.com>
>> +Description:
>> + auxiliary devices can share IRQs. This attribute indicates if
>> + the irq is shared with other SFs or exclusively used by the SF.
>> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
>> index d3a2c40c2f12..5c0efa2081b8 100644
>> --- a/drivers/base/auxiliary.c
>> +++ b/drivers/base/auxiliary.c
>> @@ -158,6 +158,167 @@
>> * };
>> */
>>
>> +#ifdef CONFIG_SYSFS
>> +/* Xarray of irqs to determine if irq is exclusive or shared. */
>> +static DEFINE_XARRAY(irqs);
>> +/* Protects insertions into the irtqs xarray. */
>> +static DEFINE_MUTEX(irqs_lock);
>> +
>> +struct auxiliary_irq_info {
>> + struct device_attribute sysfs_attr;
>> + int irq;
>> +};
>> +
>> +static struct attribute *auxiliary_irq_attrs[] = {
>> + NULL
>> +};
>> +
>> +static const struct attribute_group auxiliary_irqs_group = {
>> + .name = "irqs",
>> + .attrs = auxiliary_irq_attrs,
>> +};
>> +
>> +/**
>> + * Auxiliary devices can share IRQs. Expose to user whether the provided IRQ is
>> + * shared or exclusive.
>> + */
>> +static ssize_t auxiliary_irq_mode_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct auxiliary_irq_info *info =
>> + container_of(attr, struct auxiliary_irq_info, sysfs_attr);
>> +
>> + if (refcount_read(xa_load(&irqs, info->irq)) > 1)
>> + return sysfs_emit(buf, "%s\n", "shared");
>> + else
>> + return sysfs_emit(buf, "%s\n", "exclusive");
>> +}
>> +
>> +static void auxiliary_irq_destroy(int irq)
>> +{
>> + refcount_t *ref;
>> +
>> + xa_lock(&irqs);
>> + ref = xa_load(&irqs, irq);
>> + if (refcount_dec_and_test(ref)) {
>> + __xa_erase(&irqs, irq);
>> + kfree(ref);
>> + }
>> + xa_unlock(&irqs);
>> +}
>> +
>> +static int auxiliary_irq_create(int irq)
>> +{
>> + refcount_t *ref;
>> + int ret = 0;
>> +
>> + mutex_lock(&irqs_lock);
>> + ref = xa_load(&irqs, irq);
>> + if (ref && refcount_inc_not_zero(ref))
>> + goto out;
>> +
>> + ref = kzalloc(sizeof(ref), GFP_KERNEL);
>> + if (!ref) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + refcount_set(ref, 1);
>> + ret = xa_insert(&irqs, irq, ref, GFP_KERNEL);
>> + if (ret)
>> + kfree(ref);
>> +
>> +out:
>> + mutex_unlock(&irqs_lock);
>> + return ret;
>> +}
>> +
>> +/**
>> + * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ
>> + * @auxdev: auxiliary bus device to add the sysfs entry.
>> + * @irq: The associated Linux interrupt number.
>> + *
>> + * This function should be called after auxiliary device have successfully
>> + * received the irq.
>> + */
>> +int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq)
>> +{
>> + struct device *dev = &auxdev->dev;
>> + struct auxiliary_irq_info *info;
>> + int ret;
>> +
>> + ret = auxiliary_irq_create(irq);
>> + if (ret)
>> + return ret;
>> +
>> + info = kzalloc(sizeof(*info), GFP_KERNEL);
>> + if (!info) {
>> + ret = -ENOMEM;
>> + goto info_err;
>> + }
>> +
>> + sysfs_attr_init(&info->sysfs_attr.attr);
>> + info->sysfs_attr.attr.name = kasprintf(GFP_KERNEL, "%d", irq);
>> + if (!info->sysfs_attr.attr.name) {
>> + ret = -ENOMEM;
>> + goto name_err;
>> + }
>> + info->irq = irq;
>> + info->sysfs_attr.attr.mode = 0444;
>> + info->sysfs_attr.show = auxiliary_irq_mode_show;
>> +
>> + ret = xa_insert(&auxdev->irqs, irq, info, GFP_KERNEL);
>> + if (ret)
>> + goto auxdev_xa_err;
>> +
>> + ret = sysfs_add_file_to_group(&dev->kobj, &info->sysfs_attr.attr,
>> + auxiliary_irqs_group.name);
>> + if (ret)
>> + goto sysfs_add_err;
>> +
>> + return 0;
>> +
>> +sysfs_add_err:
>> + xa_erase(&auxdev->irqs, irq);
>> +auxdev_xa_err:
>> + kfree(info->sysfs_attr.attr.name);
>> +name_err:
>> + kfree(info);
>> +info_err:
>> + auxiliary_irq_destroy(irq);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(auxiliary_device_sysfs_irq_add);
>> +
>> +/**
>> + * auxiliary_device_sysfs_irq_remove - remove a sysfs entry for the given IRQ
>> + * @auxdev: auxiliary bus device to add the sysfs entry.
>> + * @irq: the IRQ to remove.
>> + *
>> + * This function should be called to remove an IRQ sysfs entry.
>> + */
>> +void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq)
>> +{
>> + struct auxiliary_irq_info *info = xa_load(&auxdev->irqs, irq);
>> + struct device *dev = &auxdev->dev;
>> +
>> + if (WARN_ON(!info))
>> + return;
>> +
>> + sysfs_remove_file_from_group(&dev->kobj, &info->sysfs_attr.attr,
>> + auxiliary_irqs_group.name);
>> + xa_erase(&auxdev->irqs, irq);
>> + kfree(info->sysfs_attr.attr.name);
>> + kfree(info);
>> + auxiliary_irq_destroy(irq);
>> +}
>> +EXPORT_SYMBOL(auxiliary_device_sysfs_irq_remove);
>
> Any reason this isn't EXPORT_SYMBOL_GPL() like the rest of this file?
> Same for the other export.
missed that, will fix in v2
>
>> +#else /* CONFIG_SYSFS */
>> +int auxiliary_device_sysfs_irq_add(struct auxiliary_device *auxdev, int irq) {return 0; }
>> +void auxiliary_device_sysfs_irq_remove(struct auxiliary_device *auxdev, int irq) {}
>> +#endif
>
> Shouldn't the #ifdef stuff be in a .h file? Why .c?
missed that, will fix in v2
>
> And again, why do you think that all aux devices have irqs?
like I mention above, this is optional.
I missed to explain this in the cover letter, I will add it to v2.
>
> thanks,
>
> greg k-h
Powered by blists - more mailing lists