[PATCH v3] driver core: platform: expose numa_node to users in sysfs

[PATCH v3] driver core: platform: expose numa_node to users in sysfs

[Barry Song]

Some platform devices like ARM SMMU are memory-mapped and populated by ACPI/IORT.
In this case, NUMA topology of those platform devices are exported by firmware as
well. Software might care about the numa_node of those devices in order to achieve
NUMA locality.
This patch will show the numa_node for this kind of devices in sysfs. For those
platform devices without numa, numa_node won't be visible.

Cc: Prime Zeng <prime.zeng@hisilicon.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 -v3: rebase to 5.8-rc1; refine commit log

 Documentation/ABI/testing/sysfs-bus-platform | 10 ++++++++
 drivers/base/platform.c                      | 26 +++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform
index 5172a6124b27..194ca700e962 100644
--- a/Documentation/ABI/testing/sysfs-bus-platform
+++ b/Documentation/ABI/testing/sysfs-bus-platform
@@ -18,3 +18,13 @@ Description:
 		devices to opt-out of driver binding using a driver_override
 		name such as "none".  Only a single driver may be specified in
 		the override, there is no support for parsing delimiters.
+
+What:		/sys/bus/platform/devices/.../numa_node
+Date:		June 2020
+Contact:	Barry Song <song.bao.hua@hisilicon.com>
+Description:
+		This file contains the NUMA node to which the platform device
+		is attached. It won't be visible if the node is unknown. The
+		value comes from an ACPI _PXM method or a similar firmware
+		source. Initial users for this file would be devices like
+		arm smmu which are populated by arm64 acpi_iort.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c0d0a5490ac6..4369dfd57815 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1076,13 +1076,37 @@ static ssize_t driver_override_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(driver_override);
 
+static ssize_t numa_node_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", dev_to_node(dev));
+}
+static DEVICE_ATTR_RO(numa_node);
+
+static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute *a,
+		int n)
+{
+	struct device *dev = container_of(kobj, typeof(*dev), kobj);
+
+	if (a == &dev_attr_numa_node.attr &&
+			dev_to_node(dev) == NUMA_NO_NODE)
+		return 0;
+
+	return a->mode;
+}
 
 static struct attribute *platform_dev_attrs[] = {
 	&dev_attr_modalias.attr,
+	&dev_attr_numa_node.attr,
 	&dev_attr_driver_override.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(platform_dev);
+
+static struct attribute_group platform_dev_group = {
+	.attrs = platform_dev_attrs,
+	.is_visible = platform_dev_attrs_visible,
+};
+__ATTRIBUTE_GROUPS(platform_dev);
 
 static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-- 
2.23.0

Re: [PATCH v3] driver core: platform: expose numa_node to users in sysfs

[John Garry]

On 19/06/2020 04:00, Barry Song wrote:
> Some platform devices like ARM SMMU are memory-mapped and populated by ACPI/IORT.
> In this case, NUMA topology of those platform devices are exported by firmware as
> well. Software might care about the numa_node of those devices in order to achieve
> NUMA locality.

Is it generally the case that the SMMU will be in the same NUMA node as 
the endpoint device (which you're driving)? If so, we can get this info 
from sysfs already for the endpoint, and also have a link from the 
endpoint to the iommu for pci devices (which I assume you're interested in):

root@(none)$ ls -l /sys/devices/pci0000:74/0000:74:02.0/ | grep iommu
lrwxrwxrwx 1 root  root 0 Jun 22 10:33 iommu -> 
../../platform/arm-smmu-v3.2.auto/iommu/smmu3.0x0000000140000000
lrwxrwxrwx 1 root  root 0 Jun 22 10:33 iommu_group -> 
../../../kernel/iommu_groups/0
root@(none)$

Thanks,
John

RE: [PATCH v3] driver core: platform: expose numa_node to users in sysfs

[Song Bao Hua (Barry Song)]

> -----Original Message-----
> From: John Garry
> Sent: Monday, June 22, 2020 10:49 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>;
> gregkh@linuxfoundation.org; rafael@kernel.org
> Cc: Robin Murphy <robin.murphy@arm.com>; linux-kernel@vger.kernel.org;
> Zengtao (B) <prime.zeng@hisilicon.com>; Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH v3] driver core: platform: expose numa_node to users in
> sysfs
> 
> On 19/06/2020 04:00, Barry Song wrote:
> > Some platform devices like ARM SMMU are memory-mapped and populated
> by ACPI/IORT.
> > In this case, NUMA topology of those platform devices are exported by
> firmware as
> > well. Software might care about the numa_node of those devices in order to
> achieve
> > NUMA locality.
> 

Thanks for your review, John.

> Is it generally the case that the SMMU will be in the same NUMA node as
> the endpoint device (which you're driving)? If so, we can get this info


This could be true, but I am not sure if it has to be true :-)

On the other hand, drivers/acpi/arm64/iort.c has some code to set numa node for smmu.
It doesn't assume the numa_node is directly same with the pci devices.

static int  __init arm_smmu_v3_set_proximity(struct device *dev,
					      struct acpi_iort_node *node)
{
	struct acpi_iort_smmu_v3 *smmu;

	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
		int dev_node = acpi_map_pxm_to_node(smmu->pxm);

		if (dev_node != NUMA_NO_NODE && !node_online(dev_node))
			return -EINVAL;

		set_dev_node(dev, dev_node);
		pr_info("SMMU-v3[%llx] Mapped to Proximity domain %d\n",
			smmu->base_address,
			smmu->pxm);
	}
	return 0;
}

numa_node may also extend to other platform devices once we provide a common dev_set_proximity() callback to them.
iort_add_platform_device() will set node for them:

static int __init iort_add_platform_device(struct acpi_iort_node *node,
					   const struct iort_dev_config *ops)
{
	struct fwnode_handle *fwnode;
	struct platform_device *pdev;
	struct resource *r;
	int ret, count;

	pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
	if (!pdev)
		return -ENOMEM;

	if (ops->dev_set_proximity) {
		ret = ops->dev_set_proximity(&pdev->dev, node);
		if (ret)
			goto dev_put;
	}
	...
}

It is probably worth to make dev_set_proximity() common for all iort devices.

> from sysfs already for the endpoint, and also have a link from the
> endpoint to the iommu for pci devices (which I assume you're interested in):
> 

> root@(none)$ ls -l /sys/devices/pci0000:74/0000:74:02.0/ | grep iommu
> lrwxrwxrwx 1 root  root 0 Jun 22 10:33 iommu ->
> ../../platform/arm-smmu-v3.2.auto/iommu/smmu3.0x0000000140000000
> lrwxrwxrwx 1 root  root 0 Jun 22 10:33 iommu_group ->
> ../../../kernel/iommu_groups/0
> root@(none)$

Sure there is an implicit way to figure out the numa node of smmu by various links between smmu
and devices which use the smmu if smmu and devices are luckily put in one same numa node.

However, it is still much more clear and credible to users by exposing the data directly from ACPI table. 

> 
> Thanks,
> John

Barry

RE: [PATCH v3] driver core: platform: expose numa_node to users in sysfs

[Song Bao Hua (Barry Song)]

> 
> However, it is still much more clear and credible to users by exposing the data
> directly from ACPI table.
> 

Except ARM64 iort, numa_node is actually also applicable to x86 and other architectures through general
acpi_create_platform_device() API:

drivers/acpi/scan.c:
static void acpi_default_enumeration(struct acpi_device *device)
{
	...
	if (!device->flags.enumeration_by_parent) {
		acpi_create_platform_device(device, NULL);
		acpi_device_set_enumerated(device);
	}
}

struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
					struct property_entry *properties)
{
	...

	pdev = platform_device_register_full(&pdevinfo);
	if (IS_ERR(pdev))
		dev_err(&adev->dev, "platform device creation failed: %ld\n",
			PTR_ERR(pdev));
	else {
		set_dev_node(&pdev->dev, acpi_get_node(adev->handle));
		dev_dbg(&adev->dev, "created platform device %s\n",
			dev_name(&pdev->dev));
	}

	...

	return pdev;
}

> >
> > Thanks,
> > John

Thanks
Barry