From eeedfe6c6316e8c0d58becc427e12aceb4cb3ad3 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 12 Apr 2023 19:50:58 +0100 Subject: [PATCH 01/12] hw/xen: Simplify emulated Xen platform init 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 Reviewed-by: Paul Durrant Message-Id: <20230412185102.441523-2-dwmw2@infradead.org> Signed-off-by: Anthony PERARD --- 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 3048329474..3d810dbd59 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -147,7 +147,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]; @@ -299,7 +302,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)); @@ -310,8 +313,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]); } /* @@ -336,20 +350,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); @@ -430,8 +430,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 bb62c994fa..fc52772fdd 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1332,7 +1332,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"); } @@ -1882,14 +1885,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; } From 8442232eba1b041b379ca5845df8252c1e905e43 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 12 Apr 2023 19:50:59 +0100 Subject: [PATCH 02/12] hw/xen: Fix memory leak in libxenstore_open() for Xen There was a superfluous allocation of the XS handle, leading to it being leaked on both the error path and the success path (where it gets allocated again). Spotted by Coverity (CID 1508098). Fixes: ba2a92db1ff6 ("hw/xen: Add xenstore operations to allow redirection to internal emulation") Suggested-by: Peter Maydell Signed-off-by: David Woodhouse Reviewed-by: Peter Maydell Reviewed-by: Paul Durrant Message-Id: <20230412185102.441523-3-dwmw2@infradead.org> Signed-off-by: Anthony PERARD --- hw/xen/xen-operations.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c index 4b78fbf4bd..3d213d28df 100644 --- a/hw/xen/xen-operations.c +++ b/hw/xen/xen-operations.c @@ -287,7 +287,7 @@ static void watch_event(void *opaque) static struct qemu_xs_handle *libxenstore_open(void) { struct xs_handle *xsh = xs_open(0); - struct qemu_xs_handle *h = g_new0(struct qemu_xs_handle, 1); + struct qemu_xs_handle *h; if (!xsh) { return NULL; From 2f20b1732d44c7eb1abcb611fdb07a96dbf40a17 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 12 Apr 2023 19:51:00 +0100 Subject: [PATCH 03/12] xen: Drop support for Xen versions below 4.7.1 In restructuring to allow for internal emulation of Xen functionality, I broke compatibility for Xen 4.6 and earlier. Fix this by explicitly removing support for anything older than 4.7.1, which is also ancient but it does still build, and the compatibility support for it is fairly unintrusive. Fixes: 15e283c5b684 ("hw/xen: Add foreignmem operations to allow redirection to internal emulation") Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant Message-Id: <20230412185102.441523-4-dwmw2@infradead.org> Signed-off-by: Anthony PERARD --- hw/xen/xen-operations.c | 57 +------------------ include/hw/xen/xen_native.h | 107 +----------------------------------- meson.build | 5 +- scripts/xen-detect.c | 60 -------------------- 4 files changed, 3 insertions(+), 226 deletions(-) diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c index 3d213d28df..e00983ec44 100644 --- a/hw/xen/xen-operations.c +++ b/hw/xen/xen-operations.c @@ -28,46 +28,13 @@ #include /* - * We don't support Xen prior to 4.2.0. + * We don't support Xen prior to 4.7.1. */ -/* Xen 4.2 through 4.6 */ -#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40701 - -typedef xc_evtchn xenevtchn_handle; -typedef evtchn_port_or_error_t xenevtchn_port_or_error_t; - -#define xenevtchn_open(l, f) xc_evtchn_open(l, f); -#define xenevtchn_close(h) xc_evtchn_close(h) -#define xenevtchn_fd(h) xc_evtchn_fd(h) -#define xenevtchn_pending(h) xc_evtchn_pending(h) -#define xenevtchn_notify(h, p) xc_evtchn_notify(h, p) -#define xenevtchn_bind_interdomain(h, d, p) xc_evtchn_bind_interdomain(h, d, p) -#define xenevtchn_unmask(h, p) xc_evtchn_unmask(h, p) -#define xenevtchn_unbind(h, p) xc_evtchn_unbind(h, p) - -typedef xc_gnttab xengnttab_handle; - -#define xengnttab_open(l, f) xc_gnttab_open(l, f) -#define xengnttab_close(h) xc_gnttab_close(h) -#define xengnttab_set_max_grants(h, n) xc_gnttab_set_max_grants(h, n) -#define xengnttab_map_grant_ref(h, d, r, p) xc_gnttab_map_grant_ref(h, d, r, p) -#define xengnttab_unmap(h, a, n) xc_gnttab_munmap(h, a, n) -#define xengnttab_map_grant_refs(h, c, d, r, p) \ - xc_gnttab_map_grant_refs(h, c, d, r, p) -#define xengnttab_map_domain_grant_refs(h, c, d, r, p) \ - xc_gnttab_map_domain_grant_refs(h, c, d, r, p) - -typedef xc_interface xenforeignmemory_handle; - -#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40701 */ - #include #include #include -#endif - /* Xen before 4.8 */ static int libxengnttab_fallback_grant_copy(xengnttab_handle *xgt, @@ -223,26 +190,6 @@ static struct gnttab_backend_ops libxengnttab_backend_ops = { .unmap = libxengnttab_backend_unmap, }; -#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40701 - -static void *libxenforeignmem_backend_map(uint32_t dom, void *addr, int prot, - size_t pages, xfn_pfn_t *pfns, - int *errs) -{ - if (errs) { - return xc_map_foreign_bulk(xen_xc, dom, prot, pfns, errs, pages); - } else { - return xc_map_foreign_pages(xen_xc, dom, prot, pfns, pages); - } -} - -static int libxenforeignmem_backend_unmap(void *addr, size_t pages) -{ - return munmap(addr, pages * XC_PAGE_SIZE); -} - -#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40701 */ - static void *libxenforeignmem_backend_map(uint32_t dom, void *addr, int prot, size_t pages, xen_pfn_t *pfns, int *errs) @@ -256,8 +203,6 @@ static int libxenforeignmem_backend_unmap(void *addr, size_t pages) return xenforeignmemory_unmap(xen_fmem, addr, pages); } -#endif - struct foreignmem_backend_ops libxenforeignmem_backend_ops = { .map = libxenforeignmem_backend_map, .unmap = libxenforeignmem_backend_unmap, diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h index 6bcc83baf9..f11eb423e3 100644 --- a/include/hw/xen/xen_native.h +++ b/include/hw/xen/xen_native.h @@ -24,23 +24,11 @@ extern xc_interface *xen_xc; /* - * We don't support Xen prior to 4.2.0. + * We don't support Xen prior to 4.7.1. */ -/* Xen 4.2 through 4.6 */ -#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40701 - -typedef xc_interface xenforeignmemory_handle; - -#define xenforeignmemory_open(l, f) xen_xc -#define xenforeignmemory_close(h) - -#else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40701 */ - #include -#endif - extern xenforeignmemory_handle *xen_fmem; #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40900 @@ -148,8 +136,6 @@ static inline xendevicemodel_handle *xendevicemodel_open( return xen_xc; } -#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40500 - static inline int xendevicemodel_create_ioreq_server( xendevicemodel_handle *dmod, domid_t domid, int handle_bufioreq, ioservid_t *id) @@ -211,8 +197,6 @@ static inline int xendevicemodel_set_ioreq_server_state( return xc_hvm_set_ioreq_server_state(dmod, domid, id, enabled); } -#endif /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40500 */ - static inline int xendevicemodel_set_pci_intx_level( xendevicemodel_handle *dmod, domid_t domid, uint16_t segment, uint8_t bus, uint8_t device, uint8_t intx, unsigned int level) @@ -340,15 +324,6 @@ static inline int xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom, } #endif -/* Xen before 4.6 */ -#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40600 - -#ifndef HVM_IOREQSRV_BUFIOREQ_ATOMIC -#define HVM_IOREQSRV_BUFIOREQ_ATOMIC 2 -#endif - -#endif - static inline int xen_get_default_ioreq_server_info(domid_t dom, xen_pfn_t *ioreq_pfn, xen_pfn_t *bufioreq_pfn, @@ -386,84 +361,6 @@ static inline int xen_get_default_ioreq_server_info(domid_t dom, return 0; } -/* Xen before 4.5 */ -#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40500 - -#ifndef HVM_PARAM_BUFIOREQ_EVTCHN -#define HVM_PARAM_BUFIOREQ_EVTCHN 26 -#endif - -#define IOREQ_TYPE_PCI_CONFIG 2 - -typedef uint16_t ioservid_t; - -static inline void xen_map_memory_section(domid_t dom, - ioservid_t ioservid, - MemoryRegionSection *section) -{ -} - -static inline void xen_unmap_memory_section(domid_t dom, - ioservid_t ioservid, - MemoryRegionSection *section) -{ -} - -static inline void xen_map_io_section(domid_t dom, - ioservid_t ioservid, - MemoryRegionSection *section) -{ -} - -static inline void xen_unmap_io_section(domid_t dom, - ioservid_t ioservid, - MemoryRegionSection *section) -{ -} - -static inline void xen_map_pcidev(domid_t dom, - ioservid_t ioservid, - PCIDevice *pci_dev) -{ -} - -static inline void xen_unmap_pcidev(domid_t dom, - ioservid_t ioservid, - PCIDevice *pci_dev) -{ -} - -static inline void xen_create_ioreq_server(domid_t dom, - ioservid_t *ioservid) -{ -} - -static inline void xen_destroy_ioreq_server(domid_t dom, - ioservid_t ioservid) -{ -} - -static inline int xen_get_ioreq_server_info(domid_t dom, - ioservid_t ioservid, - xen_pfn_t *ioreq_pfn, - xen_pfn_t *bufioreq_pfn, - evtchn_port_t *bufioreq_evtchn) -{ - return xen_get_default_ioreq_server_info(dom, ioreq_pfn, - bufioreq_pfn, - bufioreq_evtchn); -} - -static inline int xen_set_ioreq_server_state(domid_t dom, - ioservid_t ioservid, - bool enable) -{ - return 0; -} - -/* Xen 4.5 */ -#else - static bool use_default_ioreq_server; static inline void xen_map_memory_section(domid_t dom, @@ -624,6 +521,4 @@ static inline int xen_set_ioreq_server_state(domid_t dom, enable); } -#endif - #endif /* QEMU_HW_XEN_NATIVE_H */ diff --git a/meson.build b/meson.build index 553c8e0b9c..0984237c48 100644 --- a/meson.build +++ b/meson.build @@ -1683,16 +1683,13 @@ if get_option('xen').enabled() or (get_option('xen').auto() and have_system) endif endif if not xen.found() - xen_tests = [ '4.11.0', '4.10.0', '4.9.0', '4.8.0', '4.7.1', '4.6.0', '4.5.0', '4.2.0' ] + xen_tests = [ '4.11.0', '4.10.0', '4.9.0', '4.8.0', '4.7.1' ] xen_libs = { '4.11.0': [ 'xenstore', 'xenctrl', 'xendevicemodel', 'xenforeignmemory', 'xengnttab', 'xenevtchn', 'xentoolcore' ], '4.10.0': [ 'xenstore', 'xenctrl', 'xendevicemodel', 'xenforeignmemory', 'xengnttab', 'xenevtchn', 'xentoolcore' ], '4.9.0': [ 'xenstore', 'xenctrl', 'xendevicemodel', 'xenforeignmemory', 'xengnttab', 'xenevtchn' ], '4.8.0': [ 'xenstore', 'xenctrl', 'xenforeignmemory', 'xengnttab', 'xenevtchn' ], '4.7.1': [ 'xenstore', 'xenctrl', 'xenforeignmemory', 'xengnttab', 'xenevtchn' ], - '4.6.0': [ 'xenstore', 'xenctrl' ], - '4.5.0': [ 'xenstore', 'xenctrl' ], - '4.2.0': [ 'xenstore', 'xenctrl' ], } xen_deps = {} foreach ver: xen_tests diff --git a/scripts/xen-detect.c b/scripts/xen-detect.c index 85e8206490..db049e605c 100644 --- a/scripts/xen-detect.c +++ b/scripts/xen-detect.c @@ -138,66 +138,6 @@ return 0; } -#elif CONFIG_XEN_CTRL_INTERFACE_VERSION == 40600 - #include - #include - #include - #include - #if !defined(HVM_MAX_VCPUS) - # error HVM_MAX_VCPUS not defined - #endif - int main(void) { - xc_interface *xc; - xs_daemon_open(); - xc = xc_interface_open(0, 0, 0); - xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); - xc_gnttab_open(NULL, 0); - xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0); - xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000); - xc_hvm_create_ioreq_server(xc, 0, HVM_IOREQSRV_BUFIOREQ_ATOMIC, NULL); - xc_reserved_device_memory_map(xc, 0, 0, 0, 0, NULL, 0); - return 0; - } - -#elif CONFIG_XEN_CTRL_INTERFACE_VERSION == 40500 - #include - #include - #include - #include - #if !defined(HVM_MAX_VCPUS) - # error HVM_MAX_VCPUS not defined - #endif - int main(void) { - xc_interface *xc; - xs_daemon_open(); - xc = xc_interface_open(0, 0, 0); - xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); - xc_gnttab_open(NULL, 0); - xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0); - xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000); - xc_hvm_create_ioreq_server(xc, 0, 0, NULL); - return 0; - } - -#elif CONFIG_XEN_CTRL_INTERFACE_VERSION == 40200 - #include - #include - #include - #include - #if !defined(HVM_MAX_VCPUS) - # error HVM_MAX_VCPUS not defined - #endif - int main(void) { - xc_interface *xc; - xs_daemon_open(); - xc = xc_interface_open(0, 0, 0); - xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); - xc_gnttab_open(NULL, 0); - xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0); - xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000); - return 0; - } - #else #error invalid CONFIG_XEN_CTRL_INTERFACE_VERSION #endif From c9bdfe8d587c1a6a8fc2e0ff97343745a9f5f247 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Wed, 12 Apr 2023 19:51:02 +0100 Subject: [PATCH 04/12] hw/xen: Fix broken check for invalid state in xs_be_open() Coverity points out that if (!s && !s->impl) isn't really what we intended to do here. CID 1508131. Fixes: 032475127225 ("hw/xen: Add emulated implementation of XenStore operations") Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant Reviewed-by: Peter Maydell Message-Id: <20230412185102.441523-6-dwmw2@infradead.org> Signed-off-by: Anthony PERARD --- hw/i386/kvm/xen_xenstore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c index 0b189c6ab8..133d89e953 100644 --- a/hw/i386/kvm/xen_xenstore.c +++ b/hw/i386/kvm/xen_xenstore.c @@ -1688,7 +1688,7 @@ static struct qemu_xs_handle *xs_be_open(void) XenXenstoreState *s = xen_xenstore_singleton; struct qemu_xs_handle *h; - if (!s && !s->impl) { + if (!s || !s->impl) { errno = -ENOSYS; return NULL; } From 27047bd26633d559b9630c57e922e4e78cb2ff1d Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:18 +0200 Subject: [PATCH 05/12] include/hw/xen/xen: Rename xen_piix3_set_irq() to xen_intx_set_irq() xen_piix3_set_irq() isn't PIIX specific: PIIX is a single PCI device while xen_piix3_set_irq() maps multiple PCI devices to their respective IRQs, which is board-specific. Rename xen_piix3_set_irq() to communicate this. Also rename XEN_PIIX_NUM_PIRQS to XEN_IOAPIC_NUM_PIRQS since the Xen's IOAPIC rather than PIIX has this many interrupt routes. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Reviewed-by: Anthony PERARD Tested-by: Chuck Zmudzinski Message-Id: <20230312120221.99183-2-shentey@gmail.com> Message-Id: <20230403074124.3925-2-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/i386/xen/xen-hvm.c | 2 +- hw/isa/piix3.c | 4 ++-- include/hw/xen/xen.h | 2 +- stubs/xen-hw-stub.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index 56641a550e..ab8f1b61ee 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -143,7 +143,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) return irq_num + (PCI_SLOT(pci_dev->devfn) << 2); } -void xen_piix3_set_irq(void *opaque, int irq_num, int level) +void xen_intx_set_irq(void *opaque, int irq_num, int level) { xen_set_pci_intx_level(xen_domid, 0, 0, irq_num >> 2, irq_num & 3, level); diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index f9103ea45a..6651521a46 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -35,7 +35,7 @@ #include "migration/vmstate.h" #include "hw/acpi/acpi_aml_interface.h" -#define XEN_PIIX_NUM_PIRQS 128ULL +#define XEN_IOAPIC_NUM_PIRQS 128ULL static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { @@ -420,7 +420,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp) * connected to the IOAPIC directly. * These additional routes can be discovered through ACPI. */ - pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS); + pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_IOAPIC_NUM_PIRQS); } static void piix3_xen_class_init(ObjectClass *klass, void *data) diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index 2bd8ec742d..37ecc91fc3 100644 --- a/include/hw/xen/xen.h +++ b/include/hw/xen/xen.h @@ -39,7 +39,7 @@ extern bool xen_domid_restrict; int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num); int xen_set_pci_link_route(uint8_t link, uint8_t irq); -void xen_piix3_set_irq(void *opaque, int irq_num, int level); +void xen_intx_set_irq(void *opaque, int irq_num, int level); void xen_hvm_inject_msi(uint64_t addr, uint32_t data); int xen_is_pirq_msi(uint32_t msi_data); diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c index 34a22f2ad7..7d7ffe83a9 100644 --- a/stubs/xen-hw-stub.c +++ b/stubs/xen-hw-stub.c @@ -15,7 +15,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) return -1; } -void xen_piix3_set_irq(void *opaque, int irq_num, int level) +void xen_intx_set_irq(void *opaque, int irq_num, int level) { } From c0b59416c05fbf7a5fefee4b4757f7281e34ed02 Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:19 +0200 Subject: [PATCH 06/12] hw/pci/pci.c: Don't leak PCIBus::irq_count[] in pci_bus_irqs() When calling pci_bus_irqs() multiple times on the same object without calling pci_bus_irqs_cleanup() in between PCIBus::irq_count[] is currently leaked. Let's fix this because Xen will do just that in a few commits, and because calling pci_bus_irqs_cleanup() in between seems fragile and cumbersome. Note that pci_bus_irqs_cleanup() now has to NULL irq_count such that pci_bus_irqs() doesn't do a double free. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Tested-by: Chuck Zmudzinski Message-Id: <20230403074124.3925-3-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/pci/pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 1cc7c89036..9b7b4d7c18 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -560,6 +560,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, bus->set_irq = set_irq; bus->irq_opaque = irq_opaque; bus->nirq = nirq; + g_free(bus->irq_count); bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0])); } @@ -575,6 +576,7 @@ void pci_bus_irqs_cleanup(PCIBus *bus) bus->irq_opaque = NULL; bus->nirq = 0; g_free(bus->irq_count); + bus->irq_count = NULL; } PCIBus *pci_register_root_bus(DeviceState *parent, const char *name, From a58a31a6a18ab650c6b676eccf98b6f68ac672d0 Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:20 +0200 Subject: [PATCH 07/12] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize() This is a preparational patch for the next one to make the following more obvious: First, pci_bus_irqs() is now called twice in case of Xen where the second call overrides the pci_set_irq_fn with the Xen variant. Second, pci_bus_set_route_irq_fn() is now also called in Xen mode. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Reviewed-by: Anthony PERARD Tested-by: Chuck Zmudzinski Message-Id: <20230312120221.99183-3-shentey@gmail.com> Message-Id: <20230403074124.3925-4-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/isa/piix3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 6651521a46..800b80f2bb 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -409,7 +409,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp) PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); PCIBus *pci_bus = pci_get_bus(dev); - pci_piix3_realize(dev, errp); + piix3_realize(dev, errp); if (*errp) { return; } From 60a9eb57f337ee36cb9140154a83edb2c41f00c6 Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:21 +0200 Subject: [PATCH 08/12] hw/isa/piix3: Wire up Xen PCI IRQ handling outside of PIIX3 xen_intx_set_irq() doesn't depend on PIIX3State. In order to resolve TYPE_PIIX3_XEN_DEVICE and in order to make Xen agnostic about the precise south bridge being used, set up Xen's PCI IRQ handling of PIIX3 in the board. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Reviewed-by: Anthony PERARD Tested-by: Chuck Zmudzinski Message-Id: <20230312120221.99183-4-shentey@gmail.com> Message-Id: <20230403074124.3925-5-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/i386/pc_piix.c | 13 +++++++++++++ hw/isa/piix3.c | 24 +----------------------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index d5b0dcd1fe..11af191513 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -71,6 +71,7 @@ #include "kvm/kvm-cpu.h" #define MAX_IDE_BUS 2 +#define XEN_IOAPIC_NUM_PIRQS 128ULL #ifdef CONFIG_IDE_ISA static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; @@ -238,6 +239,18 @@ static void pc_init1(MachineState *machine, pcms->bus = pci_bus; pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); + + if (xen_enabled()) { + /* + * Xen supports additional interrupt routes from the PCI devices to + * the IOAPIC: the four pins of each PCI device on the bus are also + * connected to the IOAPIC directly. + * These additional routes can be discovered through ACPI. + */ + pci_bus_irqs(pci_bus, xen_intx_set_irq, pci_dev, + XEN_IOAPIC_NUM_PIRQS); + } + piix3 = PIIX3_PCI_DEVICE(pci_dev); piix3->pic = x86ms->gsi; piix3_devfn = piix3->dev.devfn; diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 800b80f2bb..0b7e9ade0d 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -35,8 +35,6 @@ #include "migration/vmstate.h" #include "hw/acpi/acpi_aml_interface.h" -#define XEN_IOAPIC_NUM_PIRQS 128ULL - static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { qemu_set_irq(piix3->pic[pic_irq], @@ -403,32 +401,12 @@ static const TypeInfo piix3_info = { .class_init = piix3_class_init, }; -static void piix3_xen_realize(PCIDevice *dev, Error **errp) -{ - ERRP_GUARD(); - PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev); - PCIBus *pci_bus = pci_get_bus(dev); - - piix3_realize(dev, errp); - if (*errp) { - return; - } - - /* - * Xen supports additional interrupt routes from the PCI devices to - * the IOAPIC: the four pins of each PCI device on the bus are also - * connected to the IOAPIC directly. - * These additional routes can be discovered through ACPI. - */ - pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_IOAPIC_NUM_PIRQS); -} - static void piix3_xen_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); k->config_write = piix3_write_config_xen; - k->realize = piix3_xen_realize; + k->realize = piix3_realize; } static const TypeInfo piix3_xen_info = { From 89965db43cce708216de860b3655b1a4816e9fa9 Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:22 +0200 Subject: [PATCH 09/12] hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config() Subscribe to pci_bus_fire_intx_routing_notifier() instead which allows for having a common piix3_write_config() for the PIIX3 device models. While at it, move the subscription into machine code to facilitate resolving TYPE_PIIX3_XEN_DEVICE. In a possible future followup, pci_bus_fire_intx_routing_notifier() could be adjusted in such a way that subscribing to it doesn't require knowledge of the device firing it. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Reviewed-by: Anthony PERARD Tested-by: Chuck Zmudzinski Message-Id: <20230312120221.99183-5-shentey@gmail.com> Message-Id: <20230403074124.3925-6-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/i386/pc_piix.c | 18 ++++++++++++++++++ hw/isa/piix3.c | 22 +--------------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 11af191513..2ed2b3f3c6 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -90,6 +90,21 @@ static int pc_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) return (pci_intx + slot_addend) & 3; } +static void piix_intx_routing_notifier_xen(PCIDevice *dev) +{ + int i; + + /* Scan for updates to PCI link routes (0x60-0x63). */ + for (i = 0; i < PIIX_NUM_PIRQS; i++) { + uint8_t v = dev->config_read(dev, PIIX_PIRQCA + i, 1); + if (v & 0x80) { + v = 0; + } + v &= 0xf; + xen_set_pci_link_route(i, v); + } +} + /* PC hardware initialisation */ static void pc_init1(MachineState *machine, const char *host_type, const char *pci_type) @@ -241,6 +256,9 @@ static void pc_init1(MachineState *machine, pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); if (xen_enabled()) { + pci_device_set_intx_routing_notifier( + pci_dev, piix_intx_routing_notifier_xen); + /* * Xen supports additional interrupt routes from the PCI devices to * the IOAPIC: the four pins of each PCI device on the bus are also diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 0b7e9ade0d..5e8ac98ae0 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -122,26 +122,6 @@ static void piix3_write_config(PCIDevice *dev, } } -static void piix3_write_config_xen(PCIDevice *dev, - uint32_t address, uint32_t val, int len) -{ - int i; - - /* Scan for updates to PCI link routes (0x60-0x63). */ - for (i = 0; i < len; i++) { - uint8_t v = (val >> (8 * i)) & 0xff; - if (v & 0x80) { - v = 0; - } - v &= 0xf; - if (((address + i) >= PIIX_PIRQCA) && ((address + i) <= PIIX_PIRQCD)) { - xen_set_pci_link_route(address + i - PIIX_PIRQCA, v); - } - } - - piix3_write_config(dev, address, val, len); -} - static void piix3_reset(DeviceState *dev) { PIIX3State *d = PIIX3_PCI_DEVICE(dev); @@ -405,7 +385,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->config_write = piix3_write_config_xen; + k->config_write = piix3_write_config; k->realize = piix3_realize; } From 0f3e02a2f549e8b246252610cd0c4fc4e4f501ac Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:23 +0200 Subject: [PATCH 10/12] hw/isa/piix3: Resolve redundant k->config_write assignments The previous patch unified handling of piix3_write_config() accross the PIIX3 device models which allows for assigning k->config_write once in the base class. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Reviewed-by: Anthony PERARD Tested-by: Chuck Zmudzinski Message-Id: <20230312120221.99183-6-shentey@gmail.com> Message-Id: <20230403074124.3925-7-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/isa/piix3.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 5e8ac98ae0..0549e043ca 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -322,6 +322,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass); + k->config_write = piix3_write_config; dc->reset = piix3_reset; dc->desc = "ISA bridge"; dc->vmsd = &vmstate_piix3; @@ -371,7 +372,6 @@ static void piix3_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->config_write = piix3_write_config; k->realize = piix3_realize; } @@ -385,7 +385,6 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->config_write = piix3_write_config; k->realize = piix3_realize; } From f8790f81eb8d4a1093e27485e4424ed423e2cc1b Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 3 Apr 2023 09:41:24 +0200 Subject: [PATCH 11/12] hw/isa/piix3: Resolve redundant TYPE_PIIX3_XEN_DEVICE During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of TYPE_PIIX3_DEVICE. Remove this redundancy. Signed-off-by: Bernhard Beschow Reviewed-by: Michael S. Tsirkin Reviewed-by: Anthony PERARD Tested-by: Chuck Zmudzinski Message-Id: <20230312120221.99183-7-shentey@gmail.com> Message-Id: <20230403074124.3925-8-shentey@gmail.com> Signed-off-by: Anthony PERARD --- hw/i386/pc_piix.c | 5 ++--- hw/isa/piix3.c | 15 --------------- include/hw/southbridge/piix.h | 1 - 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 2ed2b3f3c6..42af03dbb4 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -239,8 +239,6 @@ static void pc_init1(MachineState *machine, if (pcmc->pci_enabled) { PIIX3State *piix3; PCIDevice *pci_dev; - const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE - : TYPE_PIIX3_DEVICE; pci_bus = i440fx_init(pci_type, i440fx_host, @@ -253,7 +251,8 @@ static void pc_init1(MachineState *machine, : pc_pci_slot_get_pirq); pcms->bus = pci_bus; - pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); + pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, + TYPE_PIIX3_DEVICE); if (xen_enabled()) { pci_device_set_intx_routing_notifier( diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c index 0549e043ca..117024e450 100644 --- a/hw/isa/piix3.c +++ b/hw/isa/piix3.c @@ -30,7 +30,6 @@ #include "hw/irq.h" #include "hw/qdev-properties.h" #include "hw/isa/isa.h" -#include "hw/xen/xen.h" #include "sysemu/runstate.h" #include "migration/vmstate.h" #include "hw/acpi/acpi_aml_interface.h" @@ -381,24 +380,10 @@ static const TypeInfo piix3_info = { .class_init = piix3_class_init, }; -static void piix3_xen_class_init(ObjectClass *klass, void *data) -{ - PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - - k->realize = piix3_realize; -} - -static const TypeInfo piix3_xen_info = { - .name = TYPE_PIIX3_XEN_DEVICE, - .parent = TYPE_PIIX3_PCI_DEVICE, - .class_init = piix3_xen_class_init, -}; - static void piix3_register_types(void) { type_register_static(&piix3_pci_type_info); type_register_static(&piix3_info); - type_register_static(&piix3_xen_info); } type_init(piix3_register_types) diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h index a840340308..278171752f 100644 --- a/include/hw/southbridge/piix.h +++ b/include/hw/southbridge/piix.h @@ -67,7 +67,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE, TYPE_PIIX3_PCI_DEVICE) #define TYPE_PIIX3_DEVICE "PIIX3" -#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen" #define TYPE_PIIX4_PCI_DEVICE "piix4-isa" #endif From 9000666052f99ed4217e75b73636acae61e6fc2c Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Tue, 6 Jun 2023 14:16:05 +0100 Subject: [PATCH 12/12] xen-block: fix segv on unrealize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backtrace: qemu_lockcnt_lock (lockcnt=0xb4) at ../util/lockcnt.c:238 aio_set_fd_handler (ctx=0x0, fd=51, is_external=true, io_read=0x0, io_write=0x0, io_poll=0x0, io_poll_ready=0x0, opaque=0x0) at ../util/aio-posix.c:119 xen_device_unbind_event_channel (xendev=0x55c6da5b5000, channel=0x55c6da6c4c80, errp=0x7fff641ac608) at ../hw/xen/xen-bus.c:926 xen_block_dataplane_stop (dataplane=0x55c6da6ddbe0) at ../hw/block/dataplane/xen-block.c:719 xen_block_disconnect (xendev=0x55c6da5b5000, errp=0x0) at ../hw/block/xen-block.c:48 xen_block_unrealize (xendev=0x55c6da5b5000) at ../hw/block/xen-block.c:154 xen_device_unrealize (dev=0x55c6da5b5000) at ../hw/xen/xen-bus.c:956 xen_device_exit (n=0x55c6da5b50d0, data=0x0) at ../hw/xen/xen-bus.c:985 notifier_list_notify (list=0x55c6d91f9820 , data=0x0) at ../util/notify.c:39 qemu_run_exit_notifiers () at ../softmmu/runstate.c:760 Fixes: f6eac904f682 ("xen-block: implement BlockDevOps->drained_begin()") Signed-off-by: Anthony PERARD Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefan Hajnoczi Message-Id: <20230606131605.55596-1-anthony.perard@citrix.com> Signed-off-by: Anthony PERARD --- hw/xen/xen-bus.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c index 1e08cf027a..ece8ec40cd 100644 --- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -923,8 +923,10 @@ void xen_device_unbind_event_channel(XenDevice *xendev, QLIST_REMOVE(channel, list); - aio_set_fd_handler(channel->ctx, qemu_xen_evtchn_fd(channel->xeh), - NULL, NULL, NULL, NULL, NULL); + if (channel->ctx) { + aio_set_fd_handler(channel->ctx, qemu_xen_evtchn_fd(channel->xeh), + NULL, NULL, NULL, NULL, NULL); + } if (qemu_xen_evtchn_unbind(channel->xeh, channel->local_port) < 0) { error_setg_errno(errp, errno, "xenevtchn_unbind failed");