On 20.06.2024 16:31, Matthew Barnes wrote:
> There exists a bitshift in the IOAPIC code where a signed integer is
> shifted to the left by at most 31 bits. This is undefined behaviour,

s/at most/up to/ maybe?

> and can cause faults in xtf tests such as pv64-pv-iopl~hypercall.
> 
> This patch fixes this by changing the integer from signed to unsigned.
> 
> Signed-off-by: Matthew Barnes <[email protected]>
> ---
>  xen/arch/x86/io_apic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -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);

For one, can you please also take care of the similar issue in
mask_and_ack_level_ioapic_irq()? And then here and there, can you please
also address the style issue(s) on the line(s) you're touching? In both
cases it will want to be

    if ( !(v & (1U << (i & 0x1f))) )
    {

thus bringing both fully into proper Xen style afaics. Then:
Reviewed-by: Jan Beulich <[email protected]>

Jan

Reply via email to