Re: [Xen-devel] [PATCH v5 5/6] xen/x86: add PHYSDEVOP_msi_control

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.08.2019 14:43, Marek Marczykowski-Górecki  wrote:
> On Tue, Aug 06, 2019 at 02:05:48PM +0200, Jan Beulich wrote:
> > On 06.08.2019 12:53, Marek Marczykowski-Górecki  wrote:
> > > On Tue, Aug 06, 2019 at 12:33:39PM +0200, Jan Beulich wrote:
> > > > On 06.08.2019 11:46, Marek Marczykowski-Górecki  wrote:
> > > > > On Tue, Aug 06, 2019 at 07:56:39AM +0000, Jan Beulich wrote:
> > > > > > On 05.08.2019 15:44, Marek Marczykowski-Górecki  wrote:
> > > > > > > I'm trying to get it working and it isn't clear to me what should I
> > > > > > > check for "INTx is also enabled". I assumed PCI_COMMAND_INTX_DISABLE
> > > > > > > bit, but it looks like guest has no control over this bit, even in
> > > > > > > permissive mode.  This means enabling MSI(-X) always fails because guest
> > > > > > > has no way to set PCI_COMMAND_INTX_DISABLE bit before.
> > > > > > Well, the guest has no control, but in order to enable MSI{,-X} I'd
> > > > > > have expected qemu or the Dom0 kernel to set this bit up front.
> > > > > qemu would do that, when running in dom0. But in PV stubdomain it talks
> > > > > to pciback, which filters it out.
> > > > Filtering out the guest attempt is fine, but it should then set the
> > > > bit while preparing MSI/MSI-X for the guest. (I'm not sure it would
> > > > need to do so explicitly, or whether it couldn't rely on code
> > > > elsewhere in the kernel doing so.)
> > > ...
> > > > Well, I think I've made my position clear: So far we use pci_intx()
> > > > only on devices used by Xen itself. I think this should remain to be
> > > > that way. Devices in possession of domains (including Dom0) should
> > > > be under Dom0's control in this regard.
> > > The thing is, in case of using stubdomain, it's mostly stubdomain
> > > handling it. Especially all the config space interception and applying
> > > logic to it is done by qemu in stubdomain. Do you suggest duplicating /
> > > moving that part to dom0 instead? What would be the point for stubdomain
> > > then?
> > Nothing should be moved between components if possible (and if placed
> > sensibly). But isn't stubdom (being a PV DomU) relying on pciback in
> > Dom0 anyway? And hence doesn't the flow of events include
> > pci_enable_msi{,x}() invoked by pciback? I'd have expected that to
> > (also) take care of INTx.
> This was discussed in v2 of this series[1], where you also responded.
> Relevant part of Roger's email:
>
>      Oh great, that's unfortunate. Both pciback functions end up calling
>      into msi_capability_init in the Linux kernel, which does indeed more
>      than just toggling the PCI config space enable bit.
>
>      OTOH adding a bypass to pciback so the stubdom is able to write to the
>      PCI register in order to toggle the enable bit seems quite clumsy. Not
>      to mention that you would be required to update Dom0 kernel in order to
>      fix the issue.
>
>      Do you think it makes sense to add a domctl to enable/disable MSI(X)?
>
>      This way the bug could be fixed by just updating Xen (and the
>      stubdomain).
>
> [1] https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01271.html
Hmm, right. But us having closed a circle this way (as it seems)
suggests to me that bypassing Dom0 may not have been the right
idea here after all (i.e. is, as Roger said, clumsy). Also may I
ask whether the quoted there "so this most likely won’t work
correctly" has actually been proven to not work, and not be
possible to make work?

Jan

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