From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][v2][PATCH 01/14] tools: introduce some new parameters to set rdm policy Date: Mon, 08 Jun 2015 09:42:33 +0800 Message-ID: <5574F309.9080803@intel.com> References: <1432287314-4388-1-git-send-email-tiejun.chen@intel.com> <1432287314-4388-2-git-send-email-tiejun.chen@intel.com> <20150602155715.GT19403@zion.uk.xensource.com> <556E59D4.6080804@intel.com> <20150607110619.GO29102@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150607110619.GO29102@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: kevin.tian@intel.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, tim@xen.org, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, JBeulich@suse.com, yang.z.zhang@intel.com, Ian.Jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 2015/6/7 19:06, Wei Liu wrote: > 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. The policy still can be changed with "reserve=". But whatever, I mean this current format is easy to extend :) You know, the hotplug is really a complicated case, we really need to consider more next. Thanks Tiejun > >>> 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. >