From bb81bf62151031df004864eabee0431c8b8e9064 Mon Sep 17 00:00:00 2001 From: Jiasen Lin Date: Thu, 7 Nov 2019 01:35:36 -0800 Subject: NTB: Fix an error in get link status The offset of PCIe Capability Header for AMD and HYGON NTB is 0x64, but the macro which named "AMD_LINK_STATUS_OFFSET" is defined as 0x68. It is offset of Device Capabilities Reg rather than Link Control Reg. This code trigger an error in get link statsus: cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info LNK STA - 0x8fa1 Link Status - Up Link Speed - PCI-E Gen 0 Link Width - x0 This patch use pcie_capability_read_dword to get link status. After fix this issue, we can get link status accurately: cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info LNK STA - 0x11030042 Link Status - Up Link Speed - PCI-E Gen 3 Link Width - x16 Fixes: a1b3695820aa4 ("NTB: Add support for AMD PCI-Express Non-Transparent Bridge") Signed-off-by: Jiasen Lin Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 4 ++-- drivers/ntb/hw/amd/ntb_hw_amd.h | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index e52b300b2f5b..ae911053ba9c 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -855,8 +855,8 @@ static int amd_poll_link(struct amd_ntb_dev *ndev) ndev->cntl_sta = reg; - rc = pci_read_config_dword(ndev->ntb.pdev, - AMD_LINK_STATUS_OFFSET, &stat); + rc = pcie_capability_read_dword(ndev->ntb.pdev, + PCI_EXP_LNKCTL, &stat); if (rc) return 0; ndev->lnk_sta = stat; diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 139a307147bc..39e5d18e12ff 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -53,7 +53,6 @@ #include #define AMD_LINK_HB_TIMEOUT msecs_to_jiffies(1000) -#define AMD_LINK_STATUS_OFFSET 0x68 #define NTB_LIN_STA_ACTIVE_BIT 0x00000002 #define NTB_LNK_STA_SPEED_MASK 0x000F0000 #define NTB_LNK_STA_WIDTH_MASK 0x03F00000 -- cgit v1.2.3 From 99a06056124dcf5cfc4c95278b86c6ff96aaa1ec Mon Sep 17 00:00:00 2001 From: Jiasen Lin Date: Wed, 20 Nov 2019 18:28:44 -0800 Subject: NTB: ntb_perf: Fix address err in perf_copy_chunk peer->outbuf is a virtual address which is get by ioremap, it can not be converted to a physical address by virt_to_page and page_to_phys. This conversion will result in DMA error, because the destination address which is converted by page_to_phys is invalid. This patch save the MMIO address of NTB BARx in perf_setup_peer_mw, and map the BAR space to DMA address after we assign the DMA channel. Then fill the destination address of DMA descriptor with this DMA address to guarantee that the address of memory write requests fall into memory window of NBT BARx with IOMMU enabled and disabled. Fixes: 5648e56d03fa ("NTB: ntb_perf: Add full multi-port NTB API support") Signed-off-by: Jiasen Lin Reviewed-by: Logan Gunthorpe Signed-off-by: Jon Mason --- drivers/ntb/test/ntb_perf.c | 57 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c index e9b7c2dfc730..972f6d984f6d 100644 --- a/drivers/ntb/test/ntb_perf.c +++ b/drivers/ntb/test/ntb_perf.c @@ -149,7 +149,8 @@ struct perf_peer { u64 outbuf_xlat; resource_size_t outbuf_size; void __iomem *outbuf; - + phys_addr_t out_phys_addr; + dma_addr_t dma_dst_addr; /* Inbound MW params */ dma_addr_t inbuf_xlat; resource_size_t inbuf_size; @@ -782,6 +783,10 @@ static int perf_copy_chunk(struct perf_thread *pthr, struct dmaengine_unmap_data *unmap; struct device *dma_dev; int try = 0, ret = 0; + struct perf_peer *peer = pthr->perf->test_peer; + void __iomem *vbase; + void __iomem *dst_vaddr; + dma_addr_t dst_dma_addr; if (!use_dma) { memcpy_toio(dst, src, len); @@ -794,6 +799,10 @@ static int perf_copy_chunk(struct perf_thread *pthr, offset_in_page(dst), len)) return -EIO; + vbase = peer->outbuf; + dst_vaddr = dst; + dst_dma_addr = peer->dma_dst_addr + (dst_vaddr - vbase); + unmap = dmaengine_get_unmap_data(dma_dev, 2, GFP_NOWAIT); if (!unmap) return -ENOMEM; @@ -807,8 +816,7 @@ static int perf_copy_chunk(struct perf_thread *pthr, } unmap->to_cnt = 1; - unmap->addr[1] = dma_map_page(dma_dev, virt_to_page(dst), - offset_in_page(dst), len, DMA_FROM_DEVICE); + unmap->addr[1] = dst_dma_addr; if (dma_mapping_error(dma_dev, unmap->addr[1])) { ret = -EIO; goto err_free_resource; @@ -865,6 +873,7 @@ static int perf_init_test(struct perf_thread *pthr) { struct perf_ctx *perf = pthr->perf; dma_cap_mask_t dma_mask; + struct perf_peer *peer = pthr->perf->test_peer; pthr->src = kmalloc_node(perf->test_peer->outbuf_size, GFP_KERNEL, dev_to_node(&perf->ntb->dev)); @@ -882,15 +891,33 @@ static int perf_init_test(struct perf_thread *pthr) if (!pthr->dma_chan) { dev_err(&perf->ntb->dev, "%d: Failed to get DMA channel\n", pthr->tidx); - atomic_dec(&perf->tsync); - wake_up(&perf->twait); - kfree(pthr->src); - return -ENODEV; + goto err_free; } + peer->dma_dst_addr = + dma_map_resource(pthr->dma_chan->device->dev, + peer->out_phys_addr, peer->outbuf_size, + DMA_FROM_DEVICE, 0); + if (dma_mapping_error(pthr->dma_chan->device->dev, + peer->dma_dst_addr)) { + dev_err(pthr->dma_chan->device->dev, "%d: Failed to map DMA addr\n", + pthr->tidx); + peer->dma_dst_addr = 0; + dma_release_channel(pthr->dma_chan); + goto err_free; + } + dev_dbg(pthr->dma_chan->device->dev, "%d: Map MMIO %pa to DMA addr %pad\n", + pthr->tidx, + &peer->out_phys_addr, + &peer->dma_dst_addr); atomic_set(&pthr->dma_sync, 0); - return 0; + +err_free: + atomic_dec(&perf->tsync); + wake_up(&perf->twait); + kfree(pthr->src); + return -ENODEV; } static int perf_run_test(struct perf_thread *pthr) @@ -978,8 +1005,13 @@ static void perf_clear_test(struct perf_thread *pthr) * We call it anyway just to be sure of the transfers completion. */ (void)dmaengine_terminate_sync(pthr->dma_chan); - - dma_release_channel(pthr->dma_chan); + if (pthr->perf->test_peer->dma_dst_addr) + dma_unmap_resource(pthr->dma_chan->device->dev, + pthr->perf->test_peer->dma_dst_addr, + pthr->perf->test_peer->outbuf_size, + DMA_FROM_DEVICE, 0); + if (pthr->dma_chan) + dma_release_channel(pthr->dma_chan); no_dma_notify: atomic_dec(&perf->tsync); @@ -1194,6 +1226,9 @@ static ssize_t perf_dbgfs_read_info(struct file *filep, char __user *ubuf, pos += scnprintf(buf + pos, buf_size - pos, "\tOut buffer addr 0x%pK\n", peer->outbuf); + pos += scnprintf(buf + pos, buf_size - pos, + "\tOut buff phys addr %pa[p]\n", &peer->out_phys_addr); + pos += scnprintf(buf + pos, buf_size - pos, "\tOut buffer size %pa\n", &peer->outbuf_size); @@ -1388,6 +1423,8 @@ static int perf_setup_peer_mw(struct perf_peer *peer) if (!peer->outbuf) return -ENOMEM; + peer->out_phys_addr = phys_addr; + if (max_mw_size && peer->outbuf_size > max_mw_size) { peer->outbuf_size = max_mw_size; dev_warn(&peer->perf->ntb->dev, -- cgit v1.2.3 From 2ef97a6c181eba48f14c9ed98ce4398d21164683 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Tue, 14 Jan 2020 20:22:47 +0100 Subject: ntb_tool: Fix printk format The correct printk format is %pa or %pap, but not %pa[p]. Fixes: 7f46c8b3a5523 ("NTB: ntb_tool: Add full multi-port NTB API support") Signed-off-by: Helge Deller Signed-off-by: Jon Mason --- drivers/ntb/test/ntb_tool.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c index d592c0ffbd19..69da758fe64c 100644 --- a/drivers/ntb/test/ntb_tool.c +++ b/drivers/ntb/test/ntb_tool.c @@ -678,19 +678,19 @@ static ssize_t tool_mw_trans_read(struct file *filep, char __user *ubuf, &inmw->dma_base); off += scnprintf(buf + off, buf_size - off, - "Window Size \t%pa[p]\n", + "Window Size \t%pap\n", &inmw->size); off += scnprintf(buf + off, buf_size - off, - "Alignment \t%pa[p]\n", + "Alignment \t%pap\n", &addr_align); off += scnprintf(buf + off, buf_size - off, - "Size Alignment \t%pa[p]\n", + "Size Alignment \t%pap\n", &size_align); off += scnprintf(buf + off, buf_size - off, - "Size Max \t%pa[p]\n", + "Size Max \t%pap\n", &size_max); ret = simple_read_from_buffer(ubuf, size, offp, buf, off); @@ -907,16 +907,16 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep, char __user *ubuf, "Virtual address \t0x%pK\n", outmw->io_base); off += scnprintf(buf + off, buf_size - off, - "Phys Address \t%pa[p]\n", &map_base); + "Phys Address \t%pap\n", &map_base); off += scnprintf(buf + off, buf_size - off, - "Mapping Size \t%pa[p]\n", &map_size); + "Mapping Size \t%pap\n", &map_size); off += scnprintf(buf + off, buf_size - off, "Translation Address \t0x%016llx\n", outmw->tr_base); off += scnprintf(buf + off, buf_size - off, - "Window Size \t%pa[p]\n", &outmw->size); + "Window Size \t%pap\n", &outmw->size); ret = simple_read_from_buffer(ubuf, size, offp, buf, off); kfree(buf); -- cgit v1.2.3 From 788b041afd9a66ce3871734e87e908dc47739f39 Mon Sep 17 00:00:00 2001 From: Alexander Fomichev Date: Thu, 13 Feb 2020 18:05:07 +0300 Subject: ntb_hw_switchtec: Fix ntb_mw_clear_trans error if size == 0 ntb_mw_set_trans() should work as ntb_mw_clear_trans() when size == 0 and/or addr == 0. But error in xlate_pos checking condition prevents this. Fix the condition to make ntb_mw_clear_trans() working. Fixes: 87d11e645e31 (NTB: switchtec_ntb: Add memory window support) Signed-off-by: Alexander Fomichev Reviewed-by: Logan Gunthorpe Signed-off-by: Jon Mason --- drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c index 86ffa716eaf2..4c6eb61a6ac6 100644 --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c @@ -285,7 +285,7 @@ static int switchtec_ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int widx, if (widx >= switchtec_ntb_mw_count(ntb, pidx)) return -EINVAL; - if (xlate_pos < 12) + if (size != 0 && xlate_pos < 12) return -EINVAL; if (!IS_ALIGNED(addr, BIT_ULL(xlate_pos))) { -- cgit v1.2.3 From 7f78c68aa796af9e8b98880aa2079daf14e88ebf Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 11 Mar 2020 09:49:17 +0100 Subject: NTB: ntb_transport: Use scnprintf() for avoiding potential buffer overflow Since snprintf() returns the would-be-output size instead of the actual output size, the succeeding calls may go beyond the given buffer limit. Fix it by replacing with scnprintf(). Fixes: fce8a7bb5b4b (PCI-Express Non-Transparent Bridge Support) Fixes: 282a2feeb9bf (NTB: Use DMA Engine to Transmit and Receive) Fixes: a754a8fcaf38 (NTB: allocate number transport entries depending on size of ring size) Fixes: d98ef99e378b (NTB: Clean up QP stats info) Fixes: e74bfeedad08 (NTB: Add flow control to the ntb_netdev) Fixes: 569410ca756c (NTB: Use unique DMA channels for TX and RX) Signed-off-by: Takashi Iwai Reviewed-by: Logan Gunthorpe Signed-off-by: Jon Mason --- drivers/ntb/ntb_transport.c | 58 ++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c index 00a5d5764993..e6d1f5b298f3 100644 --- a/drivers/ntb/ntb_transport.c +++ b/drivers/ntb/ntb_transport.c @@ -481,70 +481,70 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count, return -ENOMEM; out_offset = 0; - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "\nNTB QP stats:\n\n"); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_bytes - \t%llu\n", qp->rx_bytes); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_pkts - \t%llu\n", qp->rx_pkts); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_memcpy - \t%llu\n", qp->rx_memcpy); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_async - \t%llu\n", qp->rx_async); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_ring_empty - %llu\n", qp->rx_ring_empty); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_err_no_buf - %llu\n", qp->rx_err_no_buf); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_err_oflow - \t%llu\n", qp->rx_err_oflow); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_err_ver - \t%llu\n", qp->rx_err_ver); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_buff - \t0x%p\n", qp->rx_buff); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_index - \t%u\n", qp->rx_index); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_max_entry - \t%u\n", qp->rx_max_entry); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "rx_alloc_entry - \t%u\n\n", qp->rx_alloc_entry); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_bytes - \t%llu\n", qp->tx_bytes); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_pkts - \t%llu\n", qp->tx_pkts); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_memcpy - \t%llu\n", qp->tx_memcpy); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_async - \t%llu\n", qp->tx_async); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_ring_full - \t%llu\n", qp->tx_ring_full); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_err_no_buf - %llu\n", qp->tx_err_no_buf); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_mw - \t0x%p\n", qp->tx_mw); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_index (H) - \t%u\n", qp->tx_index); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "RRI (T) - \t%u\n", qp->remote_rx_info->entry); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "tx_max_entry - \t%u\n", qp->tx_max_entry); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "free tx - \t%u\n", ntb_transport_tx_free_entry(qp)); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "\n"); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "Using TX DMA - \t%s\n", qp->tx_dma_chan ? "Yes" : "No"); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "Using RX DMA - \t%s\n", qp->rx_dma_chan ? "Yes" : "No"); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "QP Link - \t%s\n", qp->link_is_up ? "Up" : "Down"); - out_offset += snprintf(buf + out_offset, out_count - out_offset, + out_offset += scnprintf(buf + out_offset, out_count - out_offset, "\n"); if (out_offset > out_count) -- cgit v1.2.3 From 8ad1a2f351c1ade21ad7a2f0bc34fc887878f965 Mon Sep 17 00:00:00 2001 From: Sanjay R Mehta Date: Fri, 7 Feb 2020 03:21:58 -0600 Subject: MAINTAINERS: update maintainer list for AMD NTB driver updating with my email address. Signed-off-by: Sanjay R Mehta Signed-off-by: Shyam Sundar S K Signed-off-by: Jon Mason --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 38fe2f3f7b6f..e3554e60fb31 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11861,6 +11861,7 @@ F: scripts/nsdeps F: Documentation/core-api/symbol-namespaces.rst NTB AMD DRIVER +M: Sanjay R Mehta M: Shyam Sundar S K L: linux-ntb@googlegroups.com S: Supported -- cgit v1.2.3 From cb004c28dd2fabbb57bc03fb002f780054c28780 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:18 +0530 Subject: NTB: Fix access to link status and control register The design of AMD NTB implementation is such that NTB primary acts as an endpoint device and NTB secondary is an endpoint device behind a combination of Switch Upstream and Switch Downstream. Considering that, the link status and control register needs to be accessed differently based on the NTB topology. So in the case of NTB secondary, we first get the pointer to the Switch Downstream device for the NTB device. Then we get the pointer to the Switch Upstream device. Once we have that, we read the Link Status and Control register to get the correct status of link at the secondary. In the case of NTB primary, simply reading the Link Status and Control register of the NTB device itself will suffice. Suggested-by: Jiasen Lin Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index ae911053ba9c..9a60f34a37c2 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -842,6 +842,9 @@ static inline void ndev_init_struct(struct amd_ntb_dev *ndev, static int amd_poll_link(struct amd_ntb_dev *ndev) { void __iomem *mmio = ndev->peer_mmio; + struct pci_dev *pdev = NULL; + struct pci_dev *pci_swds = NULL; + struct pci_dev *pci_swus = NULL; u32 reg, stat; int rc; @@ -855,10 +858,41 @@ static int amd_poll_link(struct amd_ntb_dev *ndev) ndev->cntl_sta = reg; - rc = pcie_capability_read_dword(ndev->ntb.pdev, - PCI_EXP_LNKCTL, &stat); - if (rc) + if (ndev->ntb.topo == NTB_TOPO_SEC) { + /* Locate the pointer to Downstream Switch for this device */ + pci_swds = pci_upstream_bridge(ndev->ntb.pdev); + if (pci_swds) { + /* + * Locate the pointer to Upstream Switch for + * the Downstream Switch. + */ + pci_swus = pci_upstream_bridge(pci_swds); + if (pci_swus) { + rc = pcie_capability_read_dword(pci_swus, + PCI_EXP_LNKCTL, + &stat); + if (rc) + return 0; + } else { + return 0; + } + } else { + return 0; + } + } else if (ndev->ntb.topo == NTB_TOPO_PRI) { + /* + * For NTB primary, we simply read the Link Status and control + * register of the NTB device itself. + */ + pdev = ndev->ntb.pdev; + rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat); + if (rc) + return 0; + } else { + /* Catch all for everything else */ return 0; + } + ndev->lnk_sta = stat; return 1; -- cgit v1.2.3 From 52ba447889643ea25545c521e653f956b2480489 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:19 +0530 Subject: NTB: clear interrupt status register The interrupt status register should be cleared by driver once the particular event is handled. The patch fixes this. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 9a60f34a37c2..150e4db11485 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -550,6 +550,9 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) dev_info(dev, "event status = 0x%x.\n", status); break; } + + /* Clear the interrupt status */ + writel(status, mmio + AMD_INTSTAT_OFFSET); } static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec) -- cgit v1.2.3 From 8a7cedef441f56bcbdd0a0e70f73510ffb72dc5d Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:20 +0530 Subject: NTB: Enable link up and down event notification Link-Up and Link-Down events can occur irrespective of whether a data transfer is in progress or not. So we need to enable the interrupt delivery for these events early during driver load. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 150e4db11485..111f33ff2bd7 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -994,6 +994,7 @@ static enum ntb_topo amd_get_topo(struct amd_ntb_dev *ndev) static int amd_init_dev(struct amd_ntb_dev *ndev) { + void __iomem *mmio = ndev->self_mmio; struct pci_dev *pdev; int rc = 0; @@ -1015,6 +1016,10 @@ static int amd_init_dev(struct amd_ntb_dev *ndev) ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1; + /* Enable Link-Up and Link-Down event interrupts */ + ndev->int_mask &= ~(AMD_LINK_UP_EVENT | AMD_LINK_DOWN_EVENT); + writel(ndev->int_mask, mmio + AMD_INTMASK_OFFSET); + return 0; } -- cgit v1.2.3 From 5c6404d5fa74124c64661ce44d7e447f3bf3df1e Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:21 +0530 Subject: NTB: define a new function to get link status Since getting the status of link is a logically separate operation, we simply create a new function which will store the link status to be used later. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 93 ++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 111f33ff2bd7..f50537e0917b 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -195,6 +195,54 @@ static int amd_ntb_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx, return 0; } +static int amd_ntb_get_link_status(struct amd_ntb_dev *ndev) +{ + struct pci_dev *pdev = NULL; + struct pci_dev *pci_swds = NULL; + struct pci_dev *pci_swus = NULL; + u32 stat; + int rc; + + if (ndev->ntb.topo == NTB_TOPO_SEC) { + /* Locate the pointer to Downstream Switch for this device */ + pci_swds = pci_upstream_bridge(ndev->ntb.pdev); + if (pci_swds) { + /* + * Locate the pointer to Upstream Switch for + * the Downstream Switch. + */ + pci_swus = pci_upstream_bridge(pci_swds); + if (pci_swus) { + rc = pcie_capability_read_dword(pci_swus, + PCI_EXP_LNKCTL, + &stat); + if (rc) + return 0; + } else { + return 0; + } + } else { + return 0; + } + } else if (ndev->ntb.topo == NTB_TOPO_PRI) { + /* + * For NTB primary, we simply read the Link Status and control + * register of the NTB device itself. + */ + pdev = ndev->ntb.pdev; + rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat); + if (rc) + return 0; + } else { + /* Catch all for everything else */ + return 0; + } + + ndev->lnk_sta = stat; + + return 1; +} + static int amd_link_is_up(struct amd_ntb_dev *ndev) { if (!ndev->peer_sta) @@ -845,11 +893,7 @@ static inline void ndev_init_struct(struct amd_ntb_dev *ndev, static int amd_poll_link(struct amd_ntb_dev *ndev) { void __iomem *mmio = ndev->peer_mmio; - struct pci_dev *pdev = NULL; - struct pci_dev *pci_swds = NULL; - struct pci_dev *pci_swus = NULL; - u32 reg, stat; - int rc; + u32 reg; reg = readl(mmio + AMD_SIDEINFO_OFFSET); reg &= NTB_LIN_STA_ACTIVE_BIT; @@ -861,44 +905,7 @@ static int amd_poll_link(struct amd_ntb_dev *ndev) ndev->cntl_sta = reg; - if (ndev->ntb.topo == NTB_TOPO_SEC) { - /* Locate the pointer to Downstream Switch for this device */ - pci_swds = pci_upstream_bridge(ndev->ntb.pdev); - if (pci_swds) { - /* - * Locate the pointer to Upstream Switch for - * the Downstream Switch. - */ - pci_swus = pci_upstream_bridge(pci_swds); - if (pci_swus) { - rc = pcie_capability_read_dword(pci_swus, - PCI_EXP_LNKCTL, - &stat); - if (rc) - return 0; - } else { - return 0; - } - } else { - return 0; - } - } else if (ndev->ntb.topo == NTB_TOPO_PRI) { - /* - * For NTB primary, we simply read the Link Status and control - * register of the NTB device itself. - */ - pdev = ndev->ntb.pdev; - rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat); - if (rc) - return 0; - } else { - /* Catch all for everything else */ - return 0; - } - - ndev->lnk_sta = stat; - - return 1; + return amd_ntb_get_link_status(ndev); } static void amd_link_hb(struct work_struct *work) -- cgit v1.2.3 From 5cafa48502c833fa175d2d16a26d1b4e0702aa74 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:22 +0530 Subject: NTB: return the side info status from amd_poll_link Bit 1 of SIDE_INFO register is an indication that the driver on the other side of link is ready. We set this bit during driver initialization sequence. So rather than having separate macros to return the status, we can simply return the status of this bit from amd_poll_link(). So a return of 1 or 0 from this function will indicate to the caller whether the driver on the other side of link is ready or not, respectively. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 11 +++++------ drivers/ntb/hw/amd/ntb_hw_amd.h | 2 -- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index f50537e0917b..84723420d70b 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -246,7 +246,7 @@ static int amd_ntb_get_link_status(struct amd_ntb_dev *ndev) static int amd_link_is_up(struct amd_ntb_dev *ndev) { if (!ndev->peer_sta) - return NTB_LNK_STA_ACTIVE(ndev->cntl_sta); + return ndev->cntl_sta; if (ndev->peer_sta & AMD_LINK_UP_EVENT) { ndev->peer_sta = 0; @@ -896,16 +896,15 @@ static int amd_poll_link(struct amd_ntb_dev *ndev) u32 reg; reg = readl(mmio + AMD_SIDEINFO_OFFSET); - reg &= NTB_LIN_STA_ACTIVE_BIT; + reg &= AMD_SIDE_READY; dev_dbg(&ndev->ntb.pdev->dev, "%s: reg_val = 0x%x.\n", __func__, reg); - if (reg == ndev->cntl_sta) - return 0; - ndev->cntl_sta = reg; - return amd_ntb_get_link_status(ndev); + amd_ntb_get_link_status(ndev); + + return ndev->cntl_sta; } static void amd_link_hb(struct work_struct *work) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 39e5d18e12ff..156a4a92b803 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -53,10 +53,8 @@ #include #define AMD_LINK_HB_TIMEOUT msecs_to_jiffies(1000) -#define NTB_LIN_STA_ACTIVE_BIT 0x00000002 #define NTB_LNK_STA_SPEED_MASK 0x000F0000 #define NTB_LNK_STA_WIDTH_MASK 0x03F00000 -#define NTB_LNK_STA_ACTIVE(x) (!!((x) & NTB_LIN_STA_ACTIVE_BIT)) #define NTB_LNK_STA_SPEED(x) (((x) & NTB_LNK_STA_SPEED_MASK) >> 16) #define NTB_LNK_STA_WIDTH(x) (((x) & NTB_LNK_STA_WIDTH_MASK) >> 20) -- cgit v1.2.3 From 2465b87ce36ea2dbd97e5fb58a0efd284c9f687e Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:23 +0530 Subject: NTB: set peer_sta within event handler itself amd_ack_smu() should only set the corresponding bits into SMUACK register. Setting the bitmask of peer_sta should be done within the event handler. They are two different things, and so should be handled differently and at different places. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 84723420d70b..b85af150f2c6 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -541,8 +541,6 @@ static void amd_ack_smu(struct amd_ntb_dev *ndev, u32 bit) reg = readl(mmio + AMD_SMUACK_OFFSET); reg |= bit; writel(reg, mmio + AMD_SMUACK_OFFSET); - - ndev->peer_sta |= bit; } static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) @@ -560,9 +558,11 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) status &= AMD_EVENT_INTMASK; switch (status) { case AMD_PEER_FLUSH_EVENT: + ndev->peer_sta |= AMD_PEER_FLUSH_EVENT; dev_info(dev, "Flush is done.\n"); break; case AMD_PEER_RESET_EVENT: + ndev->peer_sta |= AMD_PEER_RESET_EVENT; amd_ack_smu(ndev, AMD_PEER_RESET_EVENT); /* link down first */ @@ -575,6 +575,7 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) case AMD_PEER_PMETO_EVENT: case AMD_LINK_UP_EVENT: case AMD_LINK_DOWN_EVENT: + ndev->peer_sta |= status; amd_ack_smu(ndev, status); /* link down */ @@ -588,6 +589,7 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) if (status & 0x1) dev_info(dev, "Wakeup is done.\n"); + ndev->peer_sta |= AMD_PEER_D0_EVENT; amd_ack_smu(ndev, AMD_PEER_D0_EVENT); /* start a timer to poll link status */ -- cgit v1.2.3 From fdd8281fb0bc9afddba93672e267aff1b5561215 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:24 +0530 Subject: NTB: remove handling of peer_sta from amd_link_is_up amd_link_is_up() is a callback to inquire whether the NTB link is up or not. So it should not indulge itself into clearing the bitmasks of peer_sta. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index b85af150f2c6..e964442ae2c3 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -253,17 +253,6 @@ static int amd_link_is_up(struct amd_ntb_dev *ndev) return 1; } - /* If peer_sta is reset or D0 event, the ISR has - * started a timer to check link status of hardware. - * So here just clear status bit. And if peer_sta is - * D3 or PME_TO, D0/reset event will be happened when - * system wakeup/poweron, so do nothing here. - */ - if (ndev->peer_sta & AMD_PEER_RESET_EVENT) - ndev->peer_sta &= ~AMD_PEER_RESET_EVENT; - else if (ndev->peer_sta & (AMD_PEER_D0_EVENT | AMD_LINK_DOWN_EVENT)) - ndev->peer_sta = 0; - return 0; } -- cgit v1.2.3 From 60ceafd151d62eba6c19c27e617269567082a17d Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:25 +0530 Subject: NTB: handle link down event correctly Link-Up and Link-Down are mutually exclusive events. So when we receive a Link-Down event, we should also clear the bitmask for Link-Up event in peer_sta. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index e964442ae2c3..d933a1dffdc6 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -551,8 +551,12 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) dev_info(dev, "Flush is done.\n"); break; case AMD_PEER_RESET_EVENT: - ndev->peer_sta |= AMD_PEER_RESET_EVENT; - amd_ack_smu(ndev, AMD_PEER_RESET_EVENT); + case AMD_LINK_DOWN_EVENT: + ndev->peer_sta |= status; + if (status == AMD_LINK_DOWN_EVENT) + ndev->peer_sta &= ~AMD_LINK_UP_EVENT; + + amd_ack_smu(ndev, status); /* link down first */ ntb_link_event(&ndev->ntb); @@ -563,7 +567,6 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) case AMD_PEER_D3_EVENT: case AMD_PEER_PMETO_EVENT: case AMD_LINK_UP_EVENT: - case AMD_LINK_DOWN_EVENT: ndev->peer_sta |= status; amd_ack_smu(ndev, status); -- cgit v1.2.3 From 673dd0c24779fa9233fd600555132b5ffc1e19e4 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:26 +0530 Subject: NTB: handle link up, D0 and D3 events correctly Just like for Link-Down event, Link-Up and D3 events are also mutually exclusive to Link-Down and D0 events respectively. So we clear the bitmasks in peer_sta depending on event type. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index d933a1dffdc6..a1c4a21c58c3 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -568,6 +568,11 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) case AMD_PEER_PMETO_EVENT: case AMD_LINK_UP_EVENT: ndev->peer_sta |= status; + if (status == AMD_LINK_UP_EVENT) + ndev->peer_sta &= ~AMD_LINK_DOWN_EVENT; + else if (status == AMD_PEER_D3_EVENT) + ndev->peer_sta &= ~AMD_PEER_D0_EVENT; + amd_ack_smu(ndev, status); /* link down */ @@ -582,6 +587,7 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) dev_info(dev, "Wakeup is done.\n"); ndev->peer_sta |= AMD_PEER_D0_EVENT; + ndev->peer_sta &= ~AMD_PEER_D3_EVENT; amd_ack_smu(ndev, AMD_PEER_D0_EVENT); /* start a timer to poll link status */ -- cgit v1.2.3 From 92abf4cb993ddce258acac32bd0068d1813a79d1 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:27 +0530 Subject: NTB: move ntb_ctrl handling to init and deinit It does not really make sense to enable or disable the bits of NTB_CTRL register only during enable and disable link callbacks. They should be done independent of these callbacks. The correct placement for that is during the amd_init_side_info() and amd_deinit_side_info() functions, which are invoked during probe and remove respectively. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index a1c4a21c58c3..621a69a0cff2 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -290,7 +290,6 @@ static int amd_ntb_link_enable(struct ntb_dev *ntb, { struct amd_ntb_dev *ndev = ntb_ndev(ntb); void __iomem *mmio = ndev->self_mmio; - u32 ntb_ctl; /* Enable event interrupt */ ndev->int_mask &= ~AMD_EVENT_INTMASK; @@ -300,10 +299,6 @@ static int amd_ntb_link_enable(struct ntb_dev *ntb, return -EINVAL; dev_dbg(&ntb->pdev->dev, "Enabling Link.\n"); - ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); - ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL); - writel(ntb_ctl, mmio + AMD_CNTL_OFFSET); - return 0; } @@ -311,7 +306,6 @@ static int amd_ntb_link_disable(struct ntb_dev *ntb) { struct amd_ntb_dev *ndev = ntb_ndev(ntb); void __iomem *mmio = ndev->self_mmio; - u32 ntb_ctl; /* Disable event interrupt */ ndev->int_mask |= AMD_EVENT_INTMASK; @@ -321,10 +315,6 @@ static int amd_ntb_link_disable(struct ntb_dev *ntb) return -EINVAL; dev_dbg(&ntb->pdev->dev, "Enabling Link.\n"); - ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); - ntb_ctl &= ~(PMM_REG_CTL | SMM_REG_CTL); - writel(ntb_ctl, mmio + AMD_CNTL_OFFSET); - return 0; } @@ -927,18 +917,24 @@ static void amd_init_side_info(struct amd_ntb_dev *ndev) { void __iomem *mmio = ndev->self_mmio; unsigned int reg; + u32 ntb_ctl; reg = readl(mmio + AMD_SIDEINFO_OFFSET); if (!(reg & AMD_SIDE_READY)) { reg |= AMD_SIDE_READY; writel(reg, mmio + AMD_SIDEINFO_OFFSET); } + + ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); + ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL); + writel(ntb_ctl, mmio + AMD_CNTL_OFFSET); } static void amd_deinit_side_info(struct amd_ntb_dev *ndev) { void __iomem *mmio = ndev->self_mmio; unsigned int reg; + u32 ntb_ctl; reg = readl(mmio + AMD_SIDEINFO_OFFSET); if (reg & AMD_SIDE_READY) { @@ -946,6 +942,10 @@ static void amd_deinit_side_info(struct amd_ntb_dev *ndev) writel(reg, mmio + AMD_SIDEINFO_OFFSET); readl(mmio + AMD_SIDEINFO_OFFSET); } + + ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); + ntb_ctl &= ~(PMM_REG_CTL | SMM_REG_CTL); + writel(ntb_ctl, mmio + AMD_CNTL_OFFSET); } static int amd_init_ntb(struct amd_ntb_dev *ndev) -- cgit v1.2.3 From ae5f4bdccf030d524f995dfc364ea6a96c22882c Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:28 +0530 Subject: NTB: add helper functions to set and clear sideinfo We define two new helper functions to set and clear sideinfo registers respectively. These functions take an additional boolean parameter which signifies whether we want to set/clear the sideinfo register of the peer(true) or local host(false). Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 44 +++++++++++++++++++++++++++++++---------- drivers/ntb/hw/amd/ntb_hw_amd.h | 3 +++ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 621a69a0cff2..d4029d531466 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -913,28 +913,32 @@ static int amd_init_isr(struct amd_ntb_dev *ndev) return ndev_init_isr(ndev, AMD_DB_CNT, AMD_MSIX_VECTOR_CNT); } -static void amd_init_side_info(struct amd_ntb_dev *ndev) +static void amd_set_side_info_reg(struct amd_ntb_dev *ndev, bool peer) { - void __iomem *mmio = ndev->self_mmio; + void __iomem *mmio = NULL; unsigned int reg; - u32 ntb_ctl; + + if (peer) + mmio = ndev->peer_mmio; + else + mmio = ndev->self_mmio; reg = readl(mmio + AMD_SIDEINFO_OFFSET); if (!(reg & AMD_SIDE_READY)) { reg |= AMD_SIDE_READY; writel(reg, mmio + AMD_SIDEINFO_OFFSET); } - - ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); - ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL); - writel(ntb_ctl, mmio + AMD_CNTL_OFFSET); } -static void amd_deinit_side_info(struct amd_ntb_dev *ndev) +static void amd_clear_side_info_reg(struct amd_ntb_dev *ndev, bool peer) { - void __iomem *mmio = ndev->self_mmio; + void __iomem *mmio = NULL; unsigned int reg; - u32 ntb_ctl; + + if (peer) + mmio = ndev->peer_mmio; + else + mmio = ndev->self_mmio; reg = readl(mmio + AMD_SIDEINFO_OFFSET); if (reg & AMD_SIDE_READY) { @@ -942,6 +946,26 @@ static void amd_deinit_side_info(struct amd_ntb_dev *ndev) writel(reg, mmio + AMD_SIDEINFO_OFFSET); readl(mmio + AMD_SIDEINFO_OFFSET); } +} + +static void amd_init_side_info(struct amd_ntb_dev *ndev) +{ + void __iomem *mmio = ndev->self_mmio; + u32 ntb_ctl; + + amd_set_side_info_reg(ndev, false); + + ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); + ntb_ctl |= (PMM_REG_CTL | SMM_REG_CTL); + writel(ntb_ctl, mmio + AMD_CNTL_OFFSET); +} + +static void amd_deinit_side_info(struct amd_ntb_dev *ndev) +{ + void __iomem *mmio = ndev->self_mmio; + u32 ntb_ctl; + + amd_clear_side_info_reg(ndev, false); ntb_ctl = readl(mmio + AMD_CNTL_OFFSET); ntb_ctl &= ~(PMM_REG_CTL | SMM_REG_CTL); diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 156a4a92b803..62ffdf35b683 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -215,4 +215,7 @@ struct amd_ntb_dev { #define ntb_ndev(__ntb) container_of(__ntb, struct amd_ntb_dev, ntb) #define hb_ndev(__work) container_of(__work, struct amd_ntb_dev, hb_timer.work) +static void amd_set_side_info_reg(struct amd_ntb_dev *ndev, bool peer); +static void amd_clear_side_info_reg(struct amd_ntb_dev *ndev, bool peer); + #endif -- cgit v1.2.3 From 5f0856bebc6e02a57b36cd2146f056c71862f0b6 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:29 +0530 Subject: NTB: return link up status correctly for PRI and SEC Since NTB connects two physically separate systems, there can be scenarios where one system goes down while the other one remains active. In case of NTB primary, if the NTB secondary goes down, a Link-Down event is received. For the NTB secondary, if the NTB primary goes down, the PCIe hotplug mechanism ensures that the driver on the secondary side is also unloaded. But there are other scenarios to consider as well, when suppose the physical link remains active, but the driver on primary or secondary side is loaded or un-loaded. When the driver is loaded, on either side, it sets SIDE_READY bit(bit-1) of SIDE_INFO register. Similarly, when the driver is un-loaded, it resets the same bit. We consider the NTB link to be up and operational only when the driver on both sides of link are loaded and ready. But we also need to take account of Link Up and Down events which signify the physical link status. So amd_link_is_up() is modified to take care of the above scenarios. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 64 +++++++++++++++++++++++++++++++++++++---- drivers/ntb/hw/amd/ntb_hw_amd.h | 1 + 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index d4029d531466..8a9852343de6 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -245,12 +245,66 @@ static int amd_ntb_get_link_status(struct amd_ntb_dev *ndev) static int amd_link_is_up(struct amd_ntb_dev *ndev) { - if (!ndev->peer_sta) - return ndev->cntl_sta; + int ret; + + /* + * We consider the link to be up under two conditions: + * + * - When a link-up event is received. This is indicated by + * AMD_LINK_UP_EVENT set in peer_sta. + * - When driver on both sides of the link have been loaded. + * This is indicated by bit 1 being set in the peer + * SIDEINFO register. + * + * This function should return 1 when the latter of the above + * two conditions is true. + * + * Now consider the sequence of events - Link-Up event occurs, + * then the peer side driver loads. In this case, we would have + * received LINK_UP event and bit 1 of peer SIDEINFO is also + * set. What happens now if the link goes down? Bit 1 of + * peer SIDEINFO remains set, but LINK_DOWN bit is set in + * peer_sta. So we should return 0 from this function. Not only + * that, we clear bit 1 of peer SIDEINFO to 0, since the peer + * side driver did not even get a chance to clear it before + * the link went down. This can be the case of surprise link + * removal. + * + * LINK_UP event will always occur before the peer side driver + * gets loaded the very first time. So there can be a case when + * the LINK_UP event has occurred, but the peer side driver hasn't + * yet loaded. We return 0 in that case. + * + * There is also a special case when the primary side driver is + * unloaded and then loaded again. Since there is no change in + * the status of NTB secondary in this case, there is no Link-Up + * or Link-Down notification received. We recognize this condition + * with peer_sta being set to 0. + * + * If bit 1 of peer SIDEINFO register is not set, then we + * simply return 0 irrespective of the link up or down status + * set in peer_sta. + */ + ret = amd_poll_link(ndev); + if (ret) { + /* + * We need to check the below only for NTB primary. For NTB + * secondary, simply checking the result of PSIDE_INFO + * register will suffice. + */ + if (ndev->ntb.topo == NTB_TOPO_PRI) { + if ((ndev->peer_sta & AMD_LINK_UP_EVENT) || + (ndev->peer_sta == 0)) + return ret; + else if (ndev->peer_sta & AMD_LINK_DOWN_EVENT) { + /* Clear peer sideinfo register */ + amd_clear_side_info_reg(ndev, true); - if (ndev->peer_sta & AMD_LINK_UP_EVENT) { - ndev->peer_sta = 0; - return 1; + return 0; + } + } else { /* NTB_TOPO_SEC */ + return ret; + } } return 0; diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 62ffdf35b683..73959c0b9972 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -217,5 +217,6 @@ struct amd_ntb_dev { static void amd_set_side_info_reg(struct amd_ntb_dev *ndev, bool peer); static void amd_clear_side_info_reg(struct amd_ntb_dev *ndev, bool peer); +static int amd_poll_link(struct amd_ntb_dev *ndev); #endif -- cgit v1.2.3 From 41dfc3f79650504f5047f20a4faacaff217ce37b Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:30 +0530 Subject: NTB: remove redundant setting of DB valid mask db_valid_mask is set at two places, once within amd_init_ntb(), and again within amd_init_dev(). Since amd_init_ntb() is actually called from amd_init_dev(), setting db_valid_mask from former does not really make sense. So remove it. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 8a9852343de6..04be1482037b 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -1056,8 +1056,6 @@ static int amd_init_ntb(struct amd_ntb_dev *ndev) return -EINVAL; } - ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1; - /* Mask event interrupts */ writel(ndev->int_mask, mmio + AMD_INTMASK_OFFSET); -- cgit v1.2.3 From ac10d4f6c2a8247de21655f4564b0e209c3c1dd4 Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:31 +0530 Subject: NTB: send DB event when driver is loaded or un-loaded When the driver on the local side is loaded, it sets SIDE_READY bit in SIDE_INFO register. Likewise, when it is un-loaded, it clears the bit. Also just after being loaded, the driver polls for peer SIDE_READY bit to be set. Since that bit is set when the peer side driver has loaded, the polling on local side breaks as soon as this condition is met. But the situation is different when the driver is un-loaded. Since the polling has already been stopped as mentioned before, if the peer side driver gets un-loaded, the driver on the local side is not notified implicitly. So, we improvise using existing doorbell mechanism. We reserve the highest order bit of the DB register to send a notification to peer when the driver on local side is un-loaded. This also means that now we are one short of 16 DB events and that is taken care of in the valid DB mask. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 57 +++++++++++++++++++++++++++++++++++++++-- drivers/ntb/hw/amd/ntb_hw_amd.h | 1 + 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index 04be1482037b..c6cea0005553 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -647,6 +647,36 @@ static void amd_handle_event(struct amd_ntb_dev *ndev, int vec) writel(status, mmio + AMD_INTSTAT_OFFSET); } +static void amd_handle_db_event(struct amd_ntb_dev *ndev, int vec) +{ + struct device *dev = &ndev->ntb.pdev->dev; + u64 status; + + status = amd_ntb_db_read(&ndev->ntb); + + dev_dbg(dev, "status = 0x%llx and vec = %d\n", status, vec); + + /* + * Since we had reserved highest order bit of DB for signaling peer of + * a special event, this is the only status bit we should be concerned + * here now. + */ + if (status & BIT(ndev->db_last_bit)) { + ntb_db_clear(&ndev->ntb, BIT(ndev->db_last_bit)); + /* send link down event notification */ + ntb_link_event(&ndev->ntb); + + /* + * If we are here, that means the peer has signalled a special + * event which notifies that the peer driver has been + * un-loaded for some reason. Since there is a chance that the + * peer will load its driver again sometime, we schedule link + * polling routine. + */ + schedule_delayed_work(&ndev->hb_timer, AMD_LINK_HB_TIMEOUT); + } +} + static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec) { dev_dbg(&ndev->ntb.pdev->dev, "vec %d\n", vec); @@ -654,8 +684,10 @@ static irqreturn_t ndev_interrupt(struct amd_ntb_dev *ndev, int vec) if (vec > (AMD_DB_CNT - 1) || (ndev->msix_vec_count == 1)) amd_handle_event(ndev, vec); - if (vec < AMD_DB_CNT) + if (vec < AMD_DB_CNT) { + amd_handle_db_event(ndev, vec); ntb_db_event(&ndev->ntb, vec); + } return IRQ_HANDLED; } @@ -1096,6 +1128,21 @@ static int amd_init_dev(struct amd_ntb_dev *ndev) return rc; } + ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1; + /* + * We reserve the highest order bit of the DB register which will + * be used to notify peer when the driver on this side is being + * un-loaded. + */ + ndev->db_last_bit = + find_last_bit((unsigned long *)&ndev->db_valid_mask, + hweight64(ndev->db_valid_mask)); + writew((u16)~BIT(ndev->db_last_bit), mmio + AMD_DBMASK_OFFSET); + /* + * Since now there is one less bit to account for, the DB count + * and DB mask should be adjusted accordingly. + */ + ndev->db_count -= 1; ndev->db_valid_mask = BIT_ULL(ndev->db_count) - 1; /* Enable Link-Up and Link-Down event interrupts */ @@ -1235,9 +1282,15 @@ static void amd_ntb_pci_remove(struct pci_dev *pdev) { struct amd_ntb_dev *ndev = pci_get_drvdata(pdev); + /* + * Clear the READY bit in SIDEINFO register before sending DB event + * to the peer. This will make sure that when the peer handles the + * DB event, it correctly reads this bit as being 0. + */ + amd_deinit_side_info(ndev); + ntb_peer_db_set(&ndev->ntb, BIT_ULL(ndev->db_last_bit)); ntb_unregister_device(&ndev->ntb); ndev_deinit_debugfs(ndev); - amd_deinit_side_info(ndev); amd_deinit_dev(ndev); amd_ntb_deinit_pci(ndev); kfree(ndev); diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h b/drivers/ntb/hw/amd/ntb_hw_amd.h index 73959c0b9972..5f337b1572a0 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.h +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h @@ -193,6 +193,7 @@ struct amd_ntb_dev { u64 db_valid_mask; u64 db_mask; + u64 db_last_bit; u32 int_mask; struct msix_entry *msix; -- cgit v1.2.3 From b350f0a3eb264962caefeb892af56c1b727ee03f Mon Sep 17 00:00:00 2001 From: Arindam Nath Date: Wed, 5 Feb 2020 21:24:32 +0530 Subject: NTB: add pci shutdown handler for AMD NTB The PCI shutdown handler is invoked in response to system reboot or shutdown. A data transfer might still be in flight when this happens. So the very first action we take here is to send a link down notification, so that any pending data transfer is terminated. Rest of the actions are same as that of PCI remove handler. Signed-off-by: Arindam Nath Signed-off-by: Jon Mason --- drivers/ntb/hw/amd/ntb_hw_amd.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c index c6cea0005553..9e310e1ad4d0 100644 --- a/drivers/ntb/hw/amd/ntb_hw_amd.c +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c @@ -1296,6 +1296,22 @@ static void amd_ntb_pci_remove(struct pci_dev *pdev) kfree(ndev); } +static void amd_ntb_pci_shutdown(struct pci_dev *pdev) +{ + struct amd_ntb_dev *ndev = pci_get_drvdata(pdev); + + /* Send link down notification */ + ntb_link_event(&ndev->ntb); + + amd_deinit_side_info(ndev); + ntb_peer_db_set(&ndev->ntb, BIT_ULL(ndev->db_last_bit)); + ntb_unregister_device(&ndev->ntb); + ndev_deinit_debugfs(ndev); + amd_deinit_dev(ndev); + amd_ntb_deinit_pci(ndev); + kfree(ndev); +} + static const struct file_operations amd_ntb_debugfs_info = { .owner = THIS_MODULE, .open = simple_open, @@ -1326,6 +1342,7 @@ static struct pci_driver amd_ntb_pci_driver = { .id_table = amd_ntb_pci_tbl, .probe = amd_ntb_pci_probe, .remove = amd_ntb_pci_remove, + .shutdown = amd_ntb_pci_shutdown, }; static int __init amd_ntb_pci_driver_init(void) -- cgit v1.2.3