- Stratos-dev - op-lists.linaro.org

[PATCH V4] libxl: arm: Add grant_usage parameter for virtio devices

by Viresh Kumar

Currently, the grant mapping related device tree properties are added if the backend domain is not Dom0. While Dom0 is privileged and can do foreign mapping for the entire guest memory, it is still desired for Dom0 to access guest's memory via grant mappings and hence map only what is required. This commit adds the "grant_usage" parameter for virtio devices, which provides better control over the functionality. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> Reviewed-by: Anthony PERARD <anthony.perard(a)citrix.com> --- V3.1->V4: - Added Reviewed-by tags. - Other patches from the series are already applied. - No code changes. V3->V3.1: - Print "0" or "1" in xenstore instead of "True" or "False" for grant_usage. V2->V3: - Reuse `libxl_defbool` instead of defining a new type, it can take values 0 and 1. - Improved commit logs and comments. V1->V2: - Instead of just 0 or 1, the argument can take multiple values now and control the functionality in a better way. - Update .gen.go files as well. - Don't add nodes under frontend path. docs/man/xl.cfg.5.pod.in | 8 ++++++++ tools/golang/xenlight/helpers.gen.go | 6 ++++++ tools/golang/xenlight/types.gen.go | 1 + tools/libs/light/libxl_arm.c | 22 +++++++++++++--------- tools/libs/light/libxl_types.idl | 1 + tools/libs/light/libxl_virtio.c | 23 +++++++++++++++++++++-- tools/xl/xl_parse.c | 2 ++ 7 files changed, 52 insertions(+), 11 deletions(-) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 3979be2a590a..b6fc20ef01c8 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -1619,6 +1619,14 @@ hexadecimal format, without the "0x" prefix and all in lower case, like Specifies the transport mechanism for the Virtio device, only "mmio" is supported for now. +=item B<grant_usage=BOOLEAN> + +If this option is B<true>, the Xen grants are always enabled. +If this option is B<false>, the Xen grants are always disabled. + +If this option is missing, then the default grant setting will be used, +i.e. enable grants if backend-domid != 0. + =back =item B<tee="STRING"> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go index cd1a16e32eac..3ecffdf0ef3a 100644 --- a/tools/golang/xenlight/helpers.gen.go +++ b/tools/golang/xenlight/helpers.gen.go @@ -1794,6 +1794,9 @@ func (x *DeviceVirtio) fromC(xc *C.libxl_device_virtio) error { x.BackendDomname = C.GoString(xc.backend_domname) x.Type = C.GoString(xc._type) x.Transport = VirtioTransport(xc.transport) +if err := x.GrantUsage.fromC(&xc.grant_usage);err != nil { +return fmt.Errorf("converting field GrantUsage: %v", err) +} x.Devid = Devid(xc.devid) x.Irq = uint32(xc.irq) x.Base = uint64(xc.base) @@ -1811,6 +1814,9 @@ xc.backend_domname = C.CString(x.BackendDomname)} if x.Type != "" { xc._type = C.CString(x.Type)} xc.transport = C.libxl_virtio_transport(x.Transport) +if err := x.GrantUsage.toC(&xc.grant_usage); err != nil { +return fmt.Errorf("converting field GrantUsage: %v", err) +} xc.devid = C.libxl_devid(x.Devid) xc.irq = C.uint32_t(x.Irq) xc.base = C.uint64_t(x.Base) diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go index b131a7eedc9d..4f40aac28c2a 100644 --- a/tools/golang/xenlight/types.gen.go +++ b/tools/golang/xenlight/types.gen.go @@ -706,6 +706,7 @@ BackendDomid Domid BackendDomname string Type string Transport VirtioTransport +GrantUsage Defbool Devid Devid Irq uint32 Base uint64 diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 35f76dfc21e4..f98e1affa294 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -930,7 +930,8 @@ static int make_xen_iommu_node(libxl__gc *gc, void *fdt) /* The caller is responsible to complete / close the fdt node */ static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base, - uint32_t irq, uint32_t backend_domid) + uint32_t irq, uint32_t backend_domid, + bool grant_usage) { int res; gic_interrupt intr; @@ -953,7 +954,7 @@ static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base, res = fdt_property(fdt, "dma-coherent", NULL, 0); if (res) return res; - if (backend_domid != LIBXL_TOOLSTACK_DOMID) { + if (grant_usage) { uint32_t iommus_prop[2]; iommus_prop[0] = cpu_to_fdt32(GUEST_PHANDLE_IOMMU); @@ -967,11 +968,12 @@ static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base, } static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, uint64_t base, - uint32_t irq, uint32_t backend_domid) + uint32_t irq, uint32_t backend_domid, + bool grant_usage) { int res; - res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid); + res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid, grant_usage); if (res) return res; return fdt_end_node(fdt); @@ -1027,11 +1029,11 @@ static int make_virtio_mmio_node_gpio(libxl__gc *gc, void *fdt) static int make_virtio_mmio_node_device(libxl__gc *gc, void *fdt, uint64_t base, uint32_t irq, const char *type, - uint32_t backend_domid) + uint32_t backend_domid, bool grant_usage) { int res; - res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid); + res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid, grant_usage); if (res) return res; /* Add device specific nodes */ @@ -1371,7 +1373,8 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, iommu_needed = true; FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq, - disk->backend_domid) ); + disk->backend_domid, + disk->backend_domid != LIBXL_TOOLSTACK_DOMID) ); } } @@ -1381,12 +1384,13 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, if (virtio->transport != LIBXL_VIRTIO_TRANSPORT_MMIO) continue; - if (virtio->backend_domid != LIBXL_TOOLSTACK_DOMID) + if (libxl_defbool_val(virtio->grant_usage)) iommu_needed = true; FDT( make_virtio_mmio_node_device(gc, fdt, virtio->base, virtio->irq, virtio->type, - virtio->backend_domid) ); + virtio->backend_domid, + libxl_defbool_val(virtio->grant_usage)) ); } /* diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl index 9e48bb772646..700b44a164f4 100644 --- a/tools/libs/light/libxl_types.idl +++ b/tools/libs/light/libxl_types.idl @@ -762,6 +762,7 @@ libxl_device_virtio = Struct("device_virtio", [ ("backend_domname", string), ("type", string), ("transport", libxl_virtio_transport), + ("grant_usage", libxl_defbool), ("devid", libxl_devid), # Note that virtio-mmio parameters (irq and base) are for internal # use by libxl and can't be modified. diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index f8a78e22d156..e5e321adc5c4 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -23,8 +23,16 @@ static int libxl__device_virtio_setdefault(libxl__gc *gc, uint32_t domid, libxl_device_virtio *virtio, bool hotplug) { - return libxl__resolve_domid(gc, virtio->backend_domname, - &virtio->backend_domid); + int rc; + + rc = libxl__resolve_domid(gc, virtio->backend_domname, + &virtio->backend_domid); + if (rc < 0) return rc; + + libxl_defbool_setdefault(&virtio->grant_usage, + virtio->backend_domid != LIBXL_TOOLSTACK_DOMID); + + return 0; } static int libxl__device_from_virtio(libxl__gc *gc, uint32_t domid, @@ -53,6 +61,8 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid, flexarray_append_pair(back, "base", GCSPRINTF("%#"PRIx64, virtio->base)); flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type)); flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport)); + flexarray_append_pair(back, "grant_usage", + libxl_defbool_val(virtio->grant_usage) ? "1" : "0"); return 0; } @@ -104,6 +114,15 @@ static int libxl__virtio_from_xenstore(libxl__gc *gc, const char *libxl_path, } } + tmp = NULL; + rc = libxl__xs_read_checked(gc, XBT_NULL, + GCSPRINTF("%s/grant_usage", be_path), &tmp); + if (rc) goto out; + + if (tmp) { + libxl_defbool_set(&virtio->grant_usage, strtoul(tmp, NULL, 0)); + } + tmp = NULL; rc = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/type", be_path), &tmp); diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index f036e56fc239..dfd432ca02e2 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1215,6 +1215,8 @@ static int parse_virtio_config(libxl_device_virtio *virtio, char *token) } else if (MATCH_OPTION("transport", token, oparg)) { rc = libxl_virtio_transport_from_string(oparg, &virtio->transport); if (rc) return rc; + } else if (MATCH_OPTION("grant_usage", token, oparg)) { + libxl_defbool_set(&virtio->grant_usage, strtoul(oparg, NULL, 0)); } else { fprintf(stderr, "Unknown string \"%s\" in virtio spec\n", token); return -1; -- 2.31.1.272.g89b43f80a514

9 months, 3 weeks

2
2
0 0

[PATCH] xen: privcmd: Add support for irqfd

by Viresh Kumar

Xen provides support for injecting interrupts to the guests via the HYPERVISOR_dm_op() hypercall. The same is used by the Virtio based device backend implementations, in an inefficient manner currently. Generally, the Virtio backends are implemented to work with the Eventfd based mechanism. In order to make such backends work with Xen, another software layer needs to poll the Eventfds and raise an interrupt to the guest using the Xen based mechanism. This results in an extra context switch. This is not a new problem in Linux though. It is present with other hypervisors like KVM, etc. as well. The generic solution implemented in the kernel for them is to provide an IOCTL call to pass the interrupt details and eventfd, which lets the kernel take care of polling the eventfd and raising of the interrupt, instead of handling this in user space (which involves an extra context switch). This patch adds support to inject a specific interrupt to guest using the eventfd mechanism, by preventing the extra context switch. Inspired by existing implementations for KVM, etc.. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- drivers/xen/privcmd.c | 285 ++++++++++++++++++++++++++++++++++++- include/uapi/xen/privcmd.h | 14 ++ 2 files changed, 297 insertions(+), 2 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index e2f580e30a86..e8096b09c113 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -9,11 +9,16 @@ #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt +#include <linux/eventfd.h> +#include <linux/file.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mutex.h> +#include <linux/poll.h> #include <linux/sched.h> #include <linux/slab.h> #include <linux/string.h> +#include <linux/workqueue.h> #include <linux/errno.h> #include <linux/mm.h> #include <linux/mman.h> @@ -833,6 +838,266 @@ static long privcmd_ioctl_mmap_resource(struct file *file, return rc; } +/* Irqfd support */ +static struct workqueue_struct *irqfd_cleanup_wq; +static DEFINE_MUTEX(irqfds_lock); +static LIST_HEAD(irqfds_list); + +struct privcmd_kernel_irqfd { + domid_t dom; + u8 level; + u32 irq; + struct eventfd_ctx *eventfd; + struct work_struct shutdown; + wait_queue_entry_t wait; + struct list_head list; + poll_table pt; +}; + +/* From xen/include/public/hvm/dm_op.h */ +#define XEN_DMOP_set_irq_level 19 + +struct xen_dm_op_set_irq_level { + u32 irq; + /* IN - Level: 0 -> deasserted, 1 -> asserted */ + u8 level; + u8 pad[3]; +}; + +struct xen_dm_op { + u32 op; + u32 pad; + union { + /* + * There are more structures here, we won't be using them, so + * can skip adding them here. + */ + struct xen_dm_op_set_irq_level set_irq_level; + } u; +}; + +static void irqfd_deactivate(struct privcmd_kernel_irqfd *kirqfd) +{ + lockdep_assert_held(&irqfds_lock); + + list_del_init(&kirqfd->list); + queue_work(irqfd_cleanup_wq, &kirqfd->shutdown); +} + +static void irqfd_shutdown(struct work_struct *work) +{ + struct privcmd_kernel_irqfd *kirqfd = + container_of(work, struct privcmd_kernel_irqfd, shutdown); + u64 cnt; + + eventfd_ctx_remove_wait_queue(kirqfd->eventfd, &kirqfd->wait, &cnt); + eventfd_ctx_put(kirqfd->eventfd); + kfree(kirqfd); +} + +static void irqfd_inject(struct privcmd_kernel_irqfd *kirqfd) +{ + struct xen_dm_op dm_op = { + .op = XEN_DMOP_set_irq_level, + .u.set_irq_level.irq = kirqfd->irq, + .u.set_irq_level.level = kirqfd->level, + }; + struct xen_dm_op_buf xbufs = { + .size = sizeof(dm_op), + }; + u64 cnt; + + eventfd_ctx_do_read(kirqfd->eventfd, &cnt); + set_xen_guest_handle(xbufs.h, &dm_op); + + xen_preemptible_hcall_begin(); + HYPERVISOR_dm_op(kirqfd->dom, 1, &xbufs); + xen_preemptible_hcall_end(); +} + +static int +irqfd_wakeup(wait_queue_entry_t *wait, unsigned mode, int sync, void *key) +{ + struct privcmd_kernel_irqfd *kirqfd = + container_of(wait, struct privcmd_kernel_irqfd, wait); + __poll_t flags = key_to_poll(key); + + if (flags & EPOLLIN) + irqfd_inject(kirqfd); + + if (flags & EPOLLHUP) { + mutex_lock(&irqfds_lock); + irqfd_deactivate(kirqfd); + mutex_unlock(&irqfds_lock); + } + + return 0; +} + +static void +irqfd_poll_func(struct file *file, wait_queue_head_t *wqh, poll_table *pt) +{ + struct privcmd_kernel_irqfd *kirqfd = + container_of(pt, struct privcmd_kernel_irqfd, pt); + + add_wait_queue_priority(wqh, &kirqfd->wait); +} + +static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) +{ + struct privcmd_kernel_irqfd *kirqfd, *tmp; + struct eventfd_ctx *eventfd; + __poll_t events; + struct fd f; + int ret; + + kirqfd = kzalloc(sizeof(*kirqfd), GFP_KERNEL); + if (!kirqfd) + return -ENOMEM; + + kirqfd->irq = irqfd->irq; + kirqfd->dom = irqfd->dom; + kirqfd->level = irqfd->level; + INIT_LIST_HEAD(&kirqfd->list); + INIT_WORK(&kirqfd->shutdown, irqfd_shutdown); + + f = fdget(irqfd->fd); + if (!f.file) { + ret = -EBADF; + goto error_kfree; + } + + eventfd = eventfd_ctx_fileget(f.file); + if (IS_ERR(eventfd)) { + ret = PTR_ERR(eventfd); + goto error_fd_put; + } + + kirqfd->eventfd = eventfd; + + /* + * Install our own custom wake-up handling so we are notified via a + * callback whenever someone signals the underlying eventfd. + */ + init_waitqueue_func_entry(&kirqfd->wait, irqfd_wakeup); + init_poll_funcptr(&kirqfd->pt, irqfd_poll_func); + + mutex_lock(&irqfds_lock); + + list_for_each_entry(tmp, &irqfds_list, list) { + if (kirqfd->eventfd == tmp->eventfd) { + ret = -EBUSY; + mutex_unlock(&irqfds_lock); + goto error_eventfd; + } + } + + list_add_tail(&kirqfd->list, &irqfds_list); + mutex_unlock(&irqfds_lock); + + /* + * Check if there was an event already pending on the eventfd before we + * registered, and trigger it as if we didn't miss it. + */ + events = vfs_poll(f.file, &kirqfd->pt); + if (events & EPOLLIN) + irqfd_inject(kirqfd); + + /* + * Do not drop the file until the kirqfd is fully initialized, otherwise + * we might race against the EPOLLHUP. + */ + fdput(f); + return 0; + +error_eventfd: + eventfd_ctx_put(eventfd); + +error_fd_put: + fdput(f); + +error_kfree: + kfree(kirqfd); + return ret; +} + +static int privcmd_irqfd_deassign(struct privcmd_irqfd *irqfd) +{ + struct privcmd_kernel_irqfd *kirqfd, *tmp; + struct eventfd_ctx *eventfd; + + eventfd = eventfd_ctx_fdget(irqfd->fd); + if (IS_ERR(eventfd)) + return PTR_ERR(eventfd); + + mutex_lock(&irqfds_lock); + + list_for_each_entry_safe(kirqfd, tmp, &irqfds_list, list) { + if (kirqfd->eventfd == eventfd) { + irqfd_deactivate(kirqfd); + break; + } + } + + mutex_unlock(&irqfds_lock); + + eventfd_ctx_put(eventfd); + + /* + * Block until we know all outstanding shutdown jobs have completed so + * that we guarantee there will not be any more interrupts once this + * deassign function returns. + */ + flush_workqueue(irqfd_cleanup_wq); + + return 0; +} + +static long privcmd_ioctl_irqfd(struct file *file, void __user *udata) +{ + struct privcmd_data *data = file->private_data; + struct privcmd_irqfd irqfd; + + if (copy_from_user(&irqfd, udata, sizeof(irqfd))) + return -EFAULT; + + /* No other flags should be set */ + if (irqfd.flags & ~PRIVCMD_IRQFD_FLAG_DEASSIGN) + return -EINVAL; + + /* If restriction is in place, check the domid matches */ + if (data->domid != DOMID_INVALID && data->domid != irqfd.dom) + return -EPERM; + + if (irqfd.flags & PRIVCMD_IRQFD_FLAG_DEASSIGN) + return privcmd_irqfd_deassign(&irqfd); + + return privcmd_irqfd_assign(&irqfd); +} + +static int privcmd_irqfd_init(void) +{ + irqfd_cleanup_wq = alloc_workqueue("privcmd-irqfd-cleanup", 0, 0); + if (!irqfd_cleanup_wq) + return -ENOMEM; + + return 0; +} + +static void privcmd_irqfd_exit(void) +{ + struct privcmd_kernel_irqfd *kirqfd, *tmp; + + mutex_lock(&irqfds_lock); + + list_for_each_entry_safe(kirqfd, tmp, &irqfds_list, list) + irqfd_deactivate(kirqfd); + + mutex_unlock(&irqfds_lock); + + destroy_workqueue(irqfd_cleanup_wq); +} + static long privcmd_ioctl(struct file *file, unsigned int cmd, unsigned long data) { @@ -868,6 +1133,10 @@ static long privcmd_ioctl(struct file *file, ret = privcmd_ioctl_mmap_resource(file, udata); break; + case IOCTL_PRIVCMD_IRQFD: + ret = privcmd_ioctl_irqfd(file, udata); + break; + default: break; } @@ -992,15 +1261,27 @@ static int __init privcmd_init(void) err = misc_register(&xen_privcmdbuf_dev); if (err != 0) { pr_err("Could not register Xen hypercall-buf device\n"); - misc_deregister(&privcmd_dev); - return err; + goto err_privcmdbuf; + } + + err = privcmd_irqfd_init(); + if (err != 0) { + pr_err("irqfd init failed\n"); + goto err_irqfd; } return 0; + +err_irqfd: + misc_deregister(&xen_privcmdbuf_dev); +err_privcmdbuf: + misc_deregister(&privcmd_dev); + return err; } static void __exit privcmd_exit(void) { + privcmd_irqfd_exit(); misc_deregister(&privcmd_dev); misc_deregister(&xen_privcmdbuf_dev); } diff --git a/include/uapi/xen/privcmd.h b/include/uapi/xen/privcmd.h index d2029556083e..47334bb91a09 100644 --- a/include/uapi/xen/privcmd.h +++ b/include/uapi/xen/privcmd.h @@ -98,6 +98,18 @@ struct privcmd_mmap_resource { __u64 addr; }; +/* For privcmd_irqfd::flags */ +#define PRIVCMD_IRQFD_FLAG_DEASSIGN (1 << 0) + +struct privcmd_irqfd { + __u32 fd; + __u32 flags; + __u32 irq; + domid_t dom; + __u8 level; + __u8 pad; +}; + /* * @cmd: IOCTL_PRIVCMD_HYPERCALL * @arg: &privcmd_hypercall_t @@ -125,5 +137,7 @@ struct privcmd_mmap_resource { _IOC(_IOC_NONE, 'P', 6, sizeof(domid_t)) #define IOCTL_PRIVCMD_MMAP_RESOURCE \ _IOC(_IOC_NONE, 'P', 7, sizeof(struct privcmd_mmap_resource)) +#define IOCTL_PRIVCMD_IRQFD \ + _IOC(_IOC_NONE, 'P', 8, sizeof(struct privcmd_irqfd)) #endif /* __LINUX_PUBLIC_PRIVCMD_H__ */ -- 2.31.1.272.g89b43f80a514

10 months

1
0
0 0

[PATCH V3 0/3] libxl: Make grants configurable for virtio devices

by Viresh Kumar

Hi, This patchset intends to make grant mapping usage configurable for virtio devices. Currently they are forced enabled for backends running on non-Dom0 domains. This patchset adds a new `grant_usage` parameter for the virtio devices, which can be used to enable or disable grant mappings irrespective of the backend domain, while still preserving the default behavior in absence of a parameter. V2->V3: - Patch 2/3 is new and fixes ordering issues with default values. - Reuse `libxl_defbool` instead of defining a new type, it can take values 0 and 1. - Improved commit logs and comments. V1->V2: - Instead of just 0 or 1, the argument can take multiple values now and control the functionality in a better way. - Update .gen.go files as well. - Don't add nodes under frontend path. Viresh Kumar (3): libxl: virtio: Remove unused frontend nodes libxl: Call libxl__virtio_devtype.set_default() early enough libxl: arm: Add grant_usage parameter for virtio devices docs/man/xl.cfg.5.pod.in | 8 +++++++ tools/golang/xenlight/helpers.gen.go | 6 +++++ tools/golang/xenlight/types.gen.go | 1 + tools/libs/light/libxl_arm.c | 22 +++++++++++-------- tools/libs/light/libxl_create.c | 11 +++++++++- tools/libs/light/libxl_types.idl | 1 + tools/libs/light/libxl_virtio.c | 33 ++++++++++++++++++++++------ tools/xl/xl_parse.c | 2 ++ 8 files changed, 67 insertions(+), 17 deletions(-) -- 2.31.1.272.g89b43f80a514

10 months, 1 week

3
12
0 0

Re: [PATCH V3 3/3] libxl: arm: Add grant_usage parameter for virtio devices

by Viresh Kumar

On 02-06-23, 08:25, Erik Schilling wrote: > > diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go > > index 0a203d22321f..bf846dca8ec0 100644 > > --- a/tools/golang/xenlight/helpers.gen.go > > +++ b/tools/golang/xenlight/helpers.gen.go > > @@ -1792,6 +1792,9 @@ func (x *DeviceVirtio) fromC(xc *C.libxl_device_virtio) error { > > x.BackendDomname = C.GoString(xc.backend_domname) > > x.Type = C.GoString(xc._type) > > x.Transport = VirtioTransport(xc.transport) > > +if err := x.GrantUsage.fromC(&xc.grant_usage);err != nil { > > NITPICK: space after ; seems missing. This is an auto-generated file, perhaps the script has a bug :) -- viresh

11 months, 1 week

1
0
0 0

[PATCH V2 1/2] libxl: virtio: Remove unused frontend nodes

by Viresh Kumar

The libxl_virtio file works for only PV devices and the nodes under the frontend path are not used by anyone. Remove them. While at it, also add a comment to the file describing what devices this file is used for. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- V2: New patch. tools/libs/light/libxl_virtio.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index faada49e184e..eadcb7124c3f 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -10,6 +10,9 @@ * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Lesser General Public License for more details. + * + * This is used for PV (paravirtualized) devices only and the frontend isn't + * aware of the xenstore. */ #include "libxl_internal.h" @@ -49,11 +52,6 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid, flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type)); flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport)); - flexarray_append_pair(front, "irq", GCSPRINTF("%u", virtio->irq)); - flexarray_append_pair(front, "base", GCSPRINTF("%#"PRIx64, virtio->base)); - flexarray_append_pair(front, "type", GCSPRINTF("%s", virtio->type)); - flexarray_append_pair(front, "transport", GCSPRINTF("%s", transport)); - return 0; } -- 2.31.1.272.g89b43f80a514

11 months, 3 weeks

2
5
0 0

[PATCH] libxl: arm: Allow grant mappings for backends running on Dom0

by Viresh Kumar

Currently, we add grant mapping related device tree properties if the backend domain is not Dom0. While Dom0 is privileged and can do foreign mapping for the entire guest memory, it is still okay for Dom0 to access guest's memory via grant mappings and hence map only what is required. This commit adds another parameter for virtio devices, with which they can do forced grant mappings irrespective of the backend domain id. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- docs/man/xl.cfg.5.pod.in | 4 ++++ tools/libs/light/libxl_arm.c | 21 ++++++++++++--------- tools/libs/light/libxl_types.idl | 1 + tools/libs/light/libxl_virtio.c | 11 +++++++++++ tools/xl/xl_parse.c | 2 ++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 10f37990be57..4879f136aab8 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -1616,6 +1616,10 @@ properties in the Device Tree, the type field must be set to "virtio,device". Specifies the transport mechanism for the Virtio device, only "mmio" is supported for now. +=item B<forced_grant=BOOLEAN> + +Allows Xen Grant memory mapping to be done from Dom0. + =back =item B<tee="STRING"> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 97c80d7ed0fa..ec2f1844e9b3 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -922,7 +922,8 @@ static int make_xen_iommu_node(libxl__gc *gc, void *fdt) /* The caller is responsible to complete / close the fdt node */ static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base, - uint32_t irq, uint32_t backend_domid) + uint32_t irq, uint32_t backend_domid, + bool forced_grant) { int res; gic_interrupt intr; @@ -945,7 +946,7 @@ static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base, res = fdt_property(fdt, "dma-coherent", NULL, 0); if (res) return res; - if (backend_domid != LIBXL_TOOLSTACK_DOMID) { + if (forced_grant || backend_domid != LIBXL_TOOLSTACK_DOMID) { uint32_t iommus_prop[2]; iommus_prop[0] = cpu_to_fdt32(GUEST_PHANDLE_IOMMU); @@ -959,11 +960,12 @@ static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base, } static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, uint64_t base, - uint32_t irq, uint32_t backend_domid) + uint32_t irq, uint32_t backend_domid, + bool forced_grant) { int res; - res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid); + res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid, forced_grant); if (res) return res; return fdt_end_node(fdt); @@ -1019,11 +1021,11 @@ static int make_virtio_mmio_node_gpio(libxl__gc *gc, void *fdt) static int make_virtio_mmio_node_device(libxl__gc *gc, void *fdt, uint64_t base, uint32_t irq, const char *type, - uint32_t backend_domid) + uint32_t backend_domid, bool forced_grant) { int res; - res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid); + res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid, forced_grant); if (res) return res; /* Add device specific nodes */ @@ -1363,7 +1365,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, iommu_needed = true; FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq, - disk->backend_domid) ); + disk->backend_domid, false) ); } } @@ -1373,12 +1375,13 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, if (virtio->transport != LIBXL_VIRTIO_TRANSPORT_MMIO) continue; - if (virtio->backend_domid != LIBXL_TOOLSTACK_DOMID) + if (virtio->forced_grant || virtio->backend_domid != LIBXL_TOOLSTACK_DOMID) iommu_needed = true; FDT( make_virtio_mmio_node_device(gc, fdt, virtio->base, virtio->irq, virtio->type, - virtio->backend_domid) ); + virtio->backend_domid, + virtio->forced_grant) ); } /* diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl index c10292e0d7e3..cfbcd717dc7f 100644 --- a/tools/libs/light/libxl_types.idl +++ b/tools/libs/light/libxl_types.idl @@ -740,6 +740,7 @@ libxl_device_virtio = Struct("device_virtio", [ ("backend_domname", string), ("type", string), ("transport", libxl_virtio_transport), + ("forced_grant", bool), ("devid", libxl_devid), # Note that virtio-mmio parameters (irq and base) are for internal # use by libxl and can't be modified. diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index faada49e184e..e1f15344ef97 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -48,11 +48,13 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid, flexarray_append_pair(back, "base", GCSPRINTF("%#"PRIx64, virtio->base)); flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type)); flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport)); + flexarray_append_pair(back, "forced_grant", GCSPRINTF("%u", virtio->forced_grant)); flexarray_append_pair(front, "irq", GCSPRINTF("%u", virtio->irq)); flexarray_append_pair(front, "base", GCSPRINTF("%#"PRIx64, virtio->base)); flexarray_append_pair(front, "type", GCSPRINTF("%s", virtio->type)); flexarray_append_pair(front, "transport", GCSPRINTF("%s", transport)); + flexarray_append_pair(front, "forced_grant", GCSPRINTF("%u", virtio->forced_grant)); return 0; } @@ -104,6 +106,15 @@ static int libxl__virtio_from_xenstore(libxl__gc *gc, const char *libxl_path, } } + tmp = NULL; + rc = libxl__xs_read_checked(gc, XBT_NULL, + GCSPRINTF("%s/forced_grant", be_path), &tmp); + if (rc) goto out; + + if (tmp) { + virtio->forced_grant = strtoul(tmp, NULL, 0); + } + tmp = NULL; rc = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/type", be_path), &tmp); diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 1f6f47daf4e1..3e34da099785 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1215,6 +1215,8 @@ static int parse_virtio_config(libxl_device_virtio *virtio, char *token) } else if (MATCH_OPTION("transport", token, oparg)) { rc = libxl_virtio_transport_from_string(oparg, &virtio->transport); if (rc) return rc; + } else if (MATCH_OPTION("forced_grant", token, oparg)) { + virtio->forced_grant = strtoul(oparg, NULL, 0); } else { fprintf(stderr, "Unknown string \"%s\" in virtio spec\n", token); return -1; -- 2.31.1.272.g89b43f80a514

12 months

3
9
0 0

[PATCH V3 1/2] docs: Allow generic virtio device types to contain device-id

by Viresh Kumar

For generic virtio devices, where we don't need to add compatible or other special DT properties, the type field is set to "virtio,device". But this misses the case where the user sets the type with a valid virtio device id as well, like "virtio,device1a" for file system device. The complete list of virtio device ids is mentioned here: https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x… Update documentation to support that as well. Fixes: dd54ea500be8 ("docs: add documentation for generic virtio devices") Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko(a)epam.com> --- V2->V3: - Updated commit log and clarified / fixed doc. - Tag from Oleksandr. docs/man/xl.cfg.5.pod.in | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 10f37990be57..24ac92718288 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -1608,8 +1608,11 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio…> -For generic virtio devices, where we don't need to set special or compatible -properties in the Device Tree, the type field must be set to "virtio,device". +For other generic virtio devices, where we don't need to set special or +compatible properties in the Device Tree, the type field must be set to +"virtio,device" or "virtio,device<N>", where "N" is the virtio device id in +hexadecimal format, without the "0x" prefix and all in lower case, like +"virtio,device1a" for the file system device. =item B<transport=STRING> -- 2.31.1.272.g89b43f80a514

1 year

4
6
0 0

[PATCH V2] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

by Viresh Kumar

Since the driver doesn't support interrupts, we must return early when index is set to VIRTIO_CONFIG_IRQ_IDX. Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX") Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- V2: Simplify and fix comments. hw/virtio/vhost-user-i2c.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c index 60eaf0d95be0..4eef3f063376 100644 --- a/hw/virtio/vhost-user-i2c.c +++ b/hw/virtio/vhost-user-i2c.c @@ -128,6 +128,14 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) { VHostUserI2C *i2c = VHOST_USER_I2C(vdev); + /* + * We don't support interrupts, return early if index is set to + * VIRTIO_CONFIG_IRQ_IDX. + */ + if (idx == VIRTIO_CONFIG_IRQ_IDX) { + return; + } + vhost_virtqueue_mask(&i2c->vhost_dev, vdev, idx, mask); } @@ -135,6 +143,14 @@ static bool vu_i2c_guest_notifier_pending(VirtIODevice *vdev, int idx) { VHostUserI2C *i2c = VHOST_USER_I2C(vdev); + /* + * We don't support interrupts, return early if index is set to + * VIRTIO_CONFIG_IRQ_IDX. + */ + if (idx == VIRTIO_CONFIG_IRQ_IDX) { + return false; + } + return vhost_virtqueue_pending(&i2c->vhost_dev, idx); } -- 2.31.1.272.g89b43f80a514

1 year

1
0
0 0

Re: [PATCH] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

by Alex Bennée

Philippe Mathieu-Daudé <philmd(a)linaro.org> writes: > On 17/4/23 08:02, Viresh Kumar wrote: >> Since the driver doesn't support interrupts, we must return early when >> index is set to VIRTIO_CONFIG_IRQ_IDX. >> Fixes: 544f0278afca ("virtio: introduce macro >> VIRTIO_CONFIG_IRQ_IDX") >> Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> >> --- >> hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c >> index 60eaf0d95be0..45100a24953c 100644 >> --- a/hw/virtio/vhost-user-i2c.c >> +++ b/hw/virtio/vhost-user-i2c.c >> @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) >> { >> VHostUserI2C *i2c = VHOST_USER_I2C(vdev); >> + /* >> + * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1 >> + * as the Marco of configure interrupt's IDX, If this driver does not > > Copy/paste of pre-existing comment, still I wonder who is "the Marco > of configure" :P > >> + * support, the function will return >> + */ First patch of my last VirtIO series fixes these all up. -- Alex Bennée Virtualisation Tech Lead @ Linaro

1 year

1
0
0 0

Re: [PATCH] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

by Viresh Kumar

On 17-04-23, 09:36, Philippe Mathieu-Daudé wrote: > On 17/4/23 08:02, Viresh Kumar wrote: > > Since the driver doesn't support interrupts, we must return early when > > index is set to VIRTIO_CONFIG_IRQ_IDX. > > > > Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX") > > Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> > > --- > > hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c > > index 60eaf0d95be0..45100a24953c 100644 > > --- a/hw/virtio/vhost-user-i2c.c > > +++ b/hw/virtio/vhost-user-i2c.c > > @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) > > { > > VHostUserI2C *i2c = VHOST_USER_I2C(vdev); > > + /* > > + * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1 > > + * as the Marco of configure interrupt's IDX, If this driver does not > > Copy/paste of pre-existing comment, still I wonder who is "the Marco of > configure" :P > > > + * support, the function will return > > + */ Yeah the comment could be improved, I didn't touch it as it was written this way for many drivers :) Maybe a simple comment like is all we need: /* * We don't support interrupts, return early if index is set to * VIRTIO_CONFIG_IRQ_IDX. */ -- viresh

1 year

1
0
0 0

2024

2023

2022

2021

2020

Stratos-dev