On Thu, Mar 30, 2023 at 02:13:08PM +0530, Viresh Kumar wrote:
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.
I think this description is missing context. I'm not sure what it would means "from dom0" without reading the patch. Also, it says "allows", maybe this doesn't convey the meaning of "forced". How about something like:
Always use grant mapping, even when the backend is run in dom0. (grant are already used if the backend is in another domain.)
=back =item B<tee="STRING"> 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));
This "forced_grant" feels weird to me in the protocol, I feel like this use of grant or not could be handled by the backend. For example in "blkif" protocol, there's plenty of "feature-*" which allows both front-end and back-end to advertise which feature they can or want to use. But maybe the fact that the device tree needs to be modified to be able to accommodate grant mapping means that libxl needs to ask the backend to use grant or not, and the frontend needs to now if it needs to use 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);
Maybe store only !!strtoul() ? I don't think having values other that 0 or 1 is going to be good.
} else { fprintf(stderr, "Unknown string \"%s\" in virtio spec\n", token); return -1;
Thanks,