From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by dpdk.org (Postfix) with ESMTP id 22B475A06 for ; Wed, 3 Feb 2016 10:50:10 +0100 (CET) Received: by mail-pa0-f46.google.com with SMTP id uo6so11166073pac.1 for ; Wed, 03 Feb 2016 01:50:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mvista-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=XDsxgTkX7sfgu3n2k0+4kCIjdHLyQ7BQ/78x+yYPWqU=; b=0uWXijfJzYqzMmT6KOUOcnHyP0HRGR0NbnlS3Zw4tmeHNRbhtun9Xl7SA3kAKlj4bl V2LtmIaGkDWWUlOXlsA/y2nXItuWPtXDC2GsvLTht9Jh0253HkuECNM7mjBbWDZ3TVgs G/1Rk4ootHmqxyYctQqNfj7sqA1kwuzHhDgPHM9Qa1P1F2GfWFhV7HQanTnvSplgr4ix zh1T2KBCvRhwIs07p/M9IW7NnPi+T1LXCFT9PQR2KYg5dqIZrmB5EBwJLunDYQzzSr9M LjsdamyOq7Pnd2lvOJlr/6q8HbCXlJUX6iYxXBbfE8YGo8qYL+zohH616l4SfW7zLdFi BTiw== 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=XDsxgTkX7sfgu3n2k0+4kCIjdHLyQ7BQ/78x+yYPWqU=; b=fW9WWJREeCPtzuWvSpBQjtq3yj4MeTYblVgZJ8hrqHvQVRtcRculNVHYuDfriKlelQ yNAyDQTiTxyzTsS4m43zRPYJgx76+MiqnU6wUdOVmuxcRApY7dRBQqyjwRwfqqLpqSU8 apYYw6El2tqtQb+YHLSzCC6gFhR0ijx+SavHrWIn1Ra26yIQrj0wh/7+MHm0BWS2bUe8 aI4Ikc1uFmwsSTvlVwXD04+GPaizF+QIpP4S0DGI6A9CD6meopOwVnyemeUUQ/VdUH5a uPuWlYswkI2BqlVf0rqvIoQRIxw0WXRPKJVyAqmlEfboEyFSmnEq6UC+GRa2vjAKCDZg vH9w== X-Gm-Message-State: AG10YOQ3A2SVX0SKzJWfI05i/9wRgMIi7mjoCRxeLP9WsfRGeX3SUvBNnhl0IasMQNFrPl6aXIGOc9AnEuSsZ45J MIME-Version: 1.0 X-Received: by 10.66.252.163 with SMTP id zt3mr705931pac.127.1454493009371; Wed, 03 Feb 2016 01:50:09 -0800 (PST) Received: by 10.66.12.132 with HTTP; Wed, 3 Feb 2016 01:50:09 -0800 (PST) In-Reply-To: References: <1454091717-32251-1-git-send-email-sshukla@mvista.com> <20160201134854.GE4257@yliu-dev.sh.intel.com> <20160202054345.GI4257@yliu-dev.sh.intel.com> <20160202084933.GJ4257@yliu-dev.sh.intel.com> Date: Wed, 3 Feb 2016 15:20:09 +0530 Message-ID: From: Santosh Shukla To: Yuanhan Liu Content-Type: text/plain; charset=UTF-8 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region 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, 03 Feb 2016 09:50:10 -0000 On Tue, Feb 2, 2016 at 9:48 PM, Santosh Shukla wrote: > On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla wrote: >> On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu wrote: >>> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote: >>>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu wrote: >>>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote: >>>> >> Current use-case is virtio: It is used as io_bar which is first >>>> >> bar[1]. But implementation is generic, can be used to do rd/wr for >>>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars >>>> >> w/o mapping that bar, So apis will be useful for such cases in future. >>>> >> >>>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped >>>> >> then no need to do rd/wr, user can directly access the pci_memory. But >>>> >> use-case of this api entirely different: unmapped memory by >>>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}. >>>> >> >>>> >> Is above explanation convincing? Pl. let me know. >>>> > >>>> > TBH, not really. So, as you stated, it should be generic APIs to >>>> > read/write bar space, but limiting it to VFIO only and claiming >>>> > that read/write bar space is not support by other drivers (such >>>> > as UIO) while in fact it can (in some ways) doesn't seem right >>>> > to me. >>>> > >>>> > Anyway, it's just some thoughts from me. David, comments? >>>> >>>> >From the very start, same opinion. >>>> We should have a unique api to access those, and eal should hide >>>> details like kernel drivers (uio, vfio, whatever) to the pmd. >>>> >>>> Now the thing is, how to do this in an elegant and efficient way. >>> >>> I was thinking that we may just make it be IO port specific read/ >>> write functions: >>> >> >> Ok, >> >>> rte_eal_pci_ioport_read(dev, bar, buf, size) >>> { >>> >>> return if not an IO bar; >>> >>> if (has io) >>> return inb/w/l(); >>> >> >> In that case, It may be r / if (has io) / if (drv->kdrv == UIO) >> >>> if (vfio) >>> return vfio_ioport_read(); >>> >>> else, claim aloud that io port read is not allowed >>> } >>> >>> Let us not handle memory bar resource here: in such case, you should >>> go with rte_eal_pci_map_device() and do it with memory mapped io. >>> >>> Does that make any sense? >>> >> I am not entirely sure. >> Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ? >> > > Just came-up something below what Yuanhan has proposed, Does this look okay? > > int rte_eal_pci_ioport_read(const struct rte_pci_device *device, > void *buf, size_t len, > off_t offset, > int bar_idx) > { > if (bar_idx != 0) { > RTE_LOG(ERR, EAL, "not a ioport bar\n"); > return -1; > } > > switch (device->kdrv) { > case RTE_KDRV_VFIO: > return pci_vfio_ioport_read(device, buf, len, offset, bar_idx); > case RTE_KDRV_IGB_UIO: > case RTE_KDRV_UIO_GENERIC: > case RTE_KDRV_NIC_UIO: > { > switch (size) > case 1: return inb(buf /*ioport address*/); > case 2: return inw(buf /* ioport address*/); > case 4: return inl(buf /* ioport address*/); > default: > RTE_LOG(ERR, EAL, "invalid size\n"); > } > > default: > RTE_LOG(ERR, EAL, "read bar not supported by driver\n"); > return -1; > } > } > Ping? Also can someone please review rest of series. This patchset going through multiple revision, Each revision get one / two comment, It would help if I get review comment for each patch. >> >>> --yliu