Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754902AbcLNHIy (ORCPT ); Wed, 14 Dec 2016 02:08:54 -0500 Received: from esa8.dell-outbound.iphmx.com ([68.232.149.218]:20183 "EHLO esa8.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753490AbcLNHIw (ORCPT ); Wed, 14 Dec 2016 02:08:52 -0500 DomainKey-Signature: s=smtpout; d=dell.com; c=simple; q=dns; h=Received:Received:Received:X-DKIM:Received:Received:From: To:Cc:References:In-Reply-To:Subject:Date:Message-ID: MIME-Version:Content-Type:Content-Transfer-Encoding: X-Mailer:Thread-Index:Content-Language: X-RSA-Classifications:X-Sentrion-Hostname; b=N34yn0MMq4MK+kxAPhq9t3UJRAmoLJZQFCr2QyVRvcT4QyAG9OddiQyU mtdRBAWlFFxMwzPQHJg8TlCuETIak0KclgafwGeWnVEUCtZfp15WadUj6 D664NvP0ZUg1S3RUM53akrE4YxQnezKXEsJZM3cNRJKIyRKZQDs8bhFYr Y=; X-DKIM: OpenDKIM Filter v2.4.3 mailuogwprd52.lss.emc.com uBE78jeD012248 From: "Allen Hubbe" To: "'Serge Semin'" , , , Cc: , , References: <1481576902-21091-1-git-send-email-fancer.lancer@gmail.com> <1481672961-10753-1-git-send-email-fancer.lancer@gmail.com> <1481672961-10753-6-git-send-email-fancer.lancer@gmail.com> In-Reply-To: <1481672961-10753-6-git-send-email-fancer.lancer@gmail.com> Subject: RE: [PATCH v3 5/9] NTB: Alter Scratchpads API to support multi-ports devices Date: Wed, 14 Dec 2016 02:08:32 -0500 Message-ID: <000901d255d8$e26e44e0$a74acea0$@dell.com> MIME-Version: 1.0 Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQHSVZuPfNS3b7iTYE+0JeU1ICBY26EG/bBw Content-Language: en-us X-RSA-Classifications: Source Code, public X-Sentrion-Hostname: mailuogwprd52.lss.emc.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 15822 Lines: 444 From: Serge Semin > Even though there is no any real NTB hardware, which would have both more > than two ports and Scratchpad registers, it is logically correct to have > Scratchpad API accepting a peer port index as well. Intel/AMD drivers utilize > Primary and Secondary topology to split Scratchpad between connected root > devices. Since port-index API introduced, Intel/AMD NTB hardware drivers can > use device port to determine which Scratchpad registers actually belong to > local and peer devices. The same approach can be used if some potential > hardware in future will be multi-port and have some set of Scratchpads. > Here are the brief of changes in the API: > ntb_spad_count() - return number of Scratchpads per each port > ntb_peer_spad_addr(pidx, sidx) - address of Scratchpad register of the > peer device with pidx-index > ntb_peer_spad_read(pidx, sidx) - read specified Scratchpad register of the > peer with pidx-index > ntb_peer_spad_write(pidx, sidx) - write data to Scratchpad register of the > peer with pidx-index > > Since there is hardware which doesn't support Scratchpad registers, the > corresponding API methods are now made optional. > > Signed-off-by: Serge Semin Acked-by: Allen Hubbe > --- > drivers/ntb/hw/amd/ntb_hw_amd.c | 14 +++---- > drivers/ntb/hw/intel/ntb_hw_intel.c | 14 +++---- > drivers/ntb/ntb_transport.c | 17 ++++----- > drivers/ntb/test/ntb_perf.c | 6 +-- > drivers/ntb/test/ntb_pingpong.c | 8 +++- > drivers/ntb/test/ntb_tool.c | 21 ++++++++-- > include/linux/ntb.h | 76 +++++++++++++++++++++++-------------- > 7 files changed, 98 insertions(+), 58 deletions(-) > > diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c > index 6a41c38..bc537aa 100644 > --- a/drivers/ntb/hw/amd/ntb_hw_amd.c > +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c > @@ -433,30 +433,30 @@ static int amd_ntb_spad_write(struct ntb_dev *ntb, > return 0; > } > > -static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int idx) > +static u32 amd_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx) > { > struct amd_ntb_dev *ndev = ntb_ndev(ntb); > void __iomem *mmio = ndev->self_mmio; > u32 offset; > > - if (idx < 0 || idx >= ndev->spad_count) > + if (sidx < 0 || sidx >= ndev->spad_count) > return -EINVAL; > > - offset = ndev->peer_spad + (idx << 2); > + offset = ndev->peer_spad + (sidx << 2); > return readl(mmio + AMD_SPAD_OFFSET + offset); > } > > -static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, > - int idx, u32 val) > +static int amd_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx, > + int sidx, u32 val) > { > struct amd_ntb_dev *ndev = ntb_ndev(ntb); > void __iomem *mmio = ndev->self_mmio; > u32 offset; > > - if (idx < 0 || idx >= ndev->spad_count) > + if (sidx < 0 || sidx >= ndev->spad_count) > return -EINVAL; > > - offset = ndev->peer_spad + (idx << 2); > + offset = ndev->peer_spad + (sidx << 2); > writel(val, mmio + AMD_SPAD_OFFSET + offset); > > return 0; > diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c b/drivers/ntb/hw/intel/ntb_hw_intel.c > index 4b84012..7bb14cb 100644 > --- a/drivers/ntb/hw/intel/ntb_hw_intel.c > +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c > @@ -1409,30 +1409,30 @@ static int intel_ntb_spad_write(struct ntb_dev *ntb, > ndev->self_reg->spad); > } > > -static int intel_ntb_peer_spad_addr(struct ntb_dev *ntb, int idx, > +static int intel_ntb_peer_spad_addr(struct ntb_dev *ntb, int pidx, int sidx, > phys_addr_t *spad_addr) > { > struct intel_ntb_dev *ndev = ntb_ndev(ntb); > > - return ndev_spad_addr(ndev, idx, spad_addr, ndev->peer_addr, > + return ndev_spad_addr(ndev, sidx, spad_addr, ndev->peer_addr, > ndev->peer_reg->spad); > } > > -static u32 intel_ntb_peer_spad_read(struct ntb_dev *ntb, int idx) > +static u32 intel_ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx) > { > struct intel_ntb_dev *ndev = ntb_ndev(ntb); > > - return ndev_spad_read(ndev, idx, > + return ndev_spad_read(ndev, sidx, > ndev->peer_mmio + > ndev->peer_reg->spad); > } > > -static int intel_ntb_peer_spad_write(struct ntb_dev *ntb, > - int idx, u32 val) > +static int intel_ntb_peer_spad_write(struct ntb_dev *ntb, int pidx, > + int sidx, u32 val) > { > struct intel_ntb_dev *ndev = ntb_ndev(ntb); > > - return ndev_spad_write(ndev, idx, val, > + return ndev_spad_write(ndev, sidx, val, > ndev->peer_mmio + > ndev->peer_reg->spad); > } > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c > index 4d5b160..28aaba3 100644 > --- a/drivers/ntb/ntb_transport.c > +++ b/drivers/ntb/ntb_transport.c > @@ -875,17 +875,17 @@ static void ntb_transport_link_work(struct work_struct *work) > size = max_mw_size; > > spad = MW0_SZ_HIGH + (i * 2); > - ntb_peer_spad_write(ndev, spad, upper_32_bits(size)); > + ntb_peer_spad_write(ndev, PIDX, spad, upper_32_bits(size)); > > spad = MW0_SZ_LOW + (i * 2); > - ntb_peer_spad_write(ndev, spad, lower_32_bits(size)); > + ntb_peer_spad_write(ndev, PIDX, spad, lower_32_bits(size)); > } > > - ntb_peer_spad_write(ndev, NUM_MWS, nt->mw_count); > + ntb_peer_spad_write(ndev, PIDX, NUM_MWS, nt->mw_count); > > - ntb_peer_spad_write(ndev, NUM_QPS, nt->qp_count); > + ntb_peer_spad_write(ndev, PIDX, NUM_QPS, nt->qp_count); > > - ntb_peer_spad_write(ndev, VERSION, NTB_TRANSPORT_VERSION); > + ntb_peer_spad_write(ndev, PIDX, VERSION, NTB_TRANSPORT_VERSION); > > /* Query the remote side for its info */ > val = ntb_spad_read(ndev, VERSION); > @@ -961,10 +961,10 @@ static void ntb_qp_link_work(struct work_struct *work) > > val = ntb_spad_read(nt->ndev, QP_LINKS); > > - ntb_peer_spad_write(nt->ndev, QP_LINKS, val | BIT(qp->qp_num)); > + ntb_peer_spad_write(nt->ndev, PIDX, QP_LINKS, val | BIT(qp->qp_num)); > > /* query remote spad for qp ready bits */ > - ntb_peer_spad_read(nt->ndev, QP_LINKS); > + ntb_peer_spad_read(nt->ndev, PIDX, QP_LINKS); > dev_dbg_ratelimited(&pdev->dev, "Remote QP link status = %x\n", val); > > /* See if the remote side is up */ > @@ -2141,8 +2141,7 @@ void ntb_transport_link_down(struct ntb_transport_qp *qp) > > val = ntb_spad_read(qp->ndev, QP_LINKS); > > - ntb_peer_spad_write(qp->ndev, QP_LINKS, > - val & ~BIT(qp->qp_num)); > + ntb_peer_spad_write(qp->ndev, PIDX, QP_LINKS, val & ~BIT(qp->qp_num)); > > if (qp->link_is_up) > ntb_send_link_down(qp); > diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c > index cbff0b4..0a493ba 100644 > --- a/drivers/ntb/test/ntb_perf.c > +++ b/drivers/ntb/test/ntb_perf.c > @@ -516,9 +516,9 @@ static void perf_link_work(struct work_struct *work) > if (max_mw_size && size > max_mw_size) > size = max_mw_size; > > - ntb_peer_spad_write(ndev, MW_SZ_HIGH, upper_32_bits(size)); > - ntb_peer_spad_write(ndev, MW_SZ_LOW, lower_32_bits(size)); > - ntb_peer_spad_write(ndev, VERSION, PERF_VERSION); > + ntb_peer_spad_write(ndev, PIDX, MW_SZ_HIGH, upper_32_bits(size)); > + ntb_peer_spad_write(ndev, PIDX, MW_SZ_LOW, lower_32_bits(size)); > + ntb_peer_spad_write(ndev, PIDX, VERSION, PERF_VERSION); > > /* now read what peer wrote */ > val = ntb_spad_read(ndev, VERSION); > diff --git a/drivers/ntb/test/ntb_pingpong.c b/drivers/ntb/test/ntb_pingpong.c > index 12f8b40..938a18b 100644 > --- a/drivers/ntb/test/ntb_pingpong.c > +++ b/drivers/ntb/test/ntb_pingpong.c > @@ -138,7 +138,7 @@ static void pp_ping(unsigned long ctx) > "Ping bits %#llx read %#x write %#x\n", > db_bits, spad_rd, spad_wr); > > - ntb_peer_spad_write(pp->ntb, 0, spad_wr); > + ntb_peer_spad_write(pp->ntb, PIDX, 0, spad_wr); > ntb_peer_db_set(pp->ntb, db_bits); > ntb_db_clear_mask(pp->ntb, db_mask); > > @@ -225,6 +225,12 @@ static int pp_probe(struct ntb_client *client, > } > } > > + if (ntb_spad_count(ntb) < 1) { > + dev_dbg(&ntb->dev, "no enough scratchpads\n"); > + rc = -EINVAL; > + goto err_pp; > + } > + > if (ntb_spad_is_unsafe(ntb)) { > dev_dbg(&ntb->dev, "scratchpad is unsafe\n"); > if (!unsafe) { > diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c > index cb69247..f002bf4 100644 > --- a/drivers/ntb/test/ntb_tool.c > +++ b/drivers/ntb/test/ntb_tool.c > @@ -462,13 +462,22 @@ static TOOL_FOPS_RDWR(tool_spad_fops, > tool_spad_read, > tool_spad_write); > > +static u32 ntb_tool_peer_spad_read(struct ntb_dev *ntb, int sidx) > +{ > + return ntb_peer_spad_read(ntb, PIDX, sidx); > +} > + > static ssize_t tool_peer_spad_read(struct file *filep, char __user *ubuf, > size_t size, loff_t *offp) > { > struct tool_ctx *tc = filep->private_data; > > - return tool_spadfn_read(tc, ubuf, size, offp, > - tc->ntb->ops->peer_spad_read); > + return tool_spadfn_read(tc, ubuf, size, offp, ntb_tool_peer_spad_read); > +} > + > +static int ntb_tool_peer_spad_write(struct ntb_dev *ntb, int sidx, u32 val) > +{ > + return ntb_peer_spad_write(ntb, PIDX, sidx, val); > } > > static ssize_t tool_peer_spad_write(struct file *filep, const char __user *ubuf, > @@ -477,7 +486,7 @@ static ssize_t tool_peer_spad_write(struct file *filep, const char > __user *ubuf, > struct tool_ctx *tc = filep->private_data; > > return tool_spadfn_write(tc, ubuf, size, offp, > - tc->ntb->ops->peer_spad_write); > + ntb_tool_peer_spad_write); > } > > static TOOL_FOPS_RDWR(tool_peer_spad_fops, > @@ -926,6 +935,12 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb) > goto err_tc; > } > > + if (ntb_spad_count(ntb) < 1) { > + dev_dbg(&ntb->dev, "no enough scratchpads\n"); > + rc = -EINVAL; > + goto err_tc; > + } > + > if (ntb_db_is_unsafe(ntb)) > dev_dbg(&ntb->dev, "doorbell is unsafe\n"); > > diff --git a/include/linux/ntb.h b/include/linux/ntb.h > index f6ec88f..a54e2be 100644 > --- a/include/linux/ntb.h > +++ b/include/linux/ntb.h > @@ -288,13 +288,14 @@ struct ntb_dev_ops { > int (*spad_is_unsafe)(struct ntb_dev *ntb); > int (*spad_count)(struct ntb_dev *ntb); > > - u32 (*spad_read)(struct ntb_dev *ntb, int idx); > - int (*spad_write)(struct ntb_dev *ntb, int idx, u32 val); > + u32 (*spad_read)(struct ntb_dev *ntb, int sidx); > + int (*spad_write)(struct ntb_dev *ntb, int sidx, u32 val); > > - int (*peer_spad_addr)(struct ntb_dev *ntb, int idx, > + int (*peer_spad_addr)(struct ntb_dev *ntb, int pidx, int sidx, > phys_addr_t *spad_addr); > - u32 (*peer_spad_read)(struct ntb_dev *ntb, int idx); > - int (*peer_spad_write)(struct ntb_dev *ntb, int idx, u32 val); > + u32 (*peer_spad_read)(struct ntb_dev *ntb, int pidx, int sidx); > + int (*peer_spad_write)(struct ntb_dev *ntb, int pidx, int sidx, > + u32 val); > }; > > static inline int ntb_dev_ops_is_valid(const struct ntb_dev_ops *ops) > @@ -335,13 +336,12 @@ static inline int ntb_dev_ops_is_valid(const struct ntb_dev_ops > *ops) > /* ops->peer_db_read_mask && */ > /* ops->peer_db_set_mask && */ > /* ops->peer_db_clear_mask && */ > - /* ops->spad_is_unsafe && */ > - ops->spad_count && > - ops->spad_read && > - ops->spad_write && > - /* ops->peer_spad_addr && */ > - /* ops->peer_spad_read && */ > - ops->peer_spad_write && > + /* !ops->spad_is_unsafe == !ops->spad_count && */ > + !ops->spad_read == !ops->spad_count && > + !ops->spad_write == !ops->spad_count && > + /* !ops->peer_spad_addr == !ops->spad_count && */ > + /* !ops->peer_spad_read == !ops->spad_count && */ > + !ops->peer_spad_write == !ops->spad_count && > 1; > } > > @@ -1172,51 +1172,62 @@ static inline int ntb_spad_is_unsafe(struct ntb_dev *ntb) > } > > /** > - * ntb_mw_count() - get the number of scratchpads > + * ntb_spad_count() - get the number of scratchpads > * @ntb: NTB device context. > * > * Hardware and topology may support a different number of scratchpads. > + * Although it must be the same for all ports per NTB device. > * > * Return: the number of scratchpads. > */ > static inline int ntb_spad_count(struct ntb_dev *ntb) > { > + if (!ntb->ops->spad_count) > + return 0; > + > return ntb->ops->spad_count(ntb); > } > > /** > * ntb_spad_read() - read the local scratchpad register > * @ntb: NTB device context. > - * @idx: Scratchpad index. > + * @sidx: Scratchpad index. > * > * Read the local scratchpad register, and return the value. > * > * Return: The value of the local scratchpad register. > */ > -static inline u32 ntb_spad_read(struct ntb_dev *ntb, int idx) > +static inline u32 ntb_spad_read(struct ntb_dev *ntb, int sidx) > { > - return ntb->ops->spad_read(ntb, idx); > + if (!ntb->ops->spad_read) > + return ~(u32)0; > + > + return ntb->ops->spad_read(ntb, sidx); > } > > /** > * ntb_spad_write() - write the local scratchpad register > * @ntb: NTB device context. > - * @idx: Scratchpad index. > + * @sidx: Scratchpad index. > * @val: Scratchpad value. > * > * Write the value to the local scratchpad register. > * > * Return: Zero on success, otherwise an error number. > */ > -static inline int ntb_spad_write(struct ntb_dev *ntb, int idx, u32 val) > +static inline int ntb_spad_write(struct ntb_dev *ntb, int sidx, u32 val) > { > - return ntb->ops->spad_write(ntb, idx, val); > + if (!ntb->ops->spad_write) > + return -EINVAL; > + > + return ntb->ops->spad_write(ntb, sidx, val); > } > > /** > * ntb_peer_spad_addr() - address of the peer scratchpad register > * @ntb: NTB device context. > - * @idx: Scratchpad index. > + * @pidx: Port index of peer device. > + * @sidx: Scratchpad index. > * @spad_addr: OUT - The address of the peer scratchpad register. > * > * Return the address of the peer doorbell register. This may be used, for > @@ -1224,42 +1235,51 @@ static inline int ntb_spad_write(struct ntb_dev *ntb, int idx, u32 > val) > * > * Return: Zero on success, otherwise an error number. > */ > -static inline int ntb_peer_spad_addr(struct ntb_dev *ntb, int idx, > +static inline int ntb_peer_spad_addr(struct ntb_dev *ntb, int pidx, int sidx, > phys_addr_t *spad_addr) > { > if (!ntb->ops->peer_spad_addr) > return -EINVAL; > > - return ntb->ops->peer_spad_addr(ntb, idx, spad_addr); > + return ntb->ops->peer_spad_addr(ntb, pidx, sidx, spad_addr); > } > > /** > * ntb_peer_spad_read() - read the peer scratchpad register > * @ntb: NTB device context. > - * @idx: Scratchpad index. > + * @pidx: Port index of peer device. > + * @sidx: Scratchpad index. > * > * Read the peer scratchpad register, and return the value. > * > * Return: The value of the local scratchpad register. > */ > -static inline u32 ntb_peer_spad_read(struct ntb_dev *ntb, int idx) > +static inline u32 ntb_peer_spad_read(struct ntb_dev *ntb, int pidx, int sidx) > { > - return ntb->ops->peer_spad_read(ntb, idx); > + if (!ntb->ops->peer_spad_read) > + return ~(u32)0; > + > + return ntb->ops->peer_spad_read(ntb, pidx, sidx); > } > > /** > * ntb_peer_spad_write() - write the peer scratchpad register > * @ntb: NTB device context. > - * @idx: Scratchpad index. > + * @pidx: Port index of peer device. > + * @sidx: Scratchpad index. > * @val: Scratchpad value. > * > * Write the value to the peer scratchpad register. > * > * Return: Zero on success, otherwise an error number. > */ > -static inline int ntb_peer_spad_write(struct ntb_dev *ntb, int idx, u32 val) > +static inline int ntb_peer_spad_write(struct ntb_dev *ntb, int pidx, int sidx, > + u32 val) > { > - return ntb->ops->peer_spad_write(ntb, idx, val); > + if (!ntb->ops->peer_spad_write) > + return -EINVAL; > + > + return ntb->ops->peer_spad_write(ntb, pidx, sidx, val); > } > > #endif > -- > 2.6.6