On Thu, May 11, 2023 at 01:20:43PM +0530, Viresh Kumar wrote:
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 24ac92718288..0405f6efe62a 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -1619,6 +1619,18 @@ 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=STRING>
+Specifies the grant usage details for the Virtio device. This can be set to +following values:
+- "default": The default grant setting will be used, enable grants if
- backend-domid != 0.
I don't think this "default" setting is useful. We could just describe what the default is when "grant_usage" setting is missing from the configuration.
+- "enabled": The grants are always enabled.
+- "disabled": The grants are always disabled.
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl index c10292e0d7e3..17228817f9b7 100644 --- a/tools/libs/light/libxl_types.idl +++ b/tools/libs/light/libxl_types.idl @@ -283,6 +283,12 @@ libxl_virtio_transport = Enumeration("virtio_transport", [ (1, "MMIO"), ]) +libxl_virtio_grant_usage = Enumeration("virtio_grant_usage", [
- (0, "DEFAULT"),
- (1, "DISABLED"),
- (2, "ENABLED"),
libxl already provide this type, it's call "libxl_defbool". It can be set to "default", "false" or "true".
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c index 97c80d7ed0fa..9cd7dbef0237 100644 --- a/tools/libs/light/libxl_arm.c +++ b/tools/libs/light/libxl_arm.c @@ -1363,22 +1365,29 @@ 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) ); } }for (i = 0; i < d_config->num_virtios; i++) { libxl_device_virtio *virtio = &d_config->virtios[i];
bool use_grant = false;if (virtio->transport != LIBXL_VIRTIO_TRANSPORT_MMIO) continue;
if (virtio->backend_domid != LIBXL_TOOLSTACK_DOMID)
if ((virtio->grant_usage == LIBXL_VIRTIO_GRANT_USAGE_ENABLED) ||((virtio->grant_usage == LIBXL_VIRTIO_GRANT_USAGE_DEFAULT) &&(virtio->backend_domid != LIBXL_TOOLSTACK_DOMID))) {
I think libxl can select what the default value should be replace with before we start to setup the guest. There's a *_setdefault() phase were we set the correct value when a configuration value hasn't been set and thus a default value is used. I think this can be done in libxl__device_virtio_setdefault(). After that, virtio->grant_usage will be true or false, and that's the value that should be given to the virtio backend via xenstore.
diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c index eadcb7124c3f..0a0fae967a0f 100644 --- a/tools/libs/light/libxl_virtio.c +++ b/tools/libs/light/libxl_virtio.c @@ -46,11 +46,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_virtio_grant_usage_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));
Currently, this mean that we store "default" in this node. That mean that both the virtio backend and libxl have to do computation in order to figure out if "default" mean "true" or "false". And both have to find the same result. I don't think this is necessary, and libxl can just tell enabled or disable. This would be done in libxl before we run this function. See previous comment on this patch.
Thanks,