[PATCH v2] arm/its: enable LPIs before mapping the collection table

Rahul Singh posted 1 patch 2 weeks, 5 days ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/b19768d1db24123c76e50a410965582502937bb4.1651684160.git.rahul.singh@arm.com
xen/arch/arm/gic-v3.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[PATCH v2] arm/its: enable LPIs before mapping the collection table
Posted by Rahul Singh 2 weeks, 5 days ago
When Xen boots on the platform that implements the GIC 600, ITS
MAPC_LPI_OFF uncorrectable command error issue is observed.

As per the GIC-600 TRM (Revision: r1p6) MAPC_LPI_OFF command error can
be reported if the MAPC command has tried to map a collection to a core
that does not have LPIs enabled. The definition of GICR.EnableLPIs
also suggests enabling the LPIs before sending any ITS command that
involves LPIs

0b0 LPI support is disabled. Any doorbell interrupt generated as a
    result of a write to a virtual LPI register must be discarded,
    and any ITS translation requests or commands involving LPIs in
    this Redistributor are ignored.

0b1 LPI support is enabled.

To fix the MAPC command error issue, enable the LPIs using
GICR_CTLR.EnableLPIs before mapping the collection table.

gicv3_enable_lpis() is using writel_relaxed(), write to the GICR_CTLR
register may not be visible before gicv3_its_setup_collection() send the
MAPC command. Use wmb() after writel_relaxed() to make sure register
write to enable LPIs is visible.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
v2 changes:
- Add more info about issue in commit msg and specification details.
- Use wmb() after writel_relaxed() to make sure register write to enable LPIs
  is visible
---
 xen/arch/arm/gic-v3.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 3c472ed768..64b36cec25 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -696,6 +696,9 @@ static bool gicv3_enable_lpis(void)
     val = readl_relaxed(GICD_RDIST_BASE + GICR_CTLR);
     writel_relaxed(val | GICR_CTLR_ENABLE_LPIS, GICD_RDIST_BASE + GICR_CTLR);
 
+    /* Make sure the GIC has seen the above */
+    wmb();
+
     return true;
 }
 
@@ -812,11 +815,11 @@ static int gicv3_cpu_init(void)
     /* If the host has any ITSes, enable LPIs now. */
     if ( gicv3_its_host_has_its() )
     {
+        if ( !gicv3_enable_lpis() )
+            return -EBUSY;
         ret = gicv3_its_setup_collection(smp_processor_id());
         if ( ret )
             return ret;
-        if ( !gicv3_enable_lpis() )
-            return -EBUSY;
     }
 
     /* Set priority on PPI and SGI interrupts */
-- 
2.25.1
Re: [PATCH v2] arm/its: enable LPIs before mapping the collection table
Posted by Bertrand Marquis 2 weeks, 3 days ago
Hi Rahul,

> On 4 May 2022, at 18:15, Rahul Singh <Rahul.Singh@arm.com> wrote:
> 
> When Xen boots on the platform that implements the GIC 600, ITS
> MAPC_LPI_OFF uncorrectable command error issue is observed.
> 
> As per the GIC-600 TRM (Revision: r1p6) MAPC_LPI_OFF command error can
> be reported if the MAPC command has tried to map a collection to a core
> that does not have LPIs enabled. The definition of GICR.EnableLPIs
> also suggests enabling the LPIs before sending any ITS command that
> involves LPIs
> 
> 0b0 LPI support is disabled. Any doorbell interrupt generated as a
>    result of a write to a virtual LPI register must be discarded,
>    and any ITS translation requests or commands involving LPIs in
>    this Redistributor are ignored.
> 
> 0b1 LPI support is enabled.
> 
> To fix the MAPC command error issue, enable the LPIs using
> GICR_CTLR.EnableLPIs before mapping the collection table.
> 
> gicv3_enable_lpis() is using writel_relaxed(), write to the GICR_CTLR
> register may not be visible before gicv3_its_setup_collection() send the
> MAPC command. Use wmb() after writel_relaxed() to make sure register
> write to enable LPIs is visible.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand
Re: [PATCH v2] arm/its: enable LPIs before mapping the collection table
Posted by Julien Grall 2 weeks ago
On 06/05/2022 12:28, Bertrand Marquis wrote:
>> On 4 May 2022, at 18:15, Rahul Singh <Rahul.Singh@arm.com> wrote:
>>
>> When Xen boots on the platform that implements the GIC 600, ITS
>> MAPC_LPI_OFF uncorrectable command error issue is observed.
>>
>> As per the GIC-600 TRM (Revision: r1p6) MAPC_LPI_OFF command error can
>> be reported if the MAPC command has tried to map a collection to a core
>> that does not have LPIs enabled. The definition of GICR.EnableLPIs
>> also suggests enabling the LPIs before sending any ITS command that
>> involves LPIs
>>
>> 0b0 LPI support is disabled. Any doorbell interrupt generated as a
>>     result of a write to a virtual LPI register must be discarded,
>>     and any ITS translation requests or commands involving LPIs in
>>     this Redistributor are ignored.
>>
>> 0b1 LPI support is enabled.
>>
>> To fix the MAPC command error issue, enable the LPIs using
>> GICR_CTLR.EnableLPIs before mapping the collection table.
>>
>> gicv3_enable_lpis() is using writel_relaxed(), write to the GICR_CTLR
>> register may not be visible before gicv3_its_setup_collection() send the
>> MAPC command. Use wmb() after writel_relaxed() to make sure register
>> write to enable LPIs is visible.
>>
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Committed. Thanks!

Cheers,

-- 
Julien Grall
Re: [PATCH v2] arm/its: enable LPIs before mapping the collection table
Posted by Julien Grall 2 weeks, 4 days ago
Hi Rahul,

On 04/05/2022 18:15, Rahul Singh wrote:
> When Xen boots on the platform that implements the GIC 600, ITS
> MAPC_LPI_OFF uncorrectable command error issue is observed.
> 
> As per the GIC-600 TRM (Revision: r1p6) MAPC_LPI_OFF command error can
> be reported if the MAPC command has tried to map a collection to a core
> that does not have LPIs enabled. The definition of GICR.EnableLPIs
> also suggests enabling the LPIs before sending any ITS command that
> involves LPIs
> 
> 0b0 LPI support is disabled. Any doorbell interrupt generated as a
>      result of a write to a virtual LPI register must be discarded,
>      and any ITS translation requests or commands involving LPIs in
>      this Redistributor are ignored.
> 
> 0b1 LPI support is enabled.
> 
> To fix the MAPC command error issue, enable the LPIs using
> GICR_CTLR.EnableLPIs before mapping the collection table.
> 
> gicv3_enable_lpis() is using writel_relaxed(), write to the GICR_CTLR
> register may not be visible before gicv3_its_setup_collection() send the
> MAPC command. Use wmb() after writel_relaxed() to make sure register
> write to enable LPIs is visible.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall