Re: [Xen-devel] [PATCH for Xen 4.13] x86/msi: Don't panic if msix capability is missing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2019 16:30, Chao Gao wrote:
> On Mon, Sep 30, 2019 at 11:18:05AM +0200, Jan Beulich wrote:
>> On 29.09.2019 23:24, Chao Gao wrote:
>>> --- a/xen/arch/x86/msi.c
>>> +++ b/xen/arch/x86/msi.c
>>> @@ -1265,7 +1265,13 @@ int pci_msi_conf_write_intercept(struct pci_dev 
>>> *pdev, unsigned int reg,
>>>          pos = entry ? entry->msi_attrib.pos
>>>                      : pci_find_cap_offset(seg, bus, slot, func,
>>>                                            PCI_CAP_ID_MSIX);
>>> -        ASSERT(pos);
>>> +        if ( unlikely(!pos) )
>>> +        {
>>> +            printk_once(XENLOG_WARNING
>>> +                        "%04x:%02x:%02x.%u MSI-X capability is missing\n",
>>> +                        seg, bus, slot, func);
>>> +            return -EAGAIN;
>>> +        }
>>
>> Besides agreeing with Roger's comments, whose access do we
>> intercept here at the time you observe the operation above
>> producing a zero "pos"? If it's Dom0, then surely there's a bug
>> in Dom0 doing the access in the first place when a reset hasn't
>> completed yet?
>> If it's a DomU, then is the reset happening
>> behind _its_ back as well (which is not going to end well)?
> 
> Looks like it is Dom0. Xen should defend against Dom0 bugs, right?

To a degree, yes. But what you suggest above is (to me) not defense,
but simply papering over an issue. What happens with ...

> (XEN) Xen call trace:
> (XEN)    [<ffff82d08027ed90>] pci_msi_conf_write_intercept+0xd7/0x216
> (XEN)    [<ffff82d080297d99>] pci_conf_write_intercept+0x68/0x72
> (XEN)    [<ffff82d08037d40b>] emul-priv-op.c#pci_cfg_ok+0xb5/0x146
> (XEN)    [<ffff82d08037d5af>] emul-priv-op.c#guest_io_write+0x113/0x20b
> (XEN)    [<ffff82d08037db65>] emul-priv-op.c#write_io+0xda/0xe4
> (XEN)    [<ffff82d0802bf35d>] x86_emulate+0x11cf7/0x3169d
> (XEN)    [<ffff82d0802e09bd>] x86_emulate_wrapper+0x26/0x5f
> (XEN)    [<ffff82d08037f57e>] pv_emulate_privileged_op+0x150/0x271
> (XEN)    [<ffff82d0802a80bb>] do_general_protection+0x20b/0x257
> (XEN)    [<ffff82d080387a3d>] x86_64/entry.S#handle_exception_saved+0x68/0x94

... this call stack is that the request by Dom0 simply gets dropped
on the floor. How do you ensure this isn't going to cause further
problems down the road? IOW I think in this case the fix needed to
be in Dom0, and I don't think Xen necessarily has to make things
appear to have gone smoothly. What I _could_ see Xen do in this
case is actually punish Dom0, e.g. by injecting #GP(0). (Obviously
this won't be a good idea ahead of the issue actually getting
fixed _in_ Dom0.)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel