- Linuxarm - mailweb.openeuler.org

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Alex Williamson 14 May '21

14 May '21

On Thu, 13 May 2021 15:49:25 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.thodi(a)huawei.com> wrote: > > -----Original Message----- > > From: Jason Gunthorpe [mailto:jgg@nvidia.com] > > Sent: 13 May 2021 14:44 > > To: liulongfang <liulongfang(a)huawei.com> > > Cc: Alex Williamson <alex.williamson(a)redhat.com>; cohuck(a)redhat.com; > > linux-kernel(a)vger.kernel.org; linuxarm(a)openeuler.org > > Subject: [Linuxarm] Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to > > vfio > > > > On Thu, May 13, 2021 at 10:08:28AM +0800, liulongfang wrote: > > > On 2021/5/12 20:10, Jason Gunthorpe wrote: > > > > On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > > > > > > > >> Therefore, this method of limiting the length of the BAR > > > >> configuration space can prevent unsafe operations of the memory. > > > > > > > > The issue is DMA controlled by the guest accessing the secure BAR > > > > area, not the guest CPU. > > > > > > > > Jason > > > > . > > > > > > > This secure BAR area is not presented to the Guest, > > > which makes it impossible for the Guest to obtain the secure BAR area > > > when establishing the DMA mapping of the configuration space. > > > If the DMA controller accesses the secure BAR area, the access will > > > be blocked by the SMMU. > > > > There are scenarios where this is not true. > > > > At a minimum the mdev driver should refuse to work in those cases. > > > > Hi, > > I think the idea here is not a generic solution, but a quirk for this specific dev. > > Something like, > > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -866,7 +866,12 @@ static long vfio_pci_ioctl(struct vfio_device *core_vdev, > break; > case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX: > info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index); > - info.size = pci_resource_len(pdev, info.index); > + > + if (check_hisi_acc_quirk(pdev, info)) > + info.size = new_size;// BAR is limited without migration region. > + else > + info.size = pci_resource_len(pdev, info.index); > + > if (!info.size) { > info.flags = 0; > break; > > Is this an acceptable/workable solution here? As Jason says, this only restricts CPU access to the BAR, the issue is DMA access. As the hardware vendor you may be able to guarantee that a DMA transaction generated by the device targeting the remainder of the BAR will always go upstream, but can you guarantee the routing between the device and the SMMU? For instance if this device can be implemented as a plugin card, then it can be installed into a downstream port that may not support ACS. That downstream port may implement request redirection allowing the transaction to reflect back to the device without IOMMU translation. At that point the userspace driver can target the kernel driver half of the BAR and potentially expose a security risk. Thanks, Alex

1 0

[PATCH net v7 0/3] fix packet stuck problem for lockless qdisc
by Yunsheng Lin 13 May '21

13 May '21

This patchset fixes the packet stuck problem mentioned in [1]. Patch 1: Add STATE_MISSED flag to fix packet stuck problem. Patch 2: Fix a tx_action rescheduling problem after STATE_MISSED flag is added in patch 1. Patch 3: Fix the significantly higher CPU consumption problem when multiple threads are competing on a saturated outgoing device. V7: Fix netif_tx_wake_queue() data race noted by Jakub. V6: Some performance optimization in patch 1 suggested by Jakub and drop NET_XMIT_DROP checking in patch 3. V5: add patch 3 to fix the problem reported by Michal Kubecek. V4: Change STATE_NEED_RESCHEDULE to STATE_MISSED and add patch 2. [1]. https://lkml.org/lkml/2019/10/9/42 Yunsheng Lin (3): net: sched: fix packet stuck problem for lockless qdisc net: sched: fix endless tx action reschedule during deactivation net: sched: fix tx action reschedule issue with stopped queue include/net/pkt_sched.h | 7 +------ include/net/sch_generic.h | 35 ++++++++++++++++++++++++++++++++- net/core/dev.c | 29 ++++++++++++++++++++++----- net/sched/sch_generic.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 107 insertions(+), 14 deletions(-) -- 2.7.4

2 5

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Jason Gunthorpe 13 May '21

13 May '21

On Thu, May 13, 2021 at 10:08:28AM +0800, liulongfang wrote: > On 2021/5/12 20:10, Jason Gunthorpe wrote: > > On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > > > >> Therefore, this method of limiting the length of the BAR > >> configuration space can prevent unsafe operations of the memory. > > > > The issue is DMA controlled by the guest accessing the secure BAR > > area, not the guest CPU. > > > > Jason > > . > > > This secure BAR area is not presented to the Guest, > which makes it impossible for the Guest to obtain the secure BAR area > when establishing the DMA mapping of the configuration space. > If the DMA controller accesses the secure BAR area, the access will > be blocked by the SMMU. There are scenarios where this is not true. At a minimum the mdev driver should refuse to work in those cases. Jason

1 0

[PATCH net v6 0/3] fix packet stuck problem for lockless qdisc
by Yunsheng Lin 13 May '21

13 May '21

This patchset fixes the packet stuck problem mentioned in [1]. Patch 1: Add STATE_MISSED flag to fix packet stuck problem. Patch 2: Fix a tx_action rescheduling problem after STATE_MISSED flag is added in patch 1. Patch 3: Fix the significantly higher CPU consumption problem when multiple threads are competing on a saturated outgoing device. V6: Some performance optimization in patch 1 suggested by Jakub and drop NET_XMIT_DROP checking in patch 3. V5: add patch 3 to fix the problem reported by Michal Kubecek. V4: Change STATE_NEED_RESCHEDULE to STATE_MISSED and add patch 2. [1]. https://lkml.org/lkml/2019/10/9/42 Yunsheng Lin (3): net: sched: fix packet stuck problem for lockless qdisc net: sched: fix endless tx action reschedule during deactivation net: sched: fix tx action reschedule issue with stopped queue include/net/pkt_sched.h | 7 +------ include/net/sch_generic.h | 33 ++++++++++++++++++++++++++++++++- net/core/dev.c | 29 ++++++++++++++++++++++++----- net/sched/sch_generic.c | 31 +++++++++++++++++++++++++++++-- 4 files changed, 86 insertions(+), 14 deletions(-) -- 2.7.4

2 9

Re: [Intel-wired-lan] [PATCH V3 net] ice: Re-organizes reqstd/avail {R, T}XQ check/code for efficiency
by Brelinski, TonyX 12 May '21

12 May '21

> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces(a)osuosl.org> On Behalf Of > Salil Mehta > Sent: Thursday, April 22, 2021 5:00 PM > To: davem(a)davemloft.net; kuba(a)kernel.org > Cc: salil.mehta(a)huawei.com; netdev(a)vger.kernel.org; > linuxarm(a)huawei.com; linuxarm(a)openeuler.org; linux- > kernel(a)vger.kernel.org; intel-wired-lan(a)lists.osuosl.org > Subject: [Intel-wired-lan] [PATCH V3 net] ice: Re-organizes reqstd/avail {R, > T}XQ check/code for efficiency > > If user has explicitly requested the number of {R,T}XQs, then it is > unnecessary to get the count of already available {R,T}XQs from the PF > avail_{r,t}xqs bitmap. This value will get overridden by user specified value in > any case. > > Re-organize this code for improving the flow, readability and efficiency. > This scope of improvement was found during the review of the ICE driver > code. > > Fixes: 87324e747fde ("ice: Implement ethtool ops for channels") > Cc: intel-wired-lan(a)lists.osuosl.org > Tested-by: Tony Brelinski <tonyx.brelinski(a)intel.com> > Signed-off-by: Salil Mehta <salil.mehta(a)huawei.com> > --- > Change: > V2->V3 > (*) Addressed some comments from Paul Menzel > Link: https://lkml.org/lkml/2021/4/21/136 > V1->V2 > (*) Fixed the comments from Anthony Nguyen(Intel) > Link: https://lkml.org/lkml/2021/4/12/1997 > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) Tested-by: Tony Brelinski <tonyx.brelinski(a)intel.com> (A Contingent Worker at Intel)

1 0

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Jason Gunthorpe 12 May '21

12 May '21

On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > Therefore, this method of limiting the length of the BAR > configuration space can prevent unsafe operations of the memory. The issue is DMA controlled by the guest accessing the secure BAR area, not the guest CPU. Jason

1 0

[PATCH 00/17] tty: Fix some coding style issues
by Xiaofei Tan 12 May '21

12 May '21

Fix some issues reported by checkpatch.pl. All of them are coding style issues, no function changes. Xiaofei Tan (17): tty: tty_baudrate: Remove unnecessary tab and spaces in comment sentence tty: tty_baudrate: Fix coding style issues of block comments tty: tty_buffer: Add a blank line after declarations tty: tty_buffer: Remove the repeated word 'the' tty: tty_buffer: Fix coding style issues of block comments tty: tty_io: Remove spaces before tabs tty: tty_io: Add a blank line after declarations tty: tty_io: Fix spaces required around that ':' tty: tty_io: Fix trailing whitespace issues tty: tty_io: Fix coding style issues of block comments tty: tty_io: Remove the repeated word 'can' tty: tty_io: Fix an issue of code indent for conditional statements tty: tty_io: Delete a blank line before EXPORT_SYMBOL(foo) tty: tty_io: Remove return in void function tty: tty_port: Delete a blank line before EXPORT_SYMBOL(foo) tty: tty_port: Add a blank line after declarations tty: tty_port: Fix coding style issues of block comments drivers/tty/tty_baudrate.c | 13 ++++++---- drivers/tty/tty_buffer.c | 20 +++++++++++---- drivers/tty/tty_io.c | 61 ++++++++++++++++++++++++++-------------------- drivers/tty/tty_port.c | 16 +++++++----- 4 files changed, 67 insertions(+), 43 deletions(-) -- 2.8.1

1 6

[PATCH] RDMA/ucma: Cleanup to reduce duplicate code
by Xiaofei Tan 12 May '21

12 May '21

The lable "err1" does the same thing as the branch of copy_to_user() failed in the function ucma_create_id(). Just jump to the label directly to reduce duplicate code. Signed-off-by: Xiaofei Tan <tanxiaofei(a)huawei.com> --- drivers/infiniband/core/ucma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 15d57ba..1f198c1 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -468,8 +468,8 @@ static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf, resp.id = ctx->id; if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof(resp))) { - ucma_destroy_private_ctx(ctx); - return -EFAULT; + ret = -EFAULT; + goto err1; } mutex_lock(&file->mut); -- 2.8.1

3 2

Re: [dpdk-dev] [PATCH v3] eal: fix use wrong time API
by Thomas Monjalon 11 May '21

11 May '21

05/05/2021 05:43, Chengwen Feng: > Currently, the mp uses gettimeofday() API to get the time, and used as > timeout parameter. > > But the time which gets from gettimeofday() API isn't monotonically > increasing. The process may fail if the system time is changed. > > This fixes it by using clock_gettime() API with monotonic attribution. > > Fixes: 783b6e54971d ("eal: add synchronous multi-process communication") > Fixes: f05e26051c15 ("eal: add IPC asynchronous request") > Cc: stable(a)dpdk.org > > Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> > Signed-off-by: Min Hu (Connor) <humin29(a)huawei.com> > Acked-by: Morten Brørup <mb(a)smartsharesystems.com> > --- > v3: > * add acked-by. > * change patch's author. I did some comments on v2 about potential errors to catch, but you sent this v3 without participating in v2 discussion.

2 1

Re: [PATCH net v5 1/3] net: sched: fix packet stuck problem for lockless qdisc
by Jakub Kicinski 08 May '21

08 May '21

On Sat, 8 May 2021 10:55:19 +0800 Yunsheng Lin wrote: > >> + * the flag set after releasing lock and reschedule the > >> + * net_tx_action() to do the dequeuing. > > > > I don't understand why MISSED is checked before the trylock. > > Could you explain why it can't be tested directly here? > The initial thinking was: > Just like the set_bit() before the second trylock, If MISSED is set > before first trylock, it means other thread has set the MISSED flag > for this thread before doing the first trylock, so that this thread > does not need to do the set_bit(). > > But the initial thinking seems over thinking, as thread 3' setting the > MISSED before the second trylock has ensure either thread 3' second > trylock returns ture or thread 2 holding the lock will see the MISSED > flag, so thread 1 can do the test_bit() before or after the first > trylock, as below: > > thread 1 thread 2 thread 3 > holding q->seqlock > first trylock failed first trylock failed > unlock q->seqlock > test_bit(MISSED) return false > test_bit(MISSED) return false > and not reschedule > set_bit(MISSED) > trylock success > test_bit(MISSED) retun ture > and not retry second trylock > > If the above is correct, it seems we could: > 1. do test_bit(MISSED) before the first trylock to avoid doing the > first trylock for contended case. > or > 2. do test_bit(MISSED) after the first trylock to avoid doing the > test_bit() for un-contended case. > > Which one do you prefer? No strong preference but testing after the trylock seems more obvious as it saves the temporary variable. For the contended case could we potentially move or add a MISSED test before even the first try_lock()? I'm not good at optimizing things, but it could save us the atomic op, right? (at least on x86)

1 0