From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f44.google.com (mail-pa0-f44.google.com [209.85.220.44]) by dpdk.org (Postfix) with ESMTP id E4005377E for ; Mon, 8 Feb 2016 10:40:09 +0100 (CET) Received: by mail-pa0-f44.google.com with SMTP id cy9so71276861pac.0 for ; Mon, 08 Feb 2016 01:40:09 -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=kb8y5GKkC1XgdnsSRlqE10whhXOqa78lkj5wt0oK80o=; b=h5rIATRD1zvAdrzu05zUHVpOXqE6u6OV5yv9Hm5LBwHtYYtvKe6MObvMHl+d0eTZJe fBouKZmSQTzs6ybrXMTKsw3SiSGOp7U93mSixFpL9g3Khedmhrpsen3mjLyt3Z+gK7IQ yAyN074dMrmdMBzJsDmtOtQMQ4axXxImjL+4/KCx53h8KtrkNsk/vks0vd5ysYMJYyvt keApPms3rqVLaSlUyBh2Xc7cfsQC5xYfP2zHpuzYeMCwLa9oNZW2VqAhBmE43C/CzfhJ OmhgePpbCpKh19hqB6Gfn4yTbxptpYg0M2cawY9pFajAoKWNHEBpRydQrRGb8l3ZA/4G 0xYg== 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=kb8y5GKkC1XgdnsSRlqE10whhXOqa78lkj5wt0oK80o=; b=b4Sh9IJ9GATZnjmtV5FA7LxEMXPHO21dlehitoBVZFNSEHxfdBwpTU6UYOaIo/INrU vdy9rXT3t0tqgwqnkPyywu2TAmQt3t50m9hb2nXFjGreLPoHV8tLQ7VTIonHjdPGyiKk 8Zks5SeYKPTtL7h9IrRIwa7ebSEwjlDXIg89i9MpVrVdyY4eKYMrrEBW8ISIszak8iz7 nOnQvV4LYSxqEAKBvd3dua+/hYurZxelbkZxaigw4sPD2IdycjVhHsBfRy1O3wxAJ+Xs ZmWU1NL50MYlmknlFhU0Tgs4nVyfBkzq5ESxg5KaZaM+yDkkzHSbRja3ZlG3QPMLZLsC CWyA== X-Gm-Message-State: AG10YORwzxZUbVvG/yKHkat9+pVQ1ydofLnrA9UaCTRdY8mmJTrxcPRGBwWI99uQa6V7g8MAFNdzBu3V8ggOdUl3 MIME-Version: 1.0 X-Received: by 10.66.161.227 with SMTP id xv3mr41011387pab.117.1454924409337; Mon, 08 Feb 2016 01:40:09 -0800 (PST) Received: by 10.66.12.132 with HTTP; Mon, 8 Feb 2016 01:40:09 -0800 (PST) In-Reply-To: References: <1454853068-14621-1-git-send-email-sshukla@mvista.com> <1454853068-14621-5-git-send-email-sshukla@mvista.com> Date: Mon, 8 Feb 2016 15:10:09 +0530 Message-ID: From: Santosh Shukla To: David Marchand Content-Type: text/plain; charset=UTF-8 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v7 4/4] eal/linux: vfio: add pci ioport support 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: Mon, 08 Feb 2016 09:40:10 -0000 On Mon, Feb 8, 2016 at 2:21 PM, David Marchand wrote: > On Sun, Feb 7, 2016 at 2:51 PM, Santosh Shukla wrote: >> @@ -999,37 +1000,56 @@ int >> pci_vfio_ioport_map(struct rte_pci_device *dev, int bar, >> struct rte_pci_ioport *p) > > p is passed as a value, not a reference ... > >> { >> - RTE_SET_USED(dev); >> - RTE_SET_USED(bar); >> - RTE_SET_USED(p); >> - return -1; >> + if (bar < VFIO_PCI_BAR0_REGION_INDEX || >> + bar > VFIO_PCI_BAR5_REGION_INDEX) { >> + RTE_LOG(ERR, EAL, "invalid bar (%d)!\n", bar); >> + return -1; >> + } >> + >> + p = rte_zmalloc("VFIO_IOPORT", sizeof(*p), 0); > > ... so I don't think this allocation does what you expected. > > Anyway, you don't need to allocate a rte_pci_ioport object with current api. > You already have a valid object passed by caller. > You only need to initialise it. > > >> + if (p == NULL) { >> + RTE_LOG(ERR, EAL, "cannot alloc vfio ioport mem\n"); >> + return -1; >> + } >> + >> + p->dev = dev; > > Does not hurt to do this, but p->dev is already set by caller on ret > == 0 (rte_eal_pci_ioport_map). > > so far I didn't found caller setting p->dev (looked at virtio pmd driver), In-fact I tried to use w/o setting p->dev=dev and application crashed, stating segfault :). So I am keeping this one in next revision. >> >> void >> pci_vfio_ioport_read(struct rte_pci_ioport *p, >> void *data, size_t len, off_t offset) >> { >> - RTE_SET_USED(p); >> - RTE_SET_USED(data); >> - RTE_SET_USED(len); >> - RTE_SET_USED(offset); >> + const struct rte_intr_handle *intr_handle = &p->dev->intr_handle; > > Missing blank line between declaration and code. checkpatch.sh didn't ntocied though, which all param you use in checkpatch, I just do ./checkpatch.sh --no-tree *.patch > >> + if (pread64(intr_handle->vfio_dev_fd, data, >> + len, p->offset + offset) <= 0) >> + RTE_LOG(ERR, EAL, >> + "Can't read from PCI bar (%" PRIu64 ") : offset (%x)\n", >> + VFIO_GET_REGION_IDX(p->offset), (int)offset); >> } >> >> void >> pci_vfio_ioport_write(struct rte_pci_ioport *p, >> const void *data, size_t len, off_t offset) >> { >> - RTE_SET_USED(p); >> - RTE_SET_USED(data); >> - RTE_SET_USED(len); >> - RTE_SET_USED(offset); >> + const struct rte_intr_handle *intr_handle = &p->dev->intr_handle; > > Idem. > >> + if (pwrite64(intr_handle->vfio_dev_fd, data, >> + len, p->offset + offset) <= 0) >> + RTE_LOG(ERR, EAL, >> + "Can't write to PCI bar (%" PRIu64 ") : offset (%x)\n", >> + VFIO_GET_REGION_IDX(p->offset), (int)offset); >> } >> >> int >> pci_vfio_ioport_unmap(struct rte_pci_ioport *p) >> { >> - RTE_SET_USED(p); >> - return -1; >> + if (p == NULL) >> + return -1; >> + else { >> + rte_free(p); >> + return 0; >> + } >> } > > Since you have nothing to allocate, nothing to free here ? > Removed, Thanks > > -- > David Marchand