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(-)
Only the VirtIO backend will watch xenstore to find out when a new instance needs to be created for a guest, and read the parameters from there. VirtIO frontend are only virtio, so they will not do anything with the xenstore nodes. They can be removed.
While at it, also add a comment to the libxl_virtio.c file.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- tools/libs/light/libxl_virtio.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index faada49e184e..f8a78e22d156 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -1,4 +1,9 @@ /* + * Setup VirtIO backend. This is intended to interact with a VirtIO + * backend that is watching xenstore, and create new VirtIO devices + * with the parameter found in xenstore (VirtIO frontend don't + * interact with xenstore.) + * * Copyright (C) 2022 Linaro Ltd. * * This program is free software; you can redistribute it and/or modify @@ -49,11 +54,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; }
On Fri, Jun 02, 2023 at 11:19:07AM +0530, Viresh Kumar wrote:
Only the VirtIO backend will watch xenstore to find out when a new instance needs to be created for a guest, and read the parameters from there. VirtIO frontend are only virtio, so they will not do anything with the xenstore nodes. They can be removed.
While at it, also add a comment to the libxl_virtio.c file.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Reviewed-by: Anthony PERARD anthony.perard@citrix.com
Thanks,
The _setdefault() function for virtio devices is getting called after libxl__prepare_dtb(), which is late as libxl__prepare_dtb() expects the defaults to be already set by this time.
Call libxl__virtio_devtype.set_default() from libxl__domain_config_setdefault(), in a similar way as other devices like disk, etc.
Suggested-by: Anthony PERARD anthony.perard@citrix.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- tools/libs/light/libxl_create.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c index ec8eab02c207..36770af6d4ff 100644 --- a/tools/libs/light/libxl_create.c +++ b/tools/libs/light/libxl_create.c @@ -1068,7 +1068,7 @@ int libxl__domain_config_setdefault(libxl__gc *gc, uint32_t domid /* for logging, only */) { libxl_ctx *ctx = libxl__gc_owner(gc); - int ret; + int ret, i; bool pod_enabled = false; libxl_domain_create_info *c_info = &d_config->c_info;
@@ -1266,6 +1266,15 @@ int libxl__domain_config_setdefault(libxl__gc *gc, goto error_out; }
+ for (i = 0; i < d_config->num_virtios; i++) { + ret = libxl__virtio_devtype.set_default(gc, domid, + &d_config->virtios[i], false); + if (ret) { + LOGD(ERROR, domid, "Unable to set virtio defaults for device %d", i); + goto error_out; + } + } + ret = 0; error_out: return ret;
On Fri, Jun 02, 2023 at 11:19:08AM +0530, Viresh Kumar wrote:
The _setdefault() function for virtio devices is getting called after libxl__prepare_dtb(), which is late as libxl__prepare_dtb() expects the defaults to be already set by this time.
Call libxl__virtio_devtype.set_default() from libxl__domain_config_setdefault(), in a similar way as other devices like disk, etc.
Suggested-by: Anthony PERARD anthony.perard@citrix.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Reviewed-by: Anthony PERARD anthony.perard@citrix.com
Thanks,
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@linaro.org --- 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 24ac92718288..3a40ac8cb322 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 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 { +return fmt.Errorf("converting field GrantUsage: %v", err) +} x.Devid = Devid(xc.devid) x.Irq = uint32(xc.irq) x.Base = uint64(xc.base) @@ -1809,6 +1812,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 a7c17699f80e..e0c6e91bb0ef 100644 --- a/tools/golang/xenlight/types.gen.go +++ b/tools/golang/xenlight/types.gen.go @@ -683,6 +683,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 97c80d7ed0fa..bc2bd9649b95 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 grant_usage) { 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 (grant_usage) { 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 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); @@ -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 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 */ @@ -1363,7 +1365,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) ); } }
@@ -1373,12 +1376,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 c10292e0d7e3..c5c0d1f91793 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), + ("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..19d834984777 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, @@ -48,11 +56,13 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid, flexarray_t *ro_front) { const char *transport = libxl_virtio_transport_to_string(virtio->transport); + const char *grant_usage = libxl_defbool_to_string(virtio->grant_usage);
flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq)); 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", GCSPRINTF("%s", grant_usage));
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 1f6f47daf4e1..c66b11fd01b2 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;
On Fri, Jun 02, 2023 at 11:19:09AM +0530, Viresh Kumar wrote:
diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index f8a78e22d156..19d834984777 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -48,11 +56,13 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, uint32_t domid, flexarray_t *ro_front) { const char *transport = libxl_virtio_transport_to_string(virtio->transport);
- const char *grant_usage = libxl_defbool_to_string(virtio->grant_usage);
flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq)); 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", GCSPRINTF("%s", grant_usage));
It doesn't seems like a good idea to write a string like "True" or "False" in xenstore when a simple integer would work. Also I'm pretty sure all other bool are written as "0" or "1", for false or true. Could you change to write "0" or "1" instead of using libxl_defbool_to_string() ?
Beside this, patch looks good to me.
Cheers,
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@linaro.org --- V3->V3.1: - Print "0" or "1" in xenstore instead of "True" or "False" for grant_usage.
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 24ac92718288..3a40ac8cb322 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 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 { +return fmt.Errorf("converting field GrantUsage: %v", err) +} x.Devid = Devid(xc.devid) x.Irq = uint32(xc.irq) x.Base = uint64(xc.base) @@ -1809,6 +1812,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 a7c17699f80e..e0c6e91bb0ef 100644 --- a/tools/golang/xenlight/types.gen.go +++ b/tools/golang/xenlight/types.gen.go @@ -683,6 +683,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 97c80d7ed0fa..bc2bd9649b95 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 grant_usage) { 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 (grant_usage) { 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 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); @@ -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 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 */ @@ -1363,7 +1365,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) ); } }
@@ -1373,12 +1376,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 c10292e0d7e3..c5c0d1f91793 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), + ("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 1f6f47daf4e1..c66b11fd01b2 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;
On Tue, Jun 13, 2023 at 11:32:16AM +0530, Viresh Kumar wrote:
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@linaro.org
V3->V3.1:
- Print "0" or "1" in xenstore instead of "True" or "False" for grant_usage.
Reviewed-by: Anthony PERARD anthony.perard@citrix.com
Thanks,
On 13-06-23, 11:48, Anthony PERARD wrote:
On Tue, Jun 13, 2023 at 11:32:16AM +0530, Viresh Kumar wrote:
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@linaro.org
V3->V3.1:
- Print "0" or "1" in xenstore instead of "True" or "False" for grant_usage.
Reviewed-by: Anthony PERARD anthony.perard@citrix.com
Hi,
I still don't see this patch pushed upstream. Are we waiting for some release to happen ?
Thanks.
Hi,
On 30/06/2023 05:40, Viresh Kumar wrote:
On 13-06-23, 11:48, Anthony PERARD wrote:
On Tue, Jun 13, 2023 at 11:32:16AM +0530, Viresh Kumar wrote:
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@linaro.org
V3->V3.1:
- Print "0" or "1" in xenstore instead of "True" or "False" for grant_usage.
Reviewed-by: Anthony PERARD anthony.perard@citrix.com
Hi,
I still don't see this patch pushed upstream. Are we waiting for some release to happen ?
The patch is missing an ack from the golang maintainers. I noticed you didn't CC them (done now).
In the future, I would suggest to use one of the scripts in the repo to find the appropriate maintainers: * scripts/get_maintainers.pl: It will output the list of maintainers * scripts/add_maintainers.pl: It will add the maintainers for each patch files in a given directory.
Cheers,
(correcting the e-mail from Nick)
On 30/06/2023 09:40, Julien Grall wrote:
Hi,
On 30/06/2023 05:40, Viresh Kumar wrote:
On 13-06-23, 11:48, Anthony PERARD wrote:
On Tue, Jun 13, 2023 at 11:32:16AM +0530, Viresh Kumar wrote:
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@linaro.org
V3->V3.1:
- Print "0" or "1" in xenstore instead of "True" or "False" for
grant_usage.
Reviewed-by: Anthony PERARD anthony.perard@citrix.com
Hi,
I still don't see this patch pushed upstream. Are we waiting for some release to happen ?
The patch is missing an ack from the golang maintainers. I noticed you didn't CC them (done now).
In the future, I would suggest to use one of the scripts in the repo to find the appropriate maintainers: * scripts/get_maintainers.pl: It will output the list of maintainers * scripts/add_maintainers.pl: It will add the maintainers for each patch files in a given directory.
Cheers,
On 30-06-23, 09:40, Julien Grall wrote:
The patch is missing an ack from the golang maintainers. I noticed you didn't CC them (done now).
Ahh, thanks.
In the future, I would suggest to use one of the scripts in the repo to find the appropriate maintainers:
- scripts/get_maintainers.pl: It will output the list of maintainers
- scripts/add_maintainers.pl: It will add the maintainers for each patch files in a given directory.
Right.
stratos-dev@op-lists.linaro.org
-
Anthony PERARD -
Julien Grall -
Viresh Kumar