From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][v2][PATCH 08/14] tools: extend xc_assign_device() to support rdm reservation policy Date: Wed, 03 Jun 2015 10:58:31 +0800 Message-ID: <556E6D57.7020107@intel.com> References: <1432287314-4388-1-git-send-email-tiejun.chen@intel.com> <1432287314-4388-9-git-send-email-tiejun.chen@intel.com> <20150602163602.GV19403@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: <20150602163602.GV19403@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/3 0:36, Wei Liu wrote: > On Fri, May 22, 2015 at 05:35:08PM +0800, Tiejun Chen wrote: >> This patch passes rdm reservation policy to xc_assign_device() so the policy >> is checked when assigning devices to a VM. >> >> Signed-off-by: Tiejun Chen >> --- >> tools/libxc/include/xenctrl.h | 3 ++- >> tools/libxc/xc_domain.c | 4 +++- >> tools/libxl/libxl_pci.c | 11 ++++++++++- >> tools/libxl/xl_cmdimpl.c | 23 +++++++++++++++++++---- >> tools/libxl/xl_cmdtable.c | 2 +- > > Where is document for the new options you added to xl pci commands? Looks I'm missing to describe something specific to pci-attach? diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1 index 4eb929d..2ebfd54 100644 --- a/docs/man/xl.pod.1 +++ b/docs/man/xl.pod.1 @@ -1368,10 +1368,15 @@ it will also attempt to re-bind the device to its original driver, making it usable by Domain 0 again. If the device is not bound to pciback, it will return success. -=item B I I +=item B I I I Hot-plug a new pass-through pci device to the specified domain. B is the PCI Bus/Device/Function of the physical device to pass-through. +B is about how to handle conflict between reserving reserved device +memory and guest address space. "strict" means an unsolved conflict leads to +immediate VM crash, while "relaxed" allows VM moving forward with a warning +message thrown out. Here "strict" is default. + =item B [I<-f>] I I > > BTW you might want to consider rearrange patches in this series so that Yes, this is really what I intend to do. > you keep the tree bisectable. Overall, I can separate this series as several parts, #1. Introduce our policy configuration on tools side #2. Interact with Hypervisor to get rdm info #3. Implement our policy with rdm info on tool side #4. Make hvmloader to align our policy If you already see something obviously wrong, let me know. > >> tools/ocaml/libs/xc/xenctrl_stubs.c | 18 ++++++++++++++---- >> tools/python/xen/lowlevel/xc/xc.c | 29 +++++++++++++++++++---------- >> xen/drivers/passthrough/pci.c | 3 ++- >> 8 files changed, 70 insertions(+), 23 deletions(-) >> >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h >> index 5f84a62..2a447b9 100644 >> --- a/tools/libxc/include/xenctrl.h >> +++ b/tools/libxc/include/xenctrl.h >> @@ -2078,7 +2078,8 @@ int xc_hvm_destroy_ioreq_server(xc_interface *xch, >> /* HVM guest pass-through */ >> int xc_assign_device(xc_interface *xch, >> uint32_t domid, >> - uint32_t machine_sbdf); >> + uint32_t machine_sbdf, >> + uint32_t flag); >> >> int xc_get_device_group(xc_interface *xch, >> uint32_t domid, >> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c >> index c17a5a8..9761e5a 100644 >> --- a/tools/libxc/xc_domain.c >> +++ b/tools/libxc/xc_domain.c >> @@ -1704,7 +1704,8 @@ int xc_domain_setdebugging(xc_interface *xch, >> int xc_assign_device( >> xc_interface *xch, >> uint32_t domid, >> - uint32_t machine_sbdf) >> + uint32_t machine_sbdf, >> + uint32_t flag) >> { >> DECLARE_DOMCTL; >> >> @@ -1712,6 +1713,7 @@ int xc_assign_device( >> domctl.domain = domid; >> domctl.u.assign_device.dev = XEN_DOMCTL_DEV_PCI; >> domctl.u.assign_device.u.pci.machine_sbdf = machine_sbdf; >> + domctl.u.assign_device.flag = flag; >> >> return do_domctl(xch, &domctl); >> } >> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c >> index 07e84f2..ac70edc 100644 >> --- a/tools/libxl/libxl_pci.c >> +++ b/tools/libxl/libxl_pci.c >> @@ -894,6 +894,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i >> FILE *f; >> unsigned long long start, end, flags, size; >> int irq, i, rc, hvm = 0; >> + uint32_t flag; >> >> if (type == LIBXL_DOMAIN_TYPE_INVALID) >> return ERROR_FAIL; >> @@ -987,7 +988,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i >> >> out: >> if (!libxl_is_stubdom(ctx, domid, NULL)) { >> - rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev)); >> + if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_RELAXED) { >> + flag = XEN_DOMCTL_DEV_RDM_RELAXED; >> + } else if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_STRICT) { >> + flag = XEN_DOMCTL_DEV_RDM_STRICT; >> + } else { >> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unkwon rdm check flag."); > > unknown > > Couldn't continue reviewing because I don't know the expected behaviour. > But the changes look mostly mechanical. > I want to make this assignment failed so return ERROR_FAIL