From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v3][PATCH 12/16] tools/libxl: passes rdm reservation policy Date: Mon, 15 Jun 2015 09:26:30 +0800 Message-ID: <557E29C6.3000304@intel.com> References: <1433985325-16676-1-git-send-email-tiejun.chen@intel.com> <1433985325-16676-13-git-send-email-tiejun.chen@intel.com> <20150612161706.GY14606@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: <20150612161706.GY14606@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/13 0:17, Wei Liu wrote: > On Thu, Jun 11, 2015 at 09:15:21AM +0800, Tiejun Chen wrote: >> This patch passes our rdm reservation policy inside libxl >> when we assign a device or attach a device. >> >> Signed-off-by: Tiejun Chen >> --- >> docs/man/xl.pod.1 | 7 ++++++- >> tools/libxl/libxl_pci.c | 10 +++++++++- >> tools/libxl/xl_cmdimpl.c | 23 +++++++++++++++++++---- >> tools/libxl/xl_cmdtable.c | 2 +- >> 4 files changed, 35 insertions(+), 7 deletions(-) >> >> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1 >> index 4eb929d..c5c4809 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 >> >> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c >> index a00d799..d2e8911 100644 >> --- a/tools/libxl/libxl_pci.c >> +++ b/tools/libxl/libxl_pci.c >> @@ -894,7 +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 = XEN_DOMCTL_DEV_RDM_RELAXED; >> + uint32_t flag; >> >> if (type == LIBXL_DOMAIN_TYPE_INVALID) >> return ERROR_FAIL; >> @@ -988,6 +988,14 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i >> >> out: >> if (!libxl_is_stubdom(ctx, domid, NULL)) { >> + 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."); > > Typo "unkwon" and use LOG(ERROR,...). Will fix that typo, s/unkwon/unknown, but are you sure we should use LOG() here? Because this function always uses LIBXL__LOG_ERRNO(), > >> + return ERROR_FAIL; >> + } >> rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev), flag); >> if (rc < 0 && (hvm || errno != ENOSYS)) { >> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_assign_device failed"); like here. >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >> index aedbd4b..4364ba4 100644 >> --- a/tools/libxl/xl_cmdimpl.c >> +++ b/tools/libxl/xl_cmdimpl.c >> @@ -3359,7 +3359,8 @@ int main_pcidetach(int argc, char **argv) >> pcidetach(domid, bdf, force); >> return 0; >> } >> -static void pciattach(uint32_t domid, const char *bdf, const char *vs) >> +static void pciattach(uint32_t domid, const char *bdf, const char *vs, >> + uint32_t flag) >> { >> libxl_device_pci pcidev; >> XLU_Config *config; >> @@ -3369,6 +3370,7 @@ static void pciattach(uint32_t domid, const char *bdf, const char *vs) >> config = xlu_cfg_init(stderr, "command line"); >> if (!config) { perror("xlu_cfg_inig"); exit(-1); } >> >> + pcidev.rdm_reserve = flag; >> if (xlu_pci_parse_bdf(config, &pcidev, bdf)) { >> fprintf(stderr, "pci-attach: malformed BDF specification \"%s\"\n", bdf); >> exit(2); >> @@ -3381,9 +3383,9 @@ static void pciattach(uint32_t domid, const char *bdf, const char *vs) >> >> int main_pciattach(int argc, char **argv) >> { >> - uint32_t domid; >> + uint32_t domid, flag; >> int opt; >> - const char *bdf = NULL, *vs = NULL; >> + const char *bdf = NULL, *vs = NULL, *rdm_policy = NULL; >> >> SWITCH_FOREACH_OPT(opt, "", NULL, "pci-attach", 2) { >> /* No options */ >> @@ -3395,7 +3397,20 @@ int main_pciattach(int argc, char **argv) >> if (optind + 1 < argc) >> vs = argv[optind + 2]; >> >> - pciattach(domid, bdf, vs); >> + if (optind + 2 < argc) { >> + rdm_policy = argv[optind + 3]; >> + } >> + if (!strcmp(rdm_policy, "strict")) { >> + flag = LIBXL_RDM_RESERVE_FLAG_STRICT; >> + } else if (!strcmp(rdm_policy, "relaxed")) { >> + flag = LIBXL_RDM_RESERVE_FLAG_RELAXED; >> + } else { >> + fprintf(stderr, "%s is an invalid rdm policy: 'strict'|'relaxed'\n", >> + rdm_policy); >> + exit(2); >> + } >> + >> + pciattach(domid, bdf, vs, flag); >> return 0; >> } >> >> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c >> index 7f4759b..552fbec 100644 >> --- a/tools/libxl/xl_cmdtable.c >> +++ b/tools/libxl/xl_cmdtable.c >> @@ -88,7 +88,7 @@ struct cmd_spec cmd_table[] = { >> { "pci-attach", >> &main_pciattach, 0, 1, >> "Insert a new pass-through pci device", >> - " [Virtual Slot]", >> + " [Virtual Slot] ", > > Should use "[]" to indicate it's optional. > What about this? " [Virtual Slot] [policy to reserve rdm<'strice'|'relaxed'>]", Thanks Tiejun