Re: [Xen-devel] [RFC][v2][PATCH 01/14] tools: introduce some new parameters to set rdm policy

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jun 03, 2015 at 09:35:16AM +0800, Chen, Tiejun wrote:
[...]
> >
> >>+reserved regions explicitly. And using "host" to include all reserved 
> >>regions
> >>+reported on this platform which is good to handle hotplug scenario. In the
> >>+future this parameter may be further extended to allow specifying random
> >>+regions, e.g. even those belonging to another platform as a preparation
> >
> >Extending how? What's your envisaged syntax for those random regions?
> 
> We didn't go into details while discussing that design. Maybe we can do
> something like this,
> 
> rdm="type=host,reserve=strict,rdm_add=size[KMG][@offset[KMG]],size[KMG][@offset[KMG]],..."
> 

This limits the extra regions to type host and strict policy. If that's
what you want then it's fine.

> >Should you want to reserve more, an array is more useful. Could you
> 
> Yeah.
> 
> >provide some examples?
> 
> But we may have alternative approach to this when I noticed some guys are
> trying to delivery some patches about setting rmrr region by xen
> commandline. So I also would like to check this likelihood when we can step
> forward.
> 

Makes sense.

> >
> >>+for live migration with passthrough devices.
> >>+
> >>+"none" means we have nothing to do all reserved regions and ignore all 
> >>policies,
> >>+so guest work as before.

[...]

> >>diff --git a/docs/misc/vtd.txt b/docs/misc/vtd.txt
> >>index 9af0e99..7d63c47 100644
> >>--- a/docs/misc/vtd.txt
> >>+++ b/docs/misc/vtd.txt
> >>@@ -111,6 +111,30 @@ in the config file:
> >>  To override for a specific device:
> >>    pci = [ '01:00.0,msitranslate=0', '03:00.0' ]
> >>
> >>+RDM, 'reserved device memory', for PCI Device Passthrough
> >>+---------------------------------------------------------
> >>+
> >>+There are some devices the BIOS controls, for e.g. USB devices to perform
> >>+PS2 emulation. The regions of memory used for these devices are marked
> >>+reserved in the e820 map. When we turn on DMA translation, DMA to those
> >>+regions will fail. Hence BIOS uses RMRR to specify these regions along with
> >>+devices that need to access these regions. OS is expected to setup
> >>+identity mappings for these regions for these devices to access these 
> >>regions.
> >>+
> >>+While creating a VM we should reserve them in advance, and avoid any 
> >>conflicts.
> >>+So we introduce user configurable parameters to specify RDM resource and
> >>+according policies,
> >>+
> >>+To enable this globally, add "rdm" in the config file:
> >>+
> >>+    rdm = "type=host, reserve=relaxed"   (default policy is "relaxed")
> >>+
> >>+Or just for a specific device:
> >>+
> >>+    pci = [ '01:00.0,rdm_reserve=relaxed', '03:00.0,rdm_reserve=strict' ]
> >>+
> >>+For all the options available to RDM, see xl.cfg(5).
> >>+
> >>
> >>  Caveat on Conventional PCI Device Passthrough
> >>  ---------------------------------------------
> >>diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> >>index f0da7dc..d649ead 100644
> >>--- a/tools/libxl/libxl_create.c
> >>+++ b/tools/libxl/libxl_create.c
> >>@@ -100,6 +100,12 @@ static int sched_params_valid(libxl__gc *gc,
> >>      return 1;
> >>  }
> >>
> >>+void libxl__rdm_setdefault(libxl__gc *gc, libxl_domain_build_info *b_info)
> >>+{
> >>+    b_info->rdm.type = LIBXL_RDM_RESERVE_TYPE_NONE;
> 
> Based on our previous discussion, I will initial this firstly,
> 
> +libxl_rdm_reserve_type = Enumeration("rdm_reserve_type", [
> +    (0, "none"),
> +    (1, "host"),
> +    ], init_val = "LIBXL_RDM_RESERVE_TYPE_NONE")
> +
> 
> and then, I would remove this line since right now we just own two options,
> "none" or "host". And both they're fine.
> 
> >>+    b_info->rdm.reserve = LIBXL_RDM_RESERVE_FLAG_RELAXED;
> >
> >No, not like this. You set everything back to none and relaxed even if
> >it is set before this point.
> >
> >It should be
> >     if (xxx == DEFAULT_SENTINEL_VALUE)
> >         xxx = THE_DEFAULT_YOU_WANT;
> >
> >Have a look at libxl__device_nic_setdefault etc to get an idea
> >how it works. Don't hesitate to ask if I'm not clear enough.
> 
> But indeed, here we should set rdm.reserve as you said,
> 
> +void libxl__rdm_setdefault(libxl__gc *gc, libxl_domain_build_info *b_info)
> +{
> +    if (b_info->rdm.reserve == LIBXL_RDM_RESERVE_FLAG_INVALID)
> +        b_info->rdm.reserve = LIBXL_RDM_RESERVE_FLAG_RELAXED;
> +}
> +
> 

Yes. This is fine.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel