From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][PATCH 12/13] hvmloader/pci: skip reserved ranges Date: Fri, 15 May 2015 17:21:54 +0800 Message-ID: <5555BAB2.8000607@intel.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> <5555AC10020000780007A59B@mail.emea.novell.com> <5555A185.4020200@intel.com> <5555C002020000780007A6AB@mail.emea.novell.com> <5555AB42.4020004@intel.com> <5555CAF0020000780007A6FB@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5555CAF0020000780007A6FB@mail.emea.novell.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: Jan Beulich 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 2015/5/15 16:31, Jan Beulich wrote: >>>> On 15.05.15 at 10:16, wrote: >> On 2015/5/15 15:44, Jan Beulich wrote: >>>>>> On 15.05.15 at 09:34, wrote: >>>> So I think we may need to adjust pci_mem_start like this, >>>> >>>> @@ -301,6 +301,19 @@ void pci_setup(void) >>>> pci_mem_start <<= 1; >>>> } >>>> >>>> + /* Relocate PCI memory that overlaps reserved space, like RDM. */ >>>> + 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_overlap(pci_mem_start, pci_mem_end, >>>> + memory_map.map[j].addr, >>>> + memory_map.map[j].size) ) >>>> + pci_mem_start -= memory_map.map[j].size >> PAGE_SHIFT; >>>> + } >>>> + } >>>> + >>>> if ( mmio_total > (pci_mem_end - pci_mem_start) ) >>>> { >>>> printf("Low MMIO hole not large enough for all devices," >>>> >>>> Right? >>> >>> I think that gets you in the right direction, but isn't enough, as it >>> doesn't account for (unavoidable) gaps (BARs are always a power >>> of 2 in size and accordingly aligned). >> >> Right. >> >> But as you see, we always take this action, >> PAGE_SHIFT, so this means >> its always a sort of power of 2. > > No, certainly not. rdm start or size? Or anything else I'm missing? > >> Additionally, lets go back 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 mean even without rdm, the original codes don't consider handling this >> lack of space from a alignment in advance, right? > > Correct. But your change increases the chances of this getting > used. Also I think you may want to carefully look at under what > conditions this path gets taken without and with your patches. > Sure. I think I can record that max bar_sz to improve this like, @@ -60,7 +60,7 @@ void pci_setup(void) uint64_t bar_sz; } *bars = (struct bars *)scratch_start; unsigned int i, j, nr_bars = 0; - uint64_t mmio_hole_size = 0, reserved_end; + uint64_t mmio_hole_size = 0, reserved_end, max_bar_sz = 0; const char *s; /* @@ -226,6 +226,8 @@ void pci_setup(void) bars[i].devfn = devfn; bars[i].bar_reg = bar_reg; bars[i].bar_sz = bar_sz; + if ( bar_sz > max_bar_sz ) + max_bar_sz = bar_sz; if ( ((bar_data & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY) || @@ -311,6 +313,8 @@ void pci_setup(void) memory_map.map[j].addr, memory_map.map[j].size) ) pci_mem_start -= memory_map.map[j].size >> PAGE_SHIFT; + pci_mem_start = (pci_mem_start + max_bar_sz - 1) & + ~(uint64_t)(max_bar_sz - 1); } } Note you also can take close look at this change in next revision if this is not that bad with your glance :) Thanks Tiejun