qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v2 20/22] i386/xen: HVMOP_set_param / HVM_PARAM_CALLBACK_


From: Paul Durrant
Subject: Re: [RFC PATCH v2 20/22] i386/xen: HVMOP_set_param / HVM_PARAM_CALLBACK_IRQ
Date: Mon, 12 Dec 2022 16:16:33 +0000
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1

On 09/12/2022 09:56, David Woodhouse wrote:
From: Ankur Arora <ankur.a.arora@oracle.com>

The HVM_PARAM_CALLBACK_IRQ parameter controls the system-wide event
channel upcall method.  The vector support is handled by KVM internally,
when the evtchn_upcall_pending field in the vcpu_info is set.

The GSI and PCI_INTX delivery methods are not supported. yet; those
need to simulate a level-triggered event on the I/OAPIC.

That's gonna be somewhat limiting if anyone runs a Windows guest with upcall vector support turned off... which is an option at:

https://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenbus.git;a=blob;f=src/xenbus/evtchn.c;;hb=HEAD#l1928


Add a 'xen_evtchn' device to host the migration state, as we'll shortly
be adding a full event channel table there too.

Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
[dwmw2: Rework for upstream kernel changes, split from per-VCPU vector]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
  hw/i386/kvm/meson.build  |   5 +-
  hw/i386/kvm/xen_evtchn.c | 117 +++++++++++++++++++++++++++++++++++++++
  hw/i386/kvm/xen_evtchn.h |  13 +++++
  hw/i386/pc_piix.c        |   2 +
  target/i386/xen.c        |  44 +++++++++++++--
  5 files changed, 174 insertions(+), 7 deletions(-)
  create mode 100644 hw/i386/kvm/xen_evtchn.c
  create mode 100644 hw/i386/kvm/xen_evtchn.h

diff --git a/hw/i386/kvm/meson.build b/hw/i386/kvm/meson.build
index 6165cbf019..cab64df339 100644
--- a/hw/i386/kvm/meson.build
+++ b/hw/i386/kvm/meson.build
@@ -4,6 +4,9 @@ i386_kvm_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c'))
  i386_kvm_ss.add(when: 'CONFIG_I8254', if_true: files('i8254.c'))
  i386_kvm_ss.add(when: 'CONFIG_I8259', if_true: files('i8259.c'))
  i386_kvm_ss.add(when: 'CONFIG_IOAPIC', if_true: files('ioapic.c'))
-i386_kvm_ss.add(when: 'CONFIG_XEN_EMU', if_true: files('xen_overlay.c'))
+i386_kvm_ss.add(when: 'CONFIG_XEN_EMU', if_true: files(
+  'xen_overlay.c',
+  'xen_evtchn.c',
+  ))
i386_ss.add_all(when: 'CONFIG_KVM', if_true: i386_kvm_ss)
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c
new file mode 100644
index 0000000000..1ca0c034e7
--- /dev/null
+++ b/hw/i386/kvm/xen_evtchn.c
@@ -0,0 +1,117 @@
+/*
+ * QEMU Xen emulation: Shared/overlay pages support
+ *
+ * Copyright © 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Authors: David Woodhouse <dwmw2@infradead.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/host-utils.h"
+#include "qemu/module.h"
+#include "qemu/main-loop.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+#include "exec/target_page.h"
+#include "exec/address-spaces.h"
+#include "migration/vmstate.h"
+
+#include "hw/sysbus.h"
+#include "hw/xen/xen.h"
+#include "xen_evtchn.h"
+
+#include "sysemu/kvm.h"
+#include <linux/kvm.h>
+
+#include "standard-headers/xen/memory.h"
+#include "standard-headers/xen/hvm/params.h"
+
+#define TYPE_XEN_EVTCHN "xenevtchn"
+OBJECT_DECLARE_SIMPLE_TYPE(XenEvtchnState, XEN_EVTCHN)
+
+struct XenEvtchnState {
+    /*< private >*/
+    SysBusDevice busdev;
+    /*< public >*/
+
+    uint64_t callback_param;
+};
+
+struct XenEvtchnState *xen_evtchn_singleton;
+
+static int xen_evtchn_post_load(void *opaque, int version_id)
+{
+    XenEvtchnState *s = opaque;
+
+    if (s->callback_param) {
+        xen_evtchn_set_callback_param(s->callback_param);
+    }
+
+    return 0;
+}
+
+static bool xen_evtchn_is_needed(void *opaque)
+{
+    return xen_mode == XEN_EMULATE;
+}
+
+static const VMStateDescription xen_evtchn_vmstate = {
+    .name = "xen_evtchn",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = xen_evtchn_is_needed,
+    .post_load = xen_evtchn_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(callback_param, XenEvtchnState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void xen_evtchn_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &xen_evtchn_vmstate;
+}
+
+static const TypeInfo xen_evtchn_info = {
+    .name          = TYPE_XEN_EVTCHN,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XenEvtchnState),
+    .class_init    = xen_evtchn_class_init,
+};
+
+void xen_evtchn_create(void)
+{
+    xen_evtchn_singleton = XEN_EVTCHN(sysbus_create_simple(TYPE_XEN_EVTCHN, 
-1, NULL));
+}
+
+static void xen_evtchn_register_types(void)
+{
+    type_register_static(&xen_evtchn_info);
+}
+
+type_init(xen_evtchn_register_types)
+
+
+#define CALLBACK_VIA_TYPE_SHIFT       56
+
+int xen_evtchn_set_callback_param(uint64_t param)
+{
+    int ret = -ENOSYS;
+
+    if (param >> CALLBACK_VIA_TYPE_SHIFT == HVM_PARAM_CALLBACK_TYPE_VECTOR) {
+        struct kvm_xen_hvm_attr xa = {
+            .type = KVM_XEN_ATTR_TYPE_UPCALL_VECTOR,
+            .u.vector = (uint8_t)param,
+        };
+
+        ret = kvm_vm_ioctl(kvm_state, KVM_XEN_HVM_SET_ATTR, &xa);
+        if (!ret && xen_evtchn_singleton)
+            xen_evtchn_singleton->callback_param = param;
+    }
+    return ret;
+}
diff --git a/hw/i386/kvm/xen_evtchn.h b/hw/i386/kvm/xen_evtchn.h
new file mode 100644
index 0000000000..11c6ed22a0
--- /dev/null
+++ b/hw/i386/kvm/xen_evtchn.h
@@ -0,0 +1,13 @@
+/*
+ * QEMU Xen emulation: Event channel support
+ *
+ * Copyright © 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Authors: David Woodhouse <dwmw2@infradead.org>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+void xen_evtchn_create(void);
+int xen_evtchn_set_callback_param(uint64_t param);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c3c61eedde..18540084a0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,6 +60,7 @@
  #endif
  #ifdef CONFIG_XEN_EMU
  #include "hw/i386/kvm/xen_overlay.h"
+#include "hw/i386/kvm/xen_evtchn.h"
  #endif
  #include "migration/global_state.h"
  #include "migration/misc.h"
@@ -417,6 +418,7 @@ static void pc_xen_hvm_init(MachineState *machine)
  #ifdef CONFIG_XEN_EMU
      if (xen_mode == XEN_EMULATE) {
              xen_overlay_create();
+            xen_evtchn_create();
      }
  #endif
  }
diff --git a/target/i386/xen.c b/target/i386/xen.c
index 2583c00a6b..1af336d9e5 100644
--- a/target/i386/xen.c
+++ b/target/i386/xen.c
@@ -16,6 +16,8 @@
  #include "xen.h"
  #include "trace.h"
  #include "hw/i386/kvm/xen_overlay.h"
+#include "hw/i386/kvm/xen_evtchn.h"
+
  #include "standard-headers/xen/version.h"
  #include "standard-headers/xen/memory.h"
  #include "standard-headers/xen/hvm/hvm_op.h"
@@ -287,24 +289,53 @@ static bool kvm_xen_hcall_memory_op(struct kvm_xen_exit 
*exit,
      return true;
  }
+static int handle_set_param(struct kvm_xen_exit *exit, X86CPU *cpu,
+                            uint64_t arg)
+{
+    CPUState *cs = CPU(cpu);
+    struct xen_hvm_param hp;
+    int err = 0;
+
+    if (kvm_copy_from_gva(cs, arg, &hp, sizeof(hp))) {
+        err = -EFAULT;
+        goto out;
+    }
+
+    if (hp.domid != DOMID_SELF) {

Xen actually allows the domain's own id to be specified as well as the magic DOMID_SELF.

+        err = -EINVAL;

And this should be -ESRCH.

+        goto out;
+    }
+
+    switch (hp.index) {
+    case HVM_PARAM_CALLBACK_IRQ:
+        err = xen_evtchn_set_callback_param(hp.value);
+        break;
+    default:
+        return false;
+    }
+
+out:
+    exit->u.hcall.result = err;

This is a bit on the ugly side isn't it? Why not return the err and have kvm_xen_hcall_hvm_op() deal with passing it back?

+    return true;
+}
+
  static int kvm_xen_hcall_evtchn_upcall_vector(struct kvm_xen_exit *exit,
                                                X86CPU *cpu, uint64_t arg)
  {
-    struct xen_hvm_evtchn_upcall_vector *up;
+    struct xen_hvm_evtchn_upcall_vector up;
      CPUState *target_cs;
      int vector;
- up = gva_to_hva(CPU(cpu), arg);
-    if (!up) {
+    if (kvm_copy_from_gva(CPU(cpu), arg, &up, sizeof(up))) {
          return -EFAULT;
      }
- vector = up->vector;
+    vector = up.vector;
      if (vector < 0x10) {
          return -EINVAL;
      }
- target_cs = qemu_get_cpu(up->vcpu);
+    target_cs = qemu_get_cpu(up.vcpu);
      if (!target_cs) {
          return -EINVAL;
      }

These changes to kvm_xen_hcall_evtchn_upcall_vector() seem to have nothing to do with the rest of the patch. Am I missing something?

  Paul

@@ -325,7 +356,8 @@ static bool kvm_xen_hcall_hvm_op(struct kvm_xen_exit *exit, 
X86CPU *cpu,
      case HVMOP_pagetable_dying:
              ret = -ENOSYS;
              break;
-
+    case HVMOP_set_param:
+            return handle_set_param(exit, cpu, arg);
      default:
              return false;
      }




reply via email to

[Prev in Thread] Current Thread [Next in Thread]