Hi Julien,
> On 4 May 2022, at 12:59 pm, Julien Grall <jul...@xen.org> wrote:
>
> 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).
>
Ok. I will use the wmb() and will send the next version.
Regards,
Rahul
