From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v4][PATCH 4/9] tools:libxc: check if mmio BAR is out of RMRR mappings Date: Wed, 27 Aug 2014 09:46:57 +0800 Message-ID: <53FD3891.5020202@intel.com> References: <1408702186-24432-1-git-send-email-tiejun.chen@intel.com> <1408702186-24432-5-git-send-email-tiejun.chen@intel.com> <1409085374.28009.37.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1409085374.28009.37.camel@citrix.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: Ian Campbell Cc: kevin.tian@intel.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, JBeulich@suse.com, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2014/8/27 4:36, Ian Campbell wrote: > On Fri, 2014-08-22 at 18:09 +0800, Tiejun Chen wrote: > >> + /* We should check if mmio range is out of RMRR mapping. >> + * >> + * Assume we have one entry if not enough we'll expand. >> + */ > > The usual approach with such hypervisor interfaces (which I suppose > xc_reserved_device_memory_map turns into) is to first call it with NULL > to get the required size and then allocate a suitable buffer and call a > second time. Ofentimes, RMRR should be rare with one or two entries, even zero. So I think its reasonable to start posting one entry since this can cover such a scenario the platform really owns one entry. > > >> + for ( i = 0; i < rc; i++ ) >> + { >> + rmrr_start = map[i].pfn << PAGE_SHIFT; >> + rmrr_end = rmrr_start + map[i].count * PAGE_SIZE; >> + if ( check_mmio_hole(rmrr_start, map[i].count * PAGE_SIZE, > > Adding rmrr_size = map... & PAGE_SIZE could be used twice here. > Yes, so I think the follows should be expected: --- a/tools/libxc/xc_hvm_build_x86.c +++ b/tools/libxc/xc_hvm_build_x86.c @@ -251,7 +251,7 @@ static int check_rmrr_overlap(xc_interface *xch, uint64_t mmio_start, uint64_t mmio_size) { struct xen_mem_reserved_device_memory *map = NULL; - uint64_t rmrr_start = 0, rmrr_end = 0; + uint64_t rmrr_start = 0, rmrr_end = 0, rmrr_size = 0; unsigned int i = 0; int rc = 0; /* Assume we have one entry if not enough we'll expand.*/ @@ -295,9 +295,9 @@ static int check_rmrr_overlap(xc_interface *xch, uint64_t mmio_start, for ( i = 0; i < rc; i++ ) { rmrr_start = map[i].start_pfn << PAGE_SHIFT; - rmrr_end = rmrr_start + map[i].nr_pages * PAGE_SIZE; - if ( check_mmio_hole(rmrr_start, map[i].nr_pages * PAGE_SIZE, - mmio_start, mmio_size) ) + rmrr_size = map[i].nr_pages * PAGE_SIZE; + rmrr_end = rmrr_start + rmrr_size; + if ( check_mmio_hole(rmrr_start, rmrr_size, mmio_start, mmio_size) ) { PERROR("MMIO: [%lx]<->[%lx] overlap RMRR [%lx]<->[%lx]\n", mmio_start, (mmio_start + mmio_size), rmrr_start, rmrr_end); Thanks Tiejun > Ian. > >