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;
MIME-Version: 1.0
References: <20230410150130.837691-1-lulu@redhat.com> <CACGkMEvTdgvqacFmMJZD4u++YJwESgSmLF6CMdAJBBqkxpZKgg@mail.gmail.com>
 <CACLfguWKw68=wZNa7Ga+Jm8xTE93A_5za3Dc=S_z7ds9FCkRKg@mail.gmail.com> <CACGkMEv3aca0Thx+X3WZxbV2HK7514G3RzR+A0PqRu7k6Deztg@mail.gmail.com>
In-Reply-To: <CACGkMEv3aca0Thx+X3WZxbV2HK7514G3RzR+A0PqRu7k6Deztg@mail.gmail.com>
From:   Cindy Lu <lulu@redhat.com>
Date:   Wed, 12 Apr 2023 14:31:38 +0800
Message-ID: <CACLfguXBeodQ=b-RAQ4JsaSnjS_ZNutr2nbunmdv1S8Gxz8gfg@mail.gmail.com>
Subject: Re: [PATCH] vhost_vdpa: fix unmap process in no-batch mode
To:     Jason Wang <jasowang@redhat.com>
Cc:     mst@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        virtualization@lists.linux-foundation.org, netdev@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Apr 11, 2023 at 5:14=E2=80=AFPM Jason Wang <jasowang@redhat.com> wr=
ote:
>
> On Tue, Apr 11, 2023 at 3:29=E2=80=AFPM Cindy Lu <lulu@redhat.com> wrote:
> >
> > On Tue, Apr 11, 2023 at 11:10=E2=80=AFAM Jason Wang <jasowang@redhat.co=
m> wrote:
> > >
> > > On Mon, Apr 10, 2023 at 11:01=E2=80=AFPM Cindy Lu <lulu@redhat.com> w=
rote:
> > > >
> > > > 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(struc=
t vhost_dev *dev, u32 asid,
> > > >                 goto unlock;
> > > >
> > > >         if (msg->type =3D=3D VHOST_IOTLB_UPDATE ||
> > > > +           msg->type =3D=3D 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=3D12, errno=3D14 (Bad address)
> > qemu-system-x86_64: vhost_vdpa_dma_unmap(0x7fa26d1dd190, 0x0,
> > 0x80000000) =3D -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 unmap=
ped
>
> 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=3D12, errno=3D14 (Bad address)
> > ....
> > in batch mode this operation will begin with VHOST_IOTLB_BATCH_BEGIN,
> > so don't have this issue
> >
> > Thanks
> > cindy
> > > >             msg->type =3D=3D VHOST_IOTLB_BATCH_BEGIN) {
> > > >                 as =3D vhost_vdpa_find_alloc_as(v, asid);
> > > >                 if (!as) {
> > > > --
> > > > 2.34.3
> > > >
> > >
> >
>