RE: [PATCH 1/3] NTB: Add AMD PCI-Express NTB driver
> From: Allen Hubbe [mailto:alle...@gmail.com]
> Sent: Friday, December 18, 2015 12:46 AM
> To: Yu, Xiangliang
> Cc: jdma...@kudzu.us; dave.ji...@intel.com; linux-...@googlegroups.com;
> linux-kernel@vger.kernel.org; SPG_Linux_Kernel
> Subject: Re: [PATCH 1/3] NTB: Add AMD PCI-Express NTB driver
>
> On Thu, Dec 17, 2015 at 3:17 AM, Xiangliang Yu
> wrote:
> > AMD NTB support following main features:
> > (1) Three memory windows;
> > (2) Sixteen 32-bit scratch pad;
> > (3) Two 16-bit doorbell interrupt;
> > (4) Five event interrupts;
> > (5) One system can wake up opposite system of NTB;
> > (6) Flush previous request to the opposite system;
> > (7) There are reset and PME_TO mechanisms between two systems;
> >
> > Signed-off-by: Xiangliang Yu
>
> Is hardware available on which this can be tested?
No yet. Right now, verified the driver on emulator.
>
> > +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
>
> > +static u64 amd_ntb_db_read(struct ntb_dev *ntb) {
> > + struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> > +
> > + return (u64)NTB_READ_REG(DBSTAT); }
>
> DBSTAT hides the use of ndev, or ndev is unused. The code should be more
> clear, here, and in other places where NTB_READ_REG and NTB_WRITE_REG
> are used with a macro argument.
Got it, I will add ndev into macro arguments.
>
> > +static void amd_ack_SMU(struct amd_ntb_dev *ndev, u32 bit) {
> > + int reg;
> > +
> > + reg = NTB_READ_REG(SMUACK);
> > + reg |= bit;
> > + NTB_WRITE_REG(reg, SMUACK);
> > +
> > + ndev->peer_sta |= bit;
> > +}
> > +
> > +/*
> > + * flush the requests to peer side
> > + */
> > +static int amd_flush_peer_requests(struct amd_ntb_dev *ndev) {
> > + u32 reg;
> > +
> > + if (!amd_link_is_up(ndev)) {
> > + dev_err(ndev_dev(ndev), "link is down.\n");
> > + return -EINVAL;
> > + }
> > +
>
> Add reinit_completion, or this may already be "complete" from a previous
> flush.
Ok.
>
> > + reg = NTB_READ_REG(FLUSHTRIG);
> > + reg |= 0x1;
> > + NTB_WRITE_REG(reg, FLUSHTRIG);
> > +
> > + wait_for_completion(&ndev->flush_cmpl);
>
> Because of wait_for_completion, that this can only be called in a thread
> context. This is unlike other functions of ntb.h, so there should at least
> be a
> note in the api documentation.
Ok.
>
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * wake up the peer side
> > + */
> > +static int amd_wakeup_peer_side(struct amd_ntb_dev *ndev) {
> > + u32 reg;
> > +
> > + if (!amd_link_is_up(ndev)) {
> > + dev_warn(ndev_dev(ndev), "link is down.\n");
> > + return -EINVAL;
> > + }
> > +
>
> See previous comment.
>
> > + NTB_READ_REG(PMSGTRIG);
> > + reg |= 0x1;
> > + NTB_WRITE_REG(reg, PMSGTRIG);
> > +
> > + wait_for_completion(&ndev->wakeup_cmpl);
> > +
> > + return 0;
> > +}
>
>
> > +static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) {
> > + u32 status;
> > +
> > + status = NTB_READ_REG(INTSTAT);
> > + if (!(status & AMD_EVENT_INTMASK))
> > + return;
> > +
> > + dev_dbg(ndev_dev(ndev), "status = 0x%x and vec = %d\n",
> > + status, vec);
> > +
> > + status &= AMD_EVENT_INTMASK;
> > + switch (status) {
> > + case AMD_PEER_FLUSH_EVENT:
> > + complete(&ndev->flush_cmpl);
> > + break;
> > + case AMD_PEER_RESET_EVENT:
> > + amd_ack_SMU(ndev, AMD_PEER_RESET_EVENT);
> > +
> > + /* link down first */
> > + ntb_link_event(&ndev->ntb);
> > + /* polling peer status */
> > + schedule_delayed_work(&ndev->hb_timer,
> > + AMD_LINK_HB_TIMEOUT);
> > +
> > + break;
> > + case AMD_PEER_D3_EVENT:
> > + case AMD_PEER_PMETO_EVENT:
> > + amd_ack_SMU(ndev, status);
> > +
> > + /* link down */
> > + ntb_link_event(&ndev->ntb);
> > +
> > + break;
> > + case AMD_PEER_D0_EVENT:
> > + status = NTB_READ_PEER_REG(PMESTAT);
> > + /* check if this is WAKEUP event */
> > +
Re: [PATCH 1/3] NTB: Add AMD PCI-Express NTB driver
On Thu, Dec 17, 2015 at 3:17 AM, Xiangliang Yu wrote:
> AMD NTB support following main features:
> (1) Three memory windows;
> (2) Sixteen 32-bit scratch pad;
> (3) Two 16-bit doorbell interrupt;
> (4) Five event interrupts;
> (5) One system can wake up opposite system of NTB;
> (6) Flush previous request to the opposite system;
> (7) There are reset and PME_TO mechanisms between two systems;
>
> Signed-off-by: Xiangliang Yu
Is hardware available on which this can be tested?
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> +static u64 amd_ntb_db_read(struct ntb_dev *ntb)
> +{
> + struct amd_ntb_dev *ndev = ntb_ndev(ntb);
> +
> + return (u64)NTB_READ_REG(DBSTAT);
> +}
DBSTAT hides the use of ndev, or ndev is unused. The code should be
more clear, here, and in other places where NTB_READ_REG and
NTB_WRITE_REG are used with a macro argument.
> +static void amd_ack_SMU(struct amd_ntb_dev *ndev, u32 bit)
> +{
> + int reg;
> +
> + reg = NTB_READ_REG(SMUACK);
> + reg |= bit;
> + NTB_WRITE_REG(reg, SMUACK);
> +
> + ndev->peer_sta |= bit;
> +}
> +
> +/*
> + * flush the requests to peer side
> + */
> +static int amd_flush_peer_requests(struct amd_ntb_dev *ndev)
> +{
> + u32 reg;
> +
> + if (!amd_link_is_up(ndev)) {
> + dev_err(ndev_dev(ndev), "link is down.\n");
> + return -EINVAL;
> + }
> +
Add reinit_completion, or this may already be "complete" from a previous flush.
> + reg = NTB_READ_REG(FLUSHTRIG);
> + reg |= 0x1;
> + NTB_WRITE_REG(reg, FLUSHTRIG);
> +
> + wait_for_completion(&ndev->flush_cmpl);
Because of wait_for_completion, that this can only be called in a
thread context. This is unlike other functions of ntb.h, so there
should at least be a note in the api documentation.
> +
> + return 0;
> +}
> +
> +/*
> + * wake up the peer side
> + */
> +static int amd_wakeup_peer_side(struct amd_ntb_dev *ndev)
> +{
> + u32 reg;
> +
> + if (!amd_link_is_up(ndev)) {
> + dev_warn(ndev_dev(ndev), "link is down.\n");
> + return -EINVAL;
> + }
> +
See previous comment.
> + NTB_READ_REG(PMSGTRIG);
> + reg |= 0x1;
> + NTB_WRITE_REG(reg, PMSGTRIG);
> +
> + wait_for_completion(&ndev->wakeup_cmpl);
> +
> + return 0;
> +}
> +static void amd_handle_event(struct amd_ntb_dev *ndev, int vec)
> +{
> + u32 status;
> +
> + status = NTB_READ_REG(INTSTAT);
> + if (!(status & AMD_EVENT_INTMASK))
> + return;
> +
> + dev_dbg(ndev_dev(ndev), "status = 0x%x and vec = %d\n", status, vec);
> +
> + status &= AMD_EVENT_INTMASK;
> + switch (status) {
> + case AMD_PEER_FLUSH_EVENT:
> + complete(&ndev->flush_cmpl);
> + break;
> + case AMD_PEER_RESET_EVENT:
> + amd_ack_SMU(ndev, AMD_PEER_RESET_EVENT);
> +
> + /* link down first */
> + ntb_link_event(&ndev->ntb);
> + /* polling peer status */
> + schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT);
> +
> + break;
> + case AMD_PEER_D3_EVENT:
> + case AMD_PEER_PMETO_EVENT:
> + amd_ack_SMU(ndev, status);
> +
> + /* link down */
> + ntb_link_event(&ndev->ntb);
> +
> + break;
> + case AMD_PEER_D0_EVENT:
> + status = NTB_READ_PEER_REG(PMESTAT);
> + /* check if this is WAKEUP event */
> + if (status & 0x1)
> + complete(&ndev->wakeup_cmpl);
> +
> + amd_ack_SMU(ndev, AMD_PEER_D0_EVENT);
> +
> + if (amd_link_is_up(ndev))
> + ntb_link_event(&ndev->ntb);
> + else
> + schedule_delayed_work(&ndev->hb_timer,
> + AMD_LINK_HB_TIMEOUT);
> + break;
> + default:
> + pr_err("Unsupported interrupt.\n");
> + break;
> + }
> +}
> +
> +static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec)
> +{
> + dev_dbg(ndev_dev(ndev), "vec %d\n", vec);
> +
> + if (vec > 20) {
This duplicates the "default" case of amd_handle_event.
> + dev_err(ndev_dev(ndev), "Invalid interrupt.\n");
> + return IRQ_HANDLED;
> + }
> +
> + if (vec > 16 || (ndev->msix_vec_count == 1))
> + amd_handle_event(ndev, vec);
> +
> + if (vec < 16)
> + ntb_db_event(&ndev->ntb, vec);
> +
> + return IRQ_HANDLED;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] NTB: Add AMD PCI-Express NTB driver
AMD NTB support following main features:
(1) Three memory windows;
(2) Sixteen 32-bit scratch pad;
(3) Two 16-bit doorbell interrupt;
(4) Five event interrupts;
(5) One system can wake up opposite system of NTB;
(6) Flush previous request to the opposite system;
(7) There are reset and PME_TO mechanisms between two systems;
Signed-off-by: Xiangliang Yu
---
drivers/ntb/hw/amd/Kconfig |7 +
drivers/ntb/hw/amd/Makefile |1 +
drivers/ntb/hw/amd/ntb_hw_amd.c | 1225 +++
drivers/ntb/hw/amd/ntb_hw_amd.h | 266 +
4 files changed, 1499 insertions(+)
create mode 100644 drivers/ntb/hw/amd/Kconfig
create mode 100644 drivers/ntb/hw/amd/Makefile
create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.c
create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.h
diff --git a/drivers/ntb/hw/amd/Kconfig b/drivers/ntb/hw/amd/Kconfig
new file mode 100644
index 000..cfe903c
--- /dev/null
+++ b/drivers/ntb/hw/amd/Kconfig
@@ -0,0 +1,7 @@
+config NTB_AMD
+ tristate "AMD Non-Transparent Bridge support"
+ depends on X86_64
+ help
+This driver supports AMD NTB on capable Zeppelin hardware.
+
+If unsure, say N.
diff --git a/drivers/ntb/hw/amd/Makefile b/drivers/ntb/hw/amd/Makefile
new file mode 100644
index 000..ad54da9
--- /dev/null
+++ b/drivers/ntb/hw/amd/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_NTB_AMD) += ntb_hw_amd.o
diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
new file mode 100644
index 000..931dcdc
--- /dev/null
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
@@ -0,0 +1,1225 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * BSD LICENSE
+ *
+ * Copyright (C) 2015 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copy
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of AMD Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * AMD PCIe NTB Linux driver
+ *
+ * Contact Information:
+ * Xiangliang Yu
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ntb_hw_amd.h"
+
+#define NTB_NAME "ntb_hw_amd"
+#define NTB_DESC "AMD(R) PCI-E Non-Transparent Bridge Driver"
+#define NTB_VER"1.0"
+
+MODULE_DESCRIPTION(NTB_DESC);
+MODULE_VERSION(NTB_VER);
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("AMD Inc.");
+
+static const struct file_operations amd_ntb_debugfs_info;
+static struct dentry *debugfs_dir;
+
+static const struct ntb_dev_ops amd_ntb_ops = {
+ .mw_count = amd_ntb_mw_count,
+ .mw_get_range = amd_ntb_mw_get_range,
+ .mw_set_trans = amd_ntb_mw_set_trans,
+ .link_is_up = amd_ntb_link_is_up,
+ .link_enable= amd_ntb_link_enable,
+ .link_disable = amd_ntb_link_disable,
+ .db_valid_mask = amd_ntb_db_valid_mask,
+ .db_vector_count= amd_ntb_db_vector_count,
+ .db_vector_mask = amd_ntb_db_vector_mask,
+ .db_read= amd_ntb_db_read,
+ .db_clear = amd_ntb_db_clear,
+
