From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v8 12/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Date: Sun, 25 May 2014 18:04:17 +0100 Message-ID: <53822291.2080107@linaro.org> References: <1401015115-7610-1-git-send-email-avanzini.arianna@gmail.com> <1401015115-7610-13-git-send-email-avanzini.arianna@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1401015115-7610-13-git-send-email-avanzini.arianna@gmail.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: Arianna Avanzini , xen-devel@lists.xen.org Cc: julien.grall@citrix.com, paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, tim@xen.org, dario.faggioli@citrix.com, Ian.Jackson@eu.citrix.com, Ian.Campbell@eu.citrix.com, etrudeau@broadcom.com, JBeulich@suse.com, andrew.cooper3@citrix.com, viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org Hi Arianna, On 25/05/14 11:51, Arianna Avanzini wrote: > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index 369c3f3..14f9880 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -1650,6 +1650,18 @@ int xc_domain_memory_mapping( > uint32_t add_mapping) > { > DECLARE_DOMCTL; > + xc_dominfo_t info; > + > + if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ) > + { You may retrieve the wrong domain here. xc_domain_getinfo return the information of the first domain with an ID >= domid (see XEN_DOMCTL_getdomaininfo in xen/common/domctl.c). > + PERROR("Could not get info for domain"); > + return -EINVAL; > + } > + if ( !xc_core_arch_auto_translated_physmap(&info) ) > + { > + PERROR("memory_mapping only available for auto-translated domains"); PERROR doesn't make sense here. Errno is not set by xc_core_arch_auto_* Futhermore, I won't add any error message here. Otherwise, we may have spurious error log for PV with libxl (you are calling this function unconditionally in the toolstack). > + return 0; > + } > > domctl.cmd = XEN_DOMCTL_memory_mapping; > domctl.domain = domid; > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 6aa630e..4de0fb2 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -1113,6 +1113,17 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev, > "failed give dom%d access to iomem range %"PRIx64"-%"PRIx64, > domid, io->start, io->start + io->number - 1); > ret = ERROR_FAIL; > + continue; It should be a "goto error_out" rather than a continue here. I plan to send a patch fixing the other loop (see http://lists.xen.org/archives/html/xen-devel/2014-05/msg01831.html), so I'm fine if you keep as it is this part. > + } > + ret = xc_domain_memory_mapping(CTX->xch, domid, > + io->gfn, io->start, > + io->number, 1); > + if (ret < 0) { > + LOGE(ERROR, > + "failed to map to dom%d iomem range %"PRIx64"-%"PRIx64 > + " to guest address %"PRIx64, > + domid, io->start, io->start + io->number - 1, io->gfn); > + ret = ERROR_FAIL; Same remark here. Regards, -- Julien Grall