diff mbox series

[01/12] i386/xen: fix per-vCPU upcall vector for Xen emulation

Message ID 20231016151909.22133-2-dwmw2@infradead.org (mailing list archive)
State New, archived
Headers show
Series Get Xen PV shim running in qemu | expand

Commit Message

David Woodhouse Oct. 16, 2023, 3:18 p.m. UTC
From: David Woodhouse <dwmw@amazon.co.uk>

The per-vCPU upcall vector support had two problems. Firstly it was
using the wrong hypercall argument and would always return -EFAULT.
And secondly it was using the wrong ioctl() to pass the vector to
the kernel and thus the *kernel* would always return -EINVAL.

Linux doesn't (yet) use this mode so it went without decent testing
for a while.

Fixes: 105b47fdf2d0 ("i386/xen: implement HVMOP_set_evtchn_upcall_vector")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 target/i386/kvm/xen-emu.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Durrant, Paul Oct. 24, 2023, 12:16 p.m. UTC | #1
On 16/10/2023 16:18, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> The per-vCPU upcall vector support had two problems. Firstly it was
> using the wrong hypercall argument and would always return -EFAULT.
> And secondly it was using the wrong ioctl() to pass the vector to
> the kernel and thus the *kernel* would always return -EINVAL.
> 
> Linux doesn't (yet) use this mode so it went without decent testing
> for a while.
> 
> Fixes: 105b47fdf2d0 ("i386/xen: implement HVMOP_set_evtchn_upcall_vector")
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   target/i386/kvm/xen-emu.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Paul Durrant <paul@xen.org>
David Woodhouse Oct. 24, 2023, 12:58 p.m. UTC | #2
On Tue, 2023-10-24 at 13:16 +0100, Paul Durrant wrote:
> On 16/10/2023 16:18, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > The per-vCPU upcall vector support had two problems. Firstly it was
> > using the wrong hypercall argument and would always return -EFAULT.
> > And secondly it was using the wrong ioctl() to pass the vector to
> > the kernel and thus the *kernel* would always return -EINVAL.
> > 
> > Linux doesn't (yet) use this mode so it went without decent testing
> > for a while.
> > 
> > Fixes: 105b47fdf2d0 ("i386/xen: implement
> > HVMOP_set_evtchn_upcall_vector")
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >   target/i386/kvm/xen-emu.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Paul Durrant <paul@xen.org>

FWIW this patch gained a third "brown paper bag" fix this morning, when
I finally worked it out:

@@ -440,7 +440,8 @@ void kvm_xen_inject_vcpu_callback_vector(uint32_t
vcpu_id, int type)
          * deliver it as an MSI.
          */
         MSIMessage msg = {
-            .address = APIC_DEFAULT_ADDRESS | X86_CPU(cs)->apic_id,
+            .address = APIC_DEFAULT_ADDRESS |
+                       (X86_CPU(cs)->apic_id << MSI_ADDR_DEST_ID_SHIFT),
             .data = vector | (1UL << MSI_DATA_LEVEL_SHIFT),
         };
         kvm_irqchip_send_msi(kvm_state, msg);
diff mbox series

Patch

diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c
index c4fa84a982..b49a840438 100644
--- a/target/i386/kvm/xen-emu.c
+++ b/target/i386/kvm/xen-emu.c
@@ -306,7 +306,7 @@  static int kvm_xen_set_vcpu_callback_vector(CPUState *cs)
 
     trace_kvm_xen_set_vcpu_callback(cs->cpu_index, vector);
 
-    return kvm_vcpu_ioctl(cs, KVM_XEN_HVM_SET_ATTR, &xva);
+    return kvm_vcpu_ioctl(cs, KVM_XEN_VCPU_SET_ATTR, &xva);
 }
 
 static void do_set_vcpu_callback_vector(CPUState *cs, run_on_cpu_data data)
@@ -849,8 +849,7 @@  static bool kvm_xen_hcall_hvm_op(struct kvm_xen_exit *exit, X86CPU *cpu,
     int ret = -ENOSYS;
     switch (cmd) {
     case HVMOP_set_evtchn_upcall_vector:
-        ret = kvm_xen_hcall_evtchn_upcall_vector(exit, cpu,
-                                                 exit->u.hcall.params[0]);
+        ret = kvm_xen_hcall_evtchn_upcall_vector(exit, cpu, arg);
         break;
 
     case HVMOP_pagetable_dying: