| Message ID | 20241018101059.1718375-3-shaojijie@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | There are some bugfix for the HNS3 ethernet driver | expand |
On 10/18/24 12:10, Jijie Shao wrote: > From: Jian Shen <shenjian15@huawei.com> > > To avoid errors in pgtable prefectch, add a sync command to sync > io-pagtable. > > In the case of large traffic, the TX bounce buffer may be used up. It's unclear to me what do you mean for large traffic. Is that large packets instead? Skimming over the previous patch, it looks like the for the bugger H/W driver will use the bounce buffer for all packets with len < 64K. As this driver does not support big tcp, such condition means all packets. So its not clear to me the 'may' part - it looks like the critical path will always happen on the bugged H/W > At this point, we go to mapping/unmapping on TX path again. > So we added the sync command in driver to avoid hardware issue. I thought the goal of the previous patch was to avoid such sync-up. So I don't understand why it's there. A more verbose explanation will help. > Signed-off-by: Jian Shen <shenjian15@huawei.com> > Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com> > Signed-off-by: Jijie Shao <shaojijie@huawei.com> Also we need a fixes tag. Thanks, Paolo
on 2024/10/24 16:36, Paolo Abeni wrote: > On 10/18/24 12:10, Jijie Shao wrote: >> From: Jian Shen <shenjian15@huawei.com> >> >> To avoid errors in pgtable prefectch, add a sync command to sync >> io-pagtable. >> >> In the case of large traffic, the TX bounce buffer may be used up. > It's unclear to me what do you mean for large traffic. Is that large > packets instead? > > Skimming over the previous patch, it looks like the for the bugger H/W > driver will use the bounce buffer for all packets with len < 64K. As > this driver does not support big tcp, such condition means all packets. > > So its not clear to me the 'may' part - it looks like the critical path > will always happen on the bugged H/W Sorry, actually not, I mean with tools like iperf3, we can hit the speed limit. In this case, many packets are sent within a short period of time. Therefore, the TX bounce buffer may be used up. In this case, mapping/unmapping is used for packets that cannot use the TX bounce buffer. Thanks, Jijie Shao > >> At this point, we go to mapping/unmapping on TX path again. >> So we added the sync command in driver to avoid hardware issue. > I thought the goal of the previous patch was to avoid such sync-up. > > So I don't understand why it's there. > > A more verbose explanation will help. > >> Signed-off-by: Jian Shen <shenjian15@huawei.com> >> Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com> >> Signed-off-by: Jijie Shao <shaojijie@huawei.com> > Also we need a fixes tag. > > Thanks, > > Paolo > >
在 2024/10/24 16:36, Paolo Abeni 写道: > On 10/18/24 12:10, Jijie Shao wrote: >> From: Jian Shen <shenjian15@huawei.com> >> >> To avoid errors in pgtable prefectch, add a sync command to sync >> io-pagtable. >> >> In the case of large traffic, the TX bounce buffer may be used up. > It's unclear to me what do you mean for large traffic. Is that large > packets instead? > > Skimming over the previous patch, it looks like the for the bugger H/W > driver will use the bounce buffer for all packets with len < 64K. As > this driver does not support big tcp, such condition means all packets. > > So its not clear to me the 'may' part - it looks like the critical path > will always happen on the bugged H/W Sorry for the unclear commit log. Yes, we don't support big tcp, so <64K is worked for all packets. The large traffic here is just want to describe a case that tx bounce buffer is used up, and there is no enough space for new tx packets. >> At this point, we go to mapping/unmapping on TX path again. >> So we added the sync command in driver to avoid hardware issue. > I thought the goal of the previous patch was to avoid such sync-up. > > So I don't understand why it's there. > > A more verbose explanation will help. This is a supplement for the previous patch. We want all the tx packet can be handled with tx bounce buffer path. But it depends on the remain space of the spare buffer, checked by the function hns3_can_use_tx_bounce(). In most cases, maybe 99.99%, it returns true. But once it return false by no available space, the packet will be handled with the former path, which will map/unmap the skb buffer. Then we will face the smmu prefetch risk again. So I add a sync command in this case to avoid smmu prefectch, just protects corner scenes. >> Signed-off-by: Jian Shen <shenjian15@huawei.com> >> Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com> >> Signed-off-by: Jijie Shao <shaojijie@huawei.com> > Also we need a fixes tag. We considered this issue, and since this is not a software defect, we were not too sure which commit should be blamed. It makes sense to choose the commit introducing the support for the buggy H/W, we will add it. Thanks! Jian Shen > Thanks, > > Paolo > > > . >
On 10/24/24 11:38, shenjian (K) wrote: > > 在 2024/10/24 16:36, Paolo Abeni 写道: >> On 10/18/24 12:10, Jijie Shao wrote: >>> From: Jian Shen <shenjian15@huawei.com> >>> >>> To avoid errors in pgtable prefectch, add a sync command to sync >>> io-pagtable. >>> >>> In the case of large traffic, the TX bounce buffer may be used up. >> It's unclear to me what do you mean for large traffic. Is that large >> packets instead? >> >> Skimming over the previous patch, it looks like the for the bugger H/W >> driver will use the bounce buffer for all packets with len < 64K. As >> this driver does not support big tcp, such condition means all packets. >> >> So its not clear to me the 'may' part - it looks like the critical path >> will always happen on the bugged H/W > > Sorry for the unclear commit log. > > Yes, we don't support big tcp, so <64K is worked for all packets. The > large traffic > > here is just want to describe a case that tx bounce buffer is used up, > and there is > > no enough space for new tx packets. > > >>> At this point, we go to mapping/unmapping on TX path again. >>> So we added the sync command in driver to avoid hardware issue. >> I thought the goal of the previous patch was to avoid such sync-up. >> >> So I don't understand why it's there. >> >> A more verbose explanation will help. > > This is a supplement for the previous patch. We want all the tx packet can > > be handled with tx bounce buffer path. But it depends on the remain space > > of the spare buffer, checked by the function hns3_can_use_tx_bounce(). In > > most cases, maybe 99.99%, it returns true. But once it return false by no > > available space, the packet will be handled with the former path, which > > will map/unmap the skb buffer. Then we will face the smmu prefetch risk > again. > > So I add a sync command in this case to avoid smmu prefectch, > > just protects corner scenes. > > >>> Signed-off-by: Jian Shen <shenjian15@huawei.com> >>> Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com> >>> Signed-off-by: Jijie Shao <shaojijie@huawei.com> >> Also we need a fixes tag. > > We considered this issue, and since this is not a software defect, we > were not too > > sure which commit should be blamed. > > It makes sense to choose the commit introducing the support for the > buggy H/W, we will add > > it. Please additionally rephrase the commit message including the more verbose explanation above, thanks! Paolo
on 2024/10/24 19:05, Paolo Abeni wrote: > On 10/24/24 11:38, shenjian (K) wrote: >> 在 2024/10/24 16:36, Paolo Abeni 写道: >>> On 10/18/24 12:10, Jijie Shao wrote: >> We considered this issue, and since this is not a software defect, we >> were not too >> >> sure which commit should be blamed. >> >> It makes sense to choose the commit introducing the support for the >> buggy H/W, we will add >> >> it. > Please additionally rephrase the commit message including the more > verbose explanation above, thanks! > > Paolo ok, Thanks!
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index ac88e301f221..8760b4e9ade6 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -381,6 +381,24 @@ static const struct hns3_rx_ptype hns3_rx_ptype_tbl[] = { #define HNS3_INVALID_PTYPE \ ARRAY_SIZE(hns3_rx_ptype_tbl) +static void hns3_dma_map_sync(struct device *dev, unsigned long iova) +{ + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + struct iommu_iotlb_gather iotlb_gather; + size_t granule; + + if (!domain || !iommu_is_dma_domain(domain)) + return; + + granule = 1 << __ffs(domain->pgsize_bitmap); + iova = ALIGN_DOWN(iova, granule); + iotlb_gather.start = iova; + iotlb_gather.end = iova + granule - 1; + iotlb_gather.pgsize = granule; + + iommu_iotlb_sync(domain, &iotlb_gather); +} + static irqreturn_t hns3_irq_handle(int irq, void *vector) { struct hns3_enet_tqp_vector *tqp_vector = vector; @@ -1728,7 +1746,9 @@ static int hns3_map_and_fill_desc(struct hns3_enet_ring *ring, void *priv, unsigned int type) { struct hns3_desc_cb *desc_cb = &ring->desc_cb[ring->next_to_use]; + struct hnae3_handle *handle = ring->tqp->handle; struct device *dev = ring_to_dev(ring); + struct hnae3_ae_dev *ae_dev; unsigned int size; dma_addr_t dma; @@ -1760,6 +1780,13 @@ static int hns3_map_and_fill_desc(struct hns3_enet_ring *ring, void *priv, return -ENOMEM; } + /* Add a SYNC command to sync io-pgtale to avoid errors in pgtable + * prefetch + */ + ae_dev = hns3_get_ae_dev(handle); + if (ae_dev->dev_version >= HNAE3_DEVICE_VERSION_V3) + hns3_dma_map_sync(dev, dma); + desc_cb->priv = priv; desc_cb->length = size; desc_cb->dma = dma;