linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] irqchip: GICv2m Multi-MSI support
@ 2018-02-07 14:32 Marc Zyngier
  2018-02-07 14:32 ` [PATCH 1/2] irqchip/gic-v2m: Add PCI " Marc Zyngier
  2018-02-07 14:32 ` [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region Marc Zyngier
  0 siblings, 2 replies; 14+ messages in thread
From: Marc Zyngier @ 2018-02-07 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Sudeep Holla, Robin Murphy, Liviu Dudau, Lorenzo Pieralisi,
	Thomas Gleixner

It's been recently reported that GICv2m doesn't handle the Multi-MSI
variant (it only deals with a single MSI, or the full blown
MSI-X). For better or worse, Multi-MSI is still a thing, and it'd be
better if we dealt with it.

This tiny series aims at teaching this old trick to GICv2m, together
with an extra patch that enables all the v2m regions on Juno. if
nobody objects to this, I plan to take at least the initial patch
through the irqchip tree, and potentially the DT as well if people
want me to.

Marc Zyngier (1):
  irqchip/gic-v2m: Add PCI Multi-MSI support

Robin Murphy (1):
  arm64: dts: juno: Describe the full GICv2m region

 arch/arm64/boot/dts/arm/juno-base.dtsi | 19 ++++++++++++++
 drivers/irqchip/irq-gic-v2m.c          | 46 ++++++++++++++++------------------
 2 files changed, 41 insertions(+), 24 deletions(-)

-- 
2.14.2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] irqchip/gic-v2m: Add PCI Multi-MSI support
  2018-02-07 14:32 [PATCH 0/2] irqchip: GICv2m Multi-MSI support Marc Zyngier
@ 2018-02-07 14:32 ` Marc Zyngier
  2018-02-07 15:18   ` Liviu Dudau
  2018-02-07 14:32 ` [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region Marc Zyngier
  1 sibling, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2018-02-07 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Sudeep Holla, Robin Murphy, Liviu Dudau, Lorenzo Pieralisi,
	Thomas Gleixner

We'd never implemented Multi-MSI support with GICv2m, because
it is weird and clunky, and you'd think people would rather use
MSI-X.

Turns out there is still plenty of devices out there that rely
on Multi-MSI. Oh well, let's teach that trick to the v2m widget,
it is not a big deal anyway.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v2m.c | 46 +++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 993a8426a453..1ff38aff9f29 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -94,7 +94,7 @@ static struct irq_chip gicv2m_msi_irq_chip = {
 
 static struct msi_domain_info gicv2m_msi_domain_info = {
 	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
-		   MSI_FLAG_PCI_MSIX),
+		   MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
 	.chip	= &gicv2m_msi_irq_chip,
 };
 
@@ -155,18 +155,12 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
 	return 0;
 }
 
-static void gicv2m_unalloc_msi(struct v2m_data *v2m, unsigned int hwirq)
+static void gicv2m_unalloc_msi(struct v2m_data *v2m, unsigned int hwirq,
+			       int nr_irqs)
 {
-	int pos;
-
-	pos = hwirq - v2m->spi_start;
-	if (pos < 0 || pos >= v2m->nr_spis) {
-		pr_err("Failed to teardown msi. Invalid hwirq %d\n", hwirq);
-		return;
-	}
-
 	spin_lock(&v2m_lock);
-	__clear_bit(pos, v2m->bm);
+	bitmap_release_region(v2m->bm, hwirq - v2m->spi_start,
+			      get_count_order(nr_irqs));
 	spin_unlock(&v2m_lock);
 }
 
@@ -174,13 +168,13 @@ static int gicv2m_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 				   unsigned int nr_irqs, void *args)
 {
 	struct v2m_data *v2m = NULL, *tmp;
-	int hwirq, offset, err = 0;
+	int hwirq, offset, i, err = 0;
 
 	spin_lock(&v2m_lock);
 	list_for_each_entry(tmp, &v2m_nodes, entry) {
-		offset = find_first_zero_bit(tmp->bm, tmp->nr_spis);
-		if (offset < tmp->nr_spis) {
-			__set_bit(offset, tmp->bm);
+		offset = bitmap_find_free_region(tmp->bm, tmp->nr_spis,
+						 get_count_order(nr_irqs));
+		if (offset >= 0) {
 			v2m = tmp;
 			break;
 		}
@@ -192,16 +186,21 @@ static int gicv2m_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 
 	hwirq = v2m->spi_start + offset;
 
-	err = gicv2m_irq_gic_domain_alloc(domain, virq, hwirq);
-	if (err) {
-		gicv2m_unalloc_msi(v2m, hwirq);
-		return err;
-	}
+	for (i = 0; i < nr_irqs; i++) {
+		err = gicv2m_irq_gic_domain_alloc(domain, virq + i, hwirq + i);
+		if (err)
+			goto fail;
 
-	irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
-				      &gicv2m_irq_chip, v2m);
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					      &gicv2m_irq_chip, v2m);
+	}
 
 	return 0;
+
+fail:
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+	gicv2m_unalloc_msi(v2m, hwirq, get_count_order(nr_irqs));
+	return err;
 }
 
 static void gicv2m_irq_domain_free(struct irq_domain *domain,
@@ -210,8 +209,7 @@ static void gicv2m_irq_domain_free(struct irq_domain *domain,
 	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
 	struct v2m_data *v2m = irq_data_get_irq_chip_data(d);
 
-	BUG_ON(nr_irqs != 1);
-	gicv2m_unalloc_msi(v2m, d->hwirq);
+	gicv2m_unalloc_msi(v2m, d->hwirq, nr_irqs);
 	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
 }
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-07 14:32 [PATCH 0/2] irqchip: GICv2m Multi-MSI support Marc Zyngier
  2018-02-07 14:32 ` [PATCH 1/2] irqchip/gic-v2m: Add PCI " Marc Zyngier
@ 2018-02-07 14:32 ` Marc Zyngier
  2018-02-12 18:17   ` Sudeep Holla
  2018-02-28 15:48   ` [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames Sudeep Holla
  1 sibling, 2 replies; 14+ messages in thread
From: Marc Zyngier @ 2018-02-07 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Sudeep Holla, Robin Murphy, Liviu Dudau, Lorenzo Pieralisi,
	Thomas Gleixner

From: Robin Murphy <robin.murphy@arm.com>

Juno's GICv2m implementation consists of four frames providing 32
interrupts each. Since it is possible to plug in enough PCIe endpoints
to consume more than 32 MSIs, and the driver already has a bodge to
handle multiple frames, let's expose the other three as well.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index f165f04db0c9..f8088c45b060 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -68,11 +68,30 @@
 		interrupt-controller;
 		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
 		ranges = <0 0 0 0x2c1c0000 0 0x40000>;
+
 		v2m_0: v2m@0 {
 			compatible = "arm,gic-v2m-frame";
 			msi-controller;
 			reg = <0 0 0 0x1000>;
 		};
+
+		v2m@10000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0x10000 0 0x1000>;
+		};
+
+		v2m@20000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0x20000 0 0x1000>;
+		};
+
+		v2m@30000 {
+			compatible = "arm,gic-v2m-frame";
+			msi-controller;
+			reg = <0 0x30000 0 0x1000>;
+		};
 	};
 
 	timer {
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] irqchip/gic-v2m: Add PCI Multi-MSI support
  2018-02-07 14:32 ` [PATCH 1/2] irqchip/gic-v2m: Add PCI " Marc Zyngier
@ 2018-02-07 15:18   ` Liviu Dudau
  0 siblings, 0 replies; 14+ messages in thread
From: Liviu Dudau @ 2018-02-07 15:18 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, Sudeep Holla, Robin Murphy,
	Lorenzo Pieralisi, Thomas Gleixner

On Wed, Feb 07, 2018 at 02:32:54PM +0000, Marc Zyngier wrote:
> We'd never implemented Multi-MSI support with GICv2m, because
> it is weird and clunky, and you'd think people would rather use
> MSI-X.
> 
> Turns out there is still plenty of devices out there that rely
> on Multi-MSI. Oh well, let's teach that trick to the v2m widget,
> it is not a big deal anyway.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Looks good to me. Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/irqchip/irq-gic-v2m.c | 46 +++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> index 993a8426a453..1ff38aff9f29 100644
> --- a/drivers/irqchip/irq-gic-v2m.c
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -94,7 +94,7 @@ static struct irq_chip gicv2m_msi_irq_chip = {
>  
>  static struct msi_domain_info gicv2m_msi_domain_info = {
>  	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> -		   MSI_FLAG_PCI_MSIX),
> +		   MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
>  	.chip	= &gicv2m_msi_irq_chip,
>  };
>  
> @@ -155,18 +155,12 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
>  	return 0;
>  }
>  
> -static void gicv2m_unalloc_msi(struct v2m_data *v2m, unsigned int hwirq)
> +static void gicv2m_unalloc_msi(struct v2m_data *v2m, unsigned int hwirq,
> +			       int nr_irqs)
>  {
> -	int pos;
> -
> -	pos = hwirq - v2m->spi_start;
> -	if (pos < 0 || pos >= v2m->nr_spis) {
> -		pr_err("Failed to teardown msi. Invalid hwirq %d\n", hwirq);
> -		return;
> -	}
> -
>  	spin_lock(&v2m_lock);
> -	__clear_bit(pos, v2m->bm);
> +	bitmap_release_region(v2m->bm, hwirq - v2m->spi_start,
> +			      get_count_order(nr_irqs));
>  	spin_unlock(&v2m_lock);
>  }
>  
> @@ -174,13 +168,13 @@ static int gicv2m_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>  				   unsigned int nr_irqs, void *args)
>  {
>  	struct v2m_data *v2m = NULL, *tmp;
> -	int hwirq, offset, err = 0;
> +	int hwirq, offset, i, err = 0;
>  
>  	spin_lock(&v2m_lock);
>  	list_for_each_entry(tmp, &v2m_nodes, entry) {
> -		offset = find_first_zero_bit(tmp->bm, tmp->nr_spis);
> -		if (offset < tmp->nr_spis) {
> -			__set_bit(offset, tmp->bm);
> +		offset = bitmap_find_free_region(tmp->bm, tmp->nr_spis,
> +						 get_count_order(nr_irqs));
> +		if (offset >= 0) {
>  			v2m = tmp;
>  			break;
>  		}
> @@ -192,16 +186,21 @@ static int gicv2m_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>  
>  	hwirq = v2m->spi_start + offset;
>  
> -	err = gicv2m_irq_gic_domain_alloc(domain, virq, hwirq);
> -	if (err) {
> -		gicv2m_unalloc_msi(v2m, hwirq);
> -		return err;
> -	}
> +	for (i = 0; i < nr_irqs; i++) {
> +		err = gicv2m_irq_gic_domain_alloc(domain, virq + i, hwirq + i);
> +		if (err)
> +			goto fail;
>  
> -	irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> -				      &gicv2m_irq_chip, v2m);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> +					      &gicv2m_irq_chip, v2m);
> +	}
>  
>  	return 0;
> +
> +fail:
> +	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +	gicv2m_unalloc_msi(v2m, hwirq, get_count_order(nr_irqs));
> +	return err;
>  }
>  
>  static void gicv2m_irq_domain_free(struct irq_domain *domain,
> @@ -210,8 +209,7 @@ static void gicv2m_irq_domain_free(struct irq_domain *domain,
>  	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
>  	struct v2m_data *v2m = irq_data_get_irq_chip_data(d);
>  
> -	BUG_ON(nr_irqs != 1);
> -	gicv2m_unalloc_msi(v2m, d->hwirq);
> +	gicv2m_unalloc_msi(v2m, d->hwirq, nr_irqs);
>  	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
>  }
>  
> -- 
> 2.14.2
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-07 14:32 ` [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region Marc Zyngier
@ 2018-02-12 18:17   ` Sudeep Holla
  2018-02-12 18:22     ` Robin Murphy
  2018-02-12 18:27     ` Marc Zyngier
  2018-02-28 15:48   ` [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames Sudeep Holla
  1 sibling, 2 replies; 14+ messages in thread
From: Sudeep Holla @ 2018-02-12 18:17 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel, Liviu Dudau
  Cc: Sudeep Holla, Robin Murphy, Lorenzo Pieralisi, Thomas Gleixner



On 07/02/18 14:32, Marc Zyngier wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> 
> Juno's GICv2m implementation consists of four frames providing 32
> interrupts each. Since it is possible to plug in enough PCIe endpoints
> to consume more than 32 MSIs, and the driver already has a bodge to
> handle multiple frames, let's expose the other three as well.
> 

Change on it own looks good. So if you want to merge via your tree:

Acked-by: Sudeep Holla <sudeep.holla@arm.com>

Let me know if you decide not to take it via your tree and you want me
to send it to arm-soc.

On the side note I just noticed the Juno TRM[1] has 64k for each of
these MSI frames(page 3-24 section 3.3.5 Application memory map summary)

I am not sure if TRM is wrong. This patch is just copying the 4k size
from frame 0 which got added with initial Juno DTS.

-- 
Regards,
Sudeep

[1]
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0515f/DDI0515F_juno_arm_development_platform_soc_trm.pdf

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-12 18:17   ` Sudeep Holla
@ 2018-02-12 18:22     ` Robin Murphy
  2018-02-12 18:26       ` Sudeep Holla
  2018-02-12 18:27     ` Marc Zyngier
  1 sibling, 1 reply; 14+ messages in thread
From: Robin Murphy @ 2018-02-12 18:22 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier, linux-arm-kernel, linux-kernel, Liviu Dudau
  Cc: Lorenzo Pieralisi, Thomas Gleixner

On 12/02/18 18:17, Sudeep Holla wrote:
> 
> 
> On 07/02/18 14:32, Marc Zyngier wrote:
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> Juno's GICv2m implementation consists of four frames providing 32
>> interrupts each. Since it is possible to plug in enough PCIe endpoints
>> to consume more than 32 MSIs, and the driver already has a bodge to
>> handle multiple frames, let's expose the other three as well.
>>
> 
> Change on it own looks good. So if you want to merge via your tree:
> 
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> Let me know if you decide not to take it via your tree and you want me
> to send it to arm-soc.
> 
> On the side note I just noticed the Juno TRM[1] has 64k for each of
> these MSI frames(page 3-24 section 3.3.5 Application memory map summary)
> 
> I am not sure if TRM is wrong. This patch is just copying the 4k size
> from frame 0 which got added with initial Juno DTS.

Depends what your point of view is: The address map allocates 64KB worth 
of space for the device, but the device itself only decodes the first 
4KB of that. My view is that the DT is describing the actual devices, 
not the interconnect routing between them, so there's really no need to 
tell consumers to map known-useless empty space.

Robin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-12 18:22     ` Robin Murphy
@ 2018-02-12 18:26       ` Sudeep Holla
  0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2018-02-12 18:26 UTC (permalink / raw)
  To: Robin Murphy, Marc Zyngier, linux-arm-kernel, linux-kernel, Liviu Dudau
  Cc: Sudeep Holla, Lorenzo Pieralisi, Thomas Gleixner

Hi Robin,

On 12/02/18 18:22, Robin Murphy wrote:
> On 12/02/18 18:17, Sudeep Holla wrote:
>>
>>
>> On 07/02/18 14:32, Marc Zyngier wrote:
>>> From: Robin Murphy <robin.murphy@arm.com>
>>>
>>> Juno's GICv2m implementation consists of four frames providing 32
>>> interrupts each. Since it is possible to plug in enough PCIe endpoints
>>> to consume more than 32 MSIs, and the driver already has a bodge to
>>> handle multiple frames, let's expose the other three as well.
>>>
>>
>> Change on it own looks good. So if you want to merge via your tree:
>>
>> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
>>
>> Let me know if you decide not to take it via your tree and you want me
>> to send it to arm-soc.
>>
>> On the side note I just noticed the Juno TRM[1] has 64k for each of
>> these MSI frames(page 3-24 section 3.3.5 Application memory map summary)
>>
>> I am not sure if TRM is wrong. This patch is just copying the 4k size
>> from frame 0 which got added with initial Juno DTS.
> 
> Depends what your point of view is: The address map allocates 64KB worth
> of space for the device, but the device itself only decodes the first
> 4KB of that. My view is that the DT is describing the actual devices,
> not the interconnect routing between them, so there's really no need to
> tell consumers to map known-useless empty space.
> 

Thanks for the clarification.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-12 18:17   ` Sudeep Holla
  2018-02-12 18:22     ` Robin Murphy
@ 2018-02-12 18:27     ` Marc Zyngier
  2018-02-12 19:17       ` Robin Murphy
  1 sibling, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2018-02-12 18:27 UTC (permalink / raw)
  To: Sudeep Holla, linux-arm-kernel, linux-kernel, Liviu Dudau
  Cc: Robin Murphy, Lorenzo Pieralisi, Thomas Gleixner

Hi Sudeep,

On 12/02/18 18:17, Sudeep Holla wrote:
> 
> 
> On 07/02/18 14:32, Marc Zyngier wrote:
>> From: Robin Murphy <robin.murphy@arm.com>
>>
>> Juno's GICv2m implementation consists of four frames providing 32
>> interrupts each. Since it is possible to plug in enough PCIe endpoints
>> to consume more than 32 MSIs, and the driver already has a bodge to
>> handle multiple frames, let's expose the other three as well.
>>
> 
> Change on it own looks good. So if you want to merge via your tree:
> 
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> Let me know if you decide not to take it via your tree and you want me
> to send it to arm-soc.

If this would usually go via arm-soc, feel free to take it via this
route. I'll drop the patch from my tree.

> On the side note I just noticed the Juno TRM[1] has 64k for each of
> these MSI frames(page 3-24 section 3.3.5 Application memory map summary)
> 
> I am not sure if TRM is wrong. This patch is just copying the 4k size
> from frame 0 which got added with initial Juno DTS.

I can't see why the TRM would be wrong. This is actually consistent with
the expected practice of aligning all devices on a 64kB boundary and
size so that you don't get any nasty surprise when passing the device to
a VM (*cough* GIC400 *cough*).

Robin, any chance you could check this?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-12 18:27     ` Marc Zyngier
@ 2018-02-12 19:17       ` Robin Murphy
  2018-02-28 15:48         ` Sudeep Holla
  0 siblings, 1 reply; 14+ messages in thread
From: Robin Murphy @ 2018-02-12 19:17 UTC (permalink / raw)
  To: Marc Zyngier, Sudeep Holla, linux-arm-kernel, linux-kernel, Liviu Dudau
  Cc: Lorenzo Pieralisi, Thomas Gleixner

On 12/02/18 18:27, Marc Zyngier wrote:
> Hi Sudeep,
> 
> On 12/02/18 18:17, Sudeep Holla wrote:
>>
>>
>> On 07/02/18 14:32, Marc Zyngier wrote:
>>> From: Robin Murphy <robin.murphy@arm.com>
>>>
>>> Juno's GICv2m implementation consists of four frames providing 32
>>> interrupts each. Since it is possible to plug in enough PCIe endpoints
>>> to consume more than 32 MSIs, and the driver already has a bodge to
>>> handle multiple frames, let's expose the other three as well.
>>>
>>
>> Change on it own looks good. So if you want to merge via your tree:
>>
>> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
>>
>> Let me know if you decide not to take it via your tree and you want me
>> to send it to arm-soc.
> 
> If this would usually go via arm-soc, feel free to take it via this
> route. I'll drop the patch from my tree.
> 
>> On the side note I just noticed the Juno TRM[1] has 64k for each of
>> these MSI frames(page 3-24 section 3.3.5 Application memory map summary)
>>
>> I am not sure if TRM is wrong. This patch is just copying the 4k size
>> from frame 0 which got added with initial Juno DTS.
> 
> I can't see why the TRM would be wrong. This is actually consistent with
> the expected practice of aligning all devices on a 64kB boundary and
> size so that you don't get any nasty surprise when passing the device to
> a VM (*cough* GIC400 *cough*).
> 
> Robin, any chance you could check this?

Well, the engineering spec for the v2m widget does claim that only the 
bottom 12 bits of AxADDR are used, but on the other hand it also implies 
that the "real" endpoint here is a single monolithic block of 4 such 
widgets, so a third truth is that there is only a single 256KB region...

As usual, I've completely forgotten about virtualisation when it comes 
to hardware :) On reflection I do of course appreciate that whilst 60KB 
of RAZ/WI space isn't significant in terms of "a device", it is rather 
more so in terms of "not a device" - if the only reasonable way to 
communicate that is to describe the v2m devices each owning 64KB, then 
I'm quite happy for you to fix up the patch that way if you want.

Robin.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region
  2018-02-12 19:17       ` Robin Murphy
@ 2018-02-28 15:48         ` Sudeep Holla
  0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2018-02-28 15:48 UTC (permalink / raw)
  To: Robin Murphy, Marc Zyngier, linux-arm-kernel, linux-kernel, Liviu Dudau
  Cc: Sudeep Holla, Lorenzo Pieralisi, Thomas Gleixner



On 12/02/18 19:17, Robin Murphy wrote:
> On 12/02/18 18:27, Marc Zyngier wrote:
>> Hi Sudeep,
>>
>> On 12/02/18 18:17, Sudeep Holla wrote:
>>>
>>>
>>> On 07/02/18 14:32, Marc Zyngier wrote:
>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>
>>>> Juno's GICv2m implementation consists of four frames providing 32
>>>> interrupts each. Since it is possible to plug in enough PCIe endpoints
>>>> to consume more than 32 MSIs, and the driver already has a bodge to
>>>> handle multiple frames, let's expose the other three as well.
>>>>
>>>
>>> Change on it own looks good. So if you want to merge via your tree:
>>>
>>> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
>>>
>>> Let me know if you decide not to take it via your tree and you want me
>>> to send it to arm-soc.
>>
>> If this would usually go via arm-soc, feel free to take it via this
>> route. I'll drop the patch from my tree.
>>
>>> On the side note I just noticed the Juno TRM[1] has 64k for each of
>>> these MSI frames(page 3-24 section 3.3.5 Application memory map summary)
>>>
>>> I am not sure if TRM is wrong. This patch is just copying the 4k size
>>> from frame 0 which got added with initial Juno DTS.
>>
>> I can't see why the TRM would be wrong. This is actually consistent with
>> the expected practice of aligning all devices on a 64kB boundary and
>> size so that you don't get any nasty surprise when passing the device to
>> a VM (*cough* GIC400 *cough*).
>>
>> Robin, any chance you could check this?
> 
> Well, the engineering spec for the v2m widget does claim that only the
> bottom 12 bits of AxADDR are used, but on the other hand it also implies
> that the "real" endpoint here is a single monolithic block of 4 such
> widgets, so a third truth is that there is only a single 256KB region...
> 
> As usual, I've completely forgotten about virtualisation when it comes
> to hardware :) On reflection I do of course appreciate that whilst 60KB
> of RAZ/WI space isn't significant in terms of "a device", it is rather
> more so in terms of "not a device" - if the only reasonable way to
> communicate that is to describe the v2m devices each owning 64KB, then
> I'm quite happy for you to fix up the patch that way if you want.
> 

I have applied this patch as is [1] and added another patch to fix the
size to 64kB for all the frames on top as per Juno TRM. Sorry, I forgot
to send that out, will do that shortly.

-- 
Regards,
Sudeep

[1] https://git.kernel.org/sudeep.holla/linux/h/for-next/juno

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames
  2018-02-07 14:32 ` [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region Marc Zyngier
  2018-02-12 18:17   ` Sudeep Holla
@ 2018-02-28 15:48   ` Sudeep Holla
  2018-02-28 15:51     ` Robin Murphy
  2018-02-28 16:54     ` Marc Zyngier
  1 sibling, 2 replies; 14+ messages in thread
From: Sudeep Holla @ 2018-02-28 15:48 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Sudeep Holla, linux-kernel, Liviu Dudau, Robin Murphy, Marc Zyngier

Currently the size of GICv2m MSI frames are listed as 4kB while the
Juno TRM specifies 64kB for each of these MSI frames.

Though the devices connected themself might just use the first 4kB,
to be consistent with the genaral practice of 64kB boundary alignment
to all the devices, lets keep the size as 64kB. This might also help
in avoiding any surprise when passing the device to a VM.

This patch increase the size of each GICv2m MSI frames from 4kB to 64kB
as per the specification.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index f8088c45b060..eb749c50a736 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -72,25 +72,25 @@
 		v2m_0: v2m@0 {
 			compatible = "arm,gic-v2m-frame";
 			msi-controller;
-			reg = <0 0 0 0x1000>;
+			reg = <0 0 0 0x10000>;
 		};
 
 		v2m@10000 {
 			compatible = "arm,gic-v2m-frame";
 			msi-controller;
-			reg = <0 0x10000 0 0x1000>;
+			reg = <0 0x10000 0 0x10000>;
 		};
 
 		v2m@20000 {
 			compatible = "arm,gic-v2m-frame";
 			msi-controller;
-			reg = <0 0x20000 0 0x1000>;
+			reg = <0 0x20000 0 0x10000>;
 		};
 
 		v2m@30000 {
 			compatible = "arm,gic-v2m-frame";
 			msi-controller;
-			reg = <0 0x30000 0 0x1000>;
+			reg = <0 0x30000 0 0x10000>;
 		};
 	};
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames
  2018-02-28 15:48   ` [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames Sudeep Holla
@ 2018-02-28 15:51     ` Robin Murphy
  2018-02-28 15:58       ` Sudeep Holla
  2018-02-28 16:54     ` Marc Zyngier
  1 sibling, 1 reply; 14+ messages in thread
From: Robin Murphy @ 2018-02-28 15:51 UTC (permalink / raw)
  To: Sudeep Holla, linux-arm-kernel; +Cc: linux-kernel, Liviu Dudau, Marc Zyngier

Hi Sudeep,

Cheers for sorting it out...

On 28/02/18 15:48, Sudeep Holla wrote:
> Currently the size of GICv2m MSI frames are listed as 4kB while the
> Juno TRM specifies 64kB for each of these MSI frames.
> 
> Though the devices connected themself might just use the first 4kB,

s/themself/themselves/

> to be consistent with the genaral practice of 64kB boundary alignment

s/genaral/general/

> to all the devices, lets keep the size as 64kB. This might also help
> in avoiding any surprise when passing the device to a VM.
> 
> This patch increase the size of each GICv2m MSI frames from 4kB to 64kB
> as per the specification.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   arch/arm64/boot/dts/arm/juno-base.dtsi | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index f8088c45b060..eb749c50a736 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -72,25 +72,25 @@
>   		v2m_0: v2m@0 {
>   			compatible = "arm,gic-v2m-frame";
>   			msi-controller;
> -			reg = <0 0 0 0x1000>;
> +			reg = <0 0 0 0x10000>;
>   		};
>   
>   		v2m@10000 {
>   			compatible = "arm,gic-v2m-frame";
>   			msi-controller;
> -			reg = <0 0x10000 0 0x1000>;
> +			reg = <0 0x10000 0 0x10000>;
>   		};
>   
>   		v2m@20000 {
>   			compatible = "arm,gic-v2m-frame";
>   			msi-controller;
> -			reg = <0 0x20000 0 0x1000>;
> +			reg = <0 0x20000 0 0x10000>;
>   		};
>   
>   		v2m@30000 {
>   			compatible = "arm,gic-v2m-frame";
>   			msi-controller;
> -			reg = <0 0x30000 0 0x1000>;
> +			reg = <0 0x30000 0 0x10000>;
>   		};
>   	};
>   
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames
  2018-02-28 15:51     ` Robin Murphy
@ 2018-02-28 15:58       ` Sudeep Holla
  0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2018-02-28 15:58 UTC (permalink / raw)
  To: Robin Murphy, linux-arm-kernel
  Cc: Sudeep Holla, linux-kernel, Liviu Dudau, Marc Zyngier



On 28/02/18 15:51, Robin Murphy wrote:
> Hi Sudeep,
> 
> Cheers for sorting it out...
> 
> On 28/02/18 15:48, Sudeep Holla wrote:
>> Currently the size of GICv2m MSI frames are listed as 4kB while the
>> Juno TRM specifies 64kB for each of these MSI frames.
>>
>> Though the devices connected themself might just use the first 4kB,
> 
> s/themself/themselves/
> 
>> to be consistent with the genaral practice of 64kB boundary alignment
> 
> s/genaral/general/
> 

Fixed those now.

>> to all the devices, lets keep the size as 64kB. This might also help
>> in avoiding any surprise when passing the device to a VM.
>>
>> This patch increase the size of each GICv2m MSI frames from 4kB to 64kB
>> as per the specification.
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> 

Thanks.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames
  2018-02-28 15:48   ` [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames Sudeep Holla
  2018-02-28 15:51     ` Robin Murphy
@ 2018-02-28 16:54     ` Marc Zyngier
  1 sibling, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2018-02-28 16:54 UTC (permalink / raw)
  To: Sudeep Holla; +Cc: linux-arm-kernel, linux-kernel, Liviu Dudau, Robin Murphy

On Wed, 28 Feb 2018 15:48:55 +0000,
sudeep holla wrote:
> 
> Currently the size of GICv2m MSI frames are listed as 4kB while the
> Juno TRM specifies 64kB for each of these MSI frames.
> 
> Though the devices connected themself might just use the first 4kB,
> to be consistent with the genaral practice of 64kB boundary alignment
> to all the devices, lets keep the size as 64kB. This might also help
> in avoiding any surprise when passing the device to a VM.
> 
> This patch increase the size of each GICv2m MSI frames from 4kB to 64kB
> as per the specification.
> 
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-02-28 16:55 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 14:32 [PATCH 0/2] irqchip: GICv2m Multi-MSI support Marc Zyngier
2018-02-07 14:32 ` [PATCH 1/2] irqchip/gic-v2m: Add PCI " Marc Zyngier
2018-02-07 15:18   ` Liviu Dudau
2018-02-07 14:32 ` [PATCH 2/2] arm64: dts: juno: Describe the full GICv2m region Marc Zyngier
2018-02-12 18:17   ` Sudeep Holla
2018-02-12 18:22     ` Robin Murphy
2018-02-12 18:26       ` Sudeep Holla
2018-02-12 18:27     ` Marc Zyngier
2018-02-12 19:17       ` Robin Murphy
2018-02-28 15:48         ` Sudeep Holla
2018-02-28 15:48   ` [PATCH] arm64: dts: juno: fix size of GICv2m MSI frames Sudeep Holla
2018-02-28 15:51     ` Robin Murphy
2018-02-28 15:58       ` Sudeep Holla
2018-02-28 16:54     ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).