Re: [XEN PATCH for-4.19? v2] x86/apic: Fix signed shift in io_apic.c

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/06/2024 4:40 pm, Jan Beulich wrote:
> On 20.06.2024 17:36, Matthew Barnes wrote:
>> There exists bitshifts in the IOAPIC code where signed integers are
>> shifted to the left by up to 31 bits, which is undefined behaviour.
>>
>> This patch fixes this by changing the integers from signed to unsigned.
>>
>> Signed-off-by: Matthew Barnes <matthew.barnes@xxxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Only almost, ...
>
>> ---
>> Changes in v2:
>> - Correct signed shifting in mask_and_ack_level_ioapic_irq()
>> - Adjust bracket spacing to uphold Xen style
> ... as that was only half of what I had asked for. The other half was ...
>
>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -1692,7 +1692,7 @@ static void cf_check 
>> mask_and_ack_level_ioapic_irq(struct irq_desc *desc)
>>         !io_apic_level_ack_pending(desc->irq))
>>          move_masked_irq(desc);
>>  
>> -    if ( !(v & (1 << (i & 0x1f))) ) {
>> +    if ( !(v & (1U << (i & 0x1f))) ) {
>>          spin_lock(&ioapic_lock);
>>          __edge_IO_APIC_irq(desc->irq);
>>          __level_IO_APIC_irq(desc->irq);
>> @@ -1756,7 +1756,7 @@ static void cf_check end_level_ioapic_irq_new(struct 
>> irq_desc *desc, u8 vector)
>>           !io_apic_level_ack_pending(desc->irq) )
>>          move_native_irq(desc);
>>  
>> -    if (!(v & (1 << (i & 0x1f)))) {
>> +    if ( !(v & (1U << (i & 0x1f))) ) {
>>          spin_lock(&ioapic_lock);
>>          __mask_IO_APIC_irq(desc->irq);
>>          __edge_IO_APIC_irq(desc->irq);
> ... to put each opening figure brace on their own line. I guess Andrew or
> I will do that while committing then.

Yeah.  That can be fixed on commit.

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>