[PATCH v3 0/2] acpi/iort, numa: Add numa node mapping for smmuv3 devices

[PATCH v3 0/2] acpi/iort, numa: Add numa node mapping for smmuv3 devices

[Ganapatrao Kulkarni]

ARM IORT specification(rev. C) has added  provision to define proximity
domain in SMMUv3 IORT table. Adding required code to parse Proximity
domain and set numa_node of smmv3 platform devices.

v3:
  - Addressed Lorenzo Pieralisi comment.

v2:
  - Changed as per Lorenzo Pieralisi and Hanjun Guo suggestions.

v1:
  - Initial patch

Ganapatrao Kulkarni (2):
  acpica: iort: Update SMMUv3 header for proximity domain mapping
  acpi/iort: numa: Add numa node mapping for smmuv3 devices

 drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
 include/acpi/actbl2.h     |  4 ++++
 2 files changed, 30 insertions(+), 2 deletions(-)

-- 
1.8.1.4

[PATCH v3 1/2] acpica: iort: Update SMMUv3 header for proximity domain mapping

[Ganapatrao Kulkarni]

ARM IORT specification (rev. C) has added two new fields to define
proximity domain for the SMMUv3 node in the IORT table.

Proximity Domain Valid:
	Set to 1 if the value provided in the Proximity Domain field is
	valid. Set to 0 otherwise.

Proximity domain:
	If the Proximity Domain Valid flag is set to 1, this entry
	provides the proximity domain to which this SMMU
	instance belongs.

Update header file to reflect this.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 include/acpi/actbl2.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 4b306a6..389e91f 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -805,6 +805,9 @@ struct acpi_iort_smmu_v3 {
 	u32 pri_gsiv;
 	u32 gerr_gsiv;
 	u32 sync_gsiv;
+	u8 pxm;
+	u8 reserved1;
+	u16 reserved2;
 };
 
 /* Values for Model field above */
@@ -817,6 +820,7 @@ struct acpi_iort_smmu_v3 {
 
 #define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE   (1)
 #define ACPI_IORT_SMMU_V3_HTTU_OVERRIDE     (1<<1)
+#define ACPI_IORT_SMMU_V3_PXM_VALID         (1<<3)
 
 /*******************************************************************************
  *
-- 
1.8.1.4

Re: [PATCH v3 1/2] acpica: iort: Update SMMUv3 header for proximity domain mapping

[Robin Murphy]

On 08/06/17 05:44, Ganapatrao Kulkarni wrote:
> ARM IORT specification (rev. C) has added two new fields to define
> proximity domain for the SMMUv3 node in the IORT table.
> 
> Proximity Domain Valid:
> 	Set to 1 if the value provided in the Proximity Domain field is
> 	valid. Set to 0 otherwise.
> 
> Proximity domain:
> 	If the Proximity Domain Valid flag is set to 1, this entry
> 	provides the proximity domain to which this SMMU
> 	instance belongs.
> 
> Update header file to reflect this.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  include/acpi/actbl2.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 4b306a6..389e91f 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -805,6 +805,9 @@ struct acpi_iort_smmu_v3 {
>  	u32 pri_gsiv;
>  	u32 gerr_gsiv;
>  	u32 sync_gsiv;
> +	u8 pxm;
> +	u8 reserved1;
> +	u16 reserved2;

Why add the padding if you're not also adding the following field that
it aligns? (side note: could it not just be a u8[3]?)

If the actual structure definition is changing, do there not need to be
corresponding IASL changes too?

>  };
>  
>  /* Values for Model field above */
> @@ -817,6 +820,7 @@ struct acpi_iort_smmu_v3 {
>  
>  #define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE   (1)
>  #define ACPI_IORT_SMMU_V3_HTTU_OVERRIDE     (1<<1)
> +#define ACPI_IORT_SMMU_V3_PXM_VALID         (1<<3)

Maybe it's just me, but "PXM" seems incredibly cryptic.

Robin.

>  
>  /*******************************************************************************
>   *
> 

Re: [PATCH v3 1/2] acpica: iort: Update SMMUv3 header for proximity domain mapping

[Ganapatrao Kulkarni]

On Mon, Jun 19, 2017 at 11:24 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 08/06/17 05:44, Ganapatrao Kulkarni wrote:
>> ARM IORT specification (rev. C) has added two new fields to define
>> proximity domain for the SMMUv3 node in the IORT table.
>>
>> Proximity Domain Valid:
>>       Set to 1 if the value provided in the Proximity Domain field is
>>       valid. Set to 0 otherwise.
>>
>> Proximity domain:
>>       If the Proximity Domain Valid flag is set to 1, this entry
>>       provides the proximity domain to which this SMMU
>>       instance belongs.
>>
>> Update header file to reflect this.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> ---
>>  include/acpi/actbl2.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>> index 4b306a6..389e91f 100644
>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -805,6 +805,9 @@ struct acpi_iort_smmu_v3 {
>>       u32 pri_gsiv;
>>       u32 gerr_gsiv;
>>       u32 sync_gsiv;
>> +     u8 pxm;
>> +     u8 reserved1;
>> +     u16 reserved2;
>
> Why add the padding if you're not also adding the following field that
> it aligns? (side note: could it not just be a u8[3]?)

added to align as well to comply with  the spec.

>
> If the actual structure definition is changing, do there not need to be
> corresponding IASL changes too?

yes, it is been added and sent to acpica devel ML.
https://lists.acpica.org/pipermail/devel/2017-June/001243.html

>
>>  };
>>
>>  /* Values for Model field above */
>> @@ -817,6 +820,7 @@ struct acpi_iort_smmu_v3 {
>>
>>  #define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE   (1)
>>  #define ACPI_IORT_SMMU_V3_HTTU_OVERRIDE     (1<<1)
>> +#define ACPI_IORT_SMMU_V3_PXM_VALID         (1<<3)
>
> Maybe it's just me, but "PXM" seems incredibly cryptic.

PXM stands for proximity(refer ACPI spec)
it is widely used in spec and also in Linux kernel.

>
> Robin.
>
>>
>>  /*******************************************************************************
>>   *
>>
>

thanks
Ganapat

[PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Ganapatrao Kulkarni]

Add code to parse proximity domain in SMMUv3 IORT table to
set numa node mapping for smmuv3 devices.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index bba2b59..e804386 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
 	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
 }
 
+/*
+ * set numa proximity domain for smmuv3 device
+ */
+static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
+		struct device *dev)
+{
+	struct acpi_iort_smmu_v3 *smmu;
+
+	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
+		set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
+		pr_info("SMMUV3[%llx] Mapped to Proximity domain %d\n",
+			smmu->base_address,
+			smmu->pxm);
+	}
+}
+
 static int __init arm_smmu_count_resources(struct acpi_iort_node *node)
 {
 	struct acpi_iort_smmu *smmu;
@@ -951,20 +968,24 @@ struct iort_iommu_config {
 	int (*iommu_count_resources)(struct acpi_iort_node *node);
 	void (*iommu_init_resources)(struct resource *res,
 				     struct acpi_iort_node *node);
+	void (*iommu_set_proximity)(struct acpi_iort_node *node,
+				     struct device *dev);
 };
 
 static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
 	.name = "arm-smmu-v3",
 	.iommu_is_coherent = arm_smmu_v3_is_coherent,
 	.iommu_count_resources = arm_smmu_v3_count_resources,
-	.iommu_init_resources = arm_smmu_v3_init_resources
+	.iommu_init_resources = arm_smmu_v3_init_resources,
+	.iommu_set_proximity = arm_smmu_v3_set_proximity
 };
 
 static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
 	.name = "arm-smmu",
 	.iommu_is_coherent = arm_smmu_is_coherent,
 	.iommu_count_resources = arm_smmu_count_resources,
-	.iommu_init_resources = arm_smmu_init_resources
+	.iommu_init_resources = arm_smmu_init_resources,
+	.iommu_set_proximity = NULL
 };
 
 static __init
@@ -1002,6 +1023,9 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
 	if (!pdev)
 		return -ENOMEM;
 
+	if (ops->iommu_set_proximity)
+		ops->iommu_set_proximity(node, &pdev->dev);
+
 	count = ops->iommu_count_resources(node);
 
 	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
-- 
1.8.1.4

Re: [PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Lorenzo Pieralisi]

Hi,

On Thu, Jun 08, 2017 at 10:14:19AM +0530, Ganapatrao Kulkarni wrote:
> Add code to parse proximity domain in SMMUv3 IORT table to
> set numa node mapping for smmuv3 devices.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)

I am happy to take this but I want to know what we shall do with
patch 1 and related ACPICA changes first.

Thanks,
Lorenzo

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index bba2b59..e804386 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
>  	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
>  }
>  
> +/*
> + * set numa proximity domain for smmuv3 device
> + */
> +static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
> +		struct device *dev)
> +{
> +	struct acpi_iort_smmu_v3 *smmu;
> +
> +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> +		set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
> +		pr_info("SMMUV3[%llx] Mapped to Proximity domain %d\n",
> +			smmu->base_address,
> +			smmu->pxm);
> +	}
> +}
> +
>  static int __init arm_smmu_count_resources(struct acpi_iort_node *node)
>  {
>  	struct acpi_iort_smmu *smmu;
> @@ -951,20 +968,24 @@ struct iort_iommu_config {
>  	int (*iommu_count_resources)(struct acpi_iort_node *node);
>  	void (*iommu_init_resources)(struct resource *res,
>  				     struct acpi_iort_node *node);
> +	void (*iommu_set_proximity)(struct acpi_iort_node *node,
> +				     struct device *dev);
>  };
>  
>  static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
>  	.name = "arm-smmu-v3",
>  	.iommu_is_coherent = arm_smmu_v3_is_coherent,
>  	.iommu_count_resources = arm_smmu_v3_count_resources,
> -	.iommu_init_resources = arm_smmu_v3_init_resources
> +	.iommu_init_resources = arm_smmu_v3_init_resources,
> +	.iommu_set_proximity = arm_smmu_v3_set_proximity
>  };
>  
>  static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
>  	.name = "arm-smmu",
>  	.iommu_is_coherent = arm_smmu_is_coherent,
>  	.iommu_count_resources = arm_smmu_count_resources,
> -	.iommu_init_resources = arm_smmu_init_resources
> +	.iommu_init_resources = arm_smmu_init_resources,
> +	.iommu_set_proximity = NULL
>  };
>  
>  static __init
> @@ -1002,6 +1023,9 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
>  	if (!pdev)
>  		return -ENOMEM;
>  
> +	if (ops->iommu_set_proximity)
> +		ops->iommu_set_proximity(node, &pdev->dev);
> +
>  	count = ops->iommu_count_resources(node);
>  
>  	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
> -- 
> 1.8.1.4
> 

Re: [PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Robert Richter]

On 15.06.17 14:46:03, Lorenzo Pieralisi wrote:
> On Thu, Jun 08, 2017 at 10:14:19AM +0530, Ganapatrao Kulkarni wrote:
> > Add code to parse proximity domain in SMMUv3 IORT table to
> > set numa node mapping for smmuv3 devices.
> > 
> > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > ---
> >  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> I am happy to take this but I want to know what we shall do with
> patch 1 and related ACPICA changes first.

The change is now in acpica:

 https://github.com/acpica/acpica/commit/8cadc4fb500e2aa52241e367c87a0f95d9760c58

So we could guard the code with an #ifdef until that patch is pulled
in via acpica tree:

> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index bba2b59..e804386 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
> >  	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
> >  }
> >  
> > +/*
> > + * set numa proximity domain for smmuv3 device
> > + */
> > +static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
> > +		struct device *dev)
> > +{

#ifdef ACPI_IORT_SMMU_V3_PXM_VALID

> > +	struct acpi_iort_smmu_v3 *smmu;
> > +
> > +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > +	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> > +		set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
> > +		pr_info("SMMUV3[%llx] Mapped to Proximity domain %d\n",
> > +			smmu->base_address,
> > +			smmu->pxm);
> > +	}

#endif

> > +}
> > +

Could the patch be applied with this change?

Thanks,

-Robert

> >  static int __init arm_smmu_count_resources(struct acpi_iort_node *node)
> >  {
> >  	struct acpi_iort_smmu *smmu;
> > @@ -951,20 +968,24 @@ struct iort_iommu_config {
> >  	int (*iommu_count_resources)(struct acpi_iort_node *node);
> >  	void (*iommu_init_resources)(struct resource *res,
> >  				     struct acpi_iort_node *node);
> > +	void (*iommu_set_proximity)(struct acpi_iort_node *node,
> > +				     struct device *dev);
> >  };
> >  
> >  static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
> >  	.name = "arm-smmu-v3",
> >  	.iommu_is_coherent = arm_smmu_v3_is_coherent,
> >  	.iommu_count_resources = arm_smmu_v3_count_resources,
> > -	.iommu_init_resources = arm_smmu_v3_init_resources
> > +	.iommu_init_resources = arm_smmu_v3_init_resources,
> > +	.iommu_set_proximity = arm_smmu_v3_set_proximity
> >  };
> >  
> >  static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
> >  	.name = "arm-smmu",
> >  	.iommu_is_coherent = arm_smmu_is_coherent,
> >  	.iommu_count_resources = arm_smmu_count_resources,
> > -	.iommu_init_resources = arm_smmu_init_resources
> > +	.iommu_init_resources = arm_smmu_init_resources,
> > +	.iommu_set_proximity = NULL
> >  };
> >  
> >  static __init
> > @@ -1002,6 +1023,9 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
> >  	if (!pdev)
> >  		return -ENOMEM;
> >  
> > +	if (ops->iommu_set_proximity)
> > +		ops->iommu_set_proximity(node, &pdev->dev);
> > +
> >  	count = ops->iommu_count_resources(node);
> >  
> >  	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
> > -- 
> > 1.8.1.4
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Lorenzo Pieralisi]

Hi Robert,

On Wed, Jun 28, 2017 at 07:47:50PM +0200, Robert Richter wrote:
> On 15.06.17 14:46:03, Lorenzo Pieralisi wrote:
> > On Thu, Jun 08, 2017 at 10:14:19AM +0530, Ganapatrao Kulkarni wrote:
> > > Add code to parse proximity domain in SMMUv3 IORT table to
> > > set numa node mapping for smmuv3 devices.
> > > 
> > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > ---
> > >  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
> > >  1 file changed, 26 insertions(+), 2 deletions(-)
> > 
> > I am happy to take this but I want to know what we shall do with
> > patch 1 and related ACPICA changes first.
> 
> The change is now in acpica:
> 
>  https://github.com/acpica/acpica/commit/8cadc4fb500e2aa52241e367c87a0f95d9760c58
> 
> So we could guard the code with an #ifdef until that patch is pulled
> in via acpica tree:
> 
> > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > index bba2b59..e804386 100644
> > > --- a/drivers/acpi/arm64/iort.c
> > > +++ b/drivers/acpi/arm64/iort.c
> > > @@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
> > >  	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
> > >  }
> > >  
> > > +/*
> > > + * set numa proximity domain for smmuv3 device
> > > + */
> > > +static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
> > > +		struct device *dev)
> > > +{
> 
> #ifdef ACPI_IORT_SMMU_V3_PXM_VALID
> 
> > > +	struct acpi_iort_smmu_v3 *smmu;
> > > +
> > > +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > > +	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> > > +		set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
> > > +		pr_info("SMMUV3[%llx] Mapped to Proximity domain %d\n",
> > > +			smmu->base_address,
> > > +			smmu->pxm);
> > > +	}
> 
> #endif
> 
> > > +}
> > > +
> 
> Could the patch be applied with this change?

Sorry I was away (and the ARM64 tree was not accepting any new code last
week anyway), I will send the patch upstream for 4.14 (with your change
above - or whatever ACPICA guard make it safe - since we will depend on
ACPICA changes merge timing anyway - they won't be merged this cycle).

Bad timing, apologies, I hope it is not a burden to carry it for one
cycle.

Thanks,
Lorenzo

> 
> Thanks,
> 
> -Robert
> 
> > >  static int __init arm_smmu_count_resources(struct acpi_iort_node *node)
> > >  {
> > >  	struct acpi_iort_smmu *smmu;
> > > @@ -951,20 +968,24 @@ struct iort_iommu_config {
> > >  	int (*iommu_count_resources)(struct acpi_iort_node *node);
> > >  	void (*iommu_init_resources)(struct resource *res,
> > >  				     struct acpi_iort_node *node);
> > > +	void (*iommu_set_proximity)(struct acpi_iort_node *node,
> > > +				     struct device *dev);
> > >  };
> > >  
> > >  static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
> > >  	.name = "arm-smmu-v3",
> > >  	.iommu_is_coherent = arm_smmu_v3_is_coherent,
> > >  	.iommu_count_resources = arm_smmu_v3_count_resources,
> > > -	.iommu_init_resources = arm_smmu_v3_init_resources
> > > +	.iommu_init_resources = arm_smmu_v3_init_resources,
> > > +	.iommu_set_proximity = arm_smmu_v3_set_proximity
> > >  };
> > >  
> > >  static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
> > >  	.name = "arm-smmu",
> > >  	.iommu_is_coherent = arm_smmu_is_coherent,
> > >  	.iommu_count_resources = arm_smmu_count_resources,
> > > -	.iommu_init_resources = arm_smmu_init_resources
> > > +	.iommu_init_resources = arm_smmu_init_resources,
> > > +	.iommu_set_proximity = NULL
> > >  };
> > >  
> > >  static __init
> > > @@ -1002,6 +1023,9 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
> > >  	if (!pdev)
> > >  		return -ENOMEM;
> > >  
> > > +	if (ops->iommu_set_proximity)
> > > +		ops->iommu_set_proximity(node, &pdev->dev);
> > > +
> > >  	count = ops->iommu_count_resources(node);
> > >  
> > >  	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
> > > -- 
> > > 1.8.1.4
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Robert Richter]

On 04.07.17 11:07:59, Lorenzo Pieralisi wrote:
> On Wed, Jun 28, 2017 at 07:47:50PM +0200, Robert Richter wrote:
> > On 15.06.17 14:46:03, Lorenzo Pieralisi wrote:
> > > On Thu, Jun 08, 2017 at 10:14:19AM +0530, Ganapatrao Kulkarni wrote:
> > > > Add code to parse proximity domain in SMMUv3 IORT table to
> > > > set numa node mapping for smmuv3 devices.
> > > > 
> > > > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > > > ---
> > > >  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
> > > >  1 file changed, 26 insertions(+), 2 deletions(-)
> > > 
> > > I am happy to take this but I want to know what we shall do with
> > > patch 1 and related ACPICA changes first.
> > 
> > The change is now in acpica:
> > 
> >  https://github.com/acpica/acpica/commit/8cadc4fb500e2aa52241e367c87a0f95d9760c58
> > 
> > So we could guard the code with an #ifdef until that patch is pulled
> > in via acpica tree:
> > 
> > > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > > index bba2b59..e804386 100644
> > > > --- a/drivers/acpi/arm64/iort.c
> > > > +++ b/drivers/acpi/arm64/iort.c
> > > > @@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
> > > >  	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
> > > >  }
> > > >  
> > > > +/*
> > > > + * set numa proximity domain for smmuv3 device
> > > > + */
> > > > +static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
> > > > +		struct device *dev)
> > > > +{
> > 
> > #ifdef ACPI_IORT_SMMU_V3_PXM_VALID
> > 
> > > > +	struct acpi_iort_smmu_v3 *smmu;
> > > > +
> > > > +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> > > > +	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> > > > +		set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
> > > > +		pr_info("SMMUV3[%llx] Mapped to Proximity domain %d\n",
> > > > +			smmu->base_address,
> > > > +			smmu->pxm);
> > > > +	}
> > 
> > #endif
> > 
> > > > +}
> > > > +
> > 
> > Could the patch be applied with this change?
> 
> Sorry I was away (and the ARM64 tree was not accepting any new code last
> week anyway), I will send the patch upstream for 4.14 (with your change
> above - or whatever ACPICA guard make it safe - since we will depend on
> ACPICA changes merge timing anyway - they won't be merged this cycle).
> 
> Bad timing, apologies, I hope it is not a burden to carry it for one
> cycle.

No problem. Good to hear this is accepted for 4.14.

Thanks,

-Robert

Re: [PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Hanjun Guo]

Hi Ganapat,

On 2017/6/8 12:44, Ganapatrao Kulkarni wrote:
> Add code to parse proximity domain in SMMUv3 IORT table to
> set numa node mapping for smmuv3 devices.
>
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index bba2b59..e804386 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
>  	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
>  }
>  
> +/*
> + * set numa proximity domain for smmuv3 device
> + */
> +static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
> +		struct device *dev)
> +{
> +	struct acpi_iort_smmu_v3 *smmu;
> +
> +	smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +	if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
> +		set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
                                                          ^^
Will have compile error in !CONFIG_NUMA, I think we need to introduce
a stub function in acpi_numa.h.

Thanks
Hanjun

Re: [PATCH v3 2/2] acpi/iort: numa: Add numa node mapping for smmuv3 devices

[Ganapatrao Kulkarni]

On Sat, Jul 22, 2017 at 8:53 AM, Hanjun Guo <guohanjun@huawei.com> wrote:
> Hi Ganapat,
>
> On 2017/6/8 12:44, Ganapatrao Kulkarni wrote:
>> Add code to parse proximity domain in SMMUv3 IORT table to
>> set numa node mapping for smmuv3 devices.
>>
>> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>> ---
>>  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index bba2b59..e804386 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -882,6 +882,23 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
>>       return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
>>  }
>>
>> +/*
>> + * set numa proximity domain for smmuv3 device
>> + */
>> +static void  __init arm_smmu_v3_set_proximity(struct acpi_iort_node *node,
>> +             struct device *dev)
>> +{
>> +     struct acpi_iort_smmu_v3 *smmu;
>> +
>> +     smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>> +     if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
>> +             set_dev_node(dev, acpi_map_pxm_to_node(smmu->pxm));
>                                                           ^^
> Will have compile error in !CONFIG_NUMA, I think we need to introduce
> a stub function in acpi_numa.h.

thanks Hanjun, i thought it is already there in header file, however,
there is stub for acpi_map_pxm_to_online_node and not for this
function.
it is better to have ifdef. will do it in next version.

>
> Thanks
> Hanjun
>

thanks
Ganapat

Re: [PATCH v3 0/2] acpi/iort, numa: Add numa node mapping for smmuv3 devices

[John Garry]

On 08/06/2017 05:44, Ganapatrao Kulkarni wrote:
> ARM IORT specification(rev. C) has added  provision to define proximity
> domain in SMMUv3 IORT table. Adding required code to parse Proximity
> domain and set numa_node of smmv3 platform devices.
>
> v3:
>   - Addressed Lorenzo Pieralisi comment.
>
> v2:
>   - Changed as per Lorenzo Pieralisi and Hanjun Guo suggestions.
>
> v1:
>   - Initial patch
>
> Ganapatrao Kulkarni (2):
>   acpica: iort: Update SMMUv3 header for proximity domain mapping
>   acpi/iort: numa: Add numa node mapping for smmuv3 devices
>
>  drivers/acpi/arm64/iort.c | 28 ++++++++++++++++++++++++++--
>  include/acpi/actbl2.h     |  4 ++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
>

We'll try and test this in the next day or so.

John