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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Mar 20, 2015 at 04:27:29PM +0000, Jan Beulich wrote:
> 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>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> --- 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) )
>      {
> -        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;
>      }
>  
> 
> 
> 

> 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) )
>      {
> -        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


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