From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Coquelin Subject: Re: [PATCH v2] vfio: noiommu check error handling Date: Tue, 16 Jan 2018 18:01:32 +0100 Message-ID: References: <1509465586-7436-1-git-send-email-jpf@zurich.ibm.com> <2075027.JcYejM7RvO@xps> <30962267.NVMYIIJvp8@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: "Burakov, Anatoly" , dev@dpdk.org To: Jonas Pfefferle , Thomas Monjalon Return-path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 9B0821B2E8 for ; Tue, 16 Jan 2018 18:01:43 +0100 (CET) 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" Hi Jonas, On 01/16/2018 05:08 PM, Jonas Pfefferle wrote: > >  On Mon, 15 Jan 2018 17:11:58 +0100 >  Thomas Monjalon wrote: >> 15/01/2018 13:22, Jonas Pfefferle: >>> >>>   On Sat, 13 Jan 2018 23:49:30 +0100 >>>   Thomas Monjalon wrote: >>> > 13/01/2018 13:15, Burakov, Anatoly: >>> >> On 11-Jan-18 11:45 PM, Thomas Monjalon wrote: >>> >> > 07/11/2017 10:50, Jonas Pfefferle1: >>> >> >>> Is there something urgent for 17.11? >>> >> >>> Or can it be refined in 18.02? >>> >> >> >>> >> >> Nothing urgent. We can refine this for 18.02. >>> >> >> >>> >> >>> Anatoly, any thought? >>> >> > >> > Anatoly, Jonas, how do you want to proceed with this patch? >>> >> > >> >> I don't see anything to be refined here, it's a simple bug >>> fix - >>code >> assumes noiommu mode support is always available, >>> when it might not >>be >> the case on older kernels. >>> > > As a bug fix, the title must start with "fix" and a tag "Fixes:" >>> > must be added to help with backport. >>> > At the same time, the explanation of the bug must be added in >>> > the commit log please. >>> > > Thanks >>> >>> It's not really a bug fix since it does not change the semantic of >>> the function but just adds nicer error handling. >>> Regarding redefining the code: What I don't like is the special cases >>> we have to check for when using the sPAPR iommu because it does not >>> support VA mappings yet. I think we should decide which iova mode to >>> use based on the iommu types available, i.e. each iommu type should >>> report which iova type it supports. Thoughts? >> >> Have you looked at what Maxime did? >>     https://dpdk.org/dev/patchwork/patch/33650/ >> >> How does it affect this patch? >> >> > > IMO it has the same problem. We shouldn't add more exception cases in > drivers/bus/pci/linux/pci.c but instead keep all the information about > what an IOMMU can do in lib/librte_eal/linuxapp/eal/eal_vfio.c I agree adding an exception in drivers/bus/pci/linux/pci.c isn't great, but we need first to fix a regression introduced in v17.11 LTS, and IMHO, we cannot do a big rework as the fix is to be backported. Once fixed, I agree we should work on a refactoring. I don't know if eal_vfio is the right place though, as in my case for example I cannot get the information needed through vfio ioctl(). Out of curiosity, what prevents sPAPR to use VA mode for now? Maxime