From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vn0-f43.google.com (mail-vn0-f43.google.com [209.85.216.43]) by dpdk.org (Postfix) with ESMTP id 0FB39C34A for ; Wed, 1 Jul 2015 10:34:10 +0200 (CEST) Received: by vnbf190 with SMTP id f190so5412130vnb.0 for ; Wed, 01 Jul 2015 01:34:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=lWMb92qbRG0dj3Q1kw/mHhHWewD0ORViDwy71eQQkvg=; b=Pui5Dvsn4J5vobhBUqXSvkyDb6TNgaEvfTBzpjg1zwnbtd5u+hwnBLczQCfNKJJvYB etLcB4H3CTI9DVyefAc+l+8t8Wr3Ew4pAW6r2TRLiStXRZYaX4o8CzEEj4n2QIhAYpfg DT0+VxPJFvbkVE4Md3zt1w81ohQiQkdhIi8a+ngSw0Wu9PWPlkn2tjV/dOU/TdDMmj7Y Tm2cjDavTWd4z4D30v73rQl8qJ6ZG0uT0rx35WK5oeTJDgwxLEWxyH7XKxYv2eiQq+Sp ONgxyqukoUHhanzMjeiUWaiO2RgVjr59wvFATclYn25lsMyYuZMzN0Ob5VGm/nDh4DCZ DIXA== X-Gm-Message-State: ALoCoQkzOiVIYT98yY/0EZFS7e4LB92BP2+nVff04k4bRiClhtzT1NXGnJHbcJzFwiN2tUoWsMPo MIME-Version: 1.0 X-Received: by 10.52.190.131 with SMTP id gq3mr24642534vdc.70.1435739649504; Wed, 01 Jul 2015 01:34:09 -0700 (PDT) Received: by 10.52.27.242 with HTTP; Wed, 1 Jul 2015 01:34:09 -0700 (PDT) In-Reply-To: <4201419.gIoUKGuTjB@xps13> References: <61d1c9174f7f9159d4005dd2cea16c7719cec964.1434462470.git.rahul.lakkireddy@chelsio.com> <2eeeff646a329cca169033f541de3e96cec3a27a.1435067129.git.rahul.lakkireddy@chelsio.com> <4201419.gIoUKGuTjB@xps13> Date: Wed, 1 Jul 2015 09:34:09 +0100 Message-ID: From: Alejandro Lucero To: Thomas Monjalon Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: dev , Felix Marti , Nirranjan Kirubaharan , Kumar Sanghvi Subject: Re: [dpdk-dev] [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Jul 2015 08:34:10 -0000 I submitted a patch for fixing this issue on the 25th of June. I did not notice someone had reported this before. The last patch from Rahul does not solve the problem. For those cases where the MSI-X table is in one of the BARs to map, the memreg array is still in use. My fix was using unsigned long instead of uint32_t for the memreg array as this is used as a parameter for mmap system call which expects such a type for the offset (and size). This worked for me but I did not realize this has to be compiled for 32 bit systems as well. In that case unsigned long will work for the mmap but not for the VFIO kernel API which expects uint64_t for the offset and size inside the struct vfio_region_info. The point is, the offset param from the vfio_region_info has the index BAR to map. For this VFIO kernel code uses VFIO_PCI_INDEX_TO_OFFSET: #define VFIO_PCI_OFFSET_SHIFT 40 #define VFIO_PCI_INDEX_TO_OFFSET (index ) ((u64 )(index ) << VFIO_PCI_OFFSET_SHIFT ) This index will be used by the VFIO mmap implementation when the DPDK code tries to map the BARs. That code does the opposite for getting the index: index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); In this case PAGE_SHIFT needs to be used because the mmap system call modifies the offset previously. In a 32-bit system mmap system call and VFIO mmap implementation will get an unsigned long offset, as it does the struct vma_area_struct for vm_pgoff. VFIO will not be able to map the right BAR except for BAR 0. So, basically, VFIO kernel code does not work for 32 bit systems. I think we should define memreg as unsigned long and to report this problem to the VFIO kernel maintainer. On Tue, Jun 30, 2015 at 10:12 PM, Thomas Monjalon wrote: > Hi Anatoly, > Please could you review this fix to allow Chelsio using VFIO? > Thanks > > 2015-06-23 20:30, Rahul Lakkireddy: > > When using vfio, the probe fails over Chelsio T5 adapters after > > commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). > > > > While debugging further, found that the BAR region offset and size read > from > > vfio are u64, but are assigned to uint32_t variables. This results in > the u64 > > value getting truncated to 0 and passing wrong offset and size to mmap > for > > subsequent BAR regions (i.e. trying to overwrite previously allocated > BAR 0 > > region). > > > > The fix is to use these region offset and size directly rather than > assigning > > to uint32_t variables. > > > > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") > > Signed-off-by: Rahul Lakkireddy > > Signed-off-by: Kumar Sanghvi > >