Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[Marc Zyngier]

On 08/02/18 00:00, dbasehore . wrote:
> On Wed, Feb 7, 2018 at 3:22 PM, Brian Norris  wrote:
>> Hi Marc,
>>
>> I'm really not an expert on this, so take my observations with a large
>> grain of salt:
>>
>> On Wed, Feb 07, 2018 at 08:46:42AM +, Marc Zyngier wrote:
>>> On 07/02/18 01:41, Derek Basehore wrote:
 This adds functionality to resend the MAPC command to an ITS node on
 resume. If the ITS is powered down during suspend and the collections
 are not backed by memory, the ITS will lose that state. This just sets
 up the known state for the collections after the ITS is restored.

 This is enabled via the reset-on-suspend flag in the DTS for an ITS
 that has a non-zero number of collections stored in it.

 Signed-off-by: Derek Basehore 
 ---
  drivers/irqchip/irq-gic-v3-its.c   | 80 
 --
  include/linux/irqchip/arm-gic-v3.h |  1 +
  2 files changed, 43 insertions(+), 38 deletions(-)

 diff --git a/drivers/irqchip/irq-gic-v3-its.c 
 b/drivers/irqchip/irq-gic-v3-its.c
 index 5e63635e2a7b..dd6cd6e68ed0 100644
 --- a/drivers/irqchip/irq-gic-v3-its.c
 +++ b/drivers/irqchip/irq-gic-v3-its.c
 @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
 dsb(sy);
  }

 -static void its_cpu_init_collection(void)
 +static void its_cpu_init_collection(struct its_node *its)
>>
>> ...
>>
 @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
 its_write_baser(its, baser, baser->val);
 }
 writel_relaxed(its->ctlr_save, base + GITS_CTLR);
 +
 +   if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 
 0)
 +   its_cpu_init_collection(its);
>>>
>>> This isn't correct. Think of a system where half the collections are in
>>> HW, and the other half memory based (nothing in the spec forbids this).
>>> You must evaluate the CID of each collection and replay the MAPC *only*
>>> if it falls into the range [0..HCC-1]. The memory-based collections are
>>> already mapped, and remapping an already mapped collection requires
>>> extra care (see MAPC and the UNPREDICTABLE behaviour when V=1), so don't
>>> go there.
>>
>> IIUC, this is only run on CPU0 (it's in syscore resume), so implicitly,
>> CID is 0. Thus, the current condition is already doing what you ask:
>>
>> HCC > 0 == CID
>>
>> which is equivalent to:
>>
>> HCC - 1 >= CID
>>
>> Or should we really double check what CPU we're running on?
> 
> There seems to be the edge case where you hotplug CPU 0 before
> suspending. In that case, I believe you're on the lowest number CPU
> left?

I don't think the core code makes any guarantee in that respect. This is
probably what happens in practice, but I wouldn't bet anything on this
being set in stone.

> It seems that all of the CPUs that are disabled have the ITS
> reinitialized from scratch via enable_nonboot_cpus(). This code runs
> on only the CPU that firmware resumes with. If that CPU isn't CPU 0
> for whatever reason, we need to make sure that it's processor ID is
> less than HCC.

Exactly, thanks for putting it better than I initially did.

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

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[Marc Zyngier]

On 08/02/18 00:00, dbasehore . wrote:
> On Wed, Feb 7, 2018 at 3:22 PM, Brian Norris  wrote:
>> Hi Marc,
>>
>> I'm really not an expert on this, so take my observations with a large
>> grain of salt:
>>
>> On Wed, Feb 07, 2018 at 08:46:42AM +, Marc Zyngier wrote:
>>> On 07/02/18 01:41, Derek Basehore wrote:
 This adds functionality to resend the MAPC command to an ITS node on
 resume. If the ITS is powered down during suspend and the collections
 are not backed by memory, the ITS will lose that state. This just sets
 up the known state for the collections after the ITS is restored.

 This is enabled via the reset-on-suspend flag in the DTS for an ITS
 that has a non-zero number of collections stored in it.

 Signed-off-by: Derek Basehore 
 ---
  drivers/irqchip/irq-gic-v3-its.c   | 80 
 --
  include/linux/irqchip/arm-gic-v3.h |  1 +
  2 files changed, 43 insertions(+), 38 deletions(-)

 diff --git a/drivers/irqchip/irq-gic-v3-its.c 
 b/drivers/irqchip/irq-gic-v3-its.c
 index 5e63635e2a7b..dd6cd6e68ed0 100644
 --- a/drivers/irqchip/irq-gic-v3-its.c
 +++ b/drivers/irqchip/irq-gic-v3-its.c
 @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
 dsb(sy);
  }

 -static void its_cpu_init_collection(void)
 +static void its_cpu_init_collection(struct its_node *its)
>>
>> ...
>>
 @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
 its_write_baser(its, baser, baser->val);
 }
 writel_relaxed(its->ctlr_save, base + GITS_CTLR);
 +
 +   if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 
 0)
 +   its_cpu_init_collection(its);
>>>
>>> This isn't correct. Think of a system where half the collections are in
>>> HW, and the other half memory based (nothing in the spec forbids this).
>>> You must evaluate the CID of each collection and replay the MAPC *only*
>>> if it falls into the range [0..HCC-1]. The memory-based collections are
>>> already mapped, and remapping an already mapped collection requires
>>> extra care (see MAPC and the UNPREDICTABLE behaviour when V=1), so don't
>>> go there.
>>
>> IIUC, this is only run on CPU0 (it's in syscore resume), so implicitly,
>> CID is 0. Thus, the current condition is already doing what you ask:
>>
>> HCC > 0 == CID
>>
>> which is equivalent to:
>>
>> HCC - 1 >= CID
>>
>> Or should we really double check what CPU we're running on?
> 
> There seems to be the edge case where you hotplug CPU 0 before
> suspending. In that case, I believe you're on the lowest number CPU
> left?

I don't think the core code makes any guarantee in that respect. This is
probably what happens in practice, but I wouldn't bet anything on this
being set in stone.

> It seems that all of the CPUs that are disabled have the ITS
> reinitialized from scratch via enable_nonboot_cpus(). This code runs
> on only the CPU that firmware resumes with. If that CPU isn't CPU 0
> for whatever reason, we need to make sure that it's processor ID is
> less than HCC.

Exactly, thanks for putting it better than I initially did.

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

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[dbasehore .]

On Wed, Feb 7, 2018 at 3:22 PM, Brian Norris  wrote:
> Hi Marc,
>
> I'm really not an expert on this, so take my observations with a large
> grain of salt:
>
> On Wed, Feb 07, 2018 at 08:46:42AM +, Marc Zyngier wrote:
>> On 07/02/18 01:41, Derek Basehore wrote:
>> > This adds functionality to resend the MAPC command to an ITS node on
>> > resume. If the ITS is powered down during suspend and the collections
>> > are not backed by memory, the ITS will lose that state. This just sets
>> > up the known state for the collections after the ITS is restored.
>> >
>> > This is enabled via the reset-on-suspend flag in the DTS for an ITS
>> > that has a non-zero number of collections stored in it.
>> >
>> > Signed-off-by: Derek Basehore 
>> > ---
>> >  drivers/irqchip/irq-gic-v3-its.c   | 80 
>> > --
>> >  include/linux/irqchip/arm-gic-v3.h |  1 +
>> >  2 files changed, 43 insertions(+), 38 deletions(-)
>> >
>> > diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> > b/drivers/irqchip/irq-gic-v3-its.c
>> > index 5e63635e2a7b..dd6cd6e68ed0 100644
>> > --- a/drivers/irqchip/irq-gic-v3-its.c
>> > +++ b/drivers/irqchip/irq-gic-v3-its.c
>> > @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
>> > dsb(sy);
>> >  }
>> >
>> > -static void its_cpu_init_collection(void)
>> > +static void its_cpu_init_collection(struct its_node *its)
>
> ...
>
>> > @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
>> > its_write_baser(its, baser, baser->val);
>> > }
>> > writel_relaxed(its->ctlr_save, base + GITS_CTLR);
>> > +
>> > +   if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 
>> > 0)
>> > +   its_cpu_init_collection(its);
>>
>> This isn't correct. Think of a system where half the collections are in
>> HW, and the other half memory based (nothing in the spec forbids this).
>> You must evaluate the CID of each collection and replay the MAPC *only*
>> if it falls into the range [0..HCC-1]. The memory-based collections are
>> already mapped, and remapping an already mapped collection requires
>> extra care (see MAPC and the UNPREDICTABLE behaviour when V=1), so don't
>> go there.
>
> IIUC, this is only run on CPU0 (it's in syscore resume), so implicitly,
> CID is 0. Thus, the current condition is already doing what you ask:
>
> HCC > 0 == CID
>
> which is equivalent to:
>
> HCC - 1 >= CID
>
> Or should we really double check what CPU we're running on?

There seems to be the edge case where you hotplug CPU 0 before
suspending. In that case, I believe you're on the lowest number CPU
left?

It seems that all of the CPUs that are disabled have the ITS
reinitialized from scratch via enable_nonboot_cpus(). This code runs
on only the CPU that firmware resumes with. If that CPU isn't CPU 0
for whatever reason, we need to make sure that it's processor ID is
less than HCC.

>
> Brian

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[dbasehore .]

On Wed, Feb 7, 2018 at 3:22 PM, Brian Norris  wrote:
> Hi Marc,
>
> I'm really not an expert on this, so take my observations with a large
> grain of salt:
>
> On Wed, Feb 07, 2018 at 08:46:42AM +, Marc Zyngier wrote:
>> On 07/02/18 01:41, Derek Basehore wrote:
>> > This adds functionality to resend the MAPC command to an ITS node on
>> > resume. If the ITS is powered down during suspend and the collections
>> > are not backed by memory, the ITS will lose that state. This just sets
>> > up the known state for the collections after the ITS is restored.
>> >
>> > This is enabled via the reset-on-suspend flag in the DTS for an ITS
>> > that has a non-zero number of collections stored in it.
>> >
>> > Signed-off-by: Derek Basehore 
>> > ---
>> >  drivers/irqchip/irq-gic-v3-its.c   | 80 
>> > --
>> >  include/linux/irqchip/arm-gic-v3.h |  1 +
>> >  2 files changed, 43 insertions(+), 38 deletions(-)
>> >
>> > diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> > b/drivers/irqchip/irq-gic-v3-its.c
>> > index 5e63635e2a7b..dd6cd6e68ed0 100644
>> > --- a/drivers/irqchip/irq-gic-v3-its.c
>> > +++ b/drivers/irqchip/irq-gic-v3-its.c
>> > @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
>> > dsb(sy);
>> >  }
>> >
>> > -static void its_cpu_init_collection(void)
>> > +static void its_cpu_init_collection(struct its_node *its)
>
> ...
>
>> > @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
>> > its_write_baser(its, baser, baser->val);
>> > }
>> > writel_relaxed(its->ctlr_save, base + GITS_CTLR);
>> > +
>> > +   if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 
>> > 0)
>> > +   its_cpu_init_collection(its);
>>
>> This isn't correct. Think of a system where half the collections are in
>> HW, and the other half memory based (nothing in the spec forbids this).
>> You must evaluate the CID of each collection and replay the MAPC *only*
>> if it falls into the range [0..HCC-1]. The memory-based collections are
>> already mapped, and remapping an already mapped collection requires
>> extra care (see MAPC and the UNPREDICTABLE behaviour when V=1), so don't
>> go there.
>
> IIUC, this is only run on CPU0 (it's in syscore resume), so implicitly,
> CID is 0. Thus, the current condition is already doing what you ask:
>
> HCC > 0 == CID
>
> which is equivalent to:
>
> HCC - 1 >= CID
>
> Or should we really double check what CPU we're running on?

There seems to be the edge case where you hotplug CPU 0 before
suspending. In that case, I believe you're on the lowest number CPU
left?

It seems that all of the CPUs that are disabled have the ITS
reinitialized from scratch via enable_nonboot_cpus(). This code runs
on only the CPU that firmware resumes with. If that CPU isn't CPU 0
for whatever reason, we need to make sure that it's processor ID is
less than HCC.

>
> Brian

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[Brian Norris]

Hi Marc,

I'm really not an expert on this, so take my observations with a large
grain of salt:

On Wed, Feb 07, 2018 at 08:46:42AM +, Marc Zyngier wrote:
> On 07/02/18 01:41, Derek Basehore wrote:
> > This adds functionality to resend the MAPC command to an ITS node on
> > resume. If the ITS is powered down during suspend and the collections
> > are not backed by memory, the ITS will lose that state. This just sets
> > up the known state for the collections after the ITS is restored.
> > 
> > This is enabled via the reset-on-suspend flag in the DTS for an ITS
> > that has a non-zero number of collections stored in it.
> > 
> > Signed-off-by: Derek Basehore 
> > ---
> >  drivers/irqchip/irq-gic-v3-its.c   | 80 
> > --
> >  include/linux/irqchip/arm-gic-v3.h |  1 +
> >  2 files changed, 43 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> > b/drivers/irqchip/irq-gic-v3-its.c
> > index 5e63635e2a7b..dd6cd6e68ed0 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
> > dsb(sy);
> >  }
> >  
> > -static void its_cpu_init_collection(void)
> > +static void its_cpu_init_collection(struct its_node *its)

...

> > @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
> > its_write_baser(its, baser, baser->val);
> > }
> > writel_relaxed(its->ctlr_save, base + GITS_CTLR);
> > +
> > +   if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 0)
> > +   its_cpu_init_collection(its);
> 
> This isn't correct. Think of a system where half the collections are in
> HW, and the other half memory based (nothing in the spec forbids this).
> You must evaluate the CID of each collection and replay the MAPC *only*
> if it falls into the range [0..HCC-1]. The memory-based collections are
> already mapped, and remapping an already mapped collection requires
> extra care (see MAPC and the UNPREDICTABLE behaviour when V=1), so don't
> go there.

IIUC, this is only run on CPU0 (it's in syscore resume), so implicitly,
CID is 0. Thus, the current condition is already doing what you ask:

HCC > 0 == CID

which is equivalent to:

HCC - 1 >= CID

Or should we really double check what CPU we're running on?

Brian

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[Brian Norris]

Hi Marc,

I'm really not an expert on this, so take my observations with a large
grain of salt:

On Wed, Feb 07, 2018 at 08:46:42AM +, Marc Zyngier wrote:
> On 07/02/18 01:41, Derek Basehore wrote:
> > This adds functionality to resend the MAPC command to an ITS node on
> > resume. If the ITS is powered down during suspend and the collections
> > are not backed by memory, the ITS will lose that state. This just sets
> > up the known state for the collections after the ITS is restored.
> > 
> > This is enabled via the reset-on-suspend flag in the DTS for an ITS
> > that has a non-zero number of collections stored in it.
> > 
> > Signed-off-by: Derek Basehore 
> > ---
> >  drivers/irqchip/irq-gic-v3-its.c   | 80 
> > --
> >  include/linux/irqchip/arm-gic-v3.h |  1 +
> >  2 files changed, 43 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> > b/drivers/irqchip/irq-gic-v3-its.c
> > index 5e63635e2a7b..dd6cd6e68ed0 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
> > dsb(sy);
> >  }
> >  
> > -static void its_cpu_init_collection(void)
> > +static void its_cpu_init_collection(struct its_node *its)

...

> > @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
> > its_write_baser(its, baser, baser->val);
> > }
> > writel_relaxed(its->ctlr_save, base + GITS_CTLR);
> > +
> > +   if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 0)
> > +   its_cpu_init_collection(its);
> 
> This isn't correct. Think of a system where half the collections are in
> HW, and the other half memory based (nothing in the spec forbids this).
> You must evaluate the CID of each collection and replay the MAPC *only*
> if it falls into the range [0..HCC-1]. The memory-based collections are
> already mapped, and remapping an already mapped collection requires
> extra care (see MAPC and the UNPREDICTABLE behaviour when V=1), so don't
> go there.

IIUC, this is only run on CPU0 (it's in syscore resume), so implicitly,
CID is 0. Thus, the current condition is already doing what you ask:

HCC > 0 == CID

which is equivalent to:

HCC - 1 >= CID

Or should we really double check what CPU we're running on?

Brian

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[Marc Zyngier]

On 07/02/18 01:41, Derek Basehore wrote:
> This adds functionality to resend the MAPC command to an ITS node on
> resume. If the ITS is powered down during suspend and the collections
> are not backed by memory, the ITS will lose that state. This just sets
> up the known state for the collections after the ITS is restored.
> 
> This is enabled via the reset-on-suspend flag in the DTS for an ITS
> that has a non-zero number of collections stored in it.
> 
> Signed-off-by: Derek Basehore 
> ---
>  drivers/irqchip/irq-gic-v3-its.c   | 80 
> --
>  include/linux/irqchip/arm-gic-v3.h |  1 +
>  2 files changed, 43 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index 5e63635e2a7b..dd6cd6e68ed0 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
>   dsb(sy);
>  }
>  
> -static void its_cpu_init_collection(void)
> +static void its_cpu_init_collection(struct its_node *its)
>  {
> - struct its_node *its;
> - int cpu;
> -
> - spin_lock(_lock);
> - cpu = smp_processor_id();
> -
> - list_for_each_entry(its, _nodes, entry) {
> - u64 target;
> + int cpu = smp_processor_id();
> + u64 target;
>  
> - /* avoid cross node collections and its mapping */
> - if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
> - struct device_node *cpu_node;
> + /* avoid cross node collections and its mapping */
> + if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
> + struct device_node *cpu_node;
>  
> - cpu_node = of_get_cpu_node(cpu, NULL);
> - if (its->numa_node != NUMA_NO_NODE &&
> - its->numa_node != of_node_to_nid(cpu_node))
> - continue;
> - }
> + cpu_node = of_get_cpu_node(cpu, NULL);
> + if (its->numa_node != NUMA_NO_NODE &&
> + its->numa_node != of_node_to_nid(cpu_node))
> + return;
> + }
>  
> + /*
> +  * We now have to bind each collection to its target
> +  * redistributor.
> +  */
> + if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) {
>   /*
> -  * We now have to bind each collection to its target
> +  * This ITS wants the physical address of the
>* redistributor.
>*/
> - if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) {
> - /*
> -  * This ITS wants the physical address of the
> -  * redistributor.
> -  */
> - target = gic_data_rdist()->phys_base;
> - } else {
> - /*
> -  * This ITS wants a linear CPU number.
> -  */
> - target = gic_read_typer(gic_data_rdist_rd_base() + 
> GICR_TYPER);
> - target = GICR_TYPER_CPU_NUMBER(target) << 16;
> - }
> + target = gic_data_rdist()->phys_base;
> + } else {
> + /* This ITS wants a linear CPU number. */
> + target = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
> + target = GICR_TYPER_CPU_NUMBER(target) << 16;
> + }
>  
> - /* Perform collection mapping */
> - its->collections[cpu].target_address = target;
> - its->collections[cpu].col_id = cpu;
> + /* Perform collection mapping */
> + its->collections[cpu].target_address = target;
> + its->collections[cpu].col_id = cpu;
>  
> - its_send_mapc(its, >collections[cpu], 1);
> - its_send_invall(its, >collections[cpu]);
> - }
> + its_send_mapc(its, >collections[cpu], 1);
> + its_send_invall(its, >collections[cpu]);
> +}
> +
> +static void its_cpu_init_collections(void)
> +{
> + struct its_node *its;
> +
> + spin_lock(_lock);
> +
> + list_for_each_entry(its, _nodes, entry)
> + its_cpu_init_collection(its);
>  
>   spin_unlock(_lock);
>  }
> @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
>   its_write_baser(its, baser, baser->val);
>   }
>   writel_relaxed(its->ctlr_save, base + GITS_CTLR);
> +
> + if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 0)
> + its_cpu_init_collection(its);

This isn't correct. Think of a system where half the collections are in
HW, and the other half memory based (nothing in the spec forbids this).
You must evaluate the CID of each collection and replay the MAPC *only*
if it falls into the range [0..HCC-1]. The memory-based collections are
already mapped, and remapping an already mapped 

Re: [PATCH v5 4/4] irqchip/gic-v3-its: add ability to resend MAPC on resume

[Marc Zyngier]

On 07/02/18 01:41, Derek Basehore wrote:
> This adds functionality to resend the MAPC command to an ITS node on
> resume. If the ITS is powered down during suspend and the collections
> are not backed by memory, the ITS will lose that state. This just sets
> up the known state for the collections after the ITS is restored.
> 
> This is enabled via the reset-on-suspend flag in the DTS for an ITS
> that has a non-zero number of collections stored in it.
> 
> Signed-off-by: Derek Basehore 
> ---
>  drivers/irqchip/irq-gic-v3-its.c   | 80 
> --
>  include/linux/irqchip/arm-gic-v3.h |  1 +
>  2 files changed, 43 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index 5e63635e2a7b..dd6cd6e68ed0 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1942,52 +1942,53 @@ static void its_cpu_init_lpis(void)
>   dsb(sy);
>  }
>  
> -static void its_cpu_init_collection(void)
> +static void its_cpu_init_collection(struct its_node *its)
>  {
> - struct its_node *its;
> - int cpu;
> -
> - spin_lock(_lock);
> - cpu = smp_processor_id();
> -
> - list_for_each_entry(its, _nodes, entry) {
> - u64 target;
> + int cpu = smp_processor_id();
> + u64 target;
>  
> - /* avoid cross node collections and its mapping */
> - if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
> - struct device_node *cpu_node;
> + /* avoid cross node collections and its mapping */
> + if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
> + struct device_node *cpu_node;
>  
> - cpu_node = of_get_cpu_node(cpu, NULL);
> - if (its->numa_node != NUMA_NO_NODE &&
> - its->numa_node != of_node_to_nid(cpu_node))
> - continue;
> - }
> + cpu_node = of_get_cpu_node(cpu, NULL);
> + if (its->numa_node != NUMA_NO_NODE &&
> + its->numa_node != of_node_to_nid(cpu_node))
> + return;
> + }
>  
> + /*
> +  * We now have to bind each collection to its target
> +  * redistributor.
> +  */
> + if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) {
>   /*
> -  * We now have to bind each collection to its target
> +  * This ITS wants the physical address of the
>* redistributor.
>*/
> - if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) {
> - /*
> -  * This ITS wants the physical address of the
> -  * redistributor.
> -  */
> - target = gic_data_rdist()->phys_base;
> - } else {
> - /*
> -  * This ITS wants a linear CPU number.
> -  */
> - target = gic_read_typer(gic_data_rdist_rd_base() + 
> GICR_TYPER);
> - target = GICR_TYPER_CPU_NUMBER(target) << 16;
> - }
> + target = gic_data_rdist()->phys_base;
> + } else {
> + /* This ITS wants a linear CPU number. */
> + target = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
> + target = GICR_TYPER_CPU_NUMBER(target) << 16;
> + }
>  
> - /* Perform collection mapping */
> - its->collections[cpu].target_address = target;
> - its->collections[cpu].col_id = cpu;
> + /* Perform collection mapping */
> + its->collections[cpu].target_address = target;
> + its->collections[cpu].col_id = cpu;
>  
> - its_send_mapc(its, >collections[cpu], 1);
> - its_send_invall(its, >collections[cpu]);
> - }
> + its_send_mapc(its, >collections[cpu], 1);
> + its_send_invall(its, >collections[cpu]);
> +}
> +
> +static void its_cpu_init_collections(void)
> +{
> + struct its_node *its;
> +
> + spin_lock(_lock);
> +
> + list_for_each_entry(its, _nodes, entry)
> + its_cpu_init_collection(its);
>  
>   spin_unlock(_lock);
>  }
> @@ -3127,6 +3128,9 @@ static void its_restore_enable(void)
>   its_write_baser(its, baser, baser->val);
>   }
>   writel_relaxed(its->ctlr_save, base + GITS_CTLR);
> +
> + if (GITS_TYPER_HWCOLLCNT(gic_read_typer(base + GITS_TYPER)) > 0)
> + its_cpu_init_collection(its);

This isn't correct. Think of a system where half the collections are in
HW, and the other half memory based (nothing in the spec forbids this).
You must evaluate the CID of each collection and replay the MAPC *only*
if it falls into the range [0..HCC-1]. The memory-based collections are
already mapped, and remapping an already mapped collection requires
extra care