[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 20/57] ARM: GICv2: fix GICH_V2_LR definitions



Hi,

On 06/03/18 15:46, Julien Grall wrote:
> Hi Andre,
> 
> On 05/03/18 16:03, Andre Przywara wrote:
>> The bit definition for the CPUID mask in the GICv2 LR register was
>> wrong, fortunately the current implementation does not use that bit.
>> Fix it up (it's starting at bit 10, not bit 9) and clean up some
>> nearby definitions on the way.
>> This will be used by the new VGIC shortly.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> 
> Reviewed-by: Julien Grall <julien.grall@xxxxxxx>

Thanks!

> 
>> ---
>> Changelog RFC ... v1:
>> - new patch
>>   xen/arch/arm/gic-v2.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
>> index 618dd94120..031be920cc 100644
>> --- a/xen/arch/arm/gic-v2.c
>> +++ b/xen/arch/arm/gic-v2.c
>> @@ -57,10 +57,11 @@
>>   #define GICH_V2_LR_HW_MASK         0x1
>>   #define GICH_V2_LR_GRP_SHIFT       30
>>   #define GICH_V2_LR_GRP_MASK        0x1
>> -#define GICH_V2_LR_MAINTENANCE_IRQ (1<<19)
>> -#define GICH_V2_LR_GRP1            (1<<30)
>> -#define GICH_V2_LR_HW              (1<<31)
>> -#define GICH_V2_LR_CPUID_SHIFT     9
>> +#define GICH_V2_LR_MAINTENANCE_IRQ (1U << 19)
>> +#define GICH_V2_LR_GRP1            (1U << 30)
>> +#define GICH_V2_LR_HW              (1U << GICH_V2_LR_HW_SHIFT)
> 
> I think we want this patch to get backported as 1 << 31 is an undefined
> behavior.

I don't think this is necessary. While I agree that 1<<31 is bad (thus
the fix), there is only one user of that macro (down below in that very
file), and the result type is unsigned. If I understand this issue
correctly, the undefined behaviour is about a signed *result* type.

Cheers,
Andre.

>> +#define GICH_V2_LR_CPUID_SHIFT     10
>> +#define GICH_V2_LR_CPUID_MASK      0x7
>>   #define GICH_V2_VTR_NRLRGS         0x3f
>>     #define GICH_V2_VMCR_PRIORITY_MASK   0x1f
>>
> 
> Cheers,
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.