From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges Date: Wed, 15 Jul 2015 12:27:14 +0800 Message-ID: <55A5E122.7030203@intel.com> References: <1436420047-25356-1-git-send-email-tiejun.chen@intel.com> <1436420047-25356-7-git-send-email-tiejun.chen@intel.com> <55A3D5600200007800090330@mail.emea.novell.com> <55A4AE88.2000200@intel.com> <55A4F2270200007800090834@mail.emea.novell.com> <55A4EA54.60708@intel.com> <55A5138F0200007800090A71@mail.emea.novell.com> <55A5AF6F.1050305@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A5AF6F.1050305@intel.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: Wei Liu , Ian Campbell , Stefano Stabellini , Andrew Cooper , Ian Jackson , xen-devel@lists.xen.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org On 2015/7/15 8:55, Chen, Tiejun wrote: >>> I agree we'd better overhaul this since we already found >>> something unreasonable here. But one or two weeks is really not >>> enough to fix this with a bitmap framework, and although a bitmap >>> can make mmio allocation better, but more complicated if we just >>> want to allocate PCI mmio. >>> >>> So could we do this next? I just feel if you'd like to pay more >>> time help me refine our current solution, its relatively >>> realistic to this case :) And then we can go into bitmap in >>> details or work out a better solution in sufficient time slot. >> >> Looking at how long it took to get here (wasn't this series >> originally even meant to go into 4.5?) and how much time I already >> spent > > Certainly appreciate your time. > > I didn't mean its wasting time at this point. I just want to express > that its hard to implement that solution in one or two weeks to > walking into 4.6 as an exception. > > Note I know this feature is still not accepted as an exception to 4.6 > right now so I'm making an assumption. > >> reviewing all the previous versions, I don't see a point in >> wasting even more time on working out details of an approach that's >> getting too complex/ugly already anyway. > > Here I'm trying to seek such a kind of two-steps approach if > possible. > Furthermore, could we have this solution as follows? @@ -407,8 +408,29 @@ void pci_setup(void) } base = (resource->base + bar_sz - 1) & ~(uint64_t)(bar_sz - 1); + reallocate_bar: bar_data |= (uint32_t)base; bar_data_upper = (uint32_t)(base >> 32); + /* + * We need to skip those reserved regions like RMRR but this pobably + * lead the remaing allocation failed. + */ + 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(base, bar_sz, + memory_map.map[j].addr, + memory_map.map[j].size) ) + { + if ( !mmio_conflict ) + mmio_conflict = true; + base = (reserved_end + bar_sz - 1) & ~(uint64_t)(bar_sz - 1); + goto reallocate_bar; + } + } + } base += bar_sz; if ( (base < resource->base) || (base > resource->max) ) @@ -416,6 +438,11 @@ void pci_setup(void) printf("pci dev %02x:%x bar %02x size "PRIllx": no space for " "resource!\n", devfn>>3, devfn&7, bar_reg, PRIllx_arg(bar_sz)); + if ( mmio_conflict ) + { + printf("MMIO conflicts with RDM.\n"); + BUG(); + } continue; } Here we still check RDMs and skip them if any conflicts exist, but at last, if 1) no sufficient space to allocate all bars to this VM && 2) this insufficient situation is triggered by RDM, we stop creating this VM. This may change slightly current mechanism and also guarantee we don't ignore any conflicts. But I admit this is not a good solution because that alignment issue still exists but I think it may be accepted if all PCI devices specific to this VM can work. And in real world most RDMs just fall into somewhere but < 0xF0000000 (current default pci memory start address), so its not a common case to see this kind of conflict. Thanks Tiejun