| Message ID | 20231114143816.71079-7-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [PATCH-for-9.0,v2,01/19] tests/avocado: Add 'guest:xen' tag to tests running Xen guest | expand |
On 14 November 2023 09:38:02 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote: >Similarly to the restriction in hw/pci/msix.c (see commit >e1e4bf2252 "msix: fix msix_vector_masked"), restrict the >xen_is_pirq_msi() call in msi_is_masked() to Xen. > >Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Hm, we do also support the Xen abomination of snooping on MSI table writes to see if they're targeted at a Xen PIRQ, then actually unmasking the MSI from QEMU when the guest binds the corresponding event channel to that PIRQ. I think this is going to break in CI as kvm_xen_guest.py does deliberately exercise that use case, doesn't it? I deliberately *didn't* switch to testing the Xen PV net device, with a comment that testing MSI and irqchip permutations was far more entertaining. So I hope it should catch this?
On 14/11/23 16:13, David Woodhouse wrote: > On 14 November 2023 09:38:02 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote: >> Similarly to the restriction in hw/pci/msix.c (see commit >> e1e4bf2252 "msix: fix msix_vector_masked"), restrict the >> xen_is_pirq_msi() call in msi_is_masked() to Xen. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Hm, we do also support the Xen abomination of snooping on MSI table writes to see if they're targeted at a Xen PIRQ, then actually unmasking the MSI from QEMU when the guest binds the corresponding event channel to that PIRQ. > > I think this is going to break in CI as kvm_xen_guest.py does deliberately exercise that use case, doesn't it? Hmmm I see what you mean. So you mentioned these checks: - host Xen accel - Xen accel emulated to guest via KVM host accel Maybe we need here: - guest expected to run Xen Being ( Xen accel emulated to guest via KVM host accel OR host Xen accel ) If so, possibly few places incorrectly check 'xen_enabled()' instead of this 'xen_guest()'. "Xen on KVM" is a tricky case... > I deliberately *didn't* switch to testing the Xen PV net device, with a comment that testing MSI and irqchip permutations was far more entertaining. So I hope it should catch this? ¯\_(ツ)_/¯
On Tue, 2023-11-14 at 16:22 +0100, Philippe Mathieu-Daudé wrote: > > If so, possibly few places incorrectly check 'xen_enabled()' > instead of this 'xen_guest()'. Sorry, I meant to respond to that one directly. I don't *think* there are any cases of that. As I added the CONFIG_XEN_EMU support, I moved a bunch of stuff to live under CONFIG_XEN_BUS instead of CONFIG_XEN, fixing them up (and implementing the emulated versions of grant table operations, event channel operations etc. which no longer come from the actual Xen libraries). The existing cases of xen_enabled() really *are* being used to mean 'xen_accel_enabled()', AFAIK. Apart from that one you just tried to add :)
diff --git a/hw/pci/msi.c b/hw/pci/msi.c index 041b0bdbec..8104ac1d91 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -23,6 +23,7 @@ #include "hw/xen/xen.h" #include "qemu/range.h" #include "qapi/error.h" +#include "sysemu/xen.h" #include "hw/i386/kvm/xen_evtchn.h" @@ -308,7 +309,7 @@ bool msi_is_masked(const PCIDevice *dev, unsigned int vector) } data = pci_get_word(dev->config + msi_data_off(dev, msi64bit)); - if (xen_is_pirq_msi(data)) { + if (xen_enabled() && xen_is_pirq_msi(data)) { return false; }
Similarly to the restriction in hw/pci/msix.c (see commit e1e4bf2252 "msix: fix msix_vector_masked"), restrict the xen_is_pirq_msi() call in msi_is_masked() to Xen. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/pci/msi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)