From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752658AbbJEKGO (ORCPT ); Mon, 5 Oct 2015 06:06:14 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:33152 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752173AbbJEKGM (ORCPT ); Mon, 5 Oct 2015 06:06:12 -0400 Subject: Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support To: Stephen Hemminger References: <1443991398-23761-1-git-send-email-vladz@cloudius-systems.com> <1443991398-23761-3-git-send-email-vladz@cloudius-systems.com> <20151005094122.68394833@uryu.home.lan> <56123E29.4020202@cloudius-systems.com> Cc: linux-kernel@vger.kernel.org, mst@redhat.com, hjk@hansjkoch.de, corbet@lwn.net, gregkh@linuxfoundation.org, bruce.richardson@intel.com, avi@cloudius-systems.com, gleb@cloudius-systems.com, alexander.duyck@gmail.com From: Vlad Zolotarov Message-ID: <56124B91.80701@cloudius-systems.com> Date: Mon, 5 Oct 2015 13:06:09 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <56123E29.4020202@cloudius-systems.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/05/15 12:08, Vlad Zolotarov wrote: > > > On 10/05/15 11:41, Stephen Hemminger wrote: >> On Sun, 4 Oct 2015 23:43:17 +0300 >> Vlad Zolotarov wrote: >> >>> +static int setup_maps(struct pci_dev *pdev, struct uio_info *info) >>> +{ >>> + int i, m = 0, p = 0, err; >>> + static const char * const bar_names[] = { >>> + "BAR0", "BAR1", "BAR2", "BAR3", "BAR4", "BAR5", >>> + }; >>> + >>> + for (i = 0; i < ARRAY_SIZE(bar_names); i++) { >>> + unsigned long start = pci_resource_start(pdev, i); >>> + unsigned long flags = pci_resource_flags(pdev, i); >>> + unsigned long len = pci_resource_len(pdev, i); >>> + >>> + if (start == 0 || len == 0) >>> + continue; >>> + >>> + if (flags & IORESOURCE_MEM) { >>> + void __iomem *addr; >>> + >>> + if (m >= MAX_UIO_MAPS) >>> + continue; >>> + >>> + addr = ioremap(start, len); >>> + if (addr == NULL) { >>> + err = -EINVAL; >>> + goto fail; >>> + } >>> + >>> + info->mem[m].name = bar_names[i]; >>> + info->mem[m].addr = start; >>> + info->mem[m].internal_addr = addr; >>> + info->mem[m].size = len; >>> + info->mem[m].memtype = UIO_MEM_PHYS; >>> + ++m; >>> + } else if (flags & IORESOURCE_IO) { >>> + if (p >= MAX_UIO_PORT_REGIONS) >>> + continue; >>> + >>> + info->port[p].name = bar_names[i]; >>> + info->port[p].start = start; >>> + info->port[p].size = len; >>> + info->port[p].porttype = UIO_PORT_X86; >>> + ++p; >>> + } >>> + } >>> + >>> + return 0; >>> +fail: >>> + for (i = 0; i < m; i++) { >>> + iounmap(info->mem[i].internal_addr); >>> + info->mem[i].internal_addr = NULL; >>> + } >>> + >>> + return err; >>> + >> I wonder do we really have to setup all the BAR's in uio_pci_generic? >> The DPDK code works with uio_pci_generic already, and it didn't setup >> the BAR's. > > DPDK never used uio_pci_generic with MSI-X support so far and all > MSI-X capable UIO DPDK drivers like igb_uio and your newly proposed > uio_msi do map them all. > U also mentioned in the other thread that virtio requires portio bars > too. > The thing is that generally bars are needed for programming the device > therefore it's logical to have them exposed. In general different > devices have different registers layout therefore a general driver may > not know which bar exactly is going to be required for a specific > device and for a specific usage. That's why I think that "map them > all" approach is rather generic and appropriate. However if there are > other motives not to do so that I'm missing, pls., let me know. Having said all that however I'd agree if someone would say that mappings setting would rather come as a separate patch in this series... ;) it will in v4... > > thanks, > vlad > >> One possible issue is that without that maybe kernel would not know >> about the >> region used for MSI-X vectors table. >