Message ID | 20230410150130.837691-1-lulu@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1963162vqo; Mon, 10 Apr 2023 08:15:08 -0700 (PDT) X-Google-Smtp-Source: AKy350Y8xMcICTqKep6v0kW03zJWlYLoqhApZIF0n/qoxsyYYtLSefiQEHNRn6wRTbAOQP9zMeuq X-Received: by 2002:a62:5245:0:b0:635:e961:3350 with SMTP id g66-20020a625245000000b00635e9613350mr5762232pfb.19.1681139708630; Mon, 10 Apr 2023 08:15:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681139708; cv=none; d=google.com; s=arc-20160816; b=BIRthvGXG+TSM6s8LbjlhMCFWvhSzPJkICsPXu1wrbOLKr0NnMZF608mOoUMwyon21 I6TwyYwsUBaZiMwe5QZpx66xVgWFM8SxqDngYyKzwYhLTv/8Pj/+ayTyC9p9POnReBkz hIT2m45942HAk0bkjbVkH86exg49oZVWvXhb1Du0C5tOEh6mxcysarLcanEKTrqF761D Gl0RmumVdlQahf5nvanZJYaaPlyFkJPFdbMpTj6Diu22iFmU4cSlbvkxOg83qMCUmnkY rjRS6YvLG42XDMbnNRapewNw8puLcesGpyC9gRiO/0GWCFeWJ7cyYuJXPzXDNIvLOtxd /Tvg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:to:from:dkim-signature; bh=jvoKw5fddC6ImxYUVWRQGjUNBYWemWpfZTmvE3MZouI=; b=wBLL1NqiVDT5OAwvSqrg10SmfDT9JjlJKlC+uXHBpvvZonjSPr3SLRlMsT89Cm/n5N iSoQcvV2SvwXzLwGFcXKKTRcbVZUdM6Dt7djLG4ieVK1t8dc5gEKWn5881ggRTKaX7DZ AewHissMyYUcZW59LKZIWnIJ58mEE3l5xhpPNLqJHzGJEEfZ5FjsP4k3h8lsmHaSd3TM 4wp6cSoJqPCk0UC1NA092a1VWNCrQ2ik+q2ZT5R0t6xArmjlWb+WtMoXNSjv3mzx4PBF svOHfe3LXD7O66U3w9EO7pmf1hKIe+pRWxxl0hyNg6Xw82KQncYrjtzgdL110viS6Tnz JSCw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=J3fmotMV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id w4-20020aa79a04000000b0062b90348402si10906819pfj.106.2023.04.10.08.14.54; Mon, 10 Apr 2023 08:15:08 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=J3fmotMV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229962AbjDJPC2 (ORCPT <rfc822;yuanzuo1009@gmail.com> + 99 others); Mon, 10 Apr 2023 11:02:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52480 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229766AbjDJPC1 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 10 Apr 2023 11:02:27 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B15F4ED3 for <linux-kernel@vger.kernel.org>; Mon, 10 Apr 2023 08:01:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1681138904; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=jvoKw5fddC6ImxYUVWRQGjUNBYWemWpfZTmvE3MZouI=; b=J3fmotMVTaKvLfCm1xlsbvIHdJPthCs8ZbvDWF7G6Us768W6qSc8KC0vqXvraRlzbRlaUH ztR2he+Fwf0sEx1oFDUIYPxTqDsTxQulg7pkL08PjX4B7x6T4N05ulvAK/loivRVb8gLax 2K9LTtYZLbGn+i+ICdGierb+/hZ8Gcs= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-195-6Lhfy9nhOkqSBmrUFiGbxQ-1; Mon, 10 Apr 2023 11:01:41 -0400 X-MC-Unique: 6Lhfy9nhOkqSBmrUFiGbxQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3379E1C0513F; Mon, 10 Apr 2023 15:01:41 +0000 (UTC) Received: from server.redhat.com (ovpn-12-107.pek2.redhat.com [10.72.12.107]) by smtp.corp.redhat.com (Postfix) with ESMTP id D646314171B7; Mon, 10 Apr 2023 15:01:37 +0000 (UTC) From: Cindy Lu <lulu@redhat.com> To: lulu@redhat.com, jasowang@redhat.com, mst@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org Subject: [PATCH] vhost_vdpa: fix unmap process in no-batch mode Date: Mon, 10 Apr 2023 23:01:30 +0800 Message-Id: <20230410150130.837691-1-lulu@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.7 X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1762802751160947701?= X-GMAIL-MSGID: =?utf-8?q?1762802751160947701?= |
Series |
vhost_vdpa: fix unmap process in no-batch mode
|
|
Commit Message
Cindy Lu
April 10, 2023, 3:01 p.m. UTC
While using the no-batch mode, the process will not begin with
VHOST_IOTLB_BATCH_BEGIN, so we need to add the
VHOST_IOTLB_INVALIDATE to get vhost_vdpa_as, the process is the
same as VHOST_IOTLB_UPDATE
Signed-off-by: Cindy Lu <lulu@redhat.com>
---
drivers/vhost/vdpa.c | 1 +
1 file changed, 1 insertion(+)
Comments
On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <lulu@redhat.com> wrote: > > While using the no-batch mode, the process will not begin with > VHOST_IOTLB_BATCH_BEGIN, so we need to add the > VHOST_IOTLB_INVALIDATE to get vhost_vdpa_as, the process is the > same as VHOST_IOTLB_UPDATE > > Signed-off-by: Cindy Lu <lulu@redhat.com> > --- > drivers/vhost/vdpa.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 7be9d9d8f01c..32636a02a0ab 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -1074,6 +1074,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid, > goto unlock; > > if (msg->type == VHOST_IOTLB_UPDATE || > + msg->type == VHOST_IOTLB_INVALIDATE || I'm not sure I get here, invalidation doesn't need to create a new AS. Or maybe you can post the userspace code that can trigger this issue? Thanks > msg->type == VHOST_IOTLB_BATCH_BEGIN) { > as = vhost_vdpa_find_alloc_as(v, asid); > if (!as) { > -- > 2.34.3 >
On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <jasowang@redhat.com> wrote: > > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <lulu@redhat.com> wrote: > > > > While using the no-batch mode, the process will not begin with > > VHOST_IOTLB_BATCH_BEGIN, so we need to add the > > VHOST_IOTLB_INVALIDATE to get vhost_vdpa_as, the process is the > > same as VHOST_IOTLB_UPDATE > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > --- > > drivers/vhost/vdpa.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > index 7be9d9d8f01c..32636a02a0ab 100644 > > --- a/drivers/vhost/vdpa.c > > +++ b/drivers/vhost/vdpa.c > > @@ -1074,6 +1074,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid, > > goto unlock; > > > > if (msg->type == VHOST_IOTLB_UPDATE || > > + msg->type == VHOST_IOTLB_INVALIDATE || > > I'm not sure I get here, invalidation doesn't need to create a new AS. > > Or maybe you can post the userspace code that can trigger this issue? > > Thanks > sorry I didn't write it clearly For this issue can reproduce in vIOMMU no-batch mode support because while the vIOMMU enabled, it will flash a large memory to unmap, and this memory are haven't been mapped before, so this unmapping will fail qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) qemu-system-x86_64: vhost_vdpa_dma_unmap(0x7fa26d1dd190, 0x0, 0x80000000) = -5 (Bad address) qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) .... in batch mode this operation will begin with VHOST_IOTLB_BATCH_BEGIN, so don't have this issue Thanks cindy > > msg->type == VHOST_IOTLB_BATCH_BEGIN) { > > as = vhost_vdpa_find_alloc_as(v, asid); > > if (!as) { > > -- > > 2.34.3 > > >
On Tue, Apr 11, 2023 at 3:29 PM Cindy Lu <lulu@redhat.com> wrote: > > On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <jasowang@redhat.com> wrote: > > > > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <lulu@redhat.com> wrote: > > > > > > While using the no-batch mode, the process will not begin with > > > VHOST_IOTLB_BATCH_BEGIN, so we need to add the > > > VHOST_IOTLB_INVALIDATE to get vhost_vdpa_as, the process is the > > > same as VHOST_IOTLB_UPDATE > > > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > > --- > > > drivers/vhost/vdpa.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > > index 7be9d9d8f01c..32636a02a0ab 100644 > > > --- a/drivers/vhost/vdpa.c > > > +++ b/drivers/vhost/vdpa.c > > > @@ -1074,6 +1074,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid, > > > goto unlock; > > > > > > if (msg->type == VHOST_IOTLB_UPDATE || > > > + msg->type == VHOST_IOTLB_INVALIDATE || > > > > I'm not sure I get here, invalidation doesn't need to create a new AS. > > > > Or maybe you can post the userspace code that can trigger this issue? > > > > Thanks > > > sorry I didn't write it clearly > For this issue can reproduce in vIOMMU no-batch mode support because > while the vIOMMU enabled, it will > flash a large memory to unmap, and this memory are haven't been mapped > before, so this unmapping will fail > > qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) > qemu-system-x86_64: vhost_vdpa_dma_unmap(0x7fa26d1dd190, 0x0, > 0x80000000) = -5 (Bad address) So if this is a simple unmap, which error condition had you met in vhost_vdpa_process_iotlb_msg()? I think you need to trace to see what happens. For example: 1) can the code pass asid_to_iotlb() 2) if not, ASID 0 has been deleted since all the mappings have been unmapped if ASID 0 has been completely unmap, any reason we need to unmap it again? And do we need to drop the vhost_vdpa_remove_as() from both 1) vhost_vdpa_unmap() and 2) vhost_vdpa_process_iotlb_msg() ? Thanks > qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) > .... > in batch mode this operation will begin with VHOST_IOTLB_BATCH_BEGIN, > so don't have this issue > > Thanks > cindy > > > msg->type == VHOST_IOTLB_BATCH_BEGIN) { > > > as = vhost_vdpa_find_alloc_as(v, asid); > > > if (!as) { > > > -- > > > 2.34.3 > > > > > >
On Tue, Apr 11, 2023 at 5:14 PM Jason Wang <jasowang@redhat.com> wrote: > > On Tue, Apr 11, 2023 at 3:29 PM Cindy Lu <lulu@redhat.com> wrote: > > > > On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <lulu@redhat.com> wrote: > > > > > > > > While using the no-batch mode, the process will not begin with > > > > VHOST_IOTLB_BATCH_BEGIN, so we need to add the > > > > VHOST_IOTLB_INVALIDATE to get vhost_vdpa_as, the process is the > > > > same as VHOST_IOTLB_UPDATE > > > > > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > > > --- > > > > drivers/vhost/vdpa.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > > > index 7be9d9d8f01c..32636a02a0ab 100644 > > > > --- a/drivers/vhost/vdpa.c > > > > +++ b/drivers/vhost/vdpa.c > > > > @@ -1074,6 +1074,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid, > > > > goto unlock; > > > > > > > > if (msg->type == VHOST_IOTLB_UPDATE || > > > > + msg->type == VHOST_IOTLB_INVALIDATE || > > > > > > I'm not sure I get here, invalidation doesn't need to create a new AS. > > > > > > Or maybe you can post the userspace code that can trigger this issue? > > > > > > Thanks > > > > > sorry I didn't write it clearly > > For this issue can reproduce in vIOMMU no-batch mode support because > > while the vIOMMU enabled, it will > > flash a large memory to unmap, and this memory are haven't been mapped > > before, so this unmapping will fail > > > > qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) > > qemu-system-x86_64: vhost_vdpa_dma_unmap(0x7fa26d1dd190, 0x0, > > 0x80000000) = -5 (Bad address) > > So if this is a simple unmap, which error condition had you met in > vhost_vdpa_process_iotlb_msg()? > > I think you need to trace to see what happens. For example: > this happens when vIOMMU enable and vdpa binds to vfio-pci run testpmd the qemu will unmapped whole memory that was used and then mapped the iommu MR section This memory much larger than the memory mapped to vdpa(this is what actually mapped to vdpa device in no-iommu MR) > 1) can the code pass asid_to_iotlb() > 2) if not, ASID 0 has been deleted since all the mappings have been unmapped > > if ASID 0 has been completely unmap, any reason we need to unmap it > again? And do we need to drop the vhost_vdpa_remove_as() from both > > 1) vhost_vdpa_unmap() > and > 2) vhost_vdpa_process_iotlb_msg() > ? > > Thanks > the code passed the asid_to_iotlb(), The iotlb is NULL at this situation and the vhost_vdpa_process_iotlb_msg will return fail. this will cause the mapping in qemu fail thanks cindy > > qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) > > .... > > in batch mode this operation will begin with VHOST_IOTLB_BATCH_BEGIN, > > so don't have this issue > > > > Thanks > > cindy > > > > msg->type == VHOST_IOTLB_BATCH_BEGIN) { > > > > as = vhost_vdpa_find_alloc_as(v, asid); > > > > if (!as) { > > > > -- > > > > 2.34.3 > > > > > > > > > >
On Wed, Apr 12, 2023 at 2:32 PM Cindy Lu <lulu@redhat.com> wrote: > > On Tue, Apr 11, 2023 at 5:14 PM Jason Wang <jasowang@redhat.com> wrote: > > > > On Tue, Apr 11, 2023 at 3:29 PM Cindy Lu <lulu@redhat.com> wrote: > > > > > > On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > > > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <lulu@redhat.com> wrote: > > > > > > > > > > While using the no-batch mode, the process will not begin with > > > > > VHOST_IOTLB_BATCH_BEGIN, so we need to add the > > > > > VHOST_IOTLB_INVALIDATE to get vhost_vdpa_as, the process is the > > > > > same as VHOST_IOTLB_UPDATE > > > > > > > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > > > > --- > > > > > drivers/vhost/vdpa.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > > > > index 7be9d9d8f01c..32636a02a0ab 100644 > > > > > --- a/drivers/vhost/vdpa.c > > > > > +++ b/drivers/vhost/vdpa.c > > > > > @@ -1074,6 +1074,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid, > > > > > goto unlock; > > > > > > > > > > if (msg->type == VHOST_IOTLB_UPDATE || > > > > > + msg->type == VHOST_IOTLB_INVALIDATE || > > > > > > > > I'm not sure I get here, invalidation doesn't need to create a new AS. > > > > > > > > Or maybe you can post the userspace code that can trigger this issue? > > > > > > > > Thanks > > > > > > > sorry I didn't write it clearly > > > For this issue can reproduce in vIOMMU no-batch mode support because > > > while the vIOMMU enabled, it will > > > flash a large memory to unmap, and this memory are haven't been mapped > > > before, so this unmapping will fail > > > > > > qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) > > > qemu-system-x86_64: vhost_vdpa_dma_unmap(0x7fa26d1dd190, 0x0, > > > 0x80000000) = -5 (Bad address) > > > > So if this is a simple unmap, which error condition had you met in > > vhost_vdpa_process_iotlb_msg()? > > > > I think you need to trace to see what happens. For example: > > > this happens when vIOMMU enable and vdpa binds to vfio-pci run testpmd > the qemu will unmapped whole memory that was used and then mapped the > iommu MR section So it's a map after an unmap, not an invalidation? > This memory much larger than the memory mapped to vdpa(this is what > actually mapped to > vdpa device in no-iommu MR) > > > 1) can the code pass asid_to_iotlb() > > 2) if not, ASID 0 has been deleted since all the mappings have been unmapped > > > > if ASID 0 has been completely unmap, any reason we need to unmap it > > again? And do we need to drop the vhost_vdpa_remove_as() from both > > > > > 1) vhost_vdpa_unmap() > > and > > 2) vhost_vdpa_process_iotlb_msg() > > ? > > > > Thanks > > > the code passed the asid_to_iotlb(), The iotlb is NULL at this situation > and the vhost_vdpa_process_iotlb_msg will return fail. this will cause > the mapping > in qemu fail Yes, so what I meant: Instead of free the AS in vhost_vdpa_unmap() or vhost_vdpa_process_iotlb_msg() and allocate it again here. Is it better to not remove the AS in those two functions even if there's no maps? Thanks > > thanks > cindy > > > > qemu-system-x86_64: failed to write, fd=12, errno=14 (Bad address) > > > .... > > > in batch mode this operation will begin with VHOST_IOTLB_BATCH_BEGIN, > > > so don't have this issue > > > > > > Thanks > > > cindy > > > > > msg->type == VHOST_IOTLB_BATCH_BEGIN) { > > > > > as = vhost_vdpa_find_alloc_as(v, asid); > > > > > if (!as) { > > > > > -- > > > > > 2.34.3 > > > > > > > > > > > > > > >
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 7be9d9d8f01c..32636a02a0ab 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -1074,6 +1074,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid, goto unlock; if (msg->type == VHOST_IOTLB_UPDATE || + msg->type == VHOST_IOTLB_INVALIDATE || msg->type == VHOST_IOTLB_BATCH_BEGIN) { as = vhost_vdpa_find_alloc_as(v, asid); if (!as) {