[PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Julien Grall]

Currently, the GICv3 drivers is only able to send an SGI when the cpumask
is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
no cpumask is provided. Any usage of those modes will crash the hypersivor.

Move the rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
different modes:
    - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
    (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
    - SGI_TARGET_SELF: Contrawise GICv2, the SGI registers doesn't provide
    a specific field. So use gicv3_send_sgi_list and pass the cpumask of
    the current CPU
    - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
    cpumask

Reported-by: Chen Baozi <baozich@gmail.com>
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/gic-v3.c             | 25 +++++++++++++++++++++++--
 xen/include/asm-arm/gic_v3_defs.h |  2 +-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index b0f498e..e1574d8 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -808,8 +808,7 @@ out:
     return tlist;
 }
 
-static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
-                           const cpumask_t *cpumask)
+static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t *cpumask)
 {
     int cpu = 0;
     uint64_t val;
@@ -839,6 +838,28 @@ static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
     isb();
 }
 
+static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
+                           const cpumask_t *cpumask)
+{
+    switch ( mode )
+    {
+    case SGI_TARGET_OTHERS:
+        WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
+                     (register_t)sgi << ICH_SGI_IRQ_SHIFT,
+                     ICC_SGI1R_EL1);
+        isb();
+        break;
+    case SGI_TARGET_SELF:
+        gicv3_send_sgi_list(sgi, cpumask_of(smp_processor_id()));
+        break;
+    case SGI_TARGET_LIST:
+        gicv3_send_sgi_list(sgi, cpumask);
+        break;
+    default:
+        BUG();
+    }
+}
+
 /* Shut down the per-CPU GIC interface */
 static void gicv3_disable_interface(void)
 {
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index b8a1c2e..556f114 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -147,7 +147,7 @@
 
 #define ICH_SGI_IRQMODE_SHIFT        40
 #define ICH_SGI_IRQMODE_MASK         0x1
-#define ICH_SGI_TARGET_OTHERS        1
+#define ICH_SGI_TARGET_OTHERS        1UL
 #define ICH_SGI_TARGET_LIST          0
 #define ICH_SGI_IRQ_SHIFT            24
 #define ICH_SGI_IRQ_MASK             0xf
-- 
2.1.4

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Stefano Stabellini]

On Mon, 27 Apr 2015, Julien Grall wrote:
> Currently, the GICv3 drivers is only able to send an SGI when the cpumask
> is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
> no cpumask is provided. Any usage of those modes will crash the hypersivor.
> 
> Move the rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
> different modes:
>     - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
>     (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
>     - SGI_TARGET_SELF: Contrawise GICv2, the SGI registers doesn't provide
>     a specific field. So use gicv3_send_sgi_list and pass the cpumask of
>     the current CPU
>     - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
>     cpumask
> 
> Reported-by: Chen Baozi <baozich@gmail.com>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> ---
>  xen/arch/arm/gic-v3.c             | 25 +++++++++++++++++++++++--
>  xen/include/asm-arm/gic_v3_defs.h |  2 +-
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index b0f498e..e1574d8 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -808,8 +808,7 @@ out:
>      return tlist;
>  }
>  
> -static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
> -                           const cpumask_t *cpumask)
> +static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t *cpumask)
>  {
>      int cpu = 0;
>      uint64_t val;
> @@ -839,6 +838,28 @@ static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
>      isb();
>  }
>  
> +static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
> +                           const cpumask_t *cpumask)
> +{
> +    switch ( mode )
> +    {
> +    case SGI_TARGET_OTHERS:
> +        WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
> +                     (register_t)sgi << ICH_SGI_IRQ_SHIFT,
> +                     ICC_SGI1R_EL1);
> +        isb();
> +        break;
> +    case SGI_TARGET_SELF:
> +        gicv3_send_sgi_list(sgi, cpumask_of(smp_processor_id()));
> +        break;
> +    case SGI_TARGET_LIST:
> +        gicv3_send_sgi_list(sgi, cpumask);
> +        break;
> +    default:
> +        BUG();
> +    }
> +}
> +
>  /* Shut down the per-CPU GIC interface */
>  static void gicv3_disable_interface(void)
>  {
> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
> index b8a1c2e..556f114 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -147,7 +147,7 @@
>  
>  #define ICH_SGI_IRQMODE_SHIFT        40
>  #define ICH_SGI_IRQMODE_MASK         0x1
> -#define ICH_SGI_TARGET_OTHERS        1
> +#define ICH_SGI_TARGET_OTHERS        1UL

Spurious change?

Aside from this:

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

>  #define ICH_SGI_TARGET_LIST          0
>  #define ICH_SGI_IRQ_SHIFT            24
>  #define ICH_SGI_IRQ_MASK             0xf
> -- 
> 2.1.4
> 

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Julien Grall]

Hi Stefano,

On 28/04/15 11:00, Stefano Stabellini wrote:
> On Mon, 27 Apr 2015, Julien Grall wrote:
>> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
>> index b8a1c2e..556f114 100644
>> --- a/xen/include/asm-arm/gic_v3_defs.h
>> +++ b/xen/include/asm-arm/gic_v3_defs.h
>> @@ -147,7 +147,7 @@
>>  
>>  #define ICH_SGI_IRQMODE_SHIFT        40
>>  #define ICH_SGI_IRQMODE_MASK         0x1
>> -#define ICH_SGI_TARGET_OTHERS        1
>> +#define ICH_SGI_TARGET_OTHERS        1UL
> 
> Spurious change?

No, this was a mistake when the GICv3 driver has been imported. The
IRQMODE bit is 40 and 1 << 40 would throw the error:

gic-v3.c:847:9: error: left shift count >= width of type [-Werror]
         WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
         ^

> Aside from this:
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Thanks,

-- 
Julien Grall

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Ian Campbell]

On Mon, 2015-04-27 at 19:31 +0100, Julien Grall wrote:
> Currently, the GICv3 drivers is only able to send an SGI when the cpumask

"driver"

> is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
> no cpumask is provided. Any usage of those modes will crash the hypersivor.
> 
> Move the rename gicv3_send_sgi to gicv3_send_sgi_list and implement the

s/Move the r/R/

> different modes:
>     - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
>     (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
>     - SGI_TARGET_SELF: Contrawise GICv2, the SGI registers doesn't provide

"Contrariwise"? But I think you really mean "Unlike". I'd also say "the
GICv3 SGI registers" and s/doesn't/don't/.


>     a specific field. So use gicv3_send_sgi_list and pass the cpumask of
>     the current CPU
>     - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
>     cpumask
> 
> Reported-by: Chen Baozi <baozich@gmail.com>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>

Apart from the commit message nits one code comment:

[...]
> +    case SGI_TARGET_OTHERS:
> +        WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
> +                     (register_t)sgi << ICH_SGI_IRQ_SHIFT,

I don't think you want register_t here, unless ICC_SGI1R_EL1 might be a
different width on different architectures.

I think you should just cast to uint64_t and WRITE_SYSREG64.

Ian.

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Ian Campbell]

On Tue, 2015-04-28 at 11:31 +0100, Julien Grall wrote:
> Hi Stefano,
> 
> On 28/04/15 11:00, Stefano Stabellini wrote:
> > On Mon, 27 Apr 2015, Julien Grall wrote:
> >> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
> >> index b8a1c2e..556f114 100644
> >> --- a/xen/include/asm-arm/gic_v3_defs.h
> >> +++ b/xen/include/asm-arm/gic_v3_defs.h
> >> @@ -147,7 +147,7 @@
> >>  
> >>  #define ICH_SGI_IRQMODE_SHIFT        40
> >>  #define ICH_SGI_IRQMODE_MASK         0x1
> >> -#define ICH_SGI_TARGET_OTHERS        1
> >> +#define ICH_SGI_TARGET_OTHERS        1UL
> > 
> > Spurious change?
> 
> No, this was a mistake when the GICv3 driver has been imported. The
> IRQMODE bit is 40 and 1 << 40 would throw the error:
> 
> gic-v3.c:847:9: error: left shift count >= width of type [-Werror]
>          WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
>          ^

I think you want 1ULL then so it is definitely always 64-bit.

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Julien Grall]

On 08/05/15 15:02, Ian Campbell wrote:
> On Tue, 2015-04-28 at 11:31 +0100, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 28/04/15 11:00, Stefano Stabellini wrote:
>>> On Mon, 27 Apr 2015, Julien Grall wrote:
>>>> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
>>>> index b8a1c2e..556f114 100644
>>>> --- a/xen/include/asm-arm/gic_v3_defs.h
>>>> +++ b/xen/include/asm-arm/gic_v3_defs.h
>>>> @@ -147,7 +147,7 @@
>>>>  
>>>>  #define ICH_SGI_IRQMODE_SHIFT        40
>>>>  #define ICH_SGI_IRQMODE_MASK         0x1
>>>> -#define ICH_SGI_TARGET_OTHERS        1
>>>> +#define ICH_SGI_TARGET_OTHERS        1UL
>>>
>>> Spurious change?
>>
>> No, this was a mistake when the GICv3 driver has been imported. The
>> IRQMODE bit is 40 and 1 << 40 would throw the error:
>>
>> gic-v3.c:847:9: error: left shift count >= width of type [-Werror]
>>          WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
>>          ^
> 
> I think you want 1ULL then so it is definitely always 64-bit.

This header is using UL for 64-bit. I know that Vijay's is planning to
import more define for Linux which use UL for 64-bit.

I'd like to keep 1UL for consistency.
FWIW, GICv3 is only built for ARM64.

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Julien Grall]

Hi Ian,

On 08/05/15 15:01, Ian Campbell wrote:
> On Mon, 2015-04-27 at 19:31 +0100, Julien Grall wrote:
>> Currently, the GICv3 drivers is only able to send an SGI when the cpumask
> 
> "driver"
> 
>> is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
>> no cpumask is provided. Any usage of those modes will crash the hypersivor.
>>
>> Move the rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
> 
> s/Move the r/R/

I missed while reviewing the commit message. I will fix it in the next
version.

>> different modes:
>>     - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
>>     (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
>>     - SGI_TARGET_SELF: Contrawise GICv2, the SGI registers doesn't provide
> 
> "Contrariwise"? But I think you really mean "Unlike". I'd also say "the
> GICv3 SGI registers" and s/doesn't/don't/.

Yes, "unlike" is better here.

> 
>>     a specific field. So use gicv3_send_sgi_list and pass the cpumask of
>>     the current CPU
>>     - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
>>     cpumask
>>
>> Reported-by: Chen Baozi <baozich@gmail.com>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> 
> Apart from the commit message nits one code comment:
> 
> [...]
>> +    case SGI_TARGET_OTHERS:
>> +        WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
>> +                     (register_t)sgi << ICH_SGI_IRQ_SHIFT,
> 
> I don't think you want register_t here, unless ICC_SGI1R_EL1 might be a
> different width on different architectures.

Yes, ICC_SGIR1_EL1 is always 64-bit.

> I think you should just cast to uint64_t and WRITE_SYSREG64.

Right, I blindly re-use the WRITE_SYSREG as it was done on the previous
implementation.

Although, the gic-v3 code seems to always use {READ,WRITE}_SYSREG. This
seems wrong too. I will send a patch for replace the calls by the the
64-bit version.

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Ian Campbell]

On Fri, 2015-05-08 at 15:27 +0100, Julien Grall wrote:
> On 08/05/15 15:02, Ian Campbell wrote:
> > On Tue, 2015-04-28 at 11:31 +0100, Julien Grall wrote:
> >> Hi Stefano,
> >>
> >> On 28/04/15 11:00, Stefano Stabellini wrote:
> >>> On Mon, 27 Apr 2015, Julien Grall wrote:
> >>>> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
> >>>> index b8a1c2e..556f114 100644
> >>>> --- a/xen/include/asm-arm/gic_v3_defs.h
> >>>> +++ b/xen/include/asm-arm/gic_v3_defs.h
> >>>> @@ -147,7 +147,7 @@
> >>>>  
> >>>>  #define ICH_SGI_IRQMODE_SHIFT        40
> >>>>  #define ICH_SGI_IRQMODE_MASK         0x1
> >>>> -#define ICH_SGI_TARGET_OTHERS        1
> >>>> +#define ICH_SGI_TARGET_OTHERS        1UL
> >>>
> >>> Spurious change?
> >>
> >> No, this was a mistake when the GICv3 driver has been imported. The
> >> IRQMODE bit is 40 and 1 << 40 would throw the error:
> >>
> >> gic-v3.c:847:9: error: left shift count >= width of type [-Werror]
> >>          WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
> >>          ^
> > 
> > I think you want 1ULL then so it is definitely always 64-bit.
> 
> This header is using UL for 64-bit. I know that Vijay's is planning to
> import more define for Linux which use UL for 64-bit.
> 
> I'd like to keep 1UL for consistency.

OK.

> FWIW, GICv3 is only built for ARM64.
> 
> Regards,
> 

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Ian Campbell]

On Fri, 2015-05-08 at 15:34 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 08/05/15 15:01, Ian Campbell wrote:
> > On Mon, 2015-04-27 at 19:31 +0100, Julien Grall wrote:
> >> Currently, the GICv3 drivers is only able to send an SGI when the cpumask
> > 
> > "driver"
> > 
> >> is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
> >> no cpumask is provided. Any usage of those modes will crash the hypersivor.
> >>
> >> Move the rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
> > 
> > s/Move the r/R/
> 
> I missed while reviewing the commit message. I will fix it in the next
> version.
> 
> >> different modes:
> >>     - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
> >>     (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
> >>     - SGI_TARGET_SELF: Contrawise GICv2, the SGI registers doesn't provide
> > 
> > "Contrariwise"? But I think you really mean "Unlike". I'd also say "the
> > GICv3 SGI registers" and s/doesn't/don't/.
> 
> Yes, "unlike" is better here.
> 
> > 
> >>     a specific field. So use gicv3_send_sgi_list and pass the cpumask of
> >>     the current CPU
> >>     - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
> >>     cpumask
> >>
> >> Reported-by: Chen Baozi <baozich@gmail.com>
> >> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> > 
> > Apart from the commit message nits one code comment:
> > 
> > [...]
> >> +    case SGI_TARGET_OTHERS:
> >> +        WRITE_SYSREG(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
> >> +                     (register_t)sgi << ICH_SGI_IRQ_SHIFT,
> > 
> > I don't think you want register_t here, unless ICC_SGI1R_EL1 might be a
> > different width on different architectures.
> 
> Yes, ICC_SGIR1_EL1 is always 64-bit.
> 
> > I think you should just cast to uint64_t and WRITE_SYSREG64.
> 
> Right, I blindly re-use the WRITE_SYSREG as it was done on the previous
> implementation.
> 
> Although, the gic-v3 code seems to always use {READ,WRITE}_SYSREG. This
> seems wrong too. I will send a patch for replace the calls by the the
> 64-bit version.

Thanks (although do make sure each register really is 64-bit...)

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Julien Grall]

Hi Ian,

On 08/05/15 16:51, Ian Campbell wrote:
> On Fri, 2015-05-08 at 15:34 +0100, Julien Grall wrote:
>> On 08/05/15 15:01, Ian Campbell wrote:
>>> I think you should just cast to uint64_t and WRITE_SYSREG64.
>>
>> Right, I blindly re-use the WRITE_SYSREG as it was done on the previous
>> implementation.
>>
>> Although, the gic-v3 code seems to always use {READ,WRITE}_SYSREG. This
>> seems wrong too. I will send a patch for replace the calls by the the
>> 64-bit version.
> 
> Thanks (although do make sure each register really is 64-bit...)

I looked more closely to the call of READ_SYSREG/WRITE_SYSREG in the
GICv3 code. They are use for:
	- ICH_LR<n>: 64-bit on aarch64 and 32-bit on aarch32. Although, for
completion we would have to read ICH_LRC<n> for aarch32.
	- ICC_SGIR1_EL1: 64-bit on both aarch64 and aarch32.

So the former is valid although it won't work correctly if someone
decide to use GICv3 on AArch32. Although, I expect seen more issue than
this one. So I will ignore it.

I will take care of the later in the new version of this patch.

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Ian Campbell]

On Fri, 2015-05-08 at 17:55 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 08/05/15 16:51, Ian Campbell wrote:
> > On Fri, 2015-05-08 at 15:34 +0100, Julien Grall wrote:
> >> On 08/05/15 15:01, Ian Campbell wrote:
> >>> I think you should just cast to uint64_t and WRITE_SYSREG64.
> >>
> >> Right, I blindly re-use the WRITE_SYSREG as it was done on the previous
> >> implementation.
> >>
> >> Although, the gic-v3 code seems to always use {READ,WRITE}_SYSREG. This
> >> seems wrong too. I will send a patch for replace the calls by the the
> >> 64-bit version.
> > 
> > Thanks (although do make sure each register really is 64-bit...)
> 
> I looked more closely to the call of READ_SYSREG/WRITE_SYSREG in the
> GICv3 code. They are use for:
> 	- ICH_LR<n>: 64-bit on aarch64 and 32-bit on aarch32. Although, for
> completion we would have to read ICH_LRC<n> for aarch32.
> 	- ICC_SGIR1_EL1: 64-bit on both aarch64 and aarch32.
> 
> So the former is valid although it won't work correctly if someone
> decide to use GICv3 on AArch32.

Right, you would need a #define in cpregs.h for the arm32 case.

>  Although, I expect seen more issue than
> this one. So I will ignore it.
> 
> I will take care of the later in the new version of this patch.

Thanks,

Ian.

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Ian Campbell]

On Sat, 2015-05-09 at 21:13 +0800, Chen Baozi wrote:
> On Fri, May 08, 2015 at 06:01:12PM +0100, Julien Grall wrote:
> > Currently, the GICv3 driver is only able to send an SGI when the cpumask
> > is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
> > no cpumask is provided. Any usage of those modes will crash the hypersivor.
> > 
> > Rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
> > different modes:
> >     - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
> >     (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
> >     - SGI_TARGET_SELF: Unlike GICv2, the GICv3 SGI registers don't
> >     provide a specific field. So use gicv3_send_sgi_list and pass
> >     the cpumask of the current CPU
> >     - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
> >     cpumask
> > 
> > Also, use WRITE_SYSREG64 to write into ICC_SGI1R_EL1 the access is
> > 64-bit on all the architectures.
> > 
> > Reported-by: Chen Baozi <baozich@gmail.com>
> > Signed-off-by: Julien Grall <julien.grall@citrix.com>
> Tested-by: Chen Baozi <baozich@gmail.com>

Acked + applied, thanks.

Re: [PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Chen Baozi]

On Fri, May 08, 2015 at 06:01:12PM +0100, Julien Grall wrote:
> Currently, the GICv3 driver is only able to send an SGI when the cpumask
> is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
> no cpumask is provided. Any usage of those modes will crash the hypersivor.
> 
> Rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
> different modes:
>     - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
>     (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
>     - SGI_TARGET_SELF: Unlike GICv2, the GICv3 SGI registers don't
>     provide a specific field. So use gicv3_send_sgi_list and pass
>     the cpumask of the current CPU
>     - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
>     cpumask
> 
> Also, use WRITE_SYSREG64 to write into ICC_SGI1R_EL1 the access is
> 64-bit on all the architectures.
> 
> Reported-by: Chen Baozi <baozich@gmail.com>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
Tested-by: Chen Baozi <baozich@gmail.com>

Cheers,

Baozi.

[PATCH] xen/arm: gic-v3: Implement correctly the callback send_SGI

[Julien Grall]

Currently, the GICv3 driver is only able to send an SGI when the cpumask
is provided. Although with the modes SGI_TARGET_OTHERS and SGI_TARGET_SELF,
no cpumask is provided. Any usage of those modes will crash the hypersivor.

Rename gicv3_send_sgi to gicv3_send_sgi_list and implement the
different modes:
    - SGI_TARGET_OTHERS: Set the Interrupt Routing Mode (bit 40) to 1
    (see Table 4 on Section 4.2.6 PRD03-GENC-010745 24.0)
    - SGI_TARGET_SELF: Unlike GICv2, the GICv3 SGI registers don't
    provide a specific field. So use gicv3_send_sgi_list and pass
    the cpumask of the current CPU
    - SGI_TARGET_LIST: Directly call gicv3_send_sgi_list with the given
    cpumask

Also, use WRITE_SYSREG64 to write into ICC_SGI1R_EL1 the access is
64-bit on all the architectures.

Reported-by: Chen Baozi <baozich@gmail.com>
Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
    Changes in v2:
        - Use WRITE_SYSREG64 rather than WRITE_SYSREG
        - Cast sgi to uint64_t and not register_t
        - Fix typoes and grammar in the commit message
---
 xen/arch/arm/gic-v3.c             | 27 ++++++++++++++++++++++++---
 xen/include/asm-arm/gic_v3_defs.h |  2 +-
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index db498ed..30682cf 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -808,8 +808,7 @@ out:
     return tlist;
 }
 
-static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
-                           const cpumask_t *cpumask)
+static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t *cpumask)
 {
     int cpu = 0;
     uint64_t val;
@@ -833,12 +832,34 @@ static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
                MPIDR_AFFINITY_LEVEL(cluster_id, 1) << 16  |
                tlist);
 
-        WRITE_SYSREG(val, ICC_SGI1R_EL1);
+        WRITE_SYSREG64(val, ICC_SGI1R_EL1);
     }
     /* Force above writes to ICC_SGI1R_EL1 */
     isb();
 }
 
+static void gicv3_send_sgi(enum gic_sgi sgi, enum gic_sgi_mode mode,
+                           const cpumask_t *cpumask)
+{
+    switch ( mode )
+    {
+    case SGI_TARGET_OTHERS:
+        WRITE_SYSREG64(ICH_SGI_TARGET_OTHERS << ICH_SGI_IRQMODE_SHIFT |
+                       (uint64_t)sgi << ICH_SGI_IRQ_SHIFT,
+                       ICC_SGI1R_EL1);
+        isb();
+        break;
+    case SGI_TARGET_SELF:
+        gicv3_send_sgi_list(sgi, cpumask_of(smp_processor_id()));
+        break;
+    case SGI_TARGET_LIST:
+        gicv3_send_sgi_list(sgi, cpumask);
+        break;
+    default:
+        BUG();
+    }
+}
+
 /* Shut down the per-CPU GIC interface */
 static void gicv3_disable_interface(void)
 {
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index b8a1c2e..556f114 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -147,7 +147,7 @@
 
 #define ICH_SGI_IRQMODE_SHIFT        40
 #define ICH_SGI_IRQMODE_MASK         0x1
-#define ICH_SGI_TARGET_OTHERS        1
+#define ICH_SGI_TARGET_OTHERS        1UL
 #define ICH_SGI_TARGET_LIST          0
 #define ICH_SGI_IRQ_SHIFT            24
 #define ICH_SGI_IRQ_MASK             0xf
-- 
2.1.4