| Message ID | 20231016151909.22133-3-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Get Xen PV shim running in qemu | expand |
On 16/10/2023 16:18, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > A guest which has configured the per-vCPU upcall vector may set the > HVM_PARAM_CALLBACK_IRQ param to fairly much anything other than zero. > > For example, Linux v6.0+ after commit b1c3497e604 ("x86/xen: Add support > for HVMOP_set_evtchn_upcall_vector") will just do this after setting the > vector: > > /* Trick toolstack to think we are enlightened. */ > if (!cpu) > rc = xen_set_callback_via(1); > > That's explicitly setting the delivery to GSI#, but it's supposed to be > overridden by the per-vCPU vector setting. This mostly works in QEMU > *except* for the logic to enable the in-kernel handling of event channels, > which falsely determines that the kernel cannot accelerate GSI delivery > in this case. > > Add a kvm_xen_has_vcpu_callback_vector() to report whether vCPU#0 has > the vector set, and use that in xen_evtchn_set_callback_param() to > enable the kernel acceleration features even when the param *appears* > to be set to target a GSI. > > Preserve the Xen behaviour that when HVM_PARAM_CALLBACK_IRQ is set to > *zero* the event channel delivery is disabled completely. (Which is > what that bizarre guest behaviour is working round in the first place.) > > Fixes: 91cce756179 ("hw/xen: Add xen_evtchn device for event channel emulation") > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > hw/i386/kvm/xen_evtchn.c | 6 ++++++ > include/sysemu/kvm_xen.h | 1 + > target/i386/kvm/xen-emu.c | 7 +++++++ > 3 files changed, 14 insertions(+) > > diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c > index 4df973022c..d72dca6591 100644 > --- a/hw/i386/kvm/xen_evtchn.c > +++ b/hw/i386/kvm/xen_evtchn.c > @@ -490,6 +490,12 @@ int xen_evtchn_set_callback_param(uint64_t param) > break; > } > > + /* If the guest has set a per-vCPU callback vector, prefer that. */ > + if (gsi && kvm_xen_has_vcpu_callback_vector()) { > + in_kernel = kvm_xen_has_cap(EVTCHN_SEND); > + gsi = 0; > + } > + So this deals with setting the callback via after setting the upcall vector. What happens if the guest then disables the upcall vector (by setting it to zero)? Xen checks 'v->arch.hvm.evtchn_upcall_vector != 0' for every event delivery. Paul
On Tue, 2023-10-24 at 13:29 +0100, Paul Durrant wrote: > > > + /* If the guest has set a per-vCPU callback vector, prefer that. */ > > + if (gsi && kvm_xen_has_vcpu_callback_vector()) { > > + in_kernel = kvm_xen_has_cap(EVTCHN_SEND); > > + gsi = 0; > > + } > > + > > So this deals with setting the callback via after setting the upcall > vector. What happens if the guest then disables the upcall vector (by > setting it to zero)? Xen checks 'v->arch.hvm.evtchn_upcall_vector != 0' > for every event delivery. Qemu and KVM check before each delivery too. For a vCPU other than vCPU0, if it isn't the per-vCPU lapic upcall vector, and it isn't the system-wide vector, the interrupt isn't supposed to be delivered (the GSI is tied to vCPU0). However, we don't support dynamically disabling the kernel acceleration (telling it to forget about the VIRQs, IPIs so that we can handle them in userspace from now on). Not except for soft reset, when they're all torn down anyway. I don't really *want* to support that either. Better to make the kernel mode unconditional, having it signal userspace via an eventfd when vCPU0 has an upcall pending and it's GSI or INTx. But that's a *pair* of eventfds, one for the resampler... and first I have to fix Qemu's interrupt controller models to do the resampling properly on ack (qv). Right now this works for all guests in practice and I have other yaks to shave :)
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index 4df973022c..d72dca6591 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -490,6 +490,12 @@ int xen_evtchn_set_callback_param(uint64_t param) break; } + /* If the guest has set a per-vCPU callback vector, prefer that. */ + if (gsi && kvm_xen_has_vcpu_callback_vector()) { + in_kernel = kvm_xen_has_cap(EVTCHN_SEND); + gsi = 0; + } + if (!ret) { /* If vector delivery was turned *off* then tell the kernel */ if ((s->callback_param >> CALLBACK_VIA_TYPE_SHIFT) == diff --git a/include/sysemu/kvm_xen.h b/include/sysemu/kvm_xen.h index 595abfbe40..961c702c4e 100644 --- a/include/sysemu/kvm_xen.h +++ b/include/sysemu/kvm_xen.h @@ -22,6 +22,7 @@ int kvm_xen_soft_reset(void); uint32_t kvm_xen_get_caps(void); void *kvm_xen_get_vcpu_info_hva(uint32_t vcpu_id); +bool kvm_xen_has_vcpu_callback_vector(void); void kvm_xen_inject_vcpu_callback_vector(uint32_t vcpu_id, int type); void kvm_xen_set_callback_asserted(void); int kvm_xen_set_vcpu_virq(uint32_t vcpu_id, uint16_t virq, uint16_t port); diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c index b49a840438..477e93cd92 100644 --- a/target/i386/kvm/xen-emu.c +++ b/target/i386/kvm/xen-emu.c @@ -424,6 +424,13 @@ void kvm_xen_set_callback_asserted(void) } } +bool kvm_xen_has_vcpu_callback_vector(void) +{ + CPUState *cs = qemu_get_cpu(0); + + return cs && !!X86_CPU(cs)->env.xen_vcpu_callback_vector; +} + void kvm_xen_inject_vcpu_callback_vector(uint32_t vcpu_id, int type) { CPUState *cs = qemu_get_cpu(vcpu_id);