| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJaqyWeJOwcAj=mr5R4qHqWSjK1dQ-SxMNVF0HYkPtdoO2BaGQ@mail.gmail.com>
Date: Mon, 16 Jan 2023 19:03:47 +0100
From: Eugenio Perez Martin <eperezma@...hat.com>
To: Eli Cohen <elic@...dia.com>
Cc: "mst@...hat.com" <mst@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Parav Pandit <parav@...dia.com>,
"lulu@...hat.com" <lulu@...hat.com>,
"jasowang@...hat.com" <jasowang@...hat.com>,
"virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
"sgarzare@...hat.com" <sgarzare@...hat.com>,
"si-wei.liu@...cle.com" <si-wei.liu@...cle.com>
Subject: Re: [RFC 2/3] vdpa/mlx5: conditionally delete cvq iotlb in destroy_mr
On Mon, Jan 16, 2023 at 8:03 AM Eli Cohen <elic@...dia.com> wrote:
>
> > From: Eugenio Pérez <eperezma@...hat.com>
> > Sent: Thursday, 12 January 2023 16:22
> > To: mst@...hat.com; Eli Cohen <elic@...dia.com>
> > Cc: linux-kernel@...r.kernel.org; Parav Pandit <parav@...dia.com>;
> > lulu@...hat.com; jasowang@...hat.com; virtualization@...ts.linux-
> > foundation.org; sgarzare@...hat.com; si-wei.liu@...cle.com
> > Subject: [RFC 2/3] vdpa/mlx5: conditionally delete cvq iotlb in destroy_mr
> >
> > mlx5_vdpa_destroy_mr can be called by setting a map to data ASID after
> > populating control virtqueue ASID iotlb. Control vq iotlb must not be
> > cleared, since it will not be populated again.
> >
> > Adding a conditional in the function so the caller specifies if it is
> > resetting, cleaning, or just changing data memory.
> >
> > Fixes: 8fcd20c30704 ("vdpa/mlx5: Support different address spaces for
> > control and data")
> > Signed-off-by: Eugenio Pérez <eperezma@...hat.com>
> > ---
> > drivers/vdpa/mlx5/core/mlx5_vdpa.h | 2 +-
> > drivers/vdpa/mlx5/core/mr.c | 5 +++--
> > drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++------
> > 3 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > index 058fbe28107e..000b144019ec 100644
> > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> > @@ -119,7 +119,7 @@ int mlx5_vdpa_handle_set_map(struct
> > mlx5_vdpa_dev *mvdev, struct vhost_iotlb *io
> > bool *change_map, unsigned int asid);
> > int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
> > *iotlb,
> > unsigned int asid);
> > -void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev);
> > +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev, bool
> > delete_cvq_iotlb);
> >
> > #define mlx5_vdpa_warn(__dev, format, ...)
> > \
> > dev_warn((__dev)->mdev->device, "%s:%d:(pid %d) warning: "
> > format, __func__, __LINE__, \
> > diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> > index ae34dcac9a3f..878ee94efa78 100644
> > --- a/drivers/vdpa/mlx5/core/mr.c
> > +++ b/drivers/vdpa/mlx5/core/mr.c
> > @@ -491,7 +491,7 @@ static void destroy_user_mr(struct mlx5_vdpa_dev
> > *mvdev, struct mlx5_vdpa_mr *mr
> > }
> > }
> >
> > -void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
> > +void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev, bool
> > delete_cvq_iotlb)
> > {
> > struct mlx5_vdpa_mr *mr = &mvdev->mr;
> >
> > @@ -499,7 +499,8 @@ void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev
> > *mvdev)
> > if (!mr->initialized)
> > goto out;
> >
> > - prune_iotlb(mvdev);
> > + if (delete_cvq_iotlb)
> > + prune_iotlb(mvdev);
> > if (mr->user_mr)
> > destroy_user_mr(mvdev, mr);
> > else
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index 6632651b1e54..1f1f341f602b 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -2433,7 +2433,7 @@ static int mlx5_vdpa_change_map(struct
> > mlx5_vdpa_dev *mvdev,
> > goto err_mr;
> >
> > teardown_driver(ndev);
> > - mlx5_vdpa_destroy_mr(mvdev);
> > + mlx5_vdpa_destroy_mr(mvdev, mvdev-
> > >group2asid[MLX5_VDPA_CVQ_GROUP] == asid);
>
> Looks to me we need to handle this in a more generic manner. The asid should be used conditionally for either CVQ or data VQ updates. You are protecting CVQ but same thing should hold also for data VQs iotlb.
I agree. Maybe the best option is to replace the boolean indicating
the ASID we want to destroy mr? Then, at cleanup, we can iterate by
all vq groups / ASID.
> Meaning, if qemu wants to update only CVQ than data VQ translation must not be affected.
_mlx5_vdpa_create_mr. is the one that checks it If I recall correctly.
It is not obvious in this change though.
Thanks!
>
> > err = mlx5_vdpa_create_mr(mvdev, iotlb, asid);
> > if (err)
> > goto err_mr;
> > @@ -2449,7 +2449,7 @@ static int mlx5_vdpa_change_map(struct
> > mlx5_vdpa_dev *mvdev,
> > return 0;
> >
> > err_setup:
> > - mlx5_vdpa_destroy_mr(mvdev);
> > + mlx5_vdpa_destroy_mr(mvdev, mvdev-
> > >group2asid[MLX5_VDPA_CVQ_GROUP] == asid);
> > err_mr:
> > return err;
> > }
> > @@ -2578,7 +2578,7 @@ static void mlx5_vdpa_set_status(struct
> > vdpa_device *vdev, u8 status)
> > return;
> >
> > err_setup:
> > - mlx5_vdpa_destroy_mr(&ndev->mvdev);
> > + mlx5_vdpa_destroy_mr(&ndev->mvdev, true);
> > ndev->mvdev.status |= VIRTIO_CONFIG_S_FAILED;
> > err_clear:
> > up_write(&ndev->reslock);
> > @@ -2604,7 +2604,7 @@ static int mlx5_vdpa_reset(struct vdpa_device
> > *vdev)
> > down_write(&ndev->reslock);
> > teardown_driver(ndev);
> > clear_vqs_ready(ndev);
> > - mlx5_vdpa_destroy_mr(&ndev->mvdev);
> > + mlx5_vdpa_destroy_mr(&ndev->mvdev, true);
> > ndev->mvdev.status = 0;
> > ndev->cur_num_vqs = 0;
> > ndev->mvdev.cvq.received_desc = 0;
> > @@ -2691,7 +2691,7 @@ static void mlx5_vdpa_free(struct vdpa_device
> > *vdev)
> > ndev = to_mlx5_vdpa_ndev(mvdev);
> >
> > free_resources(ndev);
> > - mlx5_vdpa_destroy_mr(mvdev);
> > + mlx5_vdpa_destroy_mr(mvdev, true);
> > if (!is_zero_ether_addr(ndev->config.mac)) {
> > pfmdev = pci_get_drvdata(pci_physfn(mvdev->mdev->pdev));
> > mlx5_mpfs_del_mac(pfmdev, ndev->config.mac);
> > @@ -3214,7 +3214,7 @@ static int mlx5_vdpa_dev_add(struct
> > vdpa_mgmt_dev *v_mdev, const char *name,
> > err_res2:
> > free_resources(ndev);
> > err_mr:
> > - mlx5_vdpa_destroy_mr(mvdev);
> > + mlx5_vdpa_destroy_mr(mvdev, true);
> > err_res:
> > mlx5_vdpa_free_resources(&ndev->mvdev);
> > err_mpfs:
> > --
> > 2.31.1
>
Powered by blists - more mailing lists