* [net-next 6/9] ixgbe: fix X540 Completion timeout
From: Jeff Kirsher @ 2014-11-07 8:57 UTC (permalink / raw)
To: davem; +Cc: Don Skidmore, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1415350670-15333-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Don Skidmore <donald.c.skidmore@intel.com>
On topologies including few levels of PCIe switching X540 can run into an
unexpected completion error. We get around this by waiting after enabling
loopback a sufficient amount of time until Tx Data Fetch is sent. We then
poll the pending transaction bit to ensure we received the completion. Only
then do we go on to clear the buffers.
Signed-of-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index b5f484b..e314b53 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -3583,7 +3583,8 @@ s32 ixgbe_set_fw_drv_ver_generic(struct ixgbe_hw *hw, u8 maj, u8 min,
**/
void ixgbe_clear_tx_pending(struct ixgbe_hw *hw)
{
- u32 gcr_ext, hlreg0;
+ u32 gcr_ext, hlreg0, i, poll;
+ u16 value;
/*
* If double reset is not requested then all transactions should
@@ -3600,6 +3601,24 @@ void ixgbe_clear_tx_pending(struct ixgbe_hw *hw)
hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0 | IXGBE_HLREG0_LPBK);
+ /* wait for a last completion before clearing buffers */
+ IXGBE_WRITE_FLUSH(hw);
+ usleep_range(3000, 6000);
+
+ /* Before proceeding, make sure that the PCIe block does not have
+ * transactions pending.
+ */
+ poll = ixgbe_pcie_timeout_poll(hw);
+ for (i = 0; i < poll; i++) {
+ usleep_range(100, 200);
+ value = ixgbe_read_pci_cfg_word(hw, IXGBE_PCI_DEVICE_STATUS);
+ if (ixgbe_removed(hw->hw_addr))
+ goto out;
+ if (!(value & IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING))
+ goto out;
+ }
+
+out:
/* initiate cleaning flow for buffers in the PCIe transaction layer */
gcr_ext = IXGBE_READ_REG(hw, IXGBE_GCR_EXT);
IXGBE_WRITE_REG(hw, IXGBE_GCR_EXT,
--
1.9.3
^ permalink raw reply related
* [net-next 1/9] i40e: poll firmware slower
From: Jeff Kirsher @ 2014-11-07 8:57 UTC (permalink / raw)
To: davem; +Cc: Kamil Krawczyk, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1415350670-15333-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Kamil Krawczyk <kamil.krawczyk@intel.com>
The code was polling the firmware tail register for completion every
10 microseconds, which is way faster than the firmware can respond.
This changes the poll interval to 1ms, which reduces polling CPU
utilization, and the number of times we loop.
The maximum delay is still 100ms.
Change-ID: I4bbfa6b66d802890baf8b4154061e55942b90958
Signed-off-by: Kamil Krawczyk <kamil.krawczyk@intel.com>
Acked-by: Shannon Nelson <shannon.nelson@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 5 ++---
drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +-
drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 5 ++---
drivers/net/ethernet/intel/i40evf/i40e_adminq.h | 2 +-
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index 72f5d25..057b7bf 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -853,7 +853,6 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
*/
if (!details->async && !details->postpone) {
u32 total_delay = 0;
- u32 delay_len = 10;
do {
/* AQ designers suggest use of head for better
@@ -862,8 +861,8 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
if (i40e_asq_done(hw))
break;
/* ugh! delay while spin_lock */
- udelay(delay_len);
- total_delay += delay_len;
+ usleep_range(1000, 2000);
+ total_delay++;
} while (total_delay < hw->aq.asq_cmd_timeout);
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
index ba38a89..df0bd09 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
@@ -141,7 +141,7 @@ static inline int i40e_aq_rc_to_posix(u16 aq_rc)
/* general information */
#define I40E_AQ_LARGE_BUF 512
-#define I40E_ASQ_CMD_TIMEOUT 100000 /* usecs */
+#define I40E_ASQ_CMD_TIMEOUT 100 /* msecs */
void i40e_fill_default_direct_cmd_desc(struct i40e_aq_desc *desc,
u16 opcode);
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
index f206be9..25c846b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
@@ -801,7 +801,6 @@ i40e_status i40evf_asq_send_command(struct i40e_hw *hw,
*/
if (!details->async && !details->postpone) {
u32 total_delay = 0;
- u32 delay_len = 10;
do {
/* AQ designers suggest use of head for better
@@ -810,8 +809,8 @@ i40e_status i40evf_asq_send_command(struct i40e_hw *hw,
if (i40evf_asq_done(hw))
break;
/* ugh! delay while spin_lock */
- udelay(delay_len);
- total_delay += delay_len;
+ usleep_range(1000, 2000);
+ total_delay++;
} while (total_delay < hw->aq.asq_cmd_timeout);
}
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
index 91a5c5b..f40cfac 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
@@ -141,7 +141,7 @@ static inline int i40e_aq_rc_to_posix(u16 aq_rc)
/* general information */
#define I40E_AQ_LARGE_BUF 512
-#define I40E_ASQ_CMD_TIMEOUT 100000 /* usecs */
+#define I40E_ASQ_CMD_TIMEOUT 100 /* msecs */
void i40evf_fill_default_direct_cmd_desc(struct i40e_aq_desc *desc,
u16 opcode);
--
1.9.3
^ permalink raw reply related
* [net-next 2/9] i40e: don't do link_status or stats collection on every ARQ
From: Jeff Kirsher @ 2014-11-07 8:57 UTC (permalink / raw)
To: davem
Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, Patrick Lu,
Jeff Kirsher
In-Reply-To: <1415350670-15333-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Shannon Nelson <shannon.nelson@intel.com>
The ARQ events cause a service_task execution, and we do a link_status
check and full stats gathering for each service_task. However, when
there are a lot of ARQ events, such as when doing an NVM update, we end up
doing 10's if not 100's of these per second, thereby heavily abusing the
PCI bus and especially the Firmware. This patch adds a check to keep the
service_task from running these periodic tasks more than once per second,
while still allowing quick action to service the events.
Change-ID: Iec7670c37bfae9791c43fec26df48aea7f70b33e
Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Signed-off-by: Patrick Lu <patrick.lu@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 3 ++-
drivers/net/ethernet/intel/i40e/i40e_main.c | 14 ++++++++++----
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index f1e33f8..b7a807b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -269,7 +269,8 @@ struct i40e_pf {
u16 msg_enable;
char misc_int_name[IFNAMSIZ + 9];
u16 adminq_work_limit; /* num of admin receive queue desc to process */
- int service_timer_period;
+ unsigned long service_timer_period;
+ unsigned long service_timer_previous;
struct timer_list service_timer;
struct work_struct service_task;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1a98e23..de66463 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5449,7 +5449,7 @@ static void i40e_check_hang_subtask(struct i40e_pf *pf)
}
/**
- * i40e_watchdog_subtask - Check and bring link up
+ * i40e_watchdog_subtask - periodic checks not using event driven response
* @pf: board private structure
**/
static void i40e_watchdog_subtask(struct i40e_pf *pf)
@@ -5461,6 +5461,15 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
test_bit(__I40E_CONFIG_BUSY, &pf->state))
return;
+ /* make sure we don't do these things too often */
+ if (time_before(jiffies, (pf->service_timer_previous +
+ pf->service_timer_period)))
+ return;
+ pf->service_timer_previous = jiffies;
+
+ i40e_check_hang_subtask(pf);
+ i40e_link_event(pf);
+
/* Update the stats for active netdevs so the network stack
* can look at updated numbers whenever it cares to
*/
@@ -6325,15 +6334,12 @@ static void i40e_service_task(struct work_struct *work)
i40e_vc_process_vflr_event(pf);
i40e_watchdog_subtask(pf);
i40e_fdir_reinit_subtask(pf);
- i40e_check_hang_subtask(pf);
i40e_sync_filters_subtask(pf);
#ifdef CONFIG_I40E_VXLAN
i40e_sync_vxlan_filters_subtask(pf);
#endif
i40e_clean_adminq_subtask(pf);
- i40e_link_event(pf);
-
i40e_service_event_complete(pf);
/* If the tasks have taken longer than one timer cycle or there
--
1.9.3
^ permalink raw reply related
* [PATCHv4 1/1] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Lothar Waßmann @ 2014-11-07 9:02 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Russell King, Frank Li, Fabio Estevam,
linux-kernel, Lothar Waßmann, linux-arm-kernel
In-Reply-To: <1415350967-2238-1-git-send-email-LW@KARO-electronics.de>
commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
introduced a regression for i.MX28. The swap_buffer() function doing
the endian conversion of the received data on i.MX28 may access memory
beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
does not copy those bytes, so that the last bytes of a packet may be
filled with invalid data after swapping.
This will likely lead to checksum errors on received packets.
E.g. when trying to mount an NFS rootfs:
UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
Do the byte swapping and copying to the new skb in one go if
necessary.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec_main.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c27128d..3dca494 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -298,6 +298,16 @@ static void *swap_buffer(void *bufaddr, int len)
return bufaddr;
}
+static void swap_buffer2(void *dst_buf, void *src_buf, int len)
+{
+ int i;
+ unsigned int *src = src_buf;
+ unsigned int *dst = dst_buf;
+
+ for (i = 0; i < len; i += 4, src++, dst++)
+ *dst = swab32p(src);
+}
+
static void fec_dump(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
@@ -1307,7 +1317,7 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff
}
static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
- struct bufdesc *bdp, u32 length)
+ struct bufdesc *bdp, u32 length, bool swap)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct sk_buff *new_skb;
@@ -1322,7 +1332,10 @@ static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
FEC_ENET_RX_FRSIZE - fep->rx_align,
DMA_FROM_DEVICE);
- memcpy(new_skb->data, (*skb)->data, length);
+ if (!swap)
+ memcpy(new_skb->data, (*skb)->data, length);
+ else
+ swap_buffer2(new_skb->data, (*skb)->data, length);
*skb = new_skb;
return true;
@@ -1352,6 +1365,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
u16 vlan_tag;
int index = 0;
bool is_copybreak;
+ bool need_swap = id_entry->driver_data & FEC_QUIRK_SWAP_FRAME;
#ifdef CONFIG_M532x
flush_cache_all();
@@ -1415,7 +1429,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
* include that when passing upstream as it messes up
* bridging applications.
*/
- is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4);
+ is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4,
+ need_swap);
if (!is_copybreak) {
skb_new = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
if (unlikely(!skb_new)) {
@@ -1430,7 +1445,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
prefetch(skb->data - NET_IP_ALIGN);
skb_put(skb, pkt_len - 4);
data = skb->data;
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (!is_copybreak && need_swap)
swap_buffer(data, pkt_len);
/* Extract the enhanced buffer descriptor */
--
1.7.10.4
^ permalink raw reply related
* [PATCHv4 0/1] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Lothar Waßmann @ 2014-11-07 9:02 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Russell King, Frank Li, Fabio Estevam,
linux-kernel, Lothar Waßmann, linux-arm-kernel
Changes wrt. v1:
- added some cleanup patches
- simplify handling of 'quirks' flags as suggested by Russell King.
- remove DIV_ROUND_UP() from byte swapping loop as suggested by
Eric Dumazet
Changes wrt. v2:
- rebased against next-20141028
- added some more cleanups in fec.h
- removed unused return value from swap_buffer()
- fixed messed swab32s() call in swap_buffer2()
- fixed messed up setup of fep->quirks
Changes wrt. v3:
- removed the cleanup patches; will send later to net-next,
when the bugfix patch has been applied
^ permalink raw reply
* Re: M_CAN message RAM initialization AppNote - was: Re: [PATCH V3 3/3] can: m_can: workaround for transmit data less than 4 bytes
From: Dong Aisheng @ 2014-11-07 8:40 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Oliver Hartkopp, linux-can, wg, varkabhadram, netdev,
linux-arm-kernel
In-Reply-To: <545B6DD8.6010906@pengutronix.de>
On Thu, Nov 06, 2014 at 01:47:20PM +0100, Marc Kleine-Budde wrote:
> On 11/06/2014 01:33 PM, Oliver Hartkopp wrote:
> >> So i'm not sure memset() the entire TX FIFO element is neccesary...
> >
> > It's no big deal - so we should be defensive here.
> > And memset() is not working as Marc pointed out in another mail.
> >
> > So we would need to loop with
> >
> > m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i), 0x0);
> >
> >>
> >> Do you think we could keep the current solution firstly and updated later
> >> if needed?
> >
> > No :-)
> >
> > I would like to have all data bytes to be written at startup.
>
> Me, too. As this happens only once during ifconfig up it should not hurt
> performance, either send an incremental or new patch. I'll sort it out.
>
I will send a new patch for this.
Regards
Dong Aisheng
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Zoltan Kiss @ 2014-11-07 9:25 UTC (permalink / raw)
To: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel, Stefan Bader, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <20141106214940.GD44162@ubuntu-hedt>
Hi,
AFAIK in this scenario your skb frag is wrong. The page pointer should
point to the original compound page (not a member of it), and offset
should be set accordingly.
For example, if your compound page is 16K (4 page), then the page
pointer should point to the first page, and if the data starts at the
3rd page, then offset should be >8K
Zoli
On 06/11/14 21:49, Seth Forshee wrote:
> We've had several reports of hitting the following BUG_ON in
> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
> results of testing with 3.17):
>
> /* Grant backend access to each skb fragment page. */
> for (i = 0; i < frags; i++) {
> skb_frag_t *frag = skb_shinfo(skb)->frags + i;
> struct page *page = skb_frag_page(frag);
>
> len = skb_frag_size(frag);
> offset = frag->page_offset;
>
> /* Data must not cross a page boundary. */
> BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
>
> When this happens the page in question is a "middle" page in a compound
> page (i.e. it's a tail page but not the last tail page), and the data is
> fully contained within the compound page. The data does however cross
> the hardware page boundary, and since compound_order evaluates to 0 for
> tail pages the check fails.
>
> In going over this I've been unable to determine whether the BUG_ON in
> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
> find that it's documented anywhere, and the networking code itself is a
> bit ambiguous when it comes to compound pages. On the one hand
> __skb_fill_page_desc specifically handles adding tail pages as paged
> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
> fail with data that extends into another page.
>
> Can anyone explain what the rules are here? My best guess based on
> skb_copy_bits is that paged data should never cross the hardware page
> boundary, but I'm not really sure how all of this works out when dealing
> with compound pages.
>
> Thanks,
> Seth
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply
* RE: [net-next 1/9] i40e: poll firmware slower
From: David Laight @ 2014-11-07 9:40 UTC (permalink / raw)
To: 'Jeff Kirsher', davem@davemloft.net
Cc: Kamil Krawczyk, netdev@vger.kernel.org, nhorman@redhat.com,
sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <1415350670-15333-2-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher
> From: Kamil Krawczyk <kamil.krawczyk@intel.com>
>
> The code was polling the firmware tail register for completion every
> 10 microseconds, which is way faster than the firmware can respond.
> This changes the poll interval to 1ms, which reduces polling CPU
> utilization, and the number of times we loop.
Are you sure the code path is allowed to sleep?
> The maximum delay is still 100ms.
Actually it is now likely to be up to 200ms or more.
You could convert the maximum delay check to one that
looks at jiffies - but maybe it doesn't matter.
> Change-ID: I4bbfa6b66d802890baf8b4154061e55942b90958
> Signed-off-by: Kamil Krawczyk <kamil.krawczyk@intel.com>
> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> Tested-by: Jim Young <jamesx.m.young@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_adminq.c | 5 ++---
> drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +-
> drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 5 ++---
> drivers/net/ethernet/intel/i40evf/i40e_adminq.h | 2 +-
> 4 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
> b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
> index 72f5d25..057b7bf 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
> @@ -853,7 +853,6 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
> */
> if (!details->async && !details->postpone) {
> u32 total_delay = 0;
> - u32 delay_len = 10;
>
> do {
> /* AQ designers suggest use of head for better
> @@ -862,8 +861,8 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
> if (i40e_asq_done(hw))
> break;
> /* ugh! delay while spin_lock */
The comment is not right any more.
> - udelay(delay_len);
> - total_delay += delay_len;
> + usleep_range(1000, 2000);
> + total_delay++;
> } while (total_delay < hw->aq.asq_cmd_timeout);
> }
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> index ba38a89..df0bd09 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> @@ -141,7 +141,7 @@ static inline int i40e_aq_rc_to_posix(u16 aq_rc)
>
> /* general information */
> #define I40E_AQ_LARGE_BUF 512
> -#define I40E_ASQ_CMD_TIMEOUT 100000 /* usecs */
> +#define I40E_ASQ_CMD_TIMEOUT 100 /* msecs */
It looks like this value is written to asq_cmd_timeout, that makes
be wonder whether anything else can change it - otherwise the compile
time constant would be used.
Changing the units has broken anything else that modifies the value.
>
> void i40e_fill_default_direct_cmd_desc(struct i40e_aq_desc *desc,
> u16 opcode);
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
> b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
> index f206be9..25c846b 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.c
> @@ -801,7 +801,6 @@ i40e_status i40evf_asq_send_command(struct i40e_hw *hw,
> */
> if (!details->async && !details->postpone) {
> u32 total_delay = 0;
> - u32 delay_len = 10;
>
> do {
> /* AQ designers suggest use of head for better
> @@ -810,8 +809,8 @@ i40e_status i40evf_asq_send_command(struct i40e_hw *hw,
> if (i40evf_asq_done(hw))
> break;
> /* ugh! delay while spin_lock */
> - udelay(delay_len);
> - total_delay += delay_len;
> + usleep_range(1000, 2000);
> + total_delay++;
> } while (total_delay < hw->aq.asq_cmd_timeout);
> }
>
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
> b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
> index 91a5c5b..f40cfac 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_adminq.h
> @@ -141,7 +141,7 @@ static inline int i40e_aq_rc_to_posix(u16 aq_rc)
>
> /* general information */
> #define I40E_AQ_LARGE_BUF 512
> -#define I40E_ASQ_CMD_TIMEOUT 100000 /* usecs */
> +#define I40E_ASQ_CMD_TIMEOUT 100 /* msecs */
>
> void i40evf_fill_default_direct_cmd_desc(struct i40e_aq_desc *desc,
> u16 opcode);
> --
> 1.9.3
Hmmm.... two drivers containing the same code....
David
^ permalink raw reply
* [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Dong Aisheng @ 2014-11-07 8:45 UTC (permalink / raw)
To: linux-can
Cc: mkl, wg, varkabhadram, netdev, socketcan, b29396,
linux-arm-kernel
In-Reply-To: <1415349914-9145-1-git-send-email-b29396@freescale.com>
The M_CAN message RAM is usually equipped with a parity or ECC functionality.
But RAM cells suffer a hardware reset and can therefore hold arbitrary
content at startup - including parity and/or ECC bits.
To prevent the M_CAN controller detecting checksum errors when reading
potentially uninitialized TX message RAM content to transmit CAN
frames the TX message RAM has to be written with (any kind of) initial
data.
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
ChangeLog:
v3-v4:
* initialize the entire Message RAM in use instead of only 8 bytes
since this is a valid and needed initialization process.
Patch title also changed.
---
drivers/net/can/m_can/m_can.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index eee1533..de742d0 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
struct resource *res;
void __iomem *addr;
u32 out_val[MRAM_CFG_LEN];
- int ret;
+ int i, start, end, ret;
/* message ram could be shared */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
@@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
+ /* initialize the entire Message RAM in use to avoid possible
+ * ECC/parity checksum errors when reading an uninitialized buffer
+ */
+ start = priv->mcfg[MRAM_SIDF].off;
+ end = priv->mcfg[MRAM_TXB].off +
+ priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
+ for (i = start; i < end; i += 4)
+ writel(0x0, priv->mram_base + i);
+
return 0;
}
--
1.9.1
^ permalink raw reply related
* Re: How to make stack send broadcast ARP request when entry is STALE?
From: Brian Haley @ 2014-11-07 9:54 UTC (permalink / raw)
To: Ulf samuelsson; +Cc: Netdev
In-Reply-To: <C4AF677B-55AC-4CE0-9BFB-EBB2E0C3BF74@emagii.com>
On 11/05/2014 07:48 AM, Ulf samuelsson wrote:
> Have a problem with an HP router at a certain location, which
> is configured to only answer to broadcast ARP requests.
> That cannot be changed.
Sorry to hear about the problem, but my only suggestions would be to try
the latest firmware and/or put a call in to support. I don't happen
work in the division that makes routers...
> The first ARP request the kernel sends out, is a broadcast request,
> which is fine, but after the reply, the kernel sends unicast requests,
> which will not get any replies.
You might be able to hack this by inserting an ebtables rule - check the
dnat target section of the man page - don't know the exact syntax but it
would probably end in '-j dnat --to-destination ff:ff:ff:ff:ff:ff'
-Brian
^ permalink raw reply
* [PATCH net-next 0/2] r8152: adjust rx functions
From: Hayes Wang @ 2014-11-07 9:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
Adjust some flows and codes which are relative to r8152_submit_rx()
and rtl_start_rx().
Hayes Wang (2):
r8152: adjust r8152_submit_rx
r8152: adjust rtl_start_rx
drivers/net/usb/r8152.c | 51 +++++++++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 23 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH net-next 1/2] r8152: adjust r8152_submit_rx
From: Hayes Wang @ 2014-11-07 9:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-88-Taiwan-albertk@realtek.com>
The behavior of handling the returned status from r8152_submit_rx()
is almost same, so let r8152_submit_rx() deal with the error
directly. This could avoid the duplicate code.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 39 +++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 66b139a..ad62994 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1032,7 +1032,6 @@ static void read_bulk_callback(struct urb *urb)
int status = urb->status;
struct rx_agg *agg;
struct r8152 *tp;
- int result;
agg = urb->context;
if (!agg)
@@ -1083,16 +1082,7 @@ static void read_bulk_callback(struct urb *urb)
break;
}
- result = r8152_submit_rx(tp, agg, GFP_ATOMIC);
- if (result == -ENODEV) {
- set_bit(RTL8152_UNPLUG, &tp->flags);
- netif_device_detach(tp->netdev);
- } else if (result) {
- spin_lock(&tp->rx_lock);
- list_add_tail(&agg->list, &tp->rx_done);
- spin_unlock(&tp->rx_lock);
- tasklet_schedule(&tp->tl);
- }
+ r8152_submit_rx(tp, agg, GFP_ATOMIC);
}
static void write_bulk_callback(struct urb *urb)
@@ -1681,7 +1671,6 @@ static void rx_bottom(struct r8152 *tp)
int len_used = 0;
struct urb *urb;
u8 *rx_data;
- int ret;
list_del_init(cursor);
@@ -1734,13 +1723,7 @@ find_next_rx:
}
submit:
- ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
- if (ret && ret != -ENODEV) {
- spin_lock_irqsave(&tp->rx_lock, flags);
- list_add_tail(&agg->list, &tp->rx_done);
- spin_unlock_irqrestore(&tp->rx_lock, flags);
- tasklet_schedule(&tp->tl);
- }
+ r8152_submit_rx(tp, agg, GFP_ATOMIC);
}
}
@@ -1805,11 +1788,27 @@ static void bottom_half(unsigned long data)
static
int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
{
+ int ret = 0;
+
usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
agg->head, agg_buf_sz,
(usb_complete_t)read_bulk_callback, agg);
- return usb_submit_urb(agg->urb, mem_flags);
+ ret = usb_submit_urb(agg->urb, mem_flags);
+
+ if (ret == -ENODEV) {
+ set_bit(RTL8152_UNPLUG, &tp->flags);
+ netif_device_detach(tp->netdev);
+ } else if (ret) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&tp->rx_lock, flags);
+ list_add_tail(&agg->list, &tp->rx_done);
+ spin_unlock_irqrestore(&tp->rx_lock, flags);
+ tasklet_schedule(&tp->tl);
+ }
+
+ return ret;
}
static void rtl_drop_queued_tx(struct r8152 *tp)
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 2/2] r8152: adjust rtl_start_rx
From: Hayes Wang @ 2014-11-07 9:55 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-88-Taiwan-albertk@realtek.com>
Submit all the rx buffers, even though a error occurs. Otherwise
the buffers which are not submitted would be lost until next
rtl_start_rx() is called. Besides, the fail buffer could be
re-submitted later.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ad62994..5e0386f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1993,10 +1993,16 @@ static int rtl_start_rx(struct r8152 *tp)
INIT_LIST_HEAD(&tp->rx_done);
for (i = 0; i < RTL8152_MAX_RX; i++) {
+ int rr;
+
INIT_LIST_HEAD(&tp->rx_info[i].list);
- ret = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
- if (ret)
- break;
+
+ rr = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
+ if (rr)
+ netif_err(tp, rx_err, tp->netdev,
+ "Couldn't submit rx[%d], ret = %d\n", i, rr);
+ if (!ret)
+ ret = rr;
}
return ret;
--
1.9.3
^ permalink raw reply related
* RE: [PATCH net-next v2 2/3] r8152: clear theflagofSCHEDULE_TASKLETin tasklet
From: Hayes Wang @ 2014-11-07 10:05 UTC (permalink / raw)
To: Francois Romieu
Cc: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
nic_swsd, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20141102225307.GA19900-lmTtMILVy1jWQcoT9B9Ug5SCg42XY1Uw0E9HWUfgJXw@public.gmane.org>
Francois Romieu [mailto:romieu-W8zweXLXuWQS+FvcfC7Uqw@public.gmane.org]
> Sent: Monday, November 03, 2014 6:53 AM
[...]
> test_and_clear_bit (dense) or clear_bit would be more idiomatic.
Excuse me. Any suggestion?
Should I use clear_bit directly, or something else?
Or, do I have to remove this patch?
Best Regards,
Hayes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: How to make stack send broadcast ARP request when entry is STALE?
From: Ulf samuelsson @ 2014-11-07 10:11 UTC (permalink / raw)
To: Brian Haley; +Cc: Netdev
In-Reply-To: <545C96D6.5020204@hp.com>
The HP router is configured by a customer, and they intentionally limit replies
to broadcast, and that is how they want it.
In the previous version of the build system, the Interpeak stack was used
and this would in PROBE state send unicast ARP request, and if that failed
send broadcast ARP.
The native linux stack, when in PROBE state sends only unicast until it decides
that it should enter FAILED state.
The 'mcast_probes' variable seems to be totally ignored, except the first time,
so I do not see why it is there.
Best Regards
Ulf Samuelsson
ulf@emagii.com
+46 (722) 427 437
> 7 nov 2014 kl. 10:54 skrev Brian Haley <brian.haley@hp.com>:
>
>> On 11/05/2014 07:48 AM, Ulf samuelsson wrote:
>> Have a problem with an HP router at a certain location, which
>> is configured to only answer to broadcast ARP requests.
>> That cannot be changed.
>
> Sorry to hear about the problem, but my only suggestions would be to try the latest firmware and/or put a call in to support. I don't happen work in the division that makes routers...
>
>> The first ARP request the kernel sends out, is a broadcast request,
>> which is fine, but after the reply, the kernel sends unicast requests,
>> which will not get any replies.
>
> You might be able to hack this by inserting an ebtables rule - check the dnat target section of the man page - don't know the exact syntax but it would probably end in '-j dnat --to-destination ff:ff:ff:ff:ff:ff'
>
> -Brian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Dong Aisheng @ 2014-11-07 10:21 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-can, wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <545C9F4E.3040901@pengutronix.de>
On Fri, Nov 07, 2014 at 11:30:38AM +0100, Marc Kleine-Budde wrote:
> On 11/07/2014 09:45 AM, Dong Aisheng wrote:
> > The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> > But RAM cells suffer a hardware reset and can therefore hold arbitrary
> > content at startup - including parity and/or ECC bits.
> >
> > To prevent the M_CAN controller detecting checksum errors when reading
> > potentially uninitialized TX message RAM content to transmit CAN
> > frames the TX message RAM has to be written with (any kind of) initial
> > data.
> >
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > ---
> > ChangeLog:
> > v3-v4:
> > * initialize the entire Message RAM in use instead of only 8 bytes
> > since this is a valid and needed initialization process.
> > Patch title also changed.
> > ---
> > drivers/net/can/m_can/m_can.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index eee1533..de742d0 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> > struct resource *res;
> > void __iomem *addr;
> > u32 out_val[MRAM_CFG_LEN];
> > - int ret;
> > + int i, start, end, ret;
> >
> > /* message ram could be shared */
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> > @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> > priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
> > priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
> >
> > + /* initialize the entire Message RAM in use to avoid possible
> > + * ECC/parity checksum errors when reading an uninitialized buffer
> > + */
> > + start = priv->mcfg[MRAM_SIDF].off;
> > + end = priv->mcfg[MRAM_TXB].off +
> > + priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> > + for (i = start; i < end; i += 4)
> > + writel(0x0, priv->mram_base + i);
>
> Can we access the mram without turning on the clock?
>
Yes, we can.
Since it's CPU access, it seems it does not depend on M_CAN clocks.
What i see from M_CAN subsystem block diagram of MX6SX,
it requires CPU clock which should always be enabled.
Regards
Dong Aisheng
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
^ permalink raw reply
* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Marc Kleine-Budde @ 2014-11-07 10:30 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415349914-9145-3-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]
On 11/07/2014 09:45 AM, Dong Aisheng wrote:
> The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> But RAM cells suffer a hardware reset and can therefore hold arbitrary
> content at startup - including parity and/or ECC bits.
>
> To prevent the M_CAN controller detecting checksum errors when reading
> potentially uninitialized TX message RAM content to transmit CAN
> frames the TX message RAM has to be written with (any kind of) initial
> data.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> ChangeLog:
> v3-v4:
> * initialize the entire Message RAM in use instead of only 8 bytes
> since this is a valid and needed initialization process.
> Patch title also changed.
> ---
> drivers/net/can/m_can/m_can.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index eee1533..de742d0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> struct resource *res;
> void __iomem *addr;
> u32 out_val[MRAM_CFG_LEN];
> - int ret;
> + int i, start, end, ret;
>
> /* message ram could be shared */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
> priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
>
> + /* initialize the entire Message RAM in use to avoid possible
> + * ECC/parity checksum errors when reading an uninitialized buffer
> + */
> + start = priv->mcfg[MRAM_SIDF].off;
> + end = priv->mcfg[MRAM_TXB].off +
> + priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> + for (i = start; i < end; i += 4)
> + writel(0x0, priv->mram_base + i);
Can we access the mram without turning on the clock?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: BUG in xennet_make_frags with paged skb data
From: David Vrabel @ 2014-11-07 10:44 UTC (permalink / raw)
To: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
Stefan Bader, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <20141106214940.GD44162@ubuntu-hedt>
On 06/11/14 21:49, Seth Forshee wrote:
> We've had several reports of hitting the following BUG_ON in
> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
> results of testing with 3.17):
>
> /* Grant backend access to each skb fragment page. */
> for (i = 0; i < frags; i++) {
> skb_frag_t *frag = skb_shinfo(skb)->frags + i;
> struct page *page = skb_frag_page(frag);
>
> len = skb_frag_size(frag);
> offset = frag->page_offset;
>
> /* Data must not cross a page boundary. */
> BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
>
> When this happens the page in question is a "middle" page in a compound
> page (i.e. it's a tail page but not the last tail page), and the data is
> fully contained within the compound page. The data does however cross
> the hardware page boundary, and since compound_order evaluates to 0 for
> tail pages the check fails.
>
> In going over this I've been unable to determine whether the BUG_ON in
> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
> find that it's documented anywhere, and the networking code itself is a
> bit ambiguous when it comes to compound pages. On the one hand
> __skb_fill_page_desc specifically handles adding tail pages as paged
> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
> fail with data that extends into another page.
netfront will safely handle this case so you can remove this BUG_ON()
(and the one later on). But it would be better to find out were these
funny-looking skbs are coming from and (if necessary) fixing the bug there.
David
^ permalink raw reply
* Re: BUG in xennet_make_frags with paged skb data
From: Stefan Bader @ 2014-11-07 11:08 UTC (permalink / raw)
To: David Vrabel, netdev, David S. Miller, Konrad Rzeszutek Wilk,
Boris Ostrovsky, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545CA27F.4070400@citrix.com>
[-- Attachment #1: Type: text/plain, Size: 2802 bytes --]
On 07.11.2014 11:44, David Vrabel wrote:
> On 06/11/14 21:49, Seth Forshee wrote:
>> We've had several reports of hitting the following BUG_ON in
>> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
>> results of testing with 3.17):
>>
>> /* Grant backend access to each skb fragment page. */
>> for (i = 0; i < frags; i++) {
>> skb_frag_t *frag = skb_shinfo(skb)->frags + i;
>> struct page *page = skb_frag_page(frag);
>>
>> len = skb_frag_size(frag);
>> offset = frag->page_offset;
>>
>> /* Data must not cross a page boundary. */
>> BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
>>
>> When this happens the page in question is a "middle" page in a compound
>> page (i.e. it's a tail page but not the last tail page), and the data is
>> fully contained within the compound page. The data does however cross
>> the hardware page boundary, and since compound_order evaluates to 0 for
>> tail pages the check fails.
>>
>> In going over this I've been unable to determine whether the BUG_ON in
>> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
>> find that it's documented anywhere, and the networking code itself is a
>> bit ambiguous when it comes to compound pages. On the one hand
>> __skb_fill_page_desc specifically handles adding tail pages as paged
>> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
>> fail with data that extends into another page.
>
> netfront will safely handle this case so you can remove this BUG_ON()
> (and the one later on). But it would be better to find out were these
> funny-looking skbs are coming from and (if necessary) fixing the bug there.
Right, in fact it does ignore pages from the start of a compound page in case
the offset is big enough. So there is no difference between that case and the
one where the page pointer from the frag is adjusted the way it was observed.
It really boils down to the question what the real rules for the frags are. If
it is "only" that data may not cross the boundary of a hw or compound page this
leaves room for interpretation. The odd skb does not violate that but a check
for conformance would become a lot more complex. And netfront is not the only
place that expects the frag page to be pointing to the compound head (there is
igb for example, though it does only a warn_on upon failing).
On the other hand the __skb_fill_page_desc is written in a way that seems to
assume the frag page pointer may be pointing to a tail page. So before "blaming"
one piece of code or the other we need an authoritative answer to what we are
supposed to expect.
-Stefan
>
> David
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Eric Dumazet @ 2014-11-07 11:22 UTC (permalink / raw)
To: Zoltan Kiss
Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel, Stefan Bader, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545C9013.1090406@linaro.org>
On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
Please do not top post.
> Hi,
>
> AFAIK in this scenario your skb frag is wrong. The page pointer should
> point to the original compound page (not a member of it), and offset
> should be set accordingly.
> For example, if your compound page is 16K (4 page), then the page
> pointer should point to the first page, and if the data starts at the
> 3rd page, then offset should be >8K
This is not accurate.
This BUG_ON() is wrong.
It should instead be :
BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));
splice() code can generate such cases.
^ permalink raw reply
* [PATCH net 0/3] cxgb4/cxgb4vf: Misc. fixes for cxgb4vf
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
Hi,
For T5 use Packing and Padding Boundaries for SGE DMA transfers, move
fl_starve_thres to adpater structure, since they are different for each
adapter. The cxgb4vf driver's Free List Starvation Threshold needs to be larger
than the SGE's Egress Congestion Threshold or we'll end up in a mutual stall
where the driver waits for Ingress Packets to drive replacing Free List
Pointers and the SGE waits for Free List Pointers before pushing Ingress
Packets to the host.
The patches series is created against 'net' tree.
And includes patches on cxgb4 and cxgb4vf driver.
We have included all the maintainers of respective drivers. Kindly review the
change and let us know in case of any review comments.
Thanks
Hariprasad Shenai (3):
cxgb4vf: Move fl_starv_thres into adapter->sge data structure
cxgb4/cxgb4vf: For T5 use Packing and Padding Boundaries for SGE DMA
transfers
cxgb4vf: FL Starvation Threshold needs to be larger than the SGE's
Egress Congestion Threshold
drivers/net/ethernet/chelsio/cxgb4/sge.c | 30 ++++-
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 51 +++++++-
drivers/net/ethernet/chelsio/cxgb4/t4_regs.h | 10 ++
drivers/net/ethernet/chelsio/cxgb4vf/adapter.h | 8 +
drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 136 +++++++++++++-------
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h | 2 +
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c | 28 ++++-
7 files changed, 209 insertions(+), 56 deletions(-)
^ permalink raw reply
* [PATCH net 1/3] cxgb4vf: Move fl_starv_thres into adapter->sge data structure
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
In-Reply-To: <1415360191-25395-1-git-send-email-hariprasad@chelsio.com>
Move fl_starv_thres into adapter->sge data structure since it
_could_ be different from adapter to adapter. Also move other per-adapter
SGE values which had been treated as driver globals into adapter->sge.
Based on original work by Casey Leedom <leedom@chelsio.com>
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4vf/adapter.h | 8 ++
drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 93 ++++++++++++++----------
2 files changed, 61 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h b/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
index 68eaa9c..3d06e77 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
@@ -299,6 +299,14 @@ struct sge {
u16 timer_val[SGE_NTIMERS]; /* interrupt holdoff timer array */
u8 counter_val[SGE_NCOUNTERS]; /* interrupt RX threshold array */
+ /* Decoded Adapter Parameters.
+ */
+ u32 fl_pg_order; /* large page allocation size */
+ u32 stat_len; /* length of status page at ring end */
+ u32 pktshift; /* padding between CPL & packet data */
+ u32 fl_align; /* response queue message alignment */
+ u32 fl_starve_thres; /* Free List starvation threshold */
+
/*
* Reverse maps from Absolute Queue IDs to associated queue pointers.
* The absolute Queue IDs are in a compact range which start at a
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index 85036e6..a18830d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -51,14 +51,6 @@
#include "../cxgb4/t4_msg.h"
/*
- * Decoded Adapter Parameters.
- */
-static u32 FL_PG_ORDER; /* large page allocation size */
-static u32 STAT_LEN; /* length of status page at ring end */
-static u32 PKTSHIFT; /* padding between CPL and packet data */
-static u32 FL_ALIGN; /* response queue message alignment */
-
-/*
* Constants ...
*/
enum {
@@ -264,15 +256,19 @@ static inline unsigned int fl_cap(const struct sge_fl *fl)
/**
* fl_starving - return whether a Free List is starving.
+ * @adapter: pointer to the adapter
* @fl: the Free List
*
* Tests specified Free List to see whether the number of buffers
* available to the hardware has falled below our "starvation"
* threshold.
*/
-static inline bool fl_starving(const struct sge_fl *fl)
+static inline bool fl_starving(const struct adapter *adapter,
+ const struct sge_fl *fl)
{
- return fl->avail - fl->pend_cred <= FL_STARVE_THRES;
+ const struct sge *s = &adapter->sge;
+
+ return fl->avail - fl->pend_cred <= s->fl_starve_thres;
}
/**
@@ -457,13 +453,16 @@ static inline void reclaim_completed_tx(struct adapter *adapter,
/**
* get_buf_size - return the size of an RX Free List buffer.
+ * @adapter: pointer to the associated adapter
* @sdesc: pointer to the software buffer descriptor
*/
-static inline int get_buf_size(const struct rx_sw_desc *sdesc)
+static inline int get_buf_size(const struct adapter *adapter,
+ const struct rx_sw_desc *sdesc)
{
- return FL_PG_ORDER > 0 && (sdesc->dma_addr & RX_LARGE_BUF)
- ? (PAGE_SIZE << FL_PG_ORDER)
- : PAGE_SIZE;
+ const struct sge *s = &adapter->sge;
+
+ return (s->fl_pg_order > 0 && (sdesc->dma_addr & RX_LARGE_BUF)
+ ? (PAGE_SIZE << s->fl_pg_order) : PAGE_SIZE);
}
/**
@@ -483,7 +482,8 @@ static void free_rx_bufs(struct adapter *adapter, struct sge_fl *fl, int n)
if (is_buf_mapped(sdesc))
dma_unmap_page(adapter->pdev_dev, get_buf_addr(sdesc),
- get_buf_size(sdesc), PCI_DMA_FROMDEVICE);
+ get_buf_size(adapter, sdesc),
+ PCI_DMA_FROMDEVICE);
put_page(sdesc->page);
sdesc->page = NULL;
if (++fl->cidx == fl->size)
@@ -511,7 +511,8 @@ static void unmap_rx_buf(struct adapter *adapter, struct sge_fl *fl)
if (is_buf_mapped(sdesc))
dma_unmap_page(adapter->pdev_dev, get_buf_addr(sdesc),
- get_buf_size(sdesc), PCI_DMA_FROMDEVICE);
+ get_buf_size(adapter, sdesc),
+ PCI_DMA_FROMDEVICE);
sdesc->page = NULL;
if (++fl->cidx == fl->size)
fl->cidx = 0;
@@ -589,6 +590,7 @@ static inline void poison_buf(struct page *page, size_t sz)
static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
int n, gfp_t gfp)
{
+ struct sge *s = &adapter->sge;
struct page *page;
dma_addr_t dma_addr;
unsigned int cred = fl->avail;
@@ -608,12 +610,12 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
* If we don't support large pages, drop directly into the small page
* allocation code.
*/
- if (FL_PG_ORDER == 0)
+ if (s->fl_pg_order == 0)
goto alloc_small_pages;
while (n) {
page = alloc_pages(gfp | __GFP_COMP | __GFP_NOWARN,
- FL_PG_ORDER);
+ s->fl_pg_order);
if (unlikely(!page)) {
/*
* We've failed inour attempt to allocate a "large
@@ -623,10 +625,10 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
fl->large_alloc_failed++;
break;
}
- poison_buf(page, PAGE_SIZE << FL_PG_ORDER);
+ poison_buf(page, PAGE_SIZE << s->fl_pg_order);
dma_addr = dma_map_page(adapter->pdev_dev, page, 0,
- PAGE_SIZE << FL_PG_ORDER,
+ PAGE_SIZE << s->fl_pg_order,
PCI_DMA_FROMDEVICE);
if (unlikely(dma_mapping_error(adapter->pdev_dev, dma_addr))) {
/*
@@ -637,7 +639,7 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
* because DMA mapping resources are typically
* critical resources once they become scarse.
*/
- __free_pages(page, FL_PG_ORDER);
+ __free_pages(page, s->fl_pg_order);
goto out;
}
dma_addr |= RX_LARGE_BUF;
@@ -693,7 +695,7 @@ out:
fl->pend_cred += cred;
ring_fl_db(adapter, fl);
- if (unlikely(fl_starving(fl))) {
+ if (unlikely(fl_starving(adapter, fl))) {
smp_wmb();
set_bit(fl->cntxt_id, adapter->sge.starving_fl);
}
@@ -1468,6 +1470,8 @@ static void t4vf_pktgl_free(const struct pkt_gl *gl)
static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
const struct cpl_rx_pkt *pkt)
{
+ struct adapter *adapter = rxq->rspq.adapter;
+ struct sge *s = &adapter->sge;
int ret;
struct sk_buff *skb;
@@ -1478,8 +1482,8 @@ static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
return;
}
- copy_frags(skb, gl, PKTSHIFT);
- skb->len = gl->tot_len - PKTSHIFT;
+ copy_frags(skb, gl, s->pktshift);
+ skb->len = gl->tot_len - s->pktshift;
skb->data_len = skb->len;
skb->truesize += skb->data_len;
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -1516,6 +1520,8 @@ int t4vf_ethrx_handler(struct sge_rspq *rspq, const __be64 *rsp,
bool csum_ok = pkt->csum_calc && !pkt->err_vec &&
(rspq->netdev->features & NETIF_F_RXCSUM);
struct sge_eth_rxq *rxq = container_of(rspq, struct sge_eth_rxq, rspq);
+ struct adapter *adapter = rspq->adapter;
+ struct sge *s = &adapter->sge;
/*
* If this is a good TCP packet and we have Generic Receive Offload
@@ -1537,7 +1543,7 @@ int t4vf_ethrx_handler(struct sge_rspq *rspq, const __be64 *rsp,
rxq->stats.rx_drops++;
return 0;
}
- __skb_pull(skb, PKTSHIFT);
+ __skb_pull(skb, s->pktshift);
skb->protocol = eth_type_trans(skb, rspq->netdev);
skb_record_rx_queue(skb, rspq->idx);
rxq->stats.pkts++;
@@ -1648,6 +1654,8 @@ static inline void rspq_next(struct sge_rspq *rspq)
static int process_responses(struct sge_rspq *rspq, int budget)
{
struct sge_eth_rxq *rxq = container_of(rspq, struct sge_eth_rxq, rspq);
+ struct adapter *adapter = rspq->adapter;
+ struct sge *s = &adapter->sge;
int budget_left = budget;
while (likely(budget_left)) {
@@ -1697,7 +1705,7 @@ static int process_responses(struct sge_rspq *rspq, int budget)
BUG_ON(frag >= MAX_SKB_FRAGS);
BUG_ON(rxq->fl.avail == 0);
sdesc = &rxq->fl.sdesc[rxq->fl.cidx];
- bufsz = get_buf_size(sdesc);
+ bufsz = get_buf_size(adapter, sdesc);
fp->page = sdesc->page;
fp->offset = rspq->offset;
fp->size = min(bufsz, len);
@@ -1726,7 +1734,7 @@ static int process_responses(struct sge_rspq *rspq, int budget)
*/
ret = rspq->handler(rspq, rspq->cur_desc, &gl);
if (likely(ret == 0))
- rspq->offset += ALIGN(fp->size, FL_ALIGN);
+ rspq->offset += ALIGN(fp->size, s->fl_align);
else
restore_rx_bufs(&gl, &rxq->fl, frag);
} else if (likely(rsp_type == RSP_TYPE_CPL)) {
@@ -1963,7 +1971,7 @@ static void sge_rx_timer_cb(unsigned long data)
* schedule napi but the FL is no longer starving.
* No biggie.
*/
- if (fl_starving(fl)) {
+ if (fl_starving(adapter, fl)) {
struct sge_eth_rxq *rxq;
rxq = container_of(fl, struct sge_eth_rxq, fl);
@@ -2047,6 +2055,7 @@ int t4vf_sge_alloc_rxq(struct adapter *adapter, struct sge_rspq *rspq,
int intr_dest,
struct sge_fl *fl, rspq_handler_t hnd)
{
+ struct sge *s = &adapter->sge;
struct port_info *pi = netdev_priv(dev);
struct fw_iq_cmd cmd, rpl;
int ret, iqandst, flsz = 0;
@@ -2117,7 +2126,7 @@ int t4vf_sge_alloc_rxq(struct adapter *adapter, struct sge_rspq *rspq,
fl->size = roundup(fl->size, FL_PER_EQ_UNIT);
fl->desc = alloc_ring(adapter->pdev_dev, fl->size,
sizeof(__be64), sizeof(struct rx_sw_desc),
- &fl->addr, &fl->sdesc, STAT_LEN);
+ &fl->addr, &fl->sdesc, s->stat_len);
if (!fl->desc) {
ret = -ENOMEM;
goto err;
@@ -2129,7 +2138,7 @@ int t4vf_sge_alloc_rxq(struct adapter *adapter, struct sge_rspq *rspq,
* free list ring) in Egress Queue Units.
*/
flsz = (fl->size / FL_PER_EQ_UNIT +
- STAT_LEN / EQ_UNIT);
+ s->stat_len / EQ_UNIT);
/*
* Fill in all the relevant firmware Ingress Queue Command
@@ -2217,6 +2226,7 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
struct net_device *dev, struct netdev_queue *devq,
unsigned int iqid)
{
+ struct sge *s = &adapter->sge;
int ret, nentries;
struct fw_eq_eth_cmd cmd, rpl;
struct port_info *pi = netdev_priv(dev);
@@ -2225,7 +2235,7 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
* Calculate the size of the hardware TX Queue (including the Status
* Page on the end of the TX Queue) in units of TX Descriptors.
*/
- nentries = txq->q.size + STAT_LEN / sizeof(struct tx_desc);
+ nentries = txq->q.size + s->stat_len / sizeof(struct tx_desc);
/*
* Allocate the hardware ring for the TX ring (with space for its
@@ -2234,7 +2244,7 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
txq->q.desc = alloc_ring(adapter->pdev_dev, txq->q.size,
sizeof(struct tx_desc),
sizeof(struct tx_sw_desc),
- &txq->q.phys_addr, &txq->q.sdesc, STAT_LEN);
+ &txq->q.phys_addr, &txq->q.sdesc, s->stat_len);
if (!txq->q.desc)
return -ENOMEM;
@@ -2307,8 +2317,10 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
*/
static void free_txq(struct adapter *adapter, struct sge_txq *tq)
{
+ struct sge *s = &adapter->sge;
+
dma_free_coherent(adapter->pdev_dev,
- tq->size * sizeof(*tq->desc) + STAT_LEN,
+ tq->size * sizeof(*tq->desc) + s->stat_len,
tq->desc, tq->phys_addr);
tq->cntxt_id = 0;
tq->sdesc = NULL;
@@ -2322,6 +2334,7 @@ static void free_txq(struct adapter *adapter, struct sge_txq *tq)
static void free_rspq_fl(struct adapter *adapter, struct sge_rspq *rspq,
struct sge_fl *fl)
{
+ struct sge *s = &adapter->sge;
unsigned int flid = fl ? fl->cntxt_id : 0xffff;
t4vf_iq_free(adapter, FW_IQ_TYPE_FL_INT_CAP,
@@ -2337,7 +2350,7 @@ static void free_rspq_fl(struct adapter *adapter, struct sge_rspq *rspq,
if (fl) {
free_rx_bufs(adapter, fl, fl->avail);
dma_free_coherent(adapter->pdev_dev,
- fl->size * sizeof(*fl->desc) + STAT_LEN,
+ fl->size * sizeof(*fl->desc) + s->stat_len,
fl->desc, fl->addr);
kfree(fl->sdesc);
fl->sdesc = NULL;
@@ -2443,12 +2456,12 @@ int t4vf_sge_init(struct adapter *adapter)
* Now translate the adapter parameters into our internal forms.
*/
if (fl1)
- FL_PG_ORDER = ilog2(fl1) - PAGE_SHIFT;
- STAT_LEN = ((sge_params->sge_control & EGRSTATUSPAGESIZE_MASK)
- ? 128 : 64);
- PKTSHIFT = PKTSHIFT_GET(sge_params->sge_control);
- FL_ALIGN = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
- SGE_INGPADBOUNDARY_SHIFT);
+ s->fl_pg_order = ilog2(fl1) - PAGE_SHIFT;
+ s->stat_len = ((sge_params->sge_control & EGRSTATUSPAGESIZE_MASK)
+ ? 128 : 64);
+ s->pktshift = PKTSHIFT_GET(sge_params->sge_control);
+ s->fl_align = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
+ SGE_INGPADBOUNDARY_SHIFT);
/*
* Set up tasklet timers.
--
1.7.1
^ permalink raw reply related
* [PATCH net 2/3] cxgb4/cxgb4vf: For T5 use Packing and Padding Boundaries for SGE DMA transfers
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
In-Reply-To: <1415360191-25395-1-git-send-email-hariprasad@chelsio.com>
T5 introduces the ability to have separate Packing and Padding Boundaries
for SGE DMA transfers from the chip to Host Memory. This change set takes
advantage of that to set up a smaller Padding Boundary to conserve PCI Link
and Memory Bandwidth with T5.
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/sge.c | 30 ++++++++++-
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 51 +++++++++++++++++--
drivers/net/ethernet/chelsio/cxgb4/t4_regs.h | 10 ++++
drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 31 +++++++++++-
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h | 1 +
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c | 23 +++++++++
6 files changed, 135 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 5e1b314..39f2b13 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2914,7 +2914,8 @@ static int t4_sge_init_hard(struct adapter *adap)
int t4_sge_init(struct adapter *adap)
{
struct sge *s = &adap->sge;
- u32 sge_control, sge_conm_ctrl;
+ u32 sge_control, sge_control2, sge_conm_ctrl;
+ unsigned int ingpadboundary, ingpackboundary;
int ret, egress_threshold;
/*
@@ -2924,8 +2925,31 @@ int t4_sge_init(struct adapter *adap)
sge_control = t4_read_reg(adap, SGE_CONTROL);
s->pktshift = PKTSHIFT_GET(sge_control);
s->stat_len = (sge_control & EGRSTATUSPAGESIZE_MASK) ? 128 : 64;
- s->fl_align = 1 << (INGPADBOUNDARY_GET(sge_control) +
- X_INGPADBOUNDARY_SHIFT);
+
+ /* T4 uses a single control field to specify both the PCIe Padding and
+ * Packing Boundary. T5 introduced the ability to specify these
+ * separately. The actual Ingress Packet Data alignment boundary
+ * within Packed Buffer Mode is the maximum of these two
+ * specifications.
+ */
+ ingpadboundary = 1 << (INGPADBOUNDARY_GET(sge_control) +
+ X_INGPADBOUNDARY_SHIFT);
+ if (is_t4(adap->params.chip)) {
+ s->fl_align = ingpadboundary;
+ } else {
+ /* T5 has a different interpretation of one of the PCIe Packing
+ * Boundary values.
+ */
+ sge_control2 = t4_read_reg(adap, SGE_CONTROL2_A);
+ ingpackboundary = INGPACKBOUNDARY_G(sge_control2);
+ if (ingpackboundary == INGPACKBOUNDARY_16B_X)
+ ingpackboundary = 16;
+ else
+ ingpackboundary = 1 << (ingpackboundary +
+ INGPACKBOUNDARY_SHIFT_X);
+
+ s->fl_align = max(ingpadboundary, ingpackboundary);
+ }
if (adap->flags & USING_SOFT_PARAMS)
ret = t4_sge_init_soft(adap);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index a9d9d74..163a2a1 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -3129,12 +3129,51 @@ int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
HOSTPAGESIZEPF6(sge_hps) |
HOSTPAGESIZEPF7(sge_hps));
- t4_set_reg_field(adap, SGE_CONTROL,
- INGPADBOUNDARY_MASK |
- EGRSTATUSPAGESIZE_MASK,
- INGPADBOUNDARY(fl_align_log - 5) |
- EGRSTATUSPAGESIZE(stat_len != 64));
-
+ if (is_t4(adap->params.chip)) {
+ t4_set_reg_field(adap, SGE_CONTROL,
+ INGPADBOUNDARY_MASK |
+ EGRSTATUSPAGESIZE_MASK,
+ INGPADBOUNDARY(fl_align_log - 5) |
+ EGRSTATUSPAGESIZE(stat_len != 64));
+ } else {
+ /* T5 introduced the separation of the Free List Padding and
+ * Packing Boundaries. Thus, we can select a smaller Padding
+ * Boundary to avoid uselessly chewing up PCIe Link and Memory
+ * Bandwidth, and use a Packing Boundary which is large enough
+ * to avoid false sharing between CPUs, etc.
+ *
+ * For the PCI Link, the smaller the Padding Boundary the
+ * better. For the Memory Controller, a smaller Padding
+ * Boundary is better until we cross under the Memory Line
+ * Size (the minimum unit of transfer to/from Memory). If we
+ * have a Padding Boundary which is smaller than the Memory
+ * Line Size, that'll involve a Read-Modify-Write cycle on the
+ * Memory Controller which is never good. For T5 the smallest
+ * Padding Boundary which we can select is 32 bytes which is
+ * larger than any known Memory Controller Line Size so we'll
+ * use that.
+ *
+ * T5 has a different interpretation of the "0" value for the
+ * Packing Boundary. This corresponds to 16 bytes instead of
+ * the expected 32 bytes. We never have a Packing Boundary
+ * less than 32 bytes so we can't use that special value but
+ * on the other hand, if we wanted 32 bytes, the best we can
+ * really do is 64 bytes.
+ */
+ if (fl_align <= 32) {
+ fl_align = 64;
+ fl_align_log = 6;
+ }
+ t4_set_reg_field(adap, SGE_CONTROL,
+ INGPADBOUNDARY_MASK |
+ EGRSTATUSPAGESIZE_MASK,
+ INGPADBOUNDARY(INGPCIEBOUNDARY_32B_X) |
+ EGRSTATUSPAGESIZE(stat_len != 64));
+ t4_set_reg_field(adap, SGE_CONTROL2_A,
+ INGPACKBOUNDARY_V(INGPACKBOUNDARY_M),
+ INGPACKBOUNDARY_V(fl_align_log -
+ INGPACKBOUNDARY_SHIFT_X));
+ }
/*
* Adjust various SGE Free List Host Buffer Sizes.
*
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h b/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h
index a1024db..8d2de10 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h
@@ -95,6 +95,7 @@
#define X_INGPADBOUNDARY_SHIFT 5
#define SGE_CONTROL 0x1008
+#define SGE_CONTROL2_A 0x1124
#define DCASYSTYPE 0x00080000U
#define RXPKTCPLMODE_MASK 0x00040000U
#define RXPKTCPLMODE_SHIFT 18
@@ -106,6 +107,7 @@
#define PKTSHIFT_SHIFT 10
#define PKTSHIFT(x) ((x) << PKTSHIFT_SHIFT)
#define PKTSHIFT_GET(x) (((x) & PKTSHIFT_MASK) >> PKTSHIFT_SHIFT)
+#define INGPCIEBOUNDARY_32B_X 0
#define INGPCIEBOUNDARY_MASK 0x00000380U
#define INGPCIEBOUNDARY_SHIFT 7
#define INGPCIEBOUNDARY(x) ((x) << INGPCIEBOUNDARY_SHIFT)
@@ -114,6 +116,14 @@
#define INGPADBOUNDARY(x) ((x) << INGPADBOUNDARY_SHIFT)
#define INGPADBOUNDARY_GET(x) (((x) & INGPADBOUNDARY_MASK) \
>> INGPADBOUNDARY_SHIFT)
+#define INGPACKBOUNDARY_16B_X 0
+#define INGPACKBOUNDARY_SHIFT_X 5
+
+#define INGPACKBOUNDARY_S 16
+#define INGPACKBOUNDARY_M 0x7U
+#define INGPACKBOUNDARY_V(x) ((x) << INGPACKBOUNDARY_S)
+#define INGPACKBOUNDARY_G(x) (((x) >> INGPACKBOUNDARY_S) \
+ & INGPACKBOUNDARY_M)
#define EGRPCIEBOUNDARY_MASK 0x0000000eU
#define EGRPCIEBOUNDARY_SHIFT 1
#define EGRPCIEBOUNDARY(x) ((x) << EGRPCIEBOUNDARY_SHIFT)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index a18830d..cd5b789 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -2436,6 +2436,7 @@ int t4vf_sge_init(struct adapter *adapter)
u32 fl0 = sge_params->sge_fl_buffer_size[0];
u32 fl1 = sge_params->sge_fl_buffer_size[1];
struct sge *s = &adapter->sge;
+ unsigned int ingpadboundary, ingpackboundary;
/*
* Start by vetting the basic SGE parameters which have been set up by
@@ -2460,8 +2461,34 @@ int t4vf_sge_init(struct adapter *adapter)
s->stat_len = ((sge_params->sge_control & EGRSTATUSPAGESIZE_MASK)
? 128 : 64);
s->pktshift = PKTSHIFT_GET(sge_params->sge_control);
- s->fl_align = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
- SGE_INGPADBOUNDARY_SHIFT);
+
+ /* T4 uses a single control field to specify both the PCIe Padding and
+ * Packing Boundary. T5 introduced the ability to specify these
+ * separately. The actual Ingress Packet Data alignment boundary
+ * within Packed Buffer Mode is the maximum of these two
+ * specifications. (Note that it makes no real practical sense to
+ * have the Pading Boudary be larger than the Packing Boundary but you
+ * could set the chip up that way and, in fact, legacy T4 code would
+ * end doing this because it would initialize the Padding Boundary and
+ * leave the Packing Boundary initialized to 0 (16 bytes).)
+ */
+ ingpadboundary = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
+ X_INGPADBOUNDARY_SHIFT);
+ if (is_t4(adapter->params.chip)) {
+ s->fl_align = ingpadboundary;
+ } else {
+ /* T5 has a different interpretation of one of the PCIe Packing
+ * Boundary values.
+ */
+ ingpackboundary = INGPACKBOUNDARY_G(sge_params->sge_control2);
+ if (ingpackboundary == INGPACKBOUNDARY_16B_X)
+ ingpackboundary = 16;
+ else
+ ingpackboundary = 1 << (ingpackboundary +
+ INGPACKBOUNDARY_SHIFT_X);
+
+ s->fl_align = max(ingpadboundary, ingpackboundary);
+ }
/*
* Set up tasklet timers.
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
index 95df61d..b5c301d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
@@ -134,6 +134,7 @@ struct dev_params {
*/
struct sge_params {
u32 sge_control; /* padding, boundaries, lengths, etc. */
+ u32 sge_control2; /* T5: more of the same */
u32 sge_host_page_size; /* RDMA page sizes */
u32 sge_queues_per_page; /* RDMA queues/page */
u32 sge_user_mode_limits; /* limits for BAR2 user mode accesses */
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
index e984fdc..dc30d28 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
@@ -468,6 +468,29 @@ int t4vf_get_sge_params(struct adapter *adapter)
sge_params->sge_timer_value_2_and_3 = vals[5];
sge_params->sge_timer_value_4_and_5 = vals[6];
+ /* T4 uses a single control field to specify both the PCIe Padding and
+ * Packing Boundary. T5 introduced the ability to specify these
+ * separately with the Padding Boundary in SGE_CONTROL and and Packing
+ * Boundary in SGE_CONTROL2. So for T5 and later we need to grab
+ * SGE_CONTROL in order to determine how ingress packet data will be
+ * laid out in Packed Buffer Mode. Unfortunately, older versions of
+ * the firmware won't let us retrieve SGE_CONTROL2 so if we get a
+ * failure grabbing it we throw an error since we can't figure out the
+ * right value.
+ */
+ if (!is_t4(adapter->params.chip)) {
+ params[0] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+ FW_PARAMS_PARAM_XYZ(SGE_CONTROL2_A));
+ v = t4vf_query_params(adapter, 1, params, vals);
+ if (v != FW_SUCCESS) {
+ dev_err(adapter->pdev_dev,
+ "Unable to get SGE Control2; "
+ "probably old firmware.\n");
+ return v;
+ }
+ sge_params->sge_control2 = vals[0];
+ }
+
params[0] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
FW_PARAMS_PARAM_XYZ(SGE_INGRESS_RX_THRESHOLD));
v = t4vf_query_params(adapter, 1, params, vals);
--
1.7.1
^ permalink raw reply related
* [PATCH net 3/3] cxgb4vf: FL Starvation Threshold needs to be larger than the SGE's Egress Congestion Threshold
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
In-Reply-To: <1415360191-25395-1-git-send-email-hariprasad@chelsio.com>
Free List Starvation Threshold needs to be larger than the SGE's Egress
Congestion Threshold or we'll end up in a mutual stall where the driver waits
for Ingress Packets to drive replacing Free List Pointers and the SGE waits for
Free List Pointers before pushing Ingress Packets to the host.
Based on original work by Casey Leedom <leedom@chelsio.com>
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 16 ++++++++++------
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h | 1 +
drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c | 5 ++++-
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index cd5b789..fdd078d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -94,12 +94,6 @@ enum {
MAX_TIMER_TX_RECLAIM = 100,
/*
- * An FL with <= FL_STARVE_THRES buffers is starving and a periodic
- * timer will attempt to refill it.
- */
- FL_STARVE_THRES = 4,
-
- /*
* Suspend an Ethernet TX queue with fewer available descriptors than
* this. We always want to have room for a maximum sized packet:
* inline immediate data + MAX_SKB_FRAGS. This is the same as
@@ -2490,6 +2484,16 @@ int t4vf_sge_init(struct adapter *adapter)
s->fl_align = max(ingpadboundary, ingpackboundary);
}
+ /* A FL with <= fl_starve_thres buffers is starving and a periodic
+ * timer will attempt to refill it. This needs to be larger than the
+ * SGE's Egress Congestion Threshold. If it isn't, then we can get
+ * stuck waiting for new packets while the SGE is waiting for us to
+ * give it more Free List entries. (Note that the SGE's Egress
+ * Congestion Threshold is in units of 2 Free List pointers.)
+ */
+ s->fl_starve_thres
+ = EGRTHRESHOLD_GET(sge_params->sge_congestion_control)*2 + 1;
+
/*
* Set up tasklet timers.
*/
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
index b5c301d..4b6a6d1 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
@@ -140,6 +140,7 @@ struct sge_params {
u32 sge_user_mode_limits; /* limits for BAR2 user mode accesses */
u32 sge_fl_buffer_size[16]; /* free list buffer sizes */
u32 sge_ingress_rx_threshold; /* RX counter interrupt threshold[4] */
+ u32 sge_congestion_control; /* congestion thresholds, etc. */
u32 sge_timer_value_0_and_1; /* interrupt coalescing timer values */
u32 sge_timer_value_2_and_3;
u32 sge_timer_value_4_and_5;
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
index dc30d28..1e896b9 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
@@ -493,10 +493,13 @@ int t4vf_get_sge_params(struct adapter *adapter)
params[0] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
FW_PARAMS_PARAM_XYZ(SGE_INGRESS_RX_THRESHOLD));
- v = t4vf_query_params(adapter, 1, params, vals);
+ params[1] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+ FW_PARAMS_PARAM_XYZ(SGE_CONM_CTRL));
+ v = t4vf_query_params(adapter, 2, params, vals);
if (v)
return v;
sge_params->sge_ingress_rx_threshold = vals[0];
+ sge_params->sge_congestion_control = vals[1];
return 0;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Oliver Hartkopp @ 2014-11-07 11:53 UTC (permalink / raw)
To: Dong Aisheng, linux-can; +Cc: mkl, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <1415349914-9145-3-git-send-email-b29396@freescale.com>
Thanks!
Hopefully we can manage to push the patch series via can/master ;-)
Regards,
Oliver
On 07.11.2014 09:45, Dong Aisheng wrote:
> The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> But RAM cells suffer a hardware reset and can therefore hold arbitrary
> content at startup - including parity and/or ECC bits.
>
> To prevent the M_CAN controller detecting checksum errors when reading
> potentially uninitialized TX message RAM content to transmit CAN
> frames the TX message RAM has to be written with (any kind of) initial
> data.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> ChangeLog:
> v3-v4:
> * initialize the entire Message RAM in use instead of only 8 bytes
> since this is a valid and needed initialization process.
> Patch title also changed.
> ---
> drivers/net/can/m_can/m_can.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index eee1533..de742d0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> struct resource *res;
> void __iomem *addr;
> u32 out_val[MRAM_CFG_LEN];
> - int ret;
> + int i, start, end, ret;
>
> /* message ram could be shared */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
> priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
>
> + /* initialize the entire Message RAM in use to avoid possible
> + * ECC/parity checksum errors when reading an uninitialized buffer
> + */
> + start = priv->mcfg[MRAM_SIDF].off;
> + end = priv->mcfg[MRAM_TXB].off +
> + priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> + for (i = start; i < end; i += 4)
> + writel(0x0, priv->mram_base + i);
> +
> return 0;
> }
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox