From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH] bus/pci: check if 5-level paging is enabled when testing IOMMU address width Date: Fri, 10 Aug 2018 10:18:45 +0100 Message-ID: <3ea6737f-a49b-a3d6-5555-33af656d75c1@intel.com> References: <1533494497-16253-1-git-send-email-quzeyao@gmail.com> <20180809100344.15af7e9c@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: maxime.coquelin@redhat.com, dev@dpdk.org To: Drocula , Stephen Hemminger Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 01A9AFEB for ; Fri, 10 Aug 2018 11:18:48 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10-Aug-18 9:35 AM, Drocula wrote: > First, thanks for your suggestions. > > When using the MAP_FIXED flag, mmap will return an MMAP_FAILED if > 0xf0000000000000 is not available. > > In this case, I want mmap to return an address near 0xf0000000000000. > > I will submit v2. How can we be sure there's nothing mapped at that address? I think the original code was correct - try mapping around that address, and see if we get *something* close to it with the right bits set. MAP_FIXED seems dangerous to use without knowing that there's nothing there. Recent kernels have added a safer version of MAP_FIXED, but obviously it won't work on the majority of kernel versions we support. > > On Fri, Aug 10, 2018, 01:03 Stephen Hemminger > wrote: > >> Thanks for the patch, there are some minor style/cleanups that >> could be done. >> >>> #if defined(RTE_ARCH_X86) >> >> Isn't this going to apply to 64 bit only? >> >>> +/* >>> + * Try to detect whether the system uses 5-level page table. >>> + */ >>> +static bool >>> +system_uses_PML5(void) >>> +{ >>> + void *page_4k, *mask = (void *)0xf0000000000000; >> >> Magic constants expressed like this seem wrong. Why not use >> shift to make it obvious. >> >> Also, you are assuming a particular layout of memory on >> Linux which might be problematic. Plus if there is already >> some memory in use there, it won't work. >> >>> + page_4k = mmap(mask, 4096, PROT_READ | PROT_WRITE, >>> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >> >> Since you are probing maybe MAP_FIXED is what you want. >> >>> + >>> + if (page_4k == (void *) -1) >>> + return false; >> Use MMAP_FAILED here. >> >>> + munmap(page_4k, 4096); >>> + >>> + if ((unsigned long)page_4k & (unsigned long)mask) >>> + return true; >>> + return false; >> >> Wouldn't this work the same for what you expect? >> return page_4k == mask; >> >> I.e you expect kernel to put page where you want. >> > -- Thanks, Anatoly