Re: [PATCH v3] x86: properly handle MSI-X unmask operation from guests

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/11/13 12:13, Wu, Feng wrote:
>
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Thursday, November 21, 2013 7:44 PM
>> To: Wu, Feng
>> Cc: Jan Beulich (JBeulich@suse.com); xen-devel@lists.xen.org; Auld, Will;
>> Nakajima, Jun; Zhang, Xiantao
>> Subject: Re: [Xen-devel] [PATCH v3] x86: properly handle MSI-X unmask
>> operation from guests
>>
>> On 21/11/13 10:51, Wu, Feng wrote:
>>> patch revision history
>>> ----------------------
>>> v1: Initial patch to handle this issue involving changing the hypercall interface
>>> v2:Totally handled inside hypervisor.
>>> v3:Change some logics of handling msi-x pending unmask operations.
>>>
>>> From 78ae225e6af88b0b850980fc55640d0776aeafbc Mon Sep 17 00:00:00
>> 2001
>>> From: Feng Wu <feng.wu@intel.com>
>>> Date: Wed, 13 Nov 2013 21:43:48 -0500
>>> Subject: [PATCH] x86: properly handle MSI-X unmask operation from guests
>>>
>>> For a pass-through device with MSI-x capability, when guest tries
>>> to unmask the MSI-x interrupt for the passed through device, xen
>>> doesn't clear the mask bit for MSI-x in hardware in the following
>>> scenario, which will cause network disconnection:
>>>
>>> 1. Guest masks the MSI-x interrupt
>>> 2. Guest updates the address and data for it
>>> 3. Guest unmasks the MSI-x interrupt (This is the problematic step)
>>>
>>> In the step #3 above, Xen doesn't handle it well. When guest tries
>>> to unmask MSI-X interrupt, it traps to Xen, Xen just returns to Qemu
>>> if it notices that address or data has been modified by guest before,
>>> then Qemu will update Xen with the latest value of address/data by
>>> hypercall. However, in this whole process, the MSI-X interrupt unmask
>>> operation is missing, which means Xen doesn't clear the mask bit in
>>> hardware for the MSI-X interrupt, so it remains disabled, that is why
>>> it loses the network connection.
>>>
>>> This patch fixes this issue.
>>>
>>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>>>
>>> +};
>>> +
>>>  struct arch_vcpu
>>>  {
>>>      /*
>>> @@ -439,6 +444,8 @@ struct arch_vcpu
>>>
>>>      /* A secondary copy of the vcpu time info. */
>>>      XEN_GUEST_HANDLE(vcpu_time_info_t) time_info_guest;
>>> +
>>> +    struct pending_msix_unmask_info pending_msix_unmask;
>> What happens if multiple msix interrupts are masked, all updated with
>> addresses, then all unmasked?
> In my understanding, for a specific VCPU, if there is a pending msix unmask
> operation, it means that the Qemu emulation has not been completed yet,states
> so the guest doesn't have chance to do another msix unmask request until
> the current Qemu emulation path is finished(return to the guest). So I think
> msix unmask requests from the guest on one VCPU cannot happen at the
> same time. Correct my if my understanding is not correct! Thanks you!
>

Your patch description suggests that the problem occurs because the
address and data have changed while the MSI-X interrupt is masked. 
There is a tracking structure for a single MSI-X interrupt, which would
indicate that having two masked interrupts and updating them both cant
be correctly tracked.

Or have I misunderstood the problem?

~Andrew