From: David Woodhouse <dwmw@amazon.co.uk>
I initially put the basic platform init (overlay pages, grant tables,
event channels) into mc->kvm_type because that was the earliest place
that could sensibly test for xen_mode==XEN_EMULATE.
The intent was to do this early enough that we could then initialise the
XenBus and other parts which would have depended on them, from a generic
location for both Xen and KVM/Xen in the PC-specific code, as seen in
https://lore.kernel.org/qemu-devel/20230116221919.1124201-16-dwmw2@infradead.org/
However, then the Xen on Arm patches came along, and *they* wanted to
do the XenBus init from a 'generic' Xen-specific location instead:
https://lore.kernel.org/qemu-devel/20230210222729.957168-4-sstabellini@kernel.org/
Since there's no generic location that covers all three, I conceded to
do it for XEN_EMULATE mode in pc_basic_devices_init().
And now there's absolutely no point in having some of the platform init
done from pc_machine_kvm_type(); we can move it all up to live in a
single place in pc_basic_devices_init(). This has the added benefit that
we can drop the separate xen_evtchn_connect_gsis() function completely,
and pass just the system GSIs in directly to xen_evtchn_create().
While I'm at it, it does no harm to explicitly pass in the *number* of
said GSIs, because it does make me twitch a bit to pass an array of
impicit size. During the lifetime of the KVM/Xen patchset, that had
already changed (albeit just cosmetically) from GSI_NUM_PINS to
IOAPIC_NUM_PINS.
And document a bit better that this is for the *output* GSI for raising
CPU0's events when the per-CPU vector isn't available. The fact that
we create a whole set of them and then only waggle the one we're told
to, instead of having a single output and only *connecting* it to the
GSI that it should be connected to, is still non-intuitive for me.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
hw/i386/kvm/xen_evtchn.c | 40 ++++++++++++++++++++--------------------
hw/i386/kvm/xen_evtchn.h | 3 +--
hw/i386/pc.c | 13 ++++---------
3 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c
index 98a7b85047..f4204949fb 100644
--- a/hw/i386/kvm/xen_evtchn.c
+++ b/hw/i386/kvm/xen_evtchn.c
@@ -146,7 +146,10 @@ struct XenEvtchnState {
QemuMutex port_lock;
uint32_t nr_ports;
XenEvtchnPort port_table[EVTCHN_2L_NR_CHANNELS];
- qemu_irq gsis[IOAPIC_NUM_PINS];
+
+ /* Connected to the system GSIs for raising callback as GSI / INTx */
+ unsigned int nr_callback_gsis;
+ qemu_irq *callback_gsis;
struct xenevtchn_handle *be_handles[EVTCHN_2L_NR_CHANNELS];
@@ -298,7 +301,7 @@ static void gsi_assert_bh(void *opaque)
}
}
-void xen_evtchn_create(void)
+void xen_evtchn_create(unsigned int nr_gsis, qemu_irq *system_gsis)
{
XenEvtchnState *s = XEN_EVTCHN(sysbus_create_simple(TYPE_XEN_EVTCHN,
-1, NULL));
@@ -309,8 +312,19 @@ void xen_evtchn_create(void)
qemu_mutex_init(&s->port_lock);
s->gsi_bh = aio_bh_new(qemu_get_aio_context(), gsi_assert_bh, s);
- for (i = 0; i < IOAPIC_NUM_PINS; i++) {
- sysbus_init_irq(SYS_BUS_DEVICE(s), &s->gsis[i]);
+ /*
+ * These are the *output* GSI from event channel support, for
+ * signalling CPU0's events via GSI or PCI INTx instead of the
+ * per-CPU vector. We create a *set* of irqs and connect one to
+ * each of the system GSIs which were passed in from the platform
+ * code, and then just trigger the right one as appropriate from
+ * xen_evtchn_set_callback_level().
+ */
+ s->nr_callback_gsis = nr_gsis;
+ s->callback_gsis = g_new0(qemu_irq, nr_gsis);
+ for (i = 0; i < nr_gsis; i++) {
+ sysbus_init_irq(SYS_BUS_DEVICE(s), &s->callback_gsis[i]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(s), i, system_gsis[i]);
}
/*
@@ -335,20 +349,6 @@ void xen_evtchn_create(void)
xen_evtchn_ops = &emu_evtchn_backend_ops;
}
-void xen_evtchn_connect_gsis(qemu_irq *system_gsis)
-{
- XenEvtchnState *s = xen_evtchn_singleton;
- int i;
-
- if (!s) {
- return;
- }
-
- for (i = 0; i < IOAPIC_NUM_PINS; i++) {
- sysbus_connect_irq(SYS_BUS_DEVICE(s), i, system_gsis[i]);
- }
-}
-
static void xen_evtchn_register_types(void)
{
type_register_static(&xen_evtchn_info);
@@ -429,8 +429,8 @@ void xen_evtchn_set_callback_level(int level)
return;
}
- if (s->callback_gsi && s->callback_gsi < IOAPIC_NUM_PINS) {
- qemu_set_irq(s->gsis[s->callback_gsi], level);
+ if (s->callback_gsi && s->callback_gsi < s->nr_callback_gsis) {
+ qemu_set_irq(s->callback_gsis[s->callback_gsi], level);
if (level) {
/* Ensure the vCPU polls for deassertion */
kvm_xen_set_callback_asserted();
diff --git a/hw/i386/kvm/xen_evtchn.h b/hw/i386/kvm/xen_evtchn.h
index bfb67ac2bc..b740acfc0d 100644
--- a/hw/i386/kvm/xen_evtchn.h
+++ b/hw/i386/kvm/xen_evtchn.h
@@ -16,10 +16,9 @@
typedef uint32_t evtchn_port_t;
-void xen_evtchn_create(void);
+void xen_evtchn_create(unsigned int nr_gsis, qemu_irq *system_gsis);
int xen_evtchn_soft_reset(void);
int xen_evtchn_set_callback_param(uint64_t param);
-void xen_evtchn_connect_gsis(qemu_irq *system_gsis);
void xen_evtchn_set_callback_level(int level);
int xen_evtchn_set_port(uint16_t port);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1489abf010..25584cb8f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1319,7 +1319,10 @@ void pc_basic_device_init(struct PCMachineState *pcms,
#ifdef CONFIG_XEN_EMU
if (xen_mode == XEN_EMULATE) {
- xen_evtchn_connect_gsis(gsi);
+ xen_overlay_create();
+ xen_evtchn_create(IOAPIC_NUM_PINS, gsi);
+ xen_gnttab_create();
+ xen_xenstore_create();
if (pcms->bus) {
pci_create_simple(pcms->bus, -1, "xen-platform");
}
@@ -1868,14 +1871,6 @@ static void pc_machine_initfn(Object *obj)
int pc_machine_kvm_type(MachineState *machine, const char *kvm_type)
{
-#ifdef CONFIG_XEN_EMU
- if (xen_mode == XEN_EMULATE) {
- xen_overlay_create();
- xen_evtchn_create();
- xen_gnttab_create();
- xen_xenstore_create();
- }
-#endif
return 0;
}
--
2.34.1