From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [RFC][PATCH 12/13] hvmloader/pci: skip reserved ranges Date: Fri, 15 May 2015 07:19:28 +0100 Message-ID: <5555AC10020000780007A59B@mail.emea.novell.com> References: <1428657724-3498-1-git-send-email-tiejun.chen@intel.com> <1428657724-3498-13-git-send-email-tiejun.chen@intel.com> <553527940200007800073DA5@mail.emea.novell.com> <5555659C.4040104@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5555659C.4040104@intel.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tiejun Chen Cc: tim@xen.org, kevin.tian@intel.com, 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, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org >>> On 15.05.15 at 05:18, wrote: > On 2015/4/20 22:21, Jan Beulich wrote: >>>>> On 10.04.15 at 11:22, wrote: >>> --- a/tools/firmware/hvmloader/pci.c >>> +++ b/tools/firmware/hvmloader/pci.c >>> @@ -59,8 +59,8 @@ void pci_setup(void) >>> uint32_t bar_reg; >>> uint64_t bar_sz; >>> } *bars = (struct bars *)scratch_start; >>> - unsigned int i, nr_bars = 0; >>> - uint64_t mmio_hole_size = 0; >>> + unsigned int i, j, nr_bars = 0; >>> + uint64_t mmio_hole_size = 0, reserved_end; >>> >>> const char *s; >>> /* >>> @@ -393,8 +393,23 @@ void pci_setup(void) >>> } >>> >>> base = (resource->base + bar_sz - 1) & ~(uint64_t)(bar_sz - 1); >>> + reallocate_mmio: >>> bar_data |= (uint32_t)base; >>> bar_data_upper = (uint32_t)(base >> 32); >>> + for ( j = 0; j < memory_map.nr_map ; j++ ) >>> + { >>> + if ( memory_map.map[j].type != E820_RAM ) >>> + { >>> + reserved_end = memory_map.map[j].addr + > memory_map.map[j].size; >>> + if ( check_hole_conflict(base, bar_sz, >>> + memory_map.map[j].addr, >>> + memory_map.map[j].size) ) >>> + { >>> + base = (reserved_end + bar_sz - 1) & ~(uint64_t)(bar_sz - > 1); >>> + goto reallocate_mmio; >>> + } >>> + } >>> + } >>> base += bar_sz; >>> >>> if ( (base < resource->base) || (base > resource->max) ) >> > > Actually some original codes are missing here, > > if ( (base < resource->base) || (base > resource->max) ) > { > printf("pci dev %02x:%x bar %02x size "PRIllx": no space for " > "resource!\n", devfn>>3, devfn&7, bar_reg, > PRIllx_arg(bar_sz)); > continue; > } > > I think this can guarantee the MMIO regions just fit in the available RAM. > > Or am I wrong? The code you cite guarantees almost nothing, it simply skips assigning resources. Your changes potentially growing the space needed to fit all MMIO BARs therefore also needs to adjust the up front calculation, such that if necessary more RAM can be relocated to make the hole large enough. Jan