Re: [Xen-devel] [PATCH 1/2] x86/vMSI-X: honor all mask requests

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx
> [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich
> Sent: Saturday, March 21, 2015 12:27 AM
> To: xen-devel
> Cc: Andrew Cooper; Keir Fraser
> Subject: [Xen-devel] [PATCH 1/2] x86/vMSI-X: honor all mask requests
> 
> Commit 74fd0036de ("x86: properly handle MSI-X unmask operation from
> guests") didn't go far enough: it fixed an issue with unmasking, but
> left an issue with masking in place: Due to the (late) point in time
> when qemu requests the hypervisor to set up MSI-X interrupts (which is
> where the MMIO intercept gets put in place), the hypervisor doesn't
> see all guest writes, and hence shouldn't make assumptions on the state
> the virtual MSI-X resources are in. Bypassing the rest of the logic on
> a guest mask operation leads to
> 
> [00:04.0] pci_msix_write: Error: Can't update msix entry 1 since MSI-X is
> already enabled.
> 
> which surprisingly enough doesn't lead to the device not working
> anymore (I didn't dig in deep enough to figure out why that is). But it
> does prevent the IRQ to be migrated inside the guest, i.e. all
> interrupts will always arrive in vCPU 0.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -286,11 +286,11 @@ static int msixtbl_write(struct vcpu *v,
>          goto out;
>      }
> 
> -    /* exit to device model if address/data has been modified */
> -    if ( test_and_clear_bit(nr_entry, &entry->table_flags) )
> +    /* Exit to device model when unmasking and address/data got modified.
> */
> +    if ( !(val & PCI_MSIX_VECTOR_BITMASK) &&
> +         test_and_clear_bit(nr_entry, &entry->table_flags) )

Is it more accurate as the follows?

If ( (offset == PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET) &&
   !(val & PCI_MSIX_VECTOR_BITMASK) &&
   test_and_clear_bit(nr_entry, &entry->table_flags) )

BTW, how can I reproduce this issue?

Thanks,
Feng

>      {
> -        if ( !(val & PCI_MSIX_VECTOR_BITMASK) )
> -            v->arch.hvm_vcpu.hvm_io.msix_unmask_address = address;
> +        v->arch.hvm_vcpu.hvm_io.msix_unmask_address = address;
>          goto out;
>      }
> 
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel