Re: [PATCH] fix qemu build with xen-4.18.0
Hi Stefan, Stefan Hajnoczi writes: > On Tue, 12 Dec 2023 at 10:36, Volodymyr Babchuk > wrote: >> >> Hi Anthony >> >> Anthony PERARD writes: >> >> > On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote: >> >> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote: >> >> > On Thu, Dec 07, 2023 at 11:12:48PM +, Michael Young wrote: >> >> > > Builds of qemu-8.2.0rc2 with xen-4.18.0 are currently failing >> >> > > with errors like >> >> > > ../hw/arm/xen_arm.c:74:5: error: ‘GUEST_VIRTIO_MMIO_SPI_LAST’ >> >> > > undeclared (first use in this function) >> >> > >74 |(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST) >> >> > > | ^~ >> >> > > >> >> > > as there is an incorrect comparision in include/hw/xen/xen_native.h >> >> > > which means that settings like GUEST_VIRTIO_MMIO_SPI_LAST >> >> > > aren't being defined for xen-4.18.0 >> >> > >> >> > The conditions in arch-arm.h for xen 4.18 show: >> >> > >> >> > $ cppi arch-arm.h | grep -E '(#.*if)|MMIO' >> >> > #ifndef __XEN_PUBLIC_ARCH_ARM_H__ >> >> > # if defined(__XEN__) || defined(__XEN_TOOLS__) || defined(__GNUC__) >> >> > # endif >> >> > # ifndef __ASSEMBLY__ >> >> > # if defined(__XEN__) || defined(__XEN_TOOLS__) >> >> > # if defined(__GNUC__) && !defined(__STRICT_ANSI__) >> >> > # endif >> >> > # endif /* __XEN__ || __XEN_TOOLS__ */ >> >> > # endif >> >> > # if defined(__XEN__) || defined(__XEN_TOOLS__) >> >> > # define PSR_MODE_BIT 0x10U /* Set iff AArch32 */ >> >> > /* Virtio MMIO mappings */ >> >> > # define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x0200) >> >> > # define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x0010) >> >> > # define GUEST_VIRTIO_MMIO_SPI_FIRST 33 >> >> > # define GUEST_VIRTIO_MMIO_SPI_LAST43 >> >> > # endif >> >> > # ifndef __ASSEMBLY__ >> >> > # endif >> >> > #endif /* __XEN_PUBLIC_ARCH_ARM_H__ */ >> >> > >> >> > So the MMIO constants are available if __XEN__ or __XEN_TOOLS__ >> >> > are defined. This is no different to the condition that was >> >> > present in Xen 4.17. >> >> > >> >> > What you didn't mention was that the Fedora build failure is >> >> > seen on an x86_64 host, when building the aarch64 target QEMU, >> >> > and I think this is the key issue. >> >> >> >> Hi Daniel, thanks for looking into it. >> >> >> >> - you are building on a x86_64 host >> >> - the target is aarch64 >> >> - the target is the aarch64 Xen PVH machine (xen_arm.c) >> >> >> >> But is the resulting QEMU binary expected to be an x86 binary? Or are >> >> you cross compiling ARM binaries on a x86 host? >> >> >> >> In other word, is the resulting QEMU binary expected to run on ARM or >> >> x86? >> >> >> >> >> >> > Are we expecting to build Xen support for non-arch native QEMU >> >> > system binaries or not ? >> >> >> >> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not >> >> Xen on x86. So this is only expected to work if you are >> >> cross-compiling. But you can cross-compile both Xen and QEMU, and I am >> >> pretty sure that Yocto is able to build Xen, Xen userspace tools, and >> >> QEMU for Xen/ARM on an x86 host today. >> >> >> >> >> >> > The constants are defined in arch-arm.h, which is only included >> >> > under: >> >> > >> >> > #if defined(__i386__) || defined(__x86_64__) >> >> > #include "arch-x86/xen.h" >> >> > #elif defined(__arm__) || defined (__aarch64__) >> >> > #include "arch-arm.h" >> >> > #else >> >> > #error "Unsupported architecture" >> >> > #endif >> >> > >> >> > >> >> > When we are building on an x86_64 host, we not going to get >> >> > arch-arm.h included, even if we're trying to build the aarch64 >> >> > system emulator. >> >> >
Re: [PATCH] fix qemu build with xen-4.18.0
Hi Anthony
Anthony PERARD writes:
> On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
>> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
>> > On Thu, Dec 07, 2023 at 11:12:48PM +, Michael Young wrote:
>> > > Builds of qemu-8.2.0rc2 with xen-4.18.0 are currently failing
>> > > with errors like
>> > > ../hw/arm/xen_arm.c:74:5: error: ‘GUEST_VIRTIO_MMIO_SPI_LAST’ undeclared
>> > > (first use in this function)
>> > >74 |(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
>> > > | ^~
>> > >
>> > > as there is an incorrect comparision in include/hw/xen/xen_native.h
>> > > which means that settings like GUEST_VIRTIO_MMIO_SPI_LAST
>> > > aren't being defined for xen-4.18.0
>> >
>> > The conditions in arch-arm.h for xen 4.18 show:
>> >
>> > $ cppi arch-arm.h | grep -E '(#.*if)|MMIO'
>> > #ifndef __XEN_PUBLIC_ARCH_ARM_H__
>> > # if defined(__XEN__) || defined(__XEN_TOOLS__) || defined(__GNUC__)
>> > # endif
>> > # ifndef __ASSEMBLY__
>> > # if defined(__XEN__) || defined(__XEN_TOOLS__)
>> > # if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>> > # endif
>> > # endif /* __XEN__ || __XEN_TOOLS__ */
>> > # endif
>> > # if defined(__XEN__) || defined(__XEN_TOOLS__)
>> > # define PSR_MODE_BIT 0x10U /* Set iff AArch32 */
>> > /* Virtio MMIO mappings */
>> > # define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x0200)
>> > # define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x0010)
>> > # define GUEST_VIRTIO_MMIO_SPI_FIRST 33
>> > # define GUEST_VIRTIO_MMIO_SPI_LAST43
>> > # endif
>> > # ifndef __ASSEMBLY__
>> > # endif
>> > #endif /* __XEN_PUBLIC_ARCH_ARM_H__ */
>> >
>> > So the MMIO constants are available if __XEN__ or __XEN_TOOLS__
>> > are defined. This is no different to the condition that was
>> > present in Xen 4.17.
>> >
>> > What you didn't mention was that the Fedora build failure is
>> > seen on an x86_64 host, when building the aarch64 target QEMU,
>> > and I think this is the key issue.
>>
>> Hi Daniel, thanks for looking into it.
>>
>> - you are building on a x86_64 host
>> - the target is aarch64
>> - the target is the aarch64 Xen PVH machine (xen_arm.c)
>>
>> But is the resulting QEMU binary expected to be an x86 binary? Or are
>> you cross compiling ARM binaries on a x86 host?
>>
>> In other word, is the resulting QEMU binary expected to run on ARM or
>> x86?
>>
>>
>> > Are we expecting to build Xen support for non-arch native QEMU
>> > system binaries or not ?
>>
>> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
>> Xen on x86. So this is only expected to work if you are
>> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
>> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
>> QEMU for Xen/ARM on an x86 host today.
>>
>>
>> > The constants are defined in arch-arm.h, which is only included
>> > under:
>> >
>> > #if defined(__i386__) || defined(__x86_64__)
>> > #include "arch-x86/xen.h"
>> > #elif defined(__arm__) || defined (__aarch64__)
>> > #include "arch-arm.h"
>> > #else
>> > #error "Unsupported architecture"
>> > #endif
>> >
>> >
>> > When we are building on an x86_64 host, we not going to get
>> > arch-arm.h included, even if we're trying to build the aarch64
>> > system emulator.
>> >
>> > I don't know how this is supposed to work ?
>>
>> It looks like a host vs. target architecture mismatch: the #if defined
>> (__aarch64__) check should pass I think.
>
>
> Building qemu with something like:
> ./configure --enable-xen --cpu=x86_64
> used to work. Can we fix that? It still works with v8.1.0.
> At least, it works on x86, I never really try to build qemu for arm.
> Notice that there's no "--target-list" on the configure command line.
> I don't know if --cpu is useful here.
>
> Looks like the first commit where the build doesn't work is
> 7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").
I am currently trying to upstream this patch. It is in the QEMU mailing
list but it was never accepted. It is not reviewed in fact. I'll take a
look at it, but I don't understand how did you get in the first place.
--
WBR, Volodymyr
[PATCH v4 2/6] xen: backends: don't overwrite XenStore nodes created by toolstack
Xen PV devices in QEMU can be created in two ways: either by QEMU
itself, if they were passed via command line, or by Xen toolstack. In
the latter case, QEMU scans XenStore entries and configures devices
accordingly.
In the second case we don't want QEMU to write/delete front-end
entries for two reasons: it might have no access to those entries if
it is running in un-privileged domain and it is just incorrect to
overwrite entries already provided by Xen toolstack, because toolstack
manages those nodes. For example, it might read backend- or frontend-
state to be sure that they are both disconnected and it is safe to
destroy a domain.
This patch checks presence of xendev->backend to check if Xen PV
device was configured by Xen toolstack to decide if it should touch
frontend entries in XenStore. Also, when we need to remove XenStore
entries during device teardown only if they weren't created by Xen
toolstack. If they were created by toolstack, then it is toolstack's
job to do proper clean-up.
Suggested-by: Paul Durrant
Suggested-by: David Woodhouse
Co-Authored-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
Reviewed-by: David Woodhouse
---
Changes in v4:
- don't touch "tty" entry in the console backend
Changes in v3:
- Rephrased the commit message
---
hw/block/xen-block.c | 16 +---
hw/net/xen_nic.c | 18 ++
hw/xen/xen-bus.c | 14 +-
3 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index c2ac9db4a2..dac519a6d3 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -390,13 +390,15 @@ static void xen_block_realize(XenDevice *xendev, Error
**errp)
xen_device_backend_printf(xendev, "info", "%u", blockdev->info);
-xen_device_frontend_printf(xendev, "virtual-device", "%lu",
- vdev->number);
-xen_device_frontend_printf(xendev, "device-type", "%s",
- blockdev->device_type);
-
-xen_device_backend_printf(xendev, "sector-size", "%u",
- conf->logical_block_size);
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "virtual-device", "%lu",
+ vdev->number);
+xen_device_frontend_printf(xendev, "device-type", "%s",
+ blockdev->device_type);
+
+xen_device_backend_printf(xendev, "sector-size", "%u",
+ conf->logical_block_size);
+}
xen_block_set_size(blockdev);
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index afa10c96e8..27442bef38 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -315,14 +315,16 @@ static void xen_netdev_realize(XenDevice *xendev, Error
**errp)
qemu_macaddr_default_if_unset(>conf.macaddr);
-xen_device_frontend_printf(xendev, "mac", "%02x:%02x:%02x:%02x:%02x:%02x",
- netdev->conf.macaddr.a[0],
- netdev->conf.macaddr.a[1],
- netdev->conf.macaddr.a[2],
- netdev->conf.macaddr.a[3],
- netdev->conf.macaddr.a[4],
- netdev->conf.macaddr.a[5]);
-
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "mac",
+ "%02x:%02x:%02x:%02x:%02x:%02x",
+ netdev->conf.macaddr.a[0],
+ netdev->conf.macaddr.a[1],
+ netdev->conf.macaddr.a[2],
+ netdev->conf.macaddr.a[3],
+ netdev->conf.macaddr.a[4],
+ netdev->conf.macaddr.a[5]);
+}
netdev->nic = qemu_new_nic(_xen_info, >conf,
object_get_typename(OBJECT(xendev)),
DEVICE(xendev)->id, netdev);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index dd0171ab98..d0f17aeb27 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -599,8 +599,10 @@ static void xen_device_backend_destroy(XenDevice *xendev)
g_assert(xenbus->xsh);
-xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->backend_path,
-_err);
+if (!xendev->backend) {
+xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->backend_path,
+_err);
+}
g_free(xendev->backend_path);
xendev->backend_path = NULL;
@@ -764,8 +766,10 @@ static void xen_device_frontend_destroy(XenDevice *xendev)
g_assert(xenbus->xsh);
-xs_node_destroy(xenbus-&g
[PATCH v4 6/6] xen_arm: Add virtual PCIe host bridge support
From: Oleksandr Tyshchenko
The bridge is needed for virtio-pci support, as QEMU can emulate the
whole bridge with any virtio-pci devices connected to it.
This patch provides a flexible way to configure PCIe bridge resources
using QEMU machine properties. We made this for several reasons:
- We don't want to clash with vPCI devices, so we need information
from Xen toolstack on which PCI bus to use.
- The guest memory layout that describes these resources is not stable
and may vary between guests, so we cannot rely on static resources
to be always the same for both ends.
- Also the device-models which run in different domains and serve
virtio-pci devices for the same guest should use different host
bridge resources for Xen to distinguish. The rule for the guest
device-tree generation is one PCI host bridge per backend domain.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
Changes in v3:
- Use QOM properties instead of reading from XenStore
- Remove unneeded includes
- Move pcie_* fields into "struct cfg"
Changes in v2:
- Renamed virtio_pci_host to pcie_host entries in XenStore, because
there is nothing specific to virtio-pci: any PCI device can be
emulated via this newly created bridge.
---
hw/arm/xen_arm.c| 226
hw/xen/xen-hvm-common.c | 9 +-
include/hw/xen/xen_native.h | 8 +-
3 files changed, 240 insertions(+), 3 deletions(-)
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index b9c3ae14b6..dc6d3a1d82 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -34,6 +34,7 @@
#include "hw/xen/xen-hvm-common.h"
#include "sysemu/tpm.h"
#include "hw/xen/arch_hvm.h"
+#include "hw/pci-host/gpex.h"
#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
@@ -57,6 +58,10 @@ struct XenArmState {
struct {
uint64_t tpm_base_addr;
+MemMapEntry pcie_mmio;
+MemMapEntry pcie_ecam;
+MemMapEntry pcie_mmio_high;
+int pcie_irq_base;
} cfg;
};
@@ -73,6 +78,15 @@ static MemoryRegion ram_lo, ram_hi;
#define NR_VIRTIO_MMIO_DEVICES \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
+#define XEN_ARM_PCIE_ECAM_BASE "pcie-ecam-base"
+#define XEN_ARM_PCIE_ECAM_SIZE "pcie-ecam-size"
+#define XEN_ARM_PCIE_MEM_BASE "pcie-mem-base"
+#define XEN_ARM_PCIE_MEM_SIZE "pcie-mem-size"
+#define XEN_ARM_PCIE_PREFETCH_BASE "pcie-prefetch-mem-base"
+#define XEN_ARM_PCIE_PREFETCH_SIZE "pcie-prefetch-mem-size"
+#define XEN_ARM_PCIE_IRQ_BASE "pcie-irq-base"
+
+/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI interrupts. */
static void xen_set_irq(void *opaque, int irq, int level)
{
if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
@@ -129,6 +143,89 @@ static void xen_init_ram(MachineState *machine)
}
}
+static bool xen_validate_pcie_config(XenArmState *xam)
+{
+if (xam->cfg.pcie_ecam.base == 0 &&
+xam->cfg.pcie_ecam.size == 0 &&
+xam->cfg.pcie_mmio.base == 0 &&
+xam->cfg.pcie_mmio.size == 0 &&
+xam->cfg.pcie_mmio_high.base == 0 &&
+xam->cfg.pcie_mmio_high.size == 0 &&
+xam->cfg.pcie_irq_base == 0) {
+
+/* It's okay, user just don't want PCIe brige */
+
+return false;
+}
+
+if (xam->cfg.pcie_ecam.base == 0 ||
+xam->cfg.pcie_ecam.size == 0 ||
+xam->cfg.pcie_mmio.base == 0 ||
+xam->cfg.pcie_mmio.size == 0 ||
+xam->cfg.pcie_mmio_high.base == 0 ||
+xam->cfg.pcie_mmio_high.size == 0 ||
+xam->cfg.pcie_irq_base == 0) {
+
+/* User provided some PCIe options, but not all of them */
+
+error_printf("Incomplete PCIe bridge configuration\n");
+
+exit(1);
+}
+
+return true;
+}
+
+static void xen_create_pcie(XenArmState *xam)
+{
+MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+MemoryRegion *ecam_alias, *ecam_reg;
+DeviceState *dev;
+int i;
+
+dev = qdev_new(TYPE_GPEX_HOST);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+
+/* Map ECAM space */
+ecam_alias = g_new0(MemoryRegion, 1);
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+ ecam_reg, 0, xam->cfg.pcie_ecam.size);
+memory_region_add_subregion(get_system_memory(), xam->cfg.pcie_ecam.base,
+ecam_alias);
+
+/* Map the MMIO space */
+mmio_alias = g_new0(MemoryRegion, 1);
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_init_alias(
[PATCH v4 5/6] xen_arm: set mc->max_cpus to GUEST_MAX_VCPUS
From: Oleksandr Tyshchenko The number of vCPUs used for the IOREQ configuration (machine->smp.cpus) should really match the system value as for each vCPU we setup a dedicated evtchn for the communication with Xen at the runtime. This is needed for the IOREQ to be properly configured and work if the involved domain has more than one vCPU assigned. Set the number of current supported guest vCPUs here (128) which is defined in public header arch-arm.h. So when the toolstack pass max_vcpus using "-smp" arg, machine creation will not fail. Signed-off-by: Oleksandr Tyshchenko Signed-off-by: Volodymyr Babchuk Reviewed-by: Philippe Mathieu-Daudé --- hw/arm/xen_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c index a5631529d0..b9c3ae14b6 100644 --- a/hw/arm/xen_arm.c +++ b/hw/arm/xen_arm.c @@ -231,7 +231,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); mc->desc = "Xen Para-virtualized PC"; mc->init = xen_arm_init; -mc->max_cpus = 1; +mc->max_cpus = GUEST_MAX_VCPUS; mc->default_machine_opts = "accel=xen"; /* Set explicitly here to make sure that real ram_size is passed */ mc->default_ram_size = 0; -- 2.42.0
[RFC PATCH v4 4/6] xen: add option to disable legacy backends
This patch makes legacy backends optional. As was discussed at [1]
this is a solution to a problem when we can't run QEMU as a device
model in a non-privileged domain. This is because legacy backends
assume that they are always running in domain with ID = 0. Actually,
this may prevent running QEMU in a privileged domain with ID not equal
to zero.
With this patch it is possible to provide
"--disable-xen-legacy-backends" configure option to get QEMU binary
that can run in a driver domain. With price of not be able to use
legacy backends of course.
[1]
https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html
Signed-off-by: Volodymyr Babchuk
---
I am not sure if I made correct changes to build system, so this patch
is tagged as RFC.
Changes in v3:
- New patch in v3
---
hw/9pfs/meson.build | 4 +++-
hw/display/meson.build| 4 +++-
hw/i386/pc.c | 2 ++
hw/usb/meson.build| 5 -
hw/xen/meson.build| 11 ---
hw/xen/xen-hvm-common.c | 2 ++
hw/xenpv/xen_machine_pv.c | 2 ++
meson.build | 5 +
meson_options.txt | 2 ++
scripts/meson-buildoptions.sh | 4
10 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 2944ea63c3..e8306ba8d2 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -15,7 +15,9 @@ fs_ss.add(files(
))
fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c'))
-fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c'))
+if have_xen_legacy_backends
+ fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c'))
+endif
system_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
specific_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c'))
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 344dfe3d8c..18d657f6b3 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true: files('pl110.c'))
system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c'))
system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c'))
system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c'))
-system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
+if have_xen_legacy_backends
+ system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
+endif
system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 29b9964733..91857af428 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1263,7 +1263,9 @@ void pc_basic_device_init(struct PCMachineState *pcms,
pci_create_simple(pcms->bus, -1, "xen-platform");
}
pcms->xenbus = xen_bus_init();
+#ifdef CONFIG_XEN_LEGACY_BACKENDS
xen_be_init();
+#endif
}
#endif
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index e94149ebde..8d395745b2 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -84,6 +84,9 @@ if libusb.found()
hw_usb_modules += {'host': usbhost_ss}
endif
-system_ss.add(when: ['CONFIG_USB', 'CONFIG_XEN_BUS', libusb], if_true:
files('xen-usb.c'))
+if have_xen_legacy_backends
+ system_ss.add(when: ['CONFIG_USB', 'CONFIG_XEN_BUS', libusb],
+if_true: files('xen-usb.c'))
+endif
modules += { 'hw-usb': hw_usb_modules }
diff --git a/hw/xen/meson.build b/hw/xen/meson.build
index d887fa9ba4..964c3364f2 100644
--- a/hw/xen/meson.build
+++ b/hw/xen/meson.build
@@ -2,11 +2,16 @@ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files(
'xen-backend.c',
'xen-bus-helper.c',
'xen-bus.c',
- 'xen-legacy-backend.c',
- 'xen_devconfig.c',
- 'xen_pvdev.c',
))
+if have_xen_legacy_backends
+ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files(
+'xen_pvdev.c',
+'xen-legacy-backend.c',
+'xen_devconfig.c',
+ ))
+endif
+
system_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
'xen-operations.c',
))
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 565dc39c8f..2e7897dbd2 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -869,7 +869,9 @@ void xen_register_ioreq(XenIOState *state, unsigned int
max_cpus,
xen_bus_init();
+#ifdef CONFIG_XEN_LEGACY_BACKENDS
xen_be_init();
+#endif
return;
diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 9f9f137f99..03a55f345c 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -37,7 +37,9 @@ static void xen_init_pv(MachineState *machine)
setup_xen_backend_ops();
/* Initialize backend core & drivers */
+#ifdef CONFIG_XEN_LEGACY_BACKENDS
xen_be_init();
+#endif
switch (xen_mode) {
case XEN_ATTACH:
diff --git a/meson.build b/meson.buil
[PATCH v4 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
From: David Woodhouse
This allows a XenDevice implementation to know whether it was created
by QEMU, or merely discovered in XenStore after the toolstack created
it. This will allow us to create frontend/backend nodes only when we
should, rather than unconditionally attempting to overwrite them from
a driver domain which doesn't have privileges to do so.
As an added benefit, it also means we no longer have to call the
xen_backend_set_device() function from the device models immediately
after calling qdev_realize_and_unref(). Even though we could make
the argument that it's safe to do so, and the pointer to the unreffed
device *will* actually still be valid, it still made my skin itch to
look at it.
Signed-off-by: David Woodhouse
Reviewed-by: Paul Durrant
---
hw/block/xen-block.c | 3 +--
hw/char/xen_console.c| 2 +-
hw/net/xen_nic.c | 2 +-
hw/xen/xen-backend.c | 15 +--
hw/xen/xen-bus.c | 4
include/hw/xen/xen-backend.h | 2 --
include/hw/xen/xen-bus.h | 2 ++
7 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 6d64ede94f..c2ac9db4a2 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -1081,13 +1081,12 @@ static void xen_block_device_create(XenBackendInstance
*backend,
blockdev->iothread = iothread;
blockdev->drive = drive;
+xendev->backend = backend;
if (!qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
error_prepend(errp, "realization of device %s failed: ", type);
goto fail;
}
-
-xen_backend_set_device(backend, xendev);
return;
fail:
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 5cbee2f184..bef8a3a621 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -600,8 +600,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
goto fail;
}
+xendev->backend = backend;
if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
goto done;
}
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index af4ba3f1e6..afa10c96e8 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -627,8 +627,8 @@ static void xen_net_device_create(XenBackendInstance
*backend,
net->dev = number;
memcpy(>conf.macaddr, , sizeof(mac));
+xendev->backend = backend;
if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
return;
}
diff --git a/hw/xen/xen-backend.c b/hw/xen/xen-backend.c
index b9bf70a9f5..b2e753ebc8 100644
--- a/hw/xen/xen-backend.c
+++ b/hw/xen/xen-backend.c
@@ -88,19 +88,6 @@ static void xen_backend_list_add(XenBackendInstance *backend)
QLIST_INSERT_HEAD(_list, backend, entry);
}
-static XenBackendInstance *xen_backend_list_find(XenDevice *xendev)
-{
-XenBackendInstance *backend;
-
-QLIST_FOREACH(backend, _list, entry) {
-if (backend->xendev == xendev) {
-return backend;
-}
-}
-
-return NULL;
-}
-
bool xen_backend_exists(const char *type, const char *name)
{
const XenBackendImpl *impl = xen_backend_table_lookup(type);
@@ -170,7 +157,7 @@ XenDevice *xen_backend_get_device(XenBackendInstance
*backend)
bool xen_backend_try_device_destroy(XenDevice *xendev, Error **errp)
{
-XenBackendInstance *backend = xen_backend_list_find(xendev);
+XenBackendInstance *backend = xendev->backend;
const XenBackendImpl *impl;
if (!backend) {
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 4973e7d9c9..dd0171ab98 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1079,6 +1079,10 @@ static void xen_device_realize(DeviceState *dev, Error
**errp)
}
}
+if (xendev->backend) {
+xen_backend_set_device(xendev->backend, xendev);
+}
+
xendev->exit.notify = xen_device_exit;
qemu_add_exit_notifier(>exit);
return;
diff --git a/include/hw/xen/xen-backend.h b/include/hw/xen/xen-backend.h
index 0f01631ae7..ea080ba7c9 100644
--- a/include/hw/xen/xen-backend.h
+++ b/include/hw/xen/xen-backend.h
@@ -10,8 +10,6 @@
#include "hw/xen/xen-bus.h"
-typedef struct XenBackendInstance XenBackendInstance;
-
typedef void (*XenBackendDeviceCreate)(XenBackendInstance *backend,
QDict *opts, Error **errp);
typedef void (*XenBackendDeviceDestroy)(XenBackendInstance *backend,
diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h
index 334ddd1ff6..7647c4c38e 100644
--- a/include/hw/xen/xen-bus.h
+++ b/include/hw/xen/xen-bus.h
@@ -14,9 +14,11 @@
#include "qom/object.h"
typedef struct XenEventChannel XenEventChannel;
+typedef struct XenBackendInstance XenBackendInstance;
struct XenDevice {
DeviceState qdev;
+XenBackendInstance *backend;
domid_t frontend_id;
char *name;
struct qemu_xs_handle *xsh;
[PATCH v4 3/6] xen: decouple generic xen code from legacy backends codebase
In xen-all.c there are unneeded dependencies on xen-legacy-backend.c:
- xen_init() uses xen_pv_printf() to report errors, but it does not
provide a pointer to the struct XenLegacyDevice, so it is kind of
useless, we can use standard error_report() instead.
- xen-all.c has function xenstore_record_dm_state() which uses global
variable "xenstore" defined and initialized in xen-legacy-backend.c
It is used exactly once, so we can just open a new connection to the
xenstore, update DM state and close connection back.
Those two changes allows us to remove xen-legacy-backend.c at all,
what should be done in the future anyways. But right now this patch
moves us one step close to have QEMU build without legacy Xen
backends.
Signed-off-by: Volodymyr Babchuk
---
In v4:
- New in v4, previous was part of "xen: add option to disable legacy
backends"
- Do not move xenstore global variable from xen-legacy-backend.c,
instead use a local variable.
---
accel/xen/xen-all.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..6c2342581f 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -33,12 +33,20 @@ xendevicemodel_handle *xen_dmod;
static void xenstore_record_dm_state(const char *state)
{
char path[50];
+struct qemu_xs_handle *xsh = qemu_xen_xs_open();
+
+if (!xsh) {
+error_report("error opening xenstore");
+exit(1);
+}
snprintf(path, sizeof (path), "device-model/%u/state", xen_domid);
-if (!qemu_xen_xs_write(xenstore, XBT_NULL, path, state, strlen(state))) {
+if (!qemu_xen_xs_write(xsh, XBT_NULL, path, state, strlen(state))) {
error_report("error recording dm state");
exit(1);
}
+
+qemu_xen_xs_close(xsh);
}
@@ -80,18 +88,18 @@ static int xen_init(MachineState *ms)
xen_xc = xc_interface_open(0, 0, 0);
if (xen_xc == NULL) {
-xen_pv_printf(NULL, 0, "can't open xen interface\n");
+error_report("can't open xen interface\n");
return -1;
}
xen_fmem = xenforeignmemory_open(0, 0);
if (xen_fmem == NULL) {
-xen_pv_printf(NULL, 0, "can't open xen fmem interface\n");
+error_report("can't open xen fmem interface\n");
xc_interface_close(xen_xc);
return -1;
}
xen_dmod = xendevicemodel_open(0, 0);
if (xen_dmod == NULL) {
-xen_pv_printf(NULL, 0, "can't open xen devicemodel interface\n");
+error_report("can't open xen devicemodel interface\n");
xenforeignmemory_close(xen_fmem);
xc_interface_close(xen_xc);
return -1;
--
2.42.0
[PATCH v4 0/6] xen-arm: add support for virtio-pci
Hello, This patch series adds the basic support for virtio-pci for xen-arm guests. The main changes are in "xen_arm: Add virtual PCIe host bridge support", while most of other patches are required to make QEMU work as device model in a non-privileged domains like driver domain. New in version 4: - Patch "xen: decouple generic xen code from legacy backends codebase" is factored out from "xen: add option to disable legacy backends" - Updated patch "xen: backends: don't overwrite XenStore nodes created by toolstack" (see the patch description) New in version 3: - Use commandline/properties instead of xenstore entries to configure PCIe bridge - Instead of trying to fix legacy backends, just add option to disable them David Woodhouse (1): hw/xen: Set XenBackendInstance in the XenDevice before realizing it Oleksandr Tyshchenko (2): xen_arm: set mc->max_cpus to GUEST_MAX_VCPUS xen_arm: Add virtual PCIe host bridge support Volodymyr Babchuk (3): xen: backends: don't overwrite XenStore nodes created by toolstack xen: decouple generic xen code from legacy backends codebase xen: add option to disable legacy backends accel/xen/xen-all.c | 16 ++- hw/9pfs/meson.build | 4 +- hw/arm/xen_arm.c | 228 +- hw/block/xen-block.c | 19 +-- hw/char/xen_console.c | 2 +- hw/display/meson.build| 4 +- hw/i386/pc.c | 2 + hw/net/xen_nic.c | 20 +-- hw/usb/meson.build| 5 +- hw/xen/meson.build| 11 +- hw/xen/xen-backend.c | 15 +-- hw/xen/xen-bus.c | 18 ++- hw/xen/xen-hvm-common.c | 11 +- hw/xenpv/xen_machine_pv.c | 2 + include/hw/xen/xen-backend.h | 2 - include/hw/xen/xen-bus.h | 2 + include/hw/xen/xen_native.h | 8 +- meson.build | 5 + meson_options.txt | 2 + scripts/meson-buildoptions.sh | 4 + 20 files changed, 326 insertions(+), 54 deletions(-) -- 2.42.0
Re: [EXTERNAL] [PATCH v3 2/5] xen: backends: don't overwrite XenStore nodes created by toolstack
Hi David,
Thank you for the review
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Fri, 2023-11-24 at 23:24 +, Volodymyr Babchuk wrote:
>> Xen PV devices in QEMU can be created in two ways: either by QEMU
>> itself, if they were passed via command line, or by Xen toolstack. In
>> the latter case, QEMU scans XenStore entries and configures devices
>> accordingly.
>>
>> In the second case we don't want QEMU to write/delete front-end
>> entries for two reasons: it might have no access to those entries if
>> it is running in un-privileged domain and it is just incorrect to
>> overwrite entries already provided by Xen toolstack, because
>> toolstack
>> manages those nodes. For example, it might read backend- or frontend-
>> state to be sure that they are both disconnected and it is safe to
>> destroy a domain.
>>
>> This patch checks presence of xendev->backend to check if Xen PV
>> device was configured by Xen toolstack to decide if it should touch
>> frontend entries in XenStore. Also, when we need to remove XenStore
>> entries during device teardown only if they weren't created by Xen
>> toolstack. If they were created by toolstack, then it is toolstack's
>> job to do proper clean-up.
>>
>> Suggested-by: Paul Durrant
>> Suggested-by: David Woodhouse
>> Co-Authored-by: Oleksandr Tyshchenko
>> Signed-off-by: Volodymyr Babchuk
>
> Reviewed-by: David Woodhouse
>
> ... albeit with a couple of suggestions...
>
>> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
>> index bef8a3a621..b52abf 100644
>> --- a/hw/char/xen_console.c
>> +++ b/hw/char/xen_console.c
>> @@ -450,7 +450,7 @@ static void xen_console_realize(XenDevice *xendev, Error
>> **errp)
>>
>> trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
>>
>> - if (CHARDEV_IS_PTY(cs)) {
>> + if (CHARDEV_IS_PTY(cs) && !xendev->backend) {
>> /* Strip the leading 'pty:' */
>> xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
>> }
>
>
> It's kind of weird that that one is a frontend node at all; surely it
> should have been a backend node?
Yeah, AFAIK, console was the first PV driver, so it is a bit strange.
As I see, this frontend entry is used by "xl console" tool to find PTY
device to attach to. So yes, it should be in backend part of the
xenstore. But I don't believe we can fix this right now.
> But it is known only to QEMU once it
> actually opens /dev/ptmx and creates a new pty. It can't be populated
> by the toolstack in advance.
>
> So shouldn't the toolstack have made it writable by the driver domain?
Maybe it can lead to a weird situation when user in Dom-0 tries to use
"xl console" command to attach to a console that is absent in Dom-0,
because "tty" entry points to PTY in the driver domain.
> I think we should attempt to write this and just gracefully handle the
> failure if we can't. (In fact, xen_device_frontend_printf() will just
> use error_report_err() which is probably OK unless you feel strongly
> about silencing it).
Nope, I am fine with this approach. I'll remove this hunk in the next
version.
>
>> diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
>> index afa10c96e8..27442bef38 100644
>> --- a/hw/net/xen_nic.c
>> +++ b/hw/net/xen_nic.c
>> @@ -315,14 +315,16 @@ static void xen_netdev_realize(XenDevice *xendev,
>> Error **errp)
>>
>> qemu_macaddr_default_if_unset(>conf.macaddr);
>>
>> - xen_device_frontend_printf(xendev, "mac",
>> "%02x:%02x:%02x:%02x:%02x:%02x",
>> - netdev->conf.macaddr.a[0],
>> - netdev->conf.macaddr.a[1],
>> - netdev->conf.macaddr.a[2],
>> - netdev->conf.macaddr.a[3],
>> - netdev->conf.macaddr.a[4],
>> - netdev->conf.macaddr.a[5]);
>> -
>> + if (!xendev->backend) {
>> + xen_device_frontend_printf(xendev, "mac",
>> + "%02x:%02x:%02x:%02x:%02x:%02x",
>> + netdev->conf.macaddr.a[0],
>> + netdev->conf.macaddr.a[1],
>> + netdev->conf.macaddr.a[2],
>> + netdev->conf.macaddr.a[3],
>> + netdev->conf.macaddr.a[4],
>> +
Re: [RFC PATCH v3 3/5] xen: add option to disable legacy backends
Hi David,
"Woodhouse, David" writes:
> [[S/MIME Signed Part:Undecided]]
> On Fri, 2023-11-24 at 23:24 +, Volodymyr Babchuk wrote:
>> This patch makes legacy backends optional. As was discussed at [1]
>> this is a solution to a problem when we can't run QEMU as a device
>> model in a non-privileged domain. This is because legacy backends
>> assume that they are always running in domain with ID = 0. Actually,
>> this may prevent running QEMU in a privileged domain with ID not equal
>> to zero.
>>
>> To be able to disable legacy backends we need to alter couple of
>> source files that unintentionally depend on them. For example
>> xen-all.c used xen_pv_printf to report errors, while not providing any
>> additional like xendev pointer. Also, we need to move xenstore
>> structure from xen-legacy-backend.c, because it is apparently used in
>> xen-all.c.
>>
>> With this patch it is possible to provide
>> "--disable-xen-legacy-backends" configure option to get QEMU binary
>> that can run in a driver domain. With price of not be able to use
>> legacy backends of course.
>>
>> [1]
>> https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html
>>
>> Signed-off-by: Volodymyr Babchuk
>>
>> ---
>>
>> I am not sure if I made correct changes to the build system, thus this
>> patch is tagged as RFC.
>
> Hm, I was imagining a new CONFIG_LEGACY_XEN_BACKENDS option which would
> look a lot like CONFIG_XEN_BUS (which would now be only for the new
> XenBus code).
>
It was my original intention too. But it appears that it is not possible
to add Kconfig value and then make it configurable via ./config
script. As I understood it can be set only via defconfig file.
> This looks weird to me:
>
>> --- a/hw/display/meson.build
>> +++ b/hw/display/meson.build
>> @@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true:
>> files('pl110.c'))
>> system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c'))
>> system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c'))
>> system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c'))
>> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
>> +if have_xen_legacy_backends
>> + system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
>> +endif
>>
>> system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
>> system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))
>
> I'd prefer to see just:
>
> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
> +system_ss.add(when: 'CONFIG_XEN_LEGACY_BACKENDS', if_true: files('xenfb.c'))
I tried, but it does not work this way. I need to create Kconfig
variable to do this, but then other problems appear.
>
> Probably also better to split out the bits in accel/xen/xen-all.c and
> hw/xen/xen-legacy-backend.c to a separate preparatory commit.
Okay, will do.
--
WBR, Volodymyr
[RFC PATCH v3 3/5] xen: add option to disable legacy backends
This patch makes legacy backends optional. As was discussed at [1]
this is a solution to a problem when we can't run QEMU as a device
model in a non-privileged domain. This is because legacy backends
assume that they are always running in domain with ID = 0. Actually,
this may prevent running QEMU in a privileged domain with ID not equal
to zero.
To be able to disable legacy backends we need to alter couple of
source files that unintentionally depend on them. For example
xen-all.c used xen_pv_printf to report errors, while not providing any
additional like xendev pointer. Also, we need to move xenstore
structure from xen-legacy-backend.c, because it is apparently used in
xen-all.c.
With this patch it is possible to provide
"--disable-xen-legacy-backends" configure option to get QEMU binary
that can run in a driver domain. With price of not be able to use
legacy backends of course.
[1]
https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html
Signed-off-by: Volodymyr Babchuk
---
I am not sure if I made correct changes to the build system, thus this
patch is tagged as RFC.
Changes in v3:
- New patch in v3
---
accel/xen/xen-all.c | 13 ++---
hw/9pfs/meson.build | 4 +++-
hw/display/meson.build| 4 +++-
hw/i386/pc.c | 2 ++
hw/usb/meson.build| 5 -
hw/xen/meson.build| 11 ---
hw/xen/xen-hvm-common.c | 2 ++
hw/xen/xen-legacy-backend.c | 7 ---
hw/xenpv/xen_machine_pv.c | 2 ++
meson.build | 5 +
meson_options.txt | 2 ++
scripts/meson-buildoptions.sh | 4
12 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..188b29597f 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -29,6 +29,7 @@ bool xen_allowed;
xc_interface *xen_xc;
xenforeignmemory_handle *xen_fmem;
xendevicemodel_handle *xen_dmod;
+struct qemu_xs_handle *xenstore;
static void xenstore_record_dm_state(const char *state)
{
@@ -78,20 +79,26 @@ static int xen_init(MachineState *ms)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
+xenstore = qemu_xen_xs_open();
+if (!xenstore) {
+error_report("can't connect to xenstored\n");
+exit(1);
+}
+
xen_xc = xc_interface_open(0, 0, 0);
if (xen_xc == NULL) {
-xen_pv_printf(NULL, 0, "can't open xen interface\n");
+error_report("can't open xen interface\n");
return -1;
}
xen_fmem = xenforeignmemory_open(0, 0);
if (xen_fmem == NULL) {
-xen_pv_printf(NULL, 0, "can't open xen fmem interface\n");
+error_report("can't open xen fmem interface\n");
xc_interface_close(xen_xc);
return -1;
}
xen_dmod = xendevicemodel_open(0, 0);
if (xen_dmod == NULL) {
-xen_pv_printf(NULL, 0, "can't open xen devicemodel interface\n");
+error_report("can't open xen devicemodel interface\n");
xenforeignmemory_close(xen_fmem);
xc_interface_close(xen_xc);
return -1;
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 2944ea63c3..e8306ba8d2 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -15,7 +15,9 @@ fs_ss.add(files(
))
fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c'))
-fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c'))
+if have_xen_legacy_backends
+ fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c'))
+endif
system_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
specific_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c'))
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 344dfe3d8c..18d657f6b3 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true: files('pl110.c'))
system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c'))
system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c'))
system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c'))
-system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
+if have_xen_legacy_backends
+ system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
+endif
system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 29b9964733..91857af428 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1263,7 +1263,9 @@ void pc_basic_device_init(struct PCMachineState *pcms,
pci_create_simple(pcms->bus, -1, "xen-platform");
}
pcms->xenbus = xen_bus_init();
+#ifdef CONFIG_XEN_LEGACY_BACKENDS
xen_be_init();
+#endif
}
#endif
diff
[PATCH v3 1/5] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
From: David Woodhouse
This allows a XenDevice implementation to know whether it was created
by QEMU, or merely discovered in XenStore after the toolstack created
it. This will allow us to create frontend/backend nodes only when we
should, rather than unconditionally attempting to overwrite them from
a driver domain which doesn't have privileges to do so.
As an added benefit, it also means we no longer have to call the
xen_backend_set_device() function from the device models immediately
after calling qdev_realize_and_unref(). Even though we could make
the argument that it's safe to do so, and the pointer to the unreffed
device *will* actually still be valid, it still made my skin itch to
look at it.
Signed-off-by: David Woodhouse
Reviewed-by: Paul Durrant
---
hw/block/xen-block.c | 3 +--
hw/char/xen_console.c| 2 +-
hw/net/xen_nic.c | 2 +-
hw/xen/xen-backend.c | 15 +--
hw/xen/xen-bus.c | 4
include/hw/xen/xen-backend.h | 2 --
include/hw/xen/xen-bus.h | 2 ++
7 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 6d64ede94f..c2ac9db4a2 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -1081,13 +1081,12 @@ static void xen_block_device_create(XenBackendInstance
*backend,
blockdev->iothread = iothread;
blockdev->drive = drive;
+xendev->backend = backend;
if (!qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
error_prepend(errp, "realization of device %s failed: ", type);
goto fail;
}
-
-xen_backend_set_device(backend, xendev);
return;
fail:
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 5cbee2f184..bef8a3a621 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -600,8 +600,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
goto fail;
}
+xendev->backend = backend;
if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
goto done;
}
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index af4ba3f1e6..afa10c96e8 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -627,8 +627,8 @@ static void xen_net_device_create(XenBackendInstance
*backend,
net->dev = number;
memcpy(>conf.macaddr, , sizeof(mac));
+xendev->backend = backend;
if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
return;
}
diff --git a/hw/xen/xen-backend.c b/hw/xen/xen-backend.c
index b9bf70a9f5..b2e753ebc8 100644
--- a/hw/xen/xen-backend.c
+++ b/hw/xen/xen-backend.c
@@ -88,19 +88,6 @@ static void xen_backend_list_add(XenBackendInstance *backend)
QLIST_INSERT_HEAD(_list, backend, entry);
}
-static XenBackendInstance *xen_backend_list_find(XenDevice *xendev)
-{
-XenBackendInstance *backend;
-
-QLIST_FOREACH(backend, _list, entry) {
-if (backend->xendev == xendev) {
-return backend;
-}
-}
-
-return NULL;
-}
-
bool xen_backend_exists(const char *type, const char *name)
{
const XenBackendImpl *impl = xen_backend_table_lookup(type);
@@ -170,7 +157,7 @@ XenDevice *xen_backend_get_device(XenBackendInstance
*backend)
bool xen_backend_try_device_destroy(XenDevice *xendev, Error **errp)
{
-XenBackendInstance *backend = xen_backend_list_find(xendev);
+XenBackendInstance *backend = xendev->backend;
const XenBackendImpl *impl;
if (!backend) {
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 4973e7d9c9..dd0171ab98 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1079,6 +1079,10 @@ static void xen_device_realize(DeviceState *dev, Error
**errp)
}
}
+if (xendev->backend) {
+xen_backend_set_device(xendev->backend, xendev);
+}
+
xendev->exit.notify = xen_device_exit;
qemu_add_exit_notifier(>exit);
return;
diff --git a/include/hw/xen/xen-backend.h b/include/hw/xen/xen-backend.h
index 0f01631ae7..ea080ba7c9 100644
--- a/include/hw/xen/xen-backend.h
+++ b/include/hw/xen/xen-backend.h
@@ -10,8 +10,6 @@
#include "hw/xen/xen-bus.h"
-typedef struct XenBackendInstance XenBackendInstance;
-
typedef void (*XenBackendDeviceCreate)(XenBackendInstance *backend,
QDict *opts, Error **errp);
typedef void (*XenBackendDeviceDestroy)(XenBackendInstance *backend,
diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h
index 334ddd1ff6..7647c4c38e 100644
--- a/include/hw/xen/xen-bus.h
+++ b/include/hw/xen/xen-bus.h
@@ -14,9 +14,11 @@
#include "qom/object.h"
typedef struct XenEventChannel XenEventChannel;
+typedef struct XenBackendInstance XenBackendInstance;
struct XenDevice {
DeviceState qdev;
+XenBackendInstance *backend;
domid_t frontend_id;
char *name;
struct qemu_xs_handle *xsh;
[PATCH v3 2/5] xen: backends: don't overwrite XenStore nodes created by toolstack
Xen PV devices in QEMU can be created in two ways: either by QEMU
itself, if they were passed via command line, or by Xen toolstack. In
the latter case, QEMU scans XenStore entries and configures devices
accordingly.
In the second case we don't want QEMU to write/delete front-end
entries for two reasons: it might have no access to those entries if
it is running in un-privileged domain and it is just incorrect to
overwrite entries already provided by Xen toolstack, because toolstack
manages those nodes. For example, it might read backend- or frontend-
state to be sure that they are both disconnected and it is safe to
destroy a domain.
This patch checks presence of xendev->backend to check if Xen PV
device was configured by Xen toolstack to decide if it should touch
frontend entries in XenStore. Also, when we need to remove XenStore
entries during device teardown only if they weren't created by Xen
toolstack. If they were created by toolstack, then it is toolstack's
job to do proper clean-up.
Suggested-by: Paul Durrant
Suggested-by: David Woodhouse
Co-Authored-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
Changes in v3:
- Rephrased the commit message
---
hw/block/xen-block.c | 16 +---
hw/char/xen_console.c | 2 +-
hw/net/xen_nic.c | 18 ++
hw/xen/xen-bus.c | 14 +-
4 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index c2ac9db4a2..dac519a6d3 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -390,13 +390,15 @@ static void xen_block_realize(XenDevice *xendev, Error
**errp)
xen_device_backend_printf(xendev, "info", "%u", blockdev->info);
-xen_device_frontend_printf(xendev, "virtual-device", "%lu",
- vdev->number);
-xen_device_frontend_printf(xendev, "device-type", "%s",
- blockdev->device_type);
-
-xen_device_backend_printf(xendev, "sector-size", "%u",
- conf->logical_block_size);
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "virtual-device", "%lu",
+ vdev->number);
+xen_device_frontend_printf(xendev, "device-type", "%s",
+ blockdev->device_type);
+
+xen_device_backend_printf(xendev, "sector-size", "%u",
+ conf->logical_block_size);
+}
xen_block_set_size(blockdev);
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index bef8a3a621..b52abf 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -450,7 +450,7 @@ static void xen_console_realize(XenDevice *xendev, Error
**errp)
trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
-if (CHARDEV_IS_PTY(cs)) {
+if (CHARDEV_IS_PTY(cs) && !xendev->backend) {
/* Strip the leading 'pty:' */
xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
}
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index afa10c96e8..27442bef38 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -315,14 +315,16 @@ static void xen_netdev_realize(XenDevice *xendev, Error
**errp)
qemu_macaddr_default_if_unset(>conf.macaddr);
-xen_device_frontend_printf(xendev, "mac", "%02x:%02x:%02x:%02x:%02x:%02x",
- netdev->conf.macaddr.a[0],
- netdev->conf.macaddr.a[1],
- netdev->conf.macaddr.a[2],
- netdev->conf.macaddr.a[3],
- netdev->conf.macaddr.a[4],
- netdev->conf.macaddr.a[5]);
-
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "mac",
+ "%02x:%02x:%02x:%02x:%02x:%02x",
+ netdev->conf.macaddr.a[0],
+ netdev->conf.macaddr.a[1],
+ netdev->conf.macaddr.a[2],
+ netdev->conf.macaddr.a[3],
+ netdev->conf.macaddr.a[4],
+ netdev->conf.macaddr.a[5]);
+}
netdev->nic = qemu_new_nic(_xen_info, >conf,
object_get_typename(OBJECT(xendev)),
DEVICE(xendev)->id, netdev);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index dd0171ab98..d0f17aeb27 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -599,8 +599,10 @@ static void xen_device_backend_destroy(XenDevice *xendev)
g_assert
[PATCH v3 4/5] xen_arm: set mc->max_cpus to GUEST_MAX_VCPUS
From: Oleksandr Tyshchenko The number of vCPUs used for the IOREQ configuration (machine->smp.cpus) should really match the system value as for each vCPU we setup a dedicated evtchn for the communication with Xen at the runtime. This is needed for the IOREQ to be properly configured and work if the involved domain has more than one vCPU assigned. Set the number of current supported guest vCPUs here (128) which is defined in public header arch-arm.h. So when the toolstack pass max_vcpus using "-smp" arg, machine creation will not fail. Signed-off-by: Oleksandr Tyshchenko Signed-off-by: Volodymyr Babchuk Reviewed-by: Philippe Mathieu-Daudé --- hw/arm/xen_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c index a5631529d0..b9c3ae14b6 100644 --- a/hw/arm/xen_arm.c +++ b/hw/arm/xen_arm.c @@ -231,7 +231,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); mc->desc = "Xen Para-virtualized PC"; mc->init = xen_arm_init; -mc->max_cpus = 1; +mc->max_cpus = GUEST_MAX_VCPUS; mc->default_machine_opts = "accel=xen"; /* Set explicitly here to make sure that real ram_size is passed */ mc->default_ram_size = 0; -- 2.42.0
[PATCH v3 0/5] xen-arm: add support for virtio-pci
Hello, This patch series adds the basic support for virtio-pci for xen-arm guests. The main changes are in "xen_arm: Add virtual PCIe host bridge support", while most of other patches are required to make QEMU work as device model in a non-privileged domains like driver domain. New in version 3: - Use commandline/properties instead of xenstore entries to configure PCIe bridge - Instead of trying to fix legacy backends, just add option to disable them David Woodhouse (1): hw/xen: Set XenBackendInstance in the XenDevice before realizing it Oleksandr Tyshchenko (2): xen_arm: set mc->max_cpus to GUEST_MAX_VCPUS xen_arm: Add virtual PCIe host bridge support Volodymyr Babchuk (2): xen: backends: don't overwrite XenStore nodes created by toolstack xen: add option to disable legacy backends accel/xen/xen-all.c | 13 +- hw/9pfs/meson.build | 4 +- hw/arm/xen_arm.c | 228 +- hw/block/xen-block.c | 19 +-- hw/char/xen_console.c | 4 +- hw/display/meson.build| 4 +- hw/i386/pc.c | 2 + hw/net/xen_nic.c | 20 +-- hw/usb/meson.build| 5 +- hw/xen/meson.build| 11 +- hw/xen/xen-backend.c | 15 +-- hw/xen/xen-bus.c | 18 ++- hw/xen/xen-hvm-common.c | 11 +- hw/xen/xen-legacy-backend.c | 7 -- hw/xenpv/xen_machine_pv.c | 2 + include/hw/xen/xen-backend.h | 2 - include/hw/xen/xen-bus.h | 2 + include/hw/xen/xen_native.h | 8 +- meson.build | 5 + meson_options.txt | 2 + scripts/meson-buildoptions.sh | 4 + 21 files changed, 325 insertions(+), 61 deletions(-) -- 2.42.0
[PATCH v3 5/5] xen_arm: Add virtual PCIe host bridge support
From: Oleksandr Tyshchenko
The bridge is needed for virtio-pci support, as QEMU can emulate the
whole bridge with any virtio-pci devices connected to it.
This patch provides a flexible way to configure PCIe bridge resources
using QEMU machine properties. We made this for several reasons:
- We don't want to clash with vPCI devices, so we need information
from Xen toolstack on which PCI bus to use.
- The guest memory layout that describes these resources is not stable
and may vary between guests, so we cannot rely on static resources
to be always the same for both ends.
- Also the device-models which run in different domains and serve
virtio-pci devices for the same guest should use different host
bridge resources for Xen to distinguish. The rule for the guest
device-tree generation is one PCI host bridge per backend domain.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
Changes in v3:
- Use QOM properties instead of reading from XenStore
- Remove unneeded includes
- Move pcie_* fields into "struct cfg"
Changes in v2:
- Renamed virtio_pci_host to pcie_host entries in XenStore, because
there is nothing specific to virtio-pci: any PCI device can be
emulated via this newly created bridge.
---
hw/arm/xen_arm.c| 226
hw/xen/xen-hvm-common.c | 9 +-
include/hw/xen/xen_native.h | 8 +-
3 files changed, 240 insertions(+), 3 deletions(-)
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index b9c3ae14b6..dc6d3a1d82 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -34,6 +34,7 @@
#include "hw/xen/xen-hvm-common.h"
#include "sysemu/tpm.h"
#include "hw/xen/arch_hvm.h"
+#include "hw/pci-host/gpex.h"
#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
@@ -57,6 +58,10 @@ struct XenArmState {
struct {
uint64_t tpm_base_addr;
+MemMapEntry pcie_mmio;
+MemMapEntry pcie_ecam;
+MemMapEntry pcie_mmio_high;
+int pcie_irq_base;
} cfg;
};
@@ -73,6 +78,15 @@ static MemoryRegion ram_lo, ram_hi;
#define NR_VIRTIO_MMIO_DEVICES \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
+#define XEN_ARM_PCIE_ECAM_BASE "pcie-ecam-base"
+#define XEN_ARM_PCIE_ECAM_SIZE "pcie-ecam-size"
+#define XEN_ARM_PCIE_MEM_BASE "pcie-mem-base"
+#define XEN_ARM_PCIE_MEM_SIZE "pcie-mem-size"
+#define XEN_ARM_PCIE_PREFETCH_BASE "pcie-prefetch-mem-base"
+#define XEN_ARM_PCIE_PREFETCH_SIZE "pcie-prefetch-mem-size"
+#define XEN_ARM_PCIE_IRQ_BASE "pcie-irq-base"
+
+/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI interrupts. */
static void xen_set_irq(void *opaque, int irq, int level)
{
if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
@@ -129,6 +143,89 @@ static void xen_init_ram(MachineState *machine)
}
}
+static bool xen_validate_pcie_config(XenArmState *xam)
+{
+if (xam->cfg.pcie_ecam.base == 0 &&
+xam->cfg.pcie_ecam.size == 0 &&
+xam->cfg.pcie_mmio.base == 0 &&
+xam->cfg.pcie_mmio.size == 0 &&
+xam->cfg.pcie_mmio_high.base == 0 &&
+xam->cfg.pcie_mmio_high.size == 0 &&
+xam->cfg.pcie_irq_base == 0) {
+
+/* It's okay, user just don't want PCIe brige */
+
+return false;
+}
+
+if (xam->cfg.pcie_ecam.base == 0 ||
+xam->cfg.pcie_ecam.size == 0 ||
+xam->cfg.pcie_mmio.base == 0 ||
+xam->cfg.pcie_mmio.size == 0 ||
+xam->cfg.pcie_mmio_high.base == 0 ||
+xam->cfg.pcie_mmio_high.size == 0 ||
+xam->cfg.pcie_irq_base == 0) {
+
+/* User provided some PCIe options, but not all of them */
+
+error_printf("Incomplete PCIe bridge configuration\n");
+
+exit(1);
+}
+
+return true;
+}
+
+static void xen_create_pcie(XenArmState *xam)
+{
+MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+MemoryRegion *ecam_alias, *ecam_reg;
+DeviceState *dev;
+int i;
+
+dev = qdev_new(TYPE_GPEX_HOST);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+
+/* Map ECAM space */
+ecam_alias = g_new0(MemoryRegion, 1);
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+ ecam_reg, 0, xam->cfg.pcie_ecam.size);
+memory_region_add_subregion(get_system_memory(), xam->cfg.pcie_ecam.base,
+ecam_alias);
+
+/* Map the MMIO space */
+mmio_alias = g_new0(MemoryRegion, 1);
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_init_alias(
Re: [PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support
Hi Igor,
Thank you for the review,
Igor Mammedov writes:
> On Tue, 21 Nov 2023 22:10:28 +
> Volodymyr Babchuk wrote:
>
>> From: Oleksandr Tyshchenko
>>
>> The bridge is needed for virtio-pci support, as QEMU can emulate the
>> whole bridge with any virtio-pci devices connected to it.
>>
>> This patch provides a flexible way to configure PCIe brige resources
>> with xenstore. We made this for several reasons:
>>
>> - We don't want to clash with vPCI devices, so we need information
>> from Xen toolstack on which PCI bus to use.
>> - The guest memory layout that describes these resources is not stable
>> and may vary between guests, so we cannot rely on static resources
>> to be always the same for both ends.
>> - Also the device-models which run in different domains and serve
>> virtio-pci devices for the same guest should use different host
>> bridge resources for Xen to distinguish. The rule for the guest
>> device-tree generation is one PCI host bridge per backend domain.
>>
>> Signed-off-by: Oleksandr Tyshchenko
>> Signed-off-by: Volodymyr Babchuk
>>
>> ---
>>
>> Changes from v1:
>>
>> - Renamed virtio_pci_host to pcie_host entries in XenStore, because
>> there is nothing specific to virtio-pci: any PCI device can be
>> emulated via this newly created bridge.
>> ---
>> hw/arm/xen_arm.c| 186
>> hw/xen/xen-hvm-common.c | 9 +-
>> include/hw/xen/xen_native.h | 8 +-
>> 3 files changed, 200 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
>> index b9c3ae14b6..d506d55d0f 100644
>> --- a/hw/arm/xen_arm.c
>> +++ b/hw/arm/xen_arm.c
>> @@ -22,6 +22,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>> #include "qemu/error-report.h"
>> #include "qapi/qapi-commands-migration.h"
>> #include "qapi/visitor.h"
>> @@ -34,6 +35,9 @@
>> #include "hw/xen/xen-hvm-common.h"
>> #include "sysemu/tpm.h"
>> #include "hw/xen/arch_hvm.h"
>> +#include "exec/address-spaces.h"
>> +#include "hw/pci-host/gpex.h"
>> +#include "hw/virtio/virtio-pci.h"
>>
>> #define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
>> OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
>> @@ -58,6 +62,11 @@ struct XenArmState {
>> struct {
>> uint64_t tpm_base_addr;
>> } cfg;
>> +
>> +MemMapEntry pcie_mmio;
>> +MemMapEntry pcie_ecam;
>> +MemMapEntry pcie_mmio_high;
>> +int pcie_irq;
>> };
>>
>> static MemoryRegion ram_lo, ram_hi;
>> @@ -73,6 +82,7 @@ static MemoryRegion ram_lo, ram_hi;
>> #define NR_VIRTIO_MMIO_DEVICES \
>> (GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
>>
>> +/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI
>> interrupts. */
>> static void xen_set_irq(void *opaque, int irq, int level)
>> {
>> if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
>> @@ -129,6 +139,176 @@ static void xen_init_ram(MachineState *machine)
>> }
>> }
>>
>> +static void xen_create_pcie(XenArmState *xam)
>> +{
>> +MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
>> +MemoryRegion *ecam_alias, *ecam_reg;
>> +DeviceState *dev;
>> +int i;
>> +
>> +dev = qdev_new(TYPE_GPEX_HOST);
>> +sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
>> +
>> +/* Map ECAM space */
>> +ecam_alias = g_new0(MemoryRegion, 1);
>> +ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
>> +memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
>> + ecam_reg, 0, xam->pcie_ecam.size);
>> +memory_region_add_subregion(get_system_memory(), xam->pcie_ecam.base,
>> +ecam_alias);
>> +
>> +/* Map the MMIO space */
>> +mmio_alias = g_new0(MemoryRegion, 1);
>> +mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
>> +memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
>> + mmio_reg,
>> + xam->pcie_mmio.base,
>> + xam->pcie_mmio.size);
>> +memory_region_add_subregion(get_system_memory(), xam->pcie
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David,
David Woodhouse writes:
> On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk
> wrote:
>>
>>Hi David,
>>
>>David Woodhouse writes:
>>> Which PV backends do you care about? We already have net, block and console
>>> converted.
>>
>>Well, this is all what we need, actually. Even console only will be
>>sufficient, as we are using QEMU to provide virtio-pci backends, so both
>>storage and networking should be provided by virtio. Are you proposing
>>to just drop this patch at all? I believe we can live without it, yes.
>
> Yeah, I think you can drop anything that's only needed for the legacy
> backends. I'm tempted to make a separate config option to compile those out.
Yep, we need this. Because without the patch ("xen_pvdev: Do not assume
Dom0 when creating a directory") I can't run QEMU in the driver domain:
root@spider-domd:~# qemu-system-aarch64 -machine xenpv -m 128M
xen be core: xs_mkdir device-model/0/backends/vkbd: failed
xen be core: xs_mkdir device-model/0/backends/vkbd: failed
xen be core: xs_mkdir device-model/0/backends/9pfs: failed
xen be core: xs_mkdir device-model/0/backends/9pfs: failed
So yeah, we need something like CONFIG_XEN_LEGACY_BACKENDS option if we
don't want to fix xenstore_mkdir()
--
WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David, David Woodhouse writes: > On 23 November 2023 11:54:01 GMT, Volodymyr Babchuk > wrote: >> >>Hi Paul, >> >>Paul Durrant writes: >> >>> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >>>> Hi, >>>> Volodymyr Babchuk writes: >>>> >>>>> Hi Stefano, >>>>> >>>>> Stefano Stabellini writes: >>>>> >>>>>> On Wed, 22 Nov 2023, David Woodhouse wrote: >>>>>>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>>>>>>> On Wed, 22 Nov 2023, David Woodhouse wrote: >>>>>>>>> On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>>>>>>>>> On Wed, 22 Nov 2023, Paul Durrant wrote: >>>>>>>>>>> On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>>>>>>>>>>> From: Oleksandr Tyshchenko >>>>>>>>>>>> >>>>>>>>>>>> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>>>>>>>>>>> inherit the owner of the directory. >>>>>>>>>>> >>>>>>>>>>> Ah... so that's why the previous patch is there. >>>>>>>>>>> >>>>>>>>>>> This is not the right way to fix it. The QEMU Xen support is >>>>>>>>>>> *assuming* that >>>>>>>>>>> QEMU is either running in, or emulating, dom0. In the emulation >>>>>>>>>>> case this is >>>>>>>>>>> probably fine, but the 'real Xen' case it should be using the >>>>>>>>>>> correct domid >>>>>>>>>>> for node creation. I guess this could either be supplied on the >>>>>>>>>>> command line >>>>>>>>>>> or discerned by reading the local domain 'domid' node. >>>>>>>>>> >>>>>>>>>> yes, it should be passed as command line option to QEMU >>>>>>>>> >>>>>>>>> I'm not sure I like the idea of a command line option for something >>>>>>>>> which QEMU could discover for itself. >>>>>>>> >>>>>>>> That's fine too. I meant to say "yes, as far as I know the toolstack >>>>>>>> passes the domid to QEMU as a command line option today". >>>>>>> >>>>>>> The -xen-domid argument on the QEMU command line today is the *guest* >>>>>>> domain ID, not the domain ID in which QEMU itself is running. >>>>>>> >>>>>>> Or were you thinking of something different? >>>>>> >>>>>> Ops, you are right and I understand your comment better now. The backend >>>>>> domid is not on the command line but it should be discoverable (on >>>>>> xenstore if I remember right). >>>>> >>>>> Yes, it is just "~/domid". I'll add a function that reads it. >>>> Just a quick question to QEMU folks: is it better to add a global >>>> variable where we will store own Domain ID or it will be okay to read >>>> domid from Xenstore every time we need it? >>>> If global variable variant is better, what is proffered place to >>>> define >>>> this variable? system/globals.c ? >>>> >>> >>> Actually... is it possible for QEMU just to use a relative path for >>> the backend nodes? That way it won't need to know it's own domid, will >>> it? >> >>Well, it is possible to use relative path, AFAIK, linux-based backends >>are doing exactly this. But problem is with xenstore_mkdir() function, >>which requires domain id to correctly set owner when it causes call to >>set_permissions(). >> >>As David said, architecturally it will be better to get rid of >>xenstore_mkdir() usage, because it is used by legacy backends only. But >>to do this, someone needs to convert legacy backends to use newer API. I >>have no capacity to do this right now, so I implemented a contained >>solution: >> >>static int xenstore_read_own_domid(unsigned int *domid) >> >>in xen_pvdev.c. I believe, this new function will be removed along with >>whole xen_pvdev.c when there will be no legacy backends left. > > Which PV backends do you care about? We already have net, block and console > converted. Well, this is all what we need, actually. Even console only will be sufficient, as we are using QEMU to provide virtio-pci backends, so both storage and networking should be provided by virtio. Are you proposing to just drop this patch at all? I believe we can live without it, yes. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi Paul, Paul Durrant writes: > On 23/11/2023 00:07, Volodymyr Babchuk wrote: >> Hi, >> Volodymyr Babchuk writes: >> >>> Hi Stefano, >>> >>> Stefano Stabellini writes: >>> >>>> On Wed, 22 Nov 2023, David Woodhouse wrote: >>>>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>>>>> On Wed, 22 Nov 2023, David Woodhouse wrote: >>>>>>> On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>>>>>>> On Wed, 22 Nov 2023, Paul Durrant wrote: >>>>>>>>> On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>>>>>>>>> From: Oleksandr Tyshchenko >>>>>>>>>> >>>>>>>>>> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>>>>>>>>> inherit the owner of the directory. >>>>>>>>> >>>>>>>>> Ah... so that's why the previous patch is there. >>>>>>>>> >>>>>>>>> This is not the right way to fix it. The QEMU Xen support is >>>>>>>>> *assuming* that >>>>>>>>> QEMU is either running in, or emulating, dom0. In the emulation case >>>>>>>>> this is >>>>>>>>> probably fine, but the 'real Xen' case it should be using the correct >>>>>>>>> domid >>>>>>>>> for node creation. I guess this could either be supplied on the >>>>>>>>> command line >>>>>>>>> or discerned by reading the local domain 'domid' node. >>>>>>>> >>>>>>>> yes, it should be passed as command line option to QEMU >>>>>>> >>>>>>> I'm not sure I like the idea of a command line option for something >>>>>>> which QEMU could discover for itself. >>>>>> >>>>>> That's fine too. I meant to say "yes, as far as I know the toolstack >>>>>> passes the domid to QEMU as a command line option today". >>>>> >>>>> The -xen-domid argument on the QEMU command line today is the *guest* >>>>> domain ID, not the domain ID in which QEMU itself is running. >>>>> >>>>> Or were you thinking of something different? >>>> >>>> Ops, you are right and I understand your comment better now. The backend >>>> domid is not on the command line but it should be discoverable (on >>>> xenstore if I remember right). >>> >>> Yes, it is just "~/domid". I'll add a function that reads it. >> Just a quick question to QEMU folks: is it better to add a global >> variable where we will store own Domain ID or it will be okay to read >> domid from Xenstore every time we need it? >> If global variable variant is better, what is proffered place to >> define >> this variable? system/globals.c ? >> > > Actually... is it possible for QEMU just to use a relative path for > the backend nodes? That way it won't need to know it's own domid, will > it? Well, it is possible to use relative path, AFAIK, linux-based backends are doing exactly this. But problem is with xenstore_mkdir() function, which requires domain id to correctly set owner when it causes call to set_permissions(). As David said, architecturally it will be better to get rid of xenstore_mkdir() usage, because it is used by legacy backends only. But to do this, someone needs to convert legacy backends to use newer API. I have no capacity to do this right now, so I implemented a contained solution: static int xenstore_read_own_domid(unsigned int *domid) in xen_pvdev.c. I believe, this new function will be removed along with whole xen_pvdev.c when there will be no legacy backends left. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David, David Woodhouse writes: > [[S/MIME Signed Part:Undecided]] > On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: >> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >> > >> > Hi, >> > >> > Volodymyr Babchuk writes: >> > >> > > Hi Stefano, >> > > >> > > Stefano Stabellini writes: >> > > >> > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >> > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > > > > > > > > > From: Oleksandr Tyshchenko >> > > > > > > > > > >> > > > > > > > > > Instead of forcing the owner to domid 0, use >> > > > > > > > > > XS_PRESERVE_OWNER to >> > > > > > > > > > inherit the owner of the directory. >> > > > > > > > > >> > > > > > > > > Ah... so that's why the previous patch is there. >> > > > > > > > > >> > > > > > > > > This is not the right way to fix it. The QEMU Xen support is >> > > > > > > > > *assuming* that >> > > > > > > > > QEMU is either running in, or emulating, dom0. In the >> > > > > > > > > emulation case this is >> > > > > > > > > probably fine, but the 'real Xen' case it should be using >> > > > > > > > > the correct domid >> > > > > > > > > for node creation. I guess this could either be supplied on >> > > > > > > > > the command line >> > > > > > > > > or discerned by reading the local domain 'domid' node. >> > > > > > > > >> > > > > > > > yes, it should be passed as command line option to QEMU >> > > > > > > >> > > > > > > I'm not sure I like the idea of a command line option for >> > > > > > > something >> > > > > > > which QEMU could discover for itself. >> > > > > > >> > > > > > That's fine too. I meant to say "yes, as far as I know the >> > > > > > toolstack >> > > > > > passes the domid to QEMU as a command line option today". >> > > > > >> > > > > The -xen-domid argument on the QEMU command line today is the *guest* >> > > > > domain ID, not the domain ID in which QEMU itself is running. >> > > > > >> > > > > Or were you thinking of something different? >> > > > >> > > > Ops, you are right and I understand your comment better now. The >> > > > backend >> > > > domid is not on the command line but it should be discoverable (on >> > > > xenstore if I remember right). >> > > >> > > Yes, it is just "~/domid". I'll add a function that reads it. >> > >> > Just a quick question to QEMU folks: is it better to add a global >> > variable where we will store own Domain ID or it will be okay to read >> > domid from Xenstore every time we need it? >> > >> > If global variable variant is better, what is proffered place to define >> > this variable? system/globals.c ? >> > >> >> Actually... is it possible for QEMU just to use a relative path for the >> backend nodes? That way it won't need to know it's own domid, will it? > > That covers some of the use cases, but it may also need to know its own > domid for other purposes. Including writing the *absolute* path of the > backend, to a frontend node? Is this case possible? As I understand, QEMU writes frontend nodes only when it emulates Xen, otherwise this done by Xen toolstack. And in case of Xen emulation, QEMU always assumes role of Domain-0. -- WBR, Volodymyr
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Tue, 2023-11-21 at 22:10 +, Volodymyr Babchuk wrote:
>>
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle
>> *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, t, path, );
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>
> If the existing transaction is XBT_NULL I think you want to create a
> new transaction for it, don't you?
I must say that your comment is valid even without my
changes. xenstore_mkdir() calls qemu_xen_xs_create, providing just plain
"0" (not even XBT_NULL) as a transaction, but actual xenstore interface
implementation, like xs_be_create(), issue multiple calls to lower
layer, passing "t" downwards. For example, xs_be_create() calls
xs_impl_read, xs_impl_write and xs_impl_set_perms(). If called from
xesntore_mkdir(), those three operations will be non-atomic. I don't
know if this can lead to a problem, because apparently it was so for a
long time...
--
WBR, Volodymyr
Re: [PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support
Hi Vikram, Vikram Garhwal writes: > Hi Volodymyr, > Thank you sharing this patch. I have few comments below > On Wed, Nov 22, 2023 at 02:39:46PM -0800, Stefano Stabellini wrote: >> +Vikram >> >> On Tue, 21 Nov 2023, Volodymyr Babchuk wrote: >> > From: Oleksandr Tyshchenko >> > >> > The bridge is needed for virtio-pci support, as QEMU can emulate the >> > whole bridge with any virtio-pci devices connected to it. >> > >> > This patch provides a flexible way to configure PCIe brige resources >> > with xenstore. We made this for several reasons: >> > >> > - We don't want to clash with vPCI devices, so we need information >> > from Xen toolstack on which PCI bus to use. >> > - The guest memory layout that describes these resources is not stable >> > and may vary between guests, so we cannot rely on static resources >> > to be always the same for both ends. >> > - Also the device-models which run in different domains and serve >> > virtio-pci devices for the same guest should use different host >> > bridge resources for Xen to distinguish. The rule for the guest >> > device-tree generation is one PCI host bridge per backend domain. >> > >> > Signed-off-by: Oleksandr Tyshchenko >> > Signed-off-by: Volodymyr Babchuk >> >> There is still a discussion ongoing on xen-devel if / how to register a >> PCI Root Complex or individual BDFs. In the meantime a couple of >> comments. >> >> Typically emulated devices are configured in QEMU via QEMU command line >> parameters, not xenstore. If you are running QEMU in a domU (instead of >> Dom0) you can always read config parameters from xenstore from a wrapper >> bash script (using xenstore-read) and then pass the right command line >> options to QEMU. >> >> If you need help in adding new QEMU command line options, Vikram (CCed) >> can help. >> >> > I agree with Stefano here. Setting properties would be better and easier. Okay, I'll look at this. > I have one questions here: > 1. If there are multiple QEMU backends, which means xen tools will end up > creating lot of entries in xenstore and we need to remove those xenstore > entries when backend goes away. Is this already handled in Xen tools? Well, this is not a classic PV backend, so common code in Xen Tools does not handle those entries. I am not sure if tools remove entries right now, because I am not the original author. But we definitely will remove them in the final version of patches. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi, Volodymyr Babchuk writes: > Hi Stefano, > > Stefano Stabellini writes: > >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>> > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >>> > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> > > > > > From: Oleksandr Tyshchenko >>> > > > > > >>> > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>> > > > > > inherit the owner of the directory. >>> > > > > >>> > > > > Ah... so that's why the previous patch is there. >>> > > > > >>> > > > > This is not the right way to fix it. The QEMU Xen support is >>> > > > > *assuming* that >>> > > > > QEMU is either running in, or emulating, dom0. In the emulation >>> > > > > case this is >>> > > > > probably fine, but the 'real Xen' case it should be using the >>> > > > > correct domid >>> > > > > for node creation. I guess this could either be supplied on the >>> > > > > command line >>> > > > > or discerned by reading the local domain 'domid' node. >>> > > > >>> > > > yes, it should be passed as command line option to QEMU >>> > > >>> > > I'm not sure I like the idea of a command line option for something >>> > > which QEMU could discover for itself. >>> > >>> > That's fine too. I meant to say "yes, as far as I know the toolstack >>> > passes the domid to QEMU as a command line option today". >>> >>> The -xen-domid argument on the QEMU command line today is the *guest* >>> domain ID, not the domain ID in which QEMU itself is running. >>> >>> Or were you thinking of something different? >> >> Ops, you are right and I understand your comment better now. The backend >> domid is not on the command line but it should be discoverable (on >> xenstore if I remember right). > > Yes, it is just "~/domid". I'll add a function that reads it. Just a quick question to QEMU folks: is it better to add a global variable where we will store own Domain ID or it will be okay to read domid from Xenstore every time we need it? If global variable variant is better, what is proffered place to define this variable? system/globals.c ? -- WBR, Volodymyr
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
Hi David, "Woodhouse, David" writes: > On Wed, 2023-11-22 at 17:05 +, Paul Durrant wrote: >> On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > From: David Woodhouse >> > >> > This allows a XenDevice implementation to know whether it was created >> > by QEMU, or merely discovered in XenStore after the toolstack created >> > it. This will allow us to create frontend/backend nodes only when we >> > should, rather than unconditionally attempting to overwrite them from >> > a driver domain which doesn't have privileges to do so. >> > >> > As an added benefit, it also means we no longer have to call the >> > xen_backend_set_device() function from the device models immediately >> > after calling qdev_realize_and_unref(). Even though we could make >> > the argument that it's safe to do so, and the pointer to the unreffed >> > device *will* actually still be valid, it still made my skin itch to >> > look at it. >> > >> > Signed-off-by: David Woodhouse >> > --- >> > hw/block/xen-block.c | 3 +-- >> > hw/char/xen_console.c | 2 +- >> > hw/net/xen_nic.c | 2 +- >> > hw/xen/xen-bus.c | 4 >> > include/hw/xen/xen-backend.h | 2 -- >> > include/hw/xen/xen-bus.h | 2 ++ >> > 6 files changed, 9 insertions(+), 6 deletions(-) >> > >> >> Actually, I think you should probably update >> xen_backend_try_device_destroy() in this patch too. It currently uses >> xen_backend_list_find() to check whether the specified XenDevice has an >> associated XenBackendInstance. > > I think I did that in > https://git.infradead.org/users/dwmw2/qemu.git/commitdiff/94f1b474478ce0ede > but didn't get round to sending it out again because of travel. I can just pull it from this link, if you don't mind. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi Stefano, Stefano Stabellini writes: > On Wed, 22 Nov 2023, David Woodhouse wrote: >> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >> > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > > > > > From: Oleksandr Tyshchenko >> > > > > > >> > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >> > > > > > inherit the owner of the directory. >> > > > > >> > > > > Ah... so that's why the previous patch is there. >> > > > > >> > > > > This is not the right way to fix it. The QEMU Xen support is >> > > > > *assuming* that >> > > > > QEMU is either running in, or emulating, dom0. In the emulation case >> > > > > this is >> > > > > probably fine, but the 'real Xen' case it should be using the >> > > > > correct domid >> > > > > for node creation. I guess this could either be supplied on the >> > > > > command line >> > > > > or discerned by reading the local domain 'domid' node. >> > > > >> > > > yes, it should be passed as command line option to QEMU >> > > >> > > I'm not sure I like the idea of a command line option for something >> > > which QEMU could discover for itself. >> > >> > That's fine too. I meant to say "yes, as far as I know the toolstack >> > passes the domid to QEMU as a command line option today". >> >> The -xen-domid argument on the QEMU command line today is the *guest* >> domain ID, not the domain ID in which QEMU itself is running. >> >> Or were you thinking of something different? > > Ops, you are right and I understand your comment better now. The backend > domid is not on the command line but it should be discoverable (on > xenstore if I remember right). Yes, it is just "~/domid". I'll add a function that reads it. -- WBR, Volodymyr
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Tue, 2023-11-21 at 22:10 +, Volodymyr Babchuk wrote:
>>
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle
>> *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, t, path, );
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>
> If the existing transaction is XBT_NULL I think you want to create a
> new transaction for it, don't you?
As per Stefano's and Paul's comments I'll drop this patch
completely. Thanks for review, thought.
--
WBR, Volodymyr
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
Paul Durrant writes: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> From: David Woodhouse >> This allows a XenDevice implementation to know whether it was >> created >> by QEMU, or merely discovered in XenStore after the toolstack created >> it. This will allow us to create frontend/backend nodes only when we >> should, rather than unconditionally attempting to overwrite them from >> a driver domain which doesn't have privileges to do so. >> As an added benefit, it also means we no longer have to call the >> xen_backend_set_device() function from the device models immediately >> after calling qdev_realize_and_unref(). Even though we could make >> the argument that it's safe to do so, and the pointer to the unreffed >> device *will* actually still be valid, it still made my skin itch to >> look at it. >> Signed-off-by: David Woodhouse >> --- >> hw/block/xen-block.c | 3 +-- >> hw/char/xen_console.c| 2 +- >> hw/net/xen_nic.c | 2 +- >> hw/xen/xen-bus.c | 4 >> include/hw/xen/xen-backend.h | 2 -- >> include/hw/xen/xen-bus.h | 2 ++ >> 6 files changed, 9 insertions(+), 6 deletions(-) >> > > Actually, I think you should probably update > xen_backend_try_device_destroy() in this patch too. It currently uses > xen_backend_list_find() to check whether the specified XenDevice has > an associated XenBackendInstance. Sure. Looks like it is the only user of xen_backend_list_find(), so we can get rid of it as well. I'll drop your R-b tag, because of those additional changes in the new version. -- WBR, Volodymyr
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
Hi Paul, Paul Durrant writes: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> was created by QEMU >> Xen PV devices in QEMU can be created in two ways: either by QEMU >> itself, if they were passed via command line, or by Xen toolstack. In >> the latter case, QEMU scans XenStore entries and configures devices >> accordingly. >> In the second case we don't want QEMU to write/delete front-end >> entries for two reasons: it might have no access to those entries if >> it is running in un-privileged domain and it is just incorrect to >> overwrite entries already provided by Xen toolstack, because toolstack >> manages those nodes. For example, it might read backend- or frontend- >> state to be sure that they are both disconnected and it is safe to >> destroy a domain. >> This patch checks presence of xendev->backend to check if Xen PV >> device is acting as a backend (i.e. it was configured by Xen > > Technally *all* XenDevice objects are backends. > Yes, you are right of course. I'll rephrase this paragraph in the next version. -- WBR, Volodymyr
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
Hi Philippe, Philippe Mathieu-Daudé writes: > Hi Volodymyr, > > On 21/11/23 23:10, Volodymyr Babchuk wrote: >> was created by QEMU > > Please do not split lines between subject and content. Rewrite the > full line. Preferably restrict the subject to 72 chars. I tried to come with shorter description, but failed. I'll rewrite it in the next version. -- WBR, Volodymyr
[PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support
From: Oleksandr Tyshchenko
The bridge is needed for virtio-pci support, as QEMU can emulate the
whole bridge with any virtio-pci devices connected to it.
This patch provides a flexible way to configure PCIe brige resources
with xenstore. We made this for several reasons:
- We don't want to clash with vPCI devices, so we need information
from Xen toolstack on which PCI bus to use.
- The guest memory layout that describes these resources is not stable
and may vary between guests, so we cannot rely on static resources
to be always the same for both ends.
- Also the device-models which run in different domains and serve
virtio-pci devices for the same guest should use different host
bridge resources for Xen to distinguish. The rule for the guest
device-tree generation is one PCI host bridge per backend domain.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
Changes from v1:
- Renamed virtio_pci_host to pcie_host entries in XenStore, because
there is nothing specific to virtio-pci: any PCI device can be
emulated via this newly created bridge.
---
hw/arm/xen_arm.c| 186
hw/xen/xen-hvm-common.c | 9 +-
include/hw/xen/xen_native.h | 8 +-
3 files changed, 200 insertions(+), 3 deletions(-)
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index b9c3ae14b6..d506d55d0f 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -22,6 +22,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include "qemu/error-report.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/visitor.h"
@@ -34,6 +35,9 @@
#include "hw/xen/xen-hvm-common.h"
#include "sysemu/tpm.h"
#include "hw/xen/arch_hvm.h"
+#include "exec/address-spaces.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/virtio/virtio-pci.h"
#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
@@ -58,6 +62,11 @@ struct XenArmState {
struct {
uint64_t tpm_base_addr;
} cfg;
+
+MemMapEntry pcie_mmio;
+MemMapEntry pcie_ecam;
+MemMapEntry pcie_mmio_high;
+int pcie_irq;
};
static MemoryRegion ram_lo, ram_hi;
@@ -73,6 +82,7 @@ static MemoryRegion ram_lo, ram_hi;
#define NR_VIRTIO_MMIO_DEVICES \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
+/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI interrupts. */
static void xen_set_irq(void *opaque, int irq, int level)
{
if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
@@ -129,6 +139,176 @@ static void xen_init_ram(MachineState *machine)
}
}
+static void xen_create_pcie(XenArmState *xam)
+{
+MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+MemoryRegion *ecam_alias, *ecam_reg;
+DeviceState *dev;
+int i;
+
+dev = qdev_new(TYPE_GPEX_HOST);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+
+/* Map ECAM space */
+ecam_alias = g_new0(MemoryRegion, 1);
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+ ecam_reg, 0, xam->pcie_ecam.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_ecam.base,
+ecam_alias);
+
+/* Map the MMIO space */
+mmio_alias = g_new0(MemoryRegion, 1);
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg,
+ xam->pcie_mmio.base,
+ xam->pcie_mmio.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_mmio.base,
+mmio_alias);
+
+/* Map the MMIO_HIGH space */
+mmio_alias_high = g_new0(MemoryRegion, 1);
+memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
+ mmio_reg,
+ xam->pcie_mmio_high.base,
+ xam->pcie_mmio_high.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_mmio_high.base,
+mmio_alias_high);
+
+/* Legacy PCI interrupts (#INTA - #INTD) */
+for (i = 0; i < GPEX_NUM_IRQS; i++) {
+qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
+ xam->pcie_irq + i);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
+gpex_set_irq_num(GPEX_HOST(dev), i, xam->pcie_irq + i);
+}
+
+DPRINTF("Created PCIe host bridge\n");
+}
+
+static bool xen_read_pcie_prop(XenArmState *xam, unsigned int xen_domid,
+ const char *prop_name, unsigned long *data)
+{
+char pat
[PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
From: David Woodhouse
This allows a XenDevice implementation to know whether it was created
by QEMU, or merely discovered in XenStore after the toolstack created
it. This will allow us to create frontend/backend nodes only when we
should, rather than unconditionally attempting to overwrite them from
a driver domain which doesn't have privileges to do so.
As an added benefit, it also means we no longer have to call the
xen_backend_set_device() function from the device models immediately
after calling qdev_realize_and_unref(). Even though we could make
the argument that it's safe to do so, and the pointer to the unreffed
device *will* actually still be valid, it still made my skin itch to
look at it.
Signed-off-by: David Woodhouse
---
hw/block/xen-block.c | 3 +--
hw/char/xen_console.c| 2 +-
hw/net/xen_nic.c | 2 +-
hw/xen/xen-bus.c | 4
include/hw/xen/xen-backend.h | 2 --
include/hw/xen/xen-bus.h | 2 ++
6 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 6d64ede94f..c2ac9db4a2 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -1081,13 +1081,12 @@ static void xen_block_device_create(XenBackendInstance
*backend,
blockdev->iothread = iothread;
blockdev->drive = drive;
+xendev->backend = backend;
if (!qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
error_prepend(errp, "realization of device %s failed: ", type);
goto fail;
}
-
-xen_backend_set_device(backend, xendev);
return;
fail:
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 5cbee2f184..bef8a3a621 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -600,8 +600,8 @@ static void xen_console_device_create(XenBackendInstance
*backend,
goto fail;
}
+xendev->backend = backend;
if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
goto done;
}
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index af4ba3f1e6..afa10c96e8 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -627,8 +627,8 @@ static void xen_net_device_create(XenBackendInstance
*backend,
net->dev = number;
memcpy(>conf.macaddr, , sizeof(mac));
+xendev->backend = backend;
if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
return;
}
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 4973e7d9c9..dd0171ab98 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1079,6 +1079,10 @@ static void xen_device_realize(DeviceState *dev, Error
**errp)
}
}
+if (xendev->backend) {
+xen_backend_set_device(xendev->backend, xendev);
+}
+
xendev->exit.notify = xen_device_exit;
qemu_add_exit_notifier(>exit);
return;
diff --git a/include/hw/xen/xen-backend.h b/include/hw/xen/xen-backend.h
index 0f01631ae7..ea080ba7c9 100644
--- a/include/hw/xen/xen-backend.h
+++ b/include/hw/xen/xen-backend.h
@@ -10,8 +10,6 @@
#include "hw/xen/xen-bus.h"
-typedef struct XenBackendInstance XenBackendInstance;
-
typedef void (*XenBackendDeviceCreate)(XenBackendInstance *backend,
QDict *opts, Error **errp);
typedef void (*XenBackendDeviceDestroy)(XenBackendInstance *backend,
diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h
index 334ddd1ff6..7647c4c38e 100644
--- a/include/hw/xen/xen-bus.h
+++ b/include/hw/xen/xen-bus.h
@@ -14,9 +14,11 @@
#include "qom/object.h"
typedef struct XenEventChannel XenEventChannel;
+typedef struct XenBackendInstance XenBackendInstance;
struct XenDevice {
DeviceState qdev;
+XenBackendInstance *backend;
domid_t frontend_id;
char *name;
struct qemu_xs_handle *xsh;
--
2.42.0
[PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
From: Oleksandr Tyshchenko
Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
inherit the owner of the directory.
Note that for other than Dom0 domain (non toolstack domain) the
"driver_domain" property should be set in domain config file for the
toolstack to create required directories in advance.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/xen/xen_pvdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..42bdd4f6c8 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -60,7 +60,8 @@ void xen_config_cleanup(void)
int xenstore_mkdir(char *path, int p)
{
-if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
+if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
+xen_domid, p, path)) {
xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
return -1;
}
--
2.42.0
[PATCH v2 5/6] xen_arm: Set mc->max_cpus to GUEST_MAX_VCPUS in xen_arm_init()
From: Oleksandr Tyshchenko The number of vCPUs used for the IOREQ configuration (machine->smp.cpus) should really match the system value as for each vCPU we setup a dedicated evtchn for the communication with Xen at the runtime. This is needed for the IOREQ to be properly configured and work if the involved domain has more than one vCPU assigned. Set the number of current supported guest vCPUs here (128) which is defined in public header arch-arm.h. And the toolstack should then pass max_vcpus using "-smp" arg. Signed-off-by: Oleksandr Tyshchenko Signed-off-by: Volodymyr Babchuk --- hw/arm/xen_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c index a5631529d0..b9c3ae14b6 100644 --- a/hw/arm/xen_arm.c +++ b/hw/arm/xen_arm.c @@ -231,7 +231,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); mc->desc = "Xen Para-virtualized PC"; mc->init = xen_arm_init; -mc->max_cpus = 1; +mc->max_cpus = GUEST_MAX_VCPUS; mc->default_machine_opts = "accel=xen"; /* Set explicitly here to make sure that real ram_size is passed */ mc->default_ram_size = 0; -- 2.42.0
[PATCH v2 0/6] xen-arm: add support for virtio-pci
Hello, This patch series adds the basic support for virtio-pci for xen-arm guests. The main changes are in "xen_arm: Add virtual PCIe host bridge support", while most of other patches are required to make QEMU work as device model in a non-privileged domains like driver domain. Changes v1->v2: - Removed "xen-bus: Set offline if backend's state is XenbusStateClosed" - Included David's patch as it is the pre-req for "xen: backends: touch some XenStore nodes only if device..." --- David Woodhouse (1): hw/xen: Set XenBackendInstance in the XenDevice before realizing it Oleksandr Tyshchenko (3): xen_pvdev: Do not assume Dom0 when creating a directory xen_arm: Set mc->max_cpus to GUEST_MAX_VCPUS in xen_arm_init() xen_arm: Add virtual PCIe host bridge support Volodymyr Babchuk (2): xen: backends: touch some XenStore nodes only if device... xen: xenstore: add possibility to preserve owner hw/arm/xen_arm.c | 188 ++- hw/block/xen-block.c | 19 ++-- hw/char/xen_console.c| 4 +- hw/i386/kvm/xen_xenstore.c | 18 +++ hw/net/xen_nic.c | 20 ++-- hw/xen/xen-bus.c | 18 ++- hw/xen/xen-hvm-common.c | 9 +- hw/xen/xen-operations.c | 12 ++ hw/xen/xen_pvdev.c | 3 +- include/hw/xen/xen-backend.h | 2 - include/hw/xen/xen-bus.h | 2 + include/hw/xen/xen_backend_ops.h | 7 ++ include/hw/xen/xen_native.h | 8 +- 13 files changed, 278 insertions(+), 32 deletions(-) -- 2.42.0
[PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
Add option to preserve owner when creating an entry in Xen Store. This
may be needed in cases when Qemu is working as device model in a
domain that is Domain-0, e.g. in driver domain.
"owner" parameter for qemu_xen_xs_create() function can have special
value XS_PRESERVE_OWNER, which will make specific implementation to
get original owner of an entry and pass it back to
set_permissions() call.
Please note, that XenStore inherits permissions, so even if entry is
newly created by, it already has the owner set to match owner of entry
at previous level.
Signed-off-by: Volodymyr Babchuk
--
In v2:
- Pass transaction to xs_get_permissions() in libxenstore_create()
- Added comment before XS_PRESERVE_OWNER defintion
- Extended the commit message
---
hw/i386/kvm/xen_xenstore.c | 18 ++
hw/xen/xen-operations.c | 12
include/hw/xen/xen_backend_ops.h | 7 +++
3 files changed, 37 insertions(+)
diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
index 6e651960b3..d0fd5d4681 100644
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -1595,6 +1595,24 @@ static bool xs_be_create(struct qemu_xs_handle *h,
xs_transaction_t t,
return false;
}
+if (owner == XS_PRESERVE_OWNER) {
+GList *prev_perms;
+char letter;
+
+err = xs_impl_get_perms(h->impl, 0, t, path, _perms);
+if (err) {
+errno = err;
+return false;
+}
+
+if (sscanf(prev_perms->data, "%c%u", , ) != 2) {
+errno = EFAULT;
+g_list_free_full(prev_perms, g_free);
+return false;
+}
+g_list_free_full(prev_perms, g_free);
+}
+
perms_list = g_list_append(perms_list,
xs_perm_as_string(XS_PERM_NONE, owner));
perms_list = g_list_append(perms_list,
diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c
index e00983ec44..ae8265635f 100644
--- a/hw/xen/xen-operations.c
+++ b/hw/xen/xen-operations.c
@@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h,
xs_transaction_t t,
return false;
}
+if (owner == XS_PRESERVE_OWNER) {
+struct xs_permissions *tmp;
+unsigned int num;
+
+tmp = xs_get_permissions(h->xsh, t, path, );
+if (tmp == NULL) {
+return false;
+}
+perms_list[0].id = tmp[0].id;
+free(tmp);
+}
+
return xs_set_permissions(h->xsh, t, path, perms_list,
ARRAY_SIZE(perms_list));
}
diff --git a/include/hw/xen/xen_backend_ops.h b/include/hw/xen/xen_backend_ops.h
index 90cca85f52..79021538a3 100644
--- a/include/hw/xen/xen_backend_ops.h
+++ b/include/hw/xen/xen_backend_ops.h
@@ -266,6 +266,13 @@ typedef uint32_t xs_transaction_t;
#define XS_PERM_READ 0x01
#define XS_PERM_WRITE 0x02
+/*
+ * This is QEMU-specific special value used only by QEMU wrappers
+ * around XenStore. It can be passed to qemu_xen_xs_create() to
+ * inherit owner value from higher-level XS entry.
+ */
+#define XS_PRESERVE_OWNER0xFFFE
+
struct xenstore_backend_ops {
struct qemu_xs_handle *(*open)(void);
void (*close)(struct qemu_xs_handle *h);
--
2.42.0
[PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
was created by QEMU
Xen PV devices in QEMU can be created in two ways: either by QEMU
itself, if they were passed via command line, or by Xen toolstack. In
the latter case, QEMU scans XenStore entries and configures devices
accordingly.
In the second case we don't want QEMU to write/delete front-end
entries for two reasons: it might have no access to those entries if
it is running in un-privileged domain and it is just incorrect to
overwrite entries already provided by Xen toolstack, because toolstack
manages those nodes. For example, it might read backend- or frontend-
state to be sure that they are both disconnected and it is safe to
destroy a domain.
This patch checks presence of xendev->backend to check if Xen PV
device is acting as a backend (i.e. it was configured by Xen
toolstack) to decide if it should touch frontend entries in XenStore.
Also, when we need to remove XenStore entries during device teardown
only if they weren't created by Xen toolstack. If they were created by
toolstack, then it is toolstack's job to do proper clean-up.
Suggested-by: Paul Durrant
Suggested-by: David Woodhouse
Co-Authored-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/block/xen-block.c | 16 +---
hw/char/xen_console.c | 2 +-
hw/net/xen_nic.c | 18 ++
hw/xen/xen-bus.c | 14 +-
4 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index c2ac9db4a2..dac519a6d3 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -390,13 +390,15 @@ static void xen_block_realize(XenDevice *xendev, Error
**errp)
xen_device_backend_printf(xendev, "info", "%u", blockdev->info);
-xen_device_frontend_printf(xendev, "virtual-device", "%lu",
- vdev->number);
-xen_device_frontend_printf(xendev, "device-type", "%s",
- blockdev->device_type);
-
-xen_device_backend_printf(xendev, "sector-size", "%u",
- conf->logical_block_size);
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "virtual-device", "%lu",
+ vdev->number);
+xen_device_frontend_printf(xendev, "device-type", "%s",
+ blockdev->device_type);
+
+xen_device_backend_printf(xendev, "sector-size", "%u",
+ conf->logical_block_size);
+}
xen_block_set_size(blockdev);
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index bef8a3a621..b52abf 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -450,7 +450,7 @@ static void xen_console_realize(XenDevice *xendev, Error
**errp)
trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
-if (CHARDEV_IS_PTY(cs)) {
+if (CHARDEV_IS_PTY(cs) && !xendev->backend) {
/* Strip the leading 'pty:' */
xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
}
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index afa10c96e8..27442bef38 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -315,14 +315,16 @@ static void xen_netdev_realize(XenDevice *xendev, Error
**errp)
qemu_macaddr_default_if_unset(>conf.macaddr);
-xen_device_frontend_printf(xendev, "mac", "%02x:%02x:%02x:%02x:%02x:%02x",
- netdev->conf.macaddr.a[0],
- netdev->conf.macaddr.a[1],
- netdev->conf.macaddr.a[2],
- netdev->conf.macaddr.a[3],
- netdev->conf.macaddr.a[4],
- netdev->conf.macaddr.a[5]);
-
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "mac",
+ "%02x:%02x:%02x:%02x:%02x:%02x",
+ netdev->conf.macaddr.a[0],
+ netdev->conf.macaddr.a[1],
+ netdev->conf.macaddr.a[2],
+ netdev->conf.macaddr.a[3],
+ netdev->conf.macaddr.a[4],
+ netdev->conf.macaddr.a[5]);
+}
netdev->nic = qemu_new_nic(_xen_info, >conf,
object_get_typename(OBJECT(xendev)),
DEVICE(xendev)->id, netdev);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index dd0171ab98..d0f17aeb27 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -599,8 +599,10 @@ static void xen_device_backend_destroy(XenDevice *xendev)
g_assert
Re: [PATCH v1 4/7] xen_pvdev: Do not assume Dom0 when creating a directrory
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Fri, 2023-11-10 at 20:42 +, Volodymyr Babchuk wrote:
>> From: Oleksandr Tyshchenko
>>
>> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to save
>> the previous owner of the directory.
>>
>
> You're missing the words "... if it already exists" from that sentence.
>
> If the directory *didn't* already exist, it gets created with dom0 as
> the owner still, right? Assuming XenStore allows QEMU to do that.
If it didn't already exist, it is created and it inherits access rights
from the parent node.
> Strictly, the node gets created (if permitted) and *then*
> xs_set_permissions() attempts to set dom0 as the owner (if permitted).
Yes. I'll rephrase this as "Instead of forcing the owner to domid 0, use
XS_PRESERVE_OWNER to save the inherited owner of the directory."
will it be okay?
>
>> Note that for other than Dom0 domain (non toolstack domain) the
>> "driver_domain" property should be set in domain config file for the
>> toolstack to create required directories in advance.
>>
>> Signed-off-by: Oleksandr Tyshchenko
>> Signed-off-by: Volodymyr Babchuk
>> ---
>> hw/xen/xen_pvdev.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
>> index c5ad71e8dc..42bdd4f6c8 100644
>> --- a/hw/xen/xen_pvdev.c
>> +++ b/hw/xen/xen_pvdev.c
>> @@ -60,7 +60,8 @@ void xen_config_cleanup(void)
>>
>> int xenstore_mkdir(char *path, int p)
>> {
>> - if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
>> + if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
>> + xen_domid, p, path)) {
>> xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
>> return -1;
>> }
>
> Why bother with xenstore_mkdir()? AFAICT it's *only* used from the
> legacy XenLegacyDevice stuff, and can't we just finish the job of
> moving from that to the XenDevice model? I've done console and net
> recently; want to keep going?
Well, I am not so familiar with QEMU to accomplish this in a short
time. If you really need help, I can take alook at 9p driver, it looks
simplest of them all...
>
> And even then... the xenstore_mkdir() function is called twice from
> xen_config_dev_dirs() in hw/xen/xen_devconfig.c to create the frontend
> and backend directories — which is what the rest of your patch series
> is trying to eliminate because a driver domain doesn't have permissions
> to do that anyway.
>
> It's also called from xen_be_register() in hw/xen/xen_devconfig.c to
> create device-model/${GUEST_DOMID}/backends/${DEVICE_TYPE} (using a
> relative path, so in the driver domain's XenStore). That one presumably
> *won't* exist already, and so XS_PRESERVE_OWNER won't even have any
> effect?
As I said, it will inherit driver's domain access rights. So yeah,
Oleksandr's patch covers this case, mostly.
I agree, it would be better to drop xenstore_mkdir() at all, but this is
tangent to my task of adding virtio-pci support for ARM guests. Those
Oleksandr's patches for drive domain, that you are seeing, come to life
only because our system happens to use a separate driver domain.
--
WBR, Volodymyr
Re: [PATCH v1 1/7] xen-block: Do not write frontend nodes
Hi David, David Woodhouse writes: > On 11 November 2023 16:51:22 GMT-05:00, Andrew Cooper > wrote: >>On 11/11/2023 8:18 pm, David Woodhouse wrote: >>> On 11 November 2023 08:43:40 GMT-05:00, Andrew Cooper >>> wrote: Furthermore, the control domain doesn't always have the domid of 0. If qemu wants/needs to make changes like this, the control domain has to arrange for qemu's domain to have appropriate permissions on the nodes. >>> Right. And that's simple enough: if you are running QEMU in a >>> domain which doesn't have permission to create the backend >>> directory and/or the frontend nodes, don't ask it to *create* >>> devices. In that case it is only able to connect as the backend for >>> devices which were created *for* it by the toolstack. >>> >>> The criterion used in this patch series should be "did QEMU create this >>> device, or discover it". >>> >> >>Yeah, that sounds like the right approach. > > I think we want to kill the xen_backend_set_device() function and > instead set the backend as a property of the XenDevice *before* > realizing it. Not sure that I got this. Right now device is property of XenBackendInstance. Are you proposing to make this other way around? Right now I am looking for a place where to store the information of XenDevice creator. My plan was to add "found_in_xenbus" property to XenDevice and set it in xen_backend_device_create. -- WBR, Volodymyr
Re: [PATCH v1 3/7] xen: xenstore: add possibility to preserve owner
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Sat, 2023-11-11 at 11:01 +, David Woodhouse wrote:
>>
>> > --- a/hw/xen/xen-operations.c
>> > +++ b/hw/xen/xen-operations.c
>> > @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle
>> > *h, xs_transaction_t t,
>> > return false;
>> > }
>> >
>> > + if (owner == XS_PRESERVE_OWNER) {
>> > + struct xs_permissions *tmp;
>> > + unsigned int num;
>> > +
>> > + tmp = xs_get_permissions(h->xsh, 0, path, );
>> > + if (tmp == NULL) {
>> > + return false;
>> > + }
>> > + perms_list[0].id = tmp[0].id;
>> > + free(tmp);
>> > + }
>> > +
>>
>> Don't see what saves you from someone else changing it at this point on
>> true Xen though. Which is why I'd prefer XenStore to do it natively.
>
> I suppose maybe you could do it in a transaction *if* the transaction_t
> you're passed in isn't already XBT_NULL?
Yes, I need to pass "t" to xs_get_permissions(), of course.
> One might argue that the mkdir+set_perms in libxenstore_create() ought
> to have been within the same transaction *anyway*?
Yes, all operations should be performed inside one transaction.
--
WBR, Volodymyr
Re: [PATCH v1 3/7] xen: xenstore: add possibility to preserve owner
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Fri, 2023-11-10 at 20:42 +, Volodymyr Babchuk wrote:
>> Add option to preserve owner when creating an entry in Xen Store. This
>> may be needed in cases when Qemu is working as device model in a
>> domain that is Domain-0, e.g. in driver domain.
>>
>> "owner" parameter for qemu_xen_xs_create() function can have special
>> value XS_PRESERVE_OWNER, which will make specific implementation to
>> get original owner of an entry and pass it back to
>> set_permissions() call.
>>
>> Signed-off-by: Volodymyr Babchuk
>
> I like this, although I'd like it more if XenStore itself offered this
> facility rather than making QEMU do it.
XenStore itself ensures that access rights are inherited. The problem
is with qemu_xen_xs_create() function that does two things at a time:
creates a new entry and then assigns permissions, overwriting any
permissions that existed before.
> Can we make it abundantly clear
> that XS_PRESERVE_OWNER is a QEMU internal thing?
It is defined in xen_backend_ops.h which is internal QEMU interface for
XenStore. Do you have any suggestions how to make it even clearer?
>> hw/i386/kvm/xen_xenstore.c | 18 ++
>> hw/xen/xen-operations.c | 12
>> include/hw/xen/xen_backend_ops.h | 2 ++
>> 3 files changed, 32 insertions(+)
>>
>> diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
>> index 660d0b72f9..7b894a9884 100644
>> --- a/hw/i386/kvm/xen_xenstore.c
>> +++ b/hw/i386/kvm/xen_xenstore.c
>> @@ -1572,6 +1572,24 @@ static bool xs_be_create(struct qemu_xs_handle *h,
>> xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + GList *perms;
>> + char letter;
>> +
>> + err = xs_impl_get_perms(h->impl, 0, t, path, );
>> + if (err) {
>> + errno = err;
>> + return false;
>> + }
>
> I guess we get away without a race here because it's all internal and
> we're holding the QEMU iothread mutex? Perhaps assert that?
>
I am not quite familiar with QEMU internals, but why we do we need
assert here? xe_be_create() calls xs_impl* function before and after
this part. Is this piece of code special in some way?
>> + if (sscanf(perms->data, "%c%u", , ) != 2) {
>
> I'd be slightly happier if you used parse_perm() from xenstore_impl.c,
> but it's static so I suppose that's fair enough.
>
Yes, I wanted to use that function, but it is internal for
xenstore_impl.c
I can rename it to xs_impl_parse_perm() and make it public, if you
believe that this is a better approach.
>> + errno = EFAULT;
>> + g_list_free_full(perms, g_free);
>> + return false;
>> + }
>> + g_list_free_full(perms, g_free);
>> + }
>> +
>> perms_list = g_list_append(perms_list,
>> xs_perm_as_string(XS_PERM_NONE, owner));
>> perms_list = g_list_append(perms_list,
>> diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c
>> index e00983ec44..1df59b3c08 100644
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle
>> *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, 0, path, );
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>
> Don't see what saves you from someone else changing it at this point on
> true Xen though. Which is why I'd prefer XenStore to do it natively.
>
Oh, I missed the transaction parameter. My bad. Will fix in the next
version.
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>> diff --git a/include/hw/xen/xen_backend_ops.h
>> b/include/hw/xen/xen_backend_ops.h
>> index 90cca85f52..273e414559 100644
>> --- a/include/hw/xen/xen_backend_ops.h
>> +++ b/include/hw/xen/xen_backend_ops.h
>> @@ -266,6 +266,8 @@ typedef uint32_t xs_transaction_t;
>> #define XS_PERM_READ 0x01
>> #define XS_PERM_WRITE 0x02
>>
>> +#define XS_PRESERVE_OWNER 0xFFFE
>> +
>> struct xenstore_backend_ops {
>> struct qemu_xs_handle *(*open)(void);
>> void (*close)(struct qemu_xs_handle *h);
>
> [[End of S/MIME Signed Part]]
--
WBR, Volodymyr
Re: [PATCH v1 7/7] xen_arm: Add basic virtio-pci support
Hi David, David Woodhouse writes: > [[S/MIME Signed Part:Undecided]] > On Fri, 2023-11-10 at 20:42 +, Volodymyr Babchuk wrote: >> From: Oleksandr Tyshchenko >> >> This patch adds basic virtio-pci support for xen_arm machine. > > Why only xen_arm? Couldn't this be a fairly generic device which can be > instantiated on x86 too, both for real and emulated Xen guests? And > riscv/ppc too? This is the architecture-specific code. Actually, from QEMU point of view, this code just adds a virtual PCI bridge. For example, on x86 this is done in a some other way, so they already have virtio-pci working. -- WBR, Volodymyr
[PATCH v1 0/7] xen-arm: add support for virtio-pci
Hello, This patch series adds the basic support for virtio-pci for xen-arm guests. The main changes are in "xen_arm: Add basic virtio-pci support", while another 5 patches are adding groundwork. First 4 patches are required to run QEMU device model in a driver domain, not only in Dom0. Oleksandr Tyshchenko (6): xen-block: Do not write frontend nodes xen-bus: Do not destroy frontend/backend directories xen_pvdev: Do not assume Dom0 when creating a directrory xen-bus: Set offline if backend's state is XenbusStateClosed xen_arm: Set mc->max_cpus to GUEST_MAX_VCPUS in xen_arm_init() xen_arm: Add basic virtio-pci support Volodymyr Babchuk (1): xen: xenstore: add possibility to preserve owner hw/arm/xen_arm.c | 188 ++- hw/block/xen-block.c | 11 +- hw/i386/kvm/xen_xenstore.c | 18 +++ hw/xen/xen-bus.c | 16 ++- hw/xen/xen-hvm-common.c | 9 +- hw/xen/xen-operations.c | 12 ++ hw/xen/xen_pvdev.c | 3 +- include/hw/xen/xen_backend_ops.h | 2 + include/hw/xen/xen_native.h | 8 +- 9 files changed, 253 insertions(+), 14 deletions(-) -- 2.42.0
[PATCH v1 1/7] xen-block: Do not write frontend nodes
From: Oleksandr Tyshchenko
The PV backend running in other than Dom0 domain (non toolstack domain)
is not allowed to write frontend nodes. The more, the backend does not
need to do that at all, this is purely toolstack/xl devd business.
I do not know for what reason the backend does that here, this is not really
needed, probably it is just a leftover and all xen_device_frontend_printf()
instances should go away completely.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/block/xen-block.c | 11 +++
hw/xen/xen-bus.c | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index a07cd7eb5d..dc4d477c22 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -221,6 +221,7 @@ static void xen_block_realize(XenDevice *xendev, Error
**errp)
XenBlockVdev *vdev = >props.vdev;
BlockConf *conf = >props.conf;
BlockBackend *blk = conf->blk;
+XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) {
error_setg(errp, "vdev property not set");
@@ -280,10 +281,12 @@ static void xen_block_realize(XenDevice *xendev, Error
**errp)
xen_device_backend_printf(xendev, "info", "%u", blockdev->info);
-xen_device_frontend_printf(xendev, "virtual-device", "%lu",
- vdev->number);
-xen_device_frontend_printf(xendev, "device-type", "%s",
- blockdev->device_type);
+if (xenbus->backend_id == 0) {
+xen_device_frontend_printf(xendev, "virtual-device", "%lu",
+ vdev->number);
+xen_device_frontend_printf(xendev, "device-type", "%s",
+ blockdev->device_type);
+}
xen_device_backend_printf(xendev, "sector-size", "%u",
conf->logical_block_size);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index ece8ec40cd..06d5192aca 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1048,7 +1048,7 @@ static void xen_device_realize(DeviceState *dev, Error
**errp)
xen_device_backend_set_online(xendev, true);
xen_device_backend_set_state(xendev, XenbusStateInitWait);
-if (!xen_device_frontend_exists(xendev)) {
+if (!xen_device_frontend_exists(xendev) && xenbus->backend_id == 0) {
xen_device_frontend_printf(xendev, "backend", "%s",
xendev->backend_path);
xen_device_frontend_printf(xendev, "backend-id", "%u",
--
2.42.0
[PATCH v1 2/7] xen-bus: Do not destroy frontend/backend directories
From: Oleksandr Tyshchenko The PV backend running in other than Dom0 domain (non toolstack domain) is not allowed to destroy frontend/backend directories. The more, it does not need to do that at all, this is purely toolstack/xl devd business. I do not know for what reason the backend does that here, this is not really needed, probably it is just a leftover and all xs_node_destroy() instances should go away completely. Signed-off-by: Oleksandr Tyshchenko Signed-off-by: Volodymyr Babchuk --- hw/xen/xen-bus.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c index 06d5192aca..75474d4b43 100644 --- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -598,8 +598,9 @@ static void xen_device_backend_destroy(XenDevice *xendev) g_assert(xenbus->xsh); -xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->backend_path, -_err); +if (xenbus->backend_id == 0) +xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->backend_path, +_err); g_free(xendev->backend_path); xendev->backend_path = NULL; @@ -754,8 +755,9 @@ static void xen_device_frontend_destroy(XenDevice *xendev) g_assert(xenbus->xsh); -xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->frontend_path, -_err); +if (xenbus->backend_id == 0) +xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->frontend_path, +_err); g_free(xendev->frontend_path); xendev->frontend_path = NULL; -- 2.42.0
[PATCH v1 3/7] xen: xenstore: add possibility to preserve owner
Add option to preserve owner when creating an entry in Xen Store. This
may be needed in cases when Qemu is working as device model in a
domain that is Domain-0, e.g. in driver domain.
"owner" parameter for qemu_xen_xs_create() function can have special
value XS_PRESERVE_OWNER, which will make specific implementation to
get original owner of an entry and pass it back to
set_permissions() call.
Signed-off-by: Volodymyr Babchuk
---
hw/i386/kvm/xen_xenstore.c | 18 ++
hw/xen/xen-operations.c | 12
include/hw/xen/xen_backend_ops.h | 2 ++
3 files changed, 32 insertions(+)
diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
index 660d0b72f9..7b894a9884 100644
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -1572,6 +1572,24 @@ static bool xs_be_create(struct qemu_xs_handle *h,
xs_transaction_t t,
return false;
}
+if (owner == XS_PRESERVE_OWNER) {
+GList *perms;
+char letter;
+
+err = xs_impl_get_perms(h->impl, 0, t, path, );
+if (err) {
+errno = err;
+return false;
+}
+
+if (sscanf(perms->data, "%c%u", , ) != 2) {
+errno = EFAULT;
+g_list_free_full(perms, g_free);
+return false;
+}
+g_list_free_full(perms, g_free);
+}
+
perms_list = g_list_append(perms_list,
xs_perm_as_string(XS_PERM_NONE, owner));
perms_list = g_list_append(perms_list,
diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c
index e00983ec44..1df59b3c08 100644
--- a/hw/xen/xen-operations.c
+++ b/hw/xen/xen-operations.c
@@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h,
xs_transaction_t t,
return false;
}
+if (owner == XS_PRESERVE_OWNER) {
+struct xs_permissions *tmp;
+unsigned int num;
+
+tmp = xs_get_permissions(h->xsh, 0, path, );
+if (tmp == NULL) {
+return false;
+}
+perms_list[0].id = tmp[0].id;
+free(tmp);
+}
+
return xs_set_permissions(h->xsh, t, path, perms_list,
ARRAY_SIZE(perms_list));
}
diff --git a/include/hw/xen/xen_backend_ops.h b/include/hw/xen/xen_backend_ops.h
index 90cca85f52..273e414559 100644
--- a/include/hw/xen/xen_backend_ops.h
+++ b/include/hw/xen/xen_backend_ops.h
@@ -266,6 +266,8 @@ typedef uint32_t xs_transaction_t;
#define XS_PERM_READ 0x01
#define XS_PERM_WRITE 0x02
+#define XS_PRESERVE_OWNER0xFFFE
+
struct xenstore_backend_ops {
struct qemu_xs_handle *(*open)(void);
void (*close)(struct qemu_xs_handle *h);
--
2.42.0
[PATCH v1 4/7] xen_pvdev: Do not assume Dom0 when creating a directrory
From: Oleksandr Tyshchenko
Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to save
the previous owner of the directory.
Note that for other than Dom0 domain (non toolstack domain) the
"driver_domain" property should be set in domain config file for the
toolstack to create required directories in advance.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/xen/xen_pvdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..42bdd4f6c8 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -60,7 +60,8 @@ void xen_config_cleanup(void)
int xenstore_mkdir(char *path, int p)
{
-if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
+if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
+xen_domid, p, path)) {
xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
return -1;
}
--
2.42.0
[PATCH v1 6/7] xen_arm: Set mc->max_cpus to GUEST_MAX_VCPUS in xen_arm_init()
From: Oleksandr Tyshchenko The number of vCPUs used for the IOREQ configuration (machine->smp.cpus) should really match the system value as for each vCPU we setup a dedicated evtchn for the communication with Xen at the runtime. This is needed for the IOREQ to be properly configured and work if the involved domain has more than one vCPU assigned. Set the number of current supported guest vCPUs here (128) which is defined in public header arch-arm.h. And the toolstack should then pass max_vcpus using "-smp" arg. Signed-off-by: Oleksandr Tyshchenko Signed-off-by: Volodymyr Babchuk --- hw/arm/xen_arm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c index f83b983ec5..a2b41dc2de 100644 --- a/hw/arm/xen_arm.c +++ b/hw/arm/xen_arm.c @@ -229,7 +229,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); mc->desc = "Xen Para-virtualized PC"; mc->init = xen_arm_init; -mc->max_cpus = 1; +mc->max_cpus = GUEST_MAX_VCPUS; mc->default_machine_opts = "accel=xen"; /* Set explicitly here to make sure that real ram_size is passed */ mc->default_ram_size = 0; -- 2.42.0
[PATCH v1 5/7] xen-bus: Set offline if backend's state is XenbusStateClosed
From: Oleksandr Tyshchenko
Both state (XenbusStateClosed) and online (0) are expected by
toolstack/xl devd to completely destroy the device. But "offline"
is never being set by the backend resulting in timeout during
domain destruction, garbage in Xestore and still running Qemu
instance.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/xen/xen-bus.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 75474d4b43..6e7ec3af64 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -519,6 +519,10 @@ static void xen_device_backend_changed(void *opaque, const
char *path)
xen_device_backend_set_state(xendev, XenbusStateClosed);
}
+if (xen_device_backend_get_state(xendev) == XenbusStateClosed) {
+xen_device_backend_set_online(xendev, false);
+}
+
/*
* If a backend is still 'online' then we should leave it alone but,
* if a backend is not 'online', then the device is a candidate
--
2.42.0
[PATCH v1 7/7] xen_arm: Add basic virtio-pci support
From: Oleksandr Tyshchenko
This patch adds basic virtio-pci support for xen_arm machine.
It provides a flexible way to configure virtio-pci resources with
xenstore. We made this for several reasons:
- We don't want to clash with vPCI devices, so we need information
from Xen toolstack on which PCI bus to use.
- The guest memory layout that describes these resources is not stable
and may vary between guests, so we cannot rely on static resources
to be always the same for both ends.
- Also the device-models which run in different domains and serve
virtio-pci devices for the same guest should use different host
bridge resources for Xen to distinguish. The rule for the guest
device-tree generation is one PCI host bridge per backend domain.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/arm/xen_arm.c| 186
hw/xen/xen-hvm-common.c | 9 +-
include/hw/xen/xen_native.h | 8 +-
3 files changed, 200 insertions(+), 3 deletions(-)
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index a2b41dc2de..4a0a6726a8 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -22,6 +22,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include "qemu/error-report.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/visitor.h"
@@ -34,6 +35,9 @@
#include "hw/xen/xen-hvm-common.h"
#include "sysemu/tpm.h"
#include "hw/xen/arch_hvm.h"
+#include "exec/address-spaces.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/virtio/virtio-pci.h"
#define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
@@ -58,6 +62,11 @@ struct XenArmState {
struct {
uint64_t tpm_base_addr;
} cfg;
+
+MemMapEntry pcie_mmio;
+MemMapEntry pcie_ecam;
+MemMapEntry pcie_mmio_high;
+int pcie_irq;
};
static MemoryRegion ram_lo, ram_hi;
@@ -73,6 +82,7 @@ static MemoryRegion ram_lo, ram_hi;
#define NR_VIRTIO_MMIO_DEVICES \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
+/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI interrupts. */
static void xen_set_irq(void *opaque, int irq, int level)
{
xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level);
@@ -127,6 +137,176 @@ static void xen_init_ram(MachineState *machine)
}
}
+static void xen_create_pcie(XenArmState *xam)
+{
+MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+MemoryRegion *ecam_alias, *ecam_reg;
+DeviceState *dev;
+int i;
+
+dev = qdev_new(TYPE_GPEX_HOST);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+
+/* Map ECAM space */
+ecam_alias = g_new0(MemoryRegion, 1);
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+ ecam_reg, 0, xam->pcie_ecam.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_ecam.base,
+ecam_alias);
+
+/* Map the MMIO space */
+mmio_alias = g_new0(MemoryRegion, 1);
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg,
+ xam->pcie_mmio.base,
+ xam->pcie_mmio.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_mmio.base,
+mmio_alias);
+
+/* Map the MMIO_HIGH space */
+mmio_alias_high = g_new0(MemoryRegion, 1);
+memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
+ mmio_reg,
+ xam->pcie_mmio_high.base,
+ xam->pcie_mmio_high.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_mmio_high.base,
+mmio_alias_high);
+
+/* Legacy PCI interrupts (#INTA - #INTD) */
+for (i = 0; i < GPEX_NUM_IRQS; i++) {
+qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
+ xam->pcie_irq + i);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
+gpex_set_irq_num(GPEX_HOST(dev), i, xam->pcie_irq + i);
+}
+
+DPRINTF("Created PCIe host bridge\n");
+}
+
+static bool xen_read_pcie_prop(XenArmState *xam, unsigned int xen_domid,
+ const char *prop_name, unsigned long *data)
+{
+char path[128], *value = NULL;
+unsigned int len;
+bool ret = true;
+
+snprintf(path, sizeof(path), "device-model/%d/virtio_pci_host/%s",
+ xen_domid, prop_name);
+value = xs_read(xam->state->xenstore, XBT_NULL, path, );
+
+if (qem
