Hi,

On 04/05/2022 12:25, Rahul Singh wrote:
On 3 May 2022, at 4:58 pm, Julien Grall <jul...@xen.org> wrote:
On 27/04/2022 17:14, Rahul Singh wrote:
MAPC_LPI_OFF ITS command error can be reported to software if LPIs are
not enabled before mapping the collection table using MAPC command.
Enable the LPIs using GICR_CTLR.EnableLPIs before mapping the collection
table.
Signed-off-by: Rahul Singh <rahul.si...@arm.com>
---
xen/arch/arm/gic-v3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 3c472ed768..8fb0014b16 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -812,11 +812,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;

gicv3_enable_lpis() is using writel_relaxed(). So in theory, the write may not 
be visible before gicv3_its_setup_collection() send the command.
Agree.

So I think we need to add an smp_wmb() to ensure the ordering with a comment 
explaning why this is necessary.

Or maybe be we can change the writer_relaxed() to writel() that will also work.

-    writel_relaxed(val | GICR_CTLR_ENABLE_LPIS, GICD_RDIST_BASE + GICR_CTLR);
+    writel(val | GICR_CTLR_ENABLE_LPIS, GICD_RDIST_BASE + GICR_CTLR);

writel() guarantees the previous writes are observed before this write. But it would not guarantee that the write will be observed before the ones after.

Also, after further thoughts, I think this wants to be wmb() (system-wide) rather than smp_wmb() (innershearable).

Cheers,

--
Julien Grall

Reply via email to