From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tian, Kevin" Subject: Re: [RFC][PATCH 07/13] xen/passthrough: extend hypercall to support rdm reservation policy Date: Mon, 4 May 2015 08:15:01 +0000 Message-ID: References: <1428657724-3498-1-git-send-email-tiejun.chen@intel.com> <1428657724-3498-8-git-send-email-tiejun.chen@intel.com> <20150416154056.GI13441@deinos.phlegethon.org> <5538E65F.60903@intel.com> <553916F00200007800075328@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <553916F00200007800075328@mail.emea.novell.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , "Chen, Tiejun" Cc: Tim Deegan , "wei.liu2@citrix.com" , "ian.campbell@citrix.com" , "andrew.cooper3@citrix.com" , "Ian.Jackson@eu.citrix.com" , "xen-devel@lists.xen.org" , "stefano.stabellini@citrix.com" , "Zhang, Yang Z" List-Id: xen-devel@lists.xenproject.org > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Thursday, April 23, 2015 10:00 PM > > >>> On 23.04.15 at 14:32, wrote: > > On 2015/4/16 23:40, Tim Deegan wrote: > >> At 17:21 +0800 on 10 Apr (1428686518), Tiejun Chen wrote: > >>> @@ -1851,7 +1857,14 @@ static int rmrr_identity_mapping(struct > domain *d, bool_t map, > >>> if ( !is_hardware_domain(d) ) > >>> { > >>> if ( (err = set_identity_p2m_entry(d, base_pfn, > p2m_access_rw)) ) > >>> - return err; > >>> + { > >>> + if ( flag == XEN_DOMCTL_PCIDEV_RDM_TRY ) > >>> + { > >>> + printk(XENLOG_G_WARNING "Some devices > may work failed .\n"); > >> > >> This is a bit cryptic. How about: > >> "RMRR map failed. Device %04x:%02x:%02x.%u and domain %d may > be > > unstable.", > >> (and pass in the devfn from the caller so we can print the details of > >> the device). > > > > Got it but we can't get SBDF here directly. > > > > So just now we can have this line. > > > > { > > if ( flag == XEN_DOMCTL_PCIDEV_RDM_TRY ) > > dprintk(XENLOG_ERR VTDPREFIX, > > "RMRR mapping failed to pfn:%"PRIx64"" > > " so Dom%d may be unstable.\n", > > base_pfn, d->domain_id); > > else > > return err; > > } > > > > Certainly, we can extend rmrr_identity_mapping() to own its associated > > SBDF as an input parameter (and bring some syncs) if you still think > > this is necessary. > > I don't think we can, since a single RMRR may be associated with > more than one device. > > >>> @@ -493,6 +493,10 @@ > DEFINE_XEN_GUEST_HANDLE(xen_domctl_sendtrigger_t); > >>> /* XEN_DOMCTL_deassign_device */ > >>> struct xen_domctl_assign_device { > >>> uint32_t machine_sbdf; /* machine PCI ID of assigned device > */ > >>> + /* IN */ > >>> +#define XEN_DOMCTL_PCIDEV_RDM_TRY 0 > >>> +#define XEN_DOMCTL_PCIDEV_RDM_FORCE 1 > >> > >> "STRICT" might be a better word than "FORCE" (here and everywhere > >> else). "FORCE" sounds like either Xen will assign the device even if > >> it's unsafe, which is the opposite of what's meant IIUC. > > > > This is definitely fine to me but this is derived from our policy based > > on that previous design, > > > > Global RDM parameter: > > rdm = [ 'host, reserve=force/try' ] > > Per-device RDM parameter: > > pci = [ 'sbdf, rdm_reserve=force/try' ] > > > > Please refer to patch #1. So I guess we need a further agreement or > > comments from other guys :) > > I think I'd prefer strict/relaxed over force/try, but I certainly can > live with the latter. > agree the former is clearer. Thanks Kevin