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 <[email protected]>
---
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 ||
msg->type == VHOST_IOTLB_BATCH_BEGIN) {
as = vhost_vdpa_find_alloc_as(v, asid);
if (!as) {
--
2.34.3
On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]> wrote:
>
> On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]> wrote:
>
> On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <[email protected]> wrote:
> >
> > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > > ---
> > > 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 Wed, Apr 12, 2023 at 2:32 PM Cindy Lu <[email protected]> wrote:
>
> On Tue, Apr 11, 2023 at 5:14 PM Jason Wang <[email protected]> wrote:
> >
> > On Tue, Apr 11, 2023 at 3:29 PM Cindy Lu <[email protected]> wrote:
> > >
> > > On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <[email protected]> wrote:
> > > >
> > > > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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
> > > > >
> > > >
> > >
> >
>
On Tue, Apr 11, 2023 at 5:14 PM Jason Wang <[email protected]> wrote:
>
> On Tue, Apr 11, 2023 at 3:29 PM Cindy Lu <[email protected]> wrote:
> >
> > On Tue, Apr 11, 2023 at 11:10 AM Jason Wang <[email protected]> wrote:
> > >
> > > On Mon, Apr 10, 2023 at 11:01 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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
> > > >
> > >
> >
>