Re: [PATCH v2] interrupts: allow guest to set and clear MSI-X mask bit

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

On 05/08/13 11:44, Jan Beulich wrote:
>>>> On 23.07.13 at 19:59, Joby Poriyath <joby.poriyath@citrix.com> wrote:
>> On Tue, Jul 23, 2013 at 09:28:42AM -0400, Konrad Rzeszutek Wilk wrote:
>>> On Tue, Jul 23, 2013 at 11:54:41AM +0100, Joby Poriyath wrote:
>>>> Ping...
>>>>
>>>> Could you kindly review this?
>>> I believe Malcom reviewed and sent you an email with his response.
>> Andrew and Malcolm has reviewed the code.
>>
>>> Are you waiting for Andrew to look at it as well? Or Jan? Jan is
>>> out on vacation for a couple of weeks.
>> I was hoping that Jan would also review this, since this patch 
>> partly reverts his code. 
>>
>> This patch makes an assumption that, for a passed through device, 
>> xen needs to mask the MSI-X control bit only when it does vcpu 
>> migration. Otherwise xen leaves it in the state guest has set it.
> Exactly. And hence the patch is wrong, 

Unfortunately it is not.

The whole problem is that the VF can (and does) request that the PF
resets itself (using an out-of-band mailbox system in the hardware
itself), causing the mask bit to change (i.e. get set) without Xen's
knowledge.

At this point, it is only the guest which can rectify the situation and
clear the mask bit.

I would agree that the solution is not perfect, but SRIOV virtual
functions rely on the ability to clear the mask bit, whether or not it
is a security issue.  This means that your patch to disable writing to
the mask bit has caused a functional regression in all SRIOV operations
in Xen.

I would say that the less bad alternative is to reintroduce the ability
to change the mask bit, and fix the regression, then work out how to fix
the security aspect.

~Andrew

> re-introducing a security
> issue. If you need to give the guest control over the _virtual_
> mask bit, you need to add proper emulation / masking of the
> delivery of the virtual interrupt (i.e. the physical mask bit should
> always be under the control of Xen, but interrupts should not be
> delivered to the guest when the virtual mask bit is set).
>
> In any event - if the solution was as simple as what you propose,
> I would have implemented it right away rather than leaving the
> code known broken (which in fact was partly based on the
> observation that KVM/QEMU - at least at that time - got away
> without properly emulating the guest attempts to control this bit
> as well).
>
> Jan
>