[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH-for-9.0 v2 06/19] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
On 14 November 2023 10:22:23 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@xxxxxxxxxx> wrote: >On 14/11/23 16:13, David Woodhouse wrote: >> On 14 November 2023 09:38:02 GMT-05:00, "Philippe Mathieu-Daudé" >> <philmd@xxxxxxxxxx> 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@xxxxxxxxxx> >> >> 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()'. I think xen_is_pirq_msi() had that test built in, didn't it? Adding a 'xen_enabled() &&' prefix was technically redundant? What's the actual problem we're trying to solve here? That we had two separate implementations of xen_is_pirq_msi() (three if you count an empty stub?) which are resolved at link time and prevent you from running Xen-accel and KVM-accel VMs within the same QEMU process? >"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? > >¯\_(ツ)_/¯ I believe that if you push your branch to a gitlab tree with the right CI variables defined, it'll run all the CI? And I *hope* it fails with this patch. It's precisely the kind of thing I was *intending* to catch with the testing!
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |