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: <20220915085048.12840-1-jasowang@redhat.com> <20220915085048.12840-4-jasowang@redhat.com>
 <DM8PR12MB54006A97A99515542260B5D6AB4D9@DM8PR12MB5400.namprd12.prod.outlook.com>
In-Reply-To: <DM8PR12MB54006A97A99515542260B5D6AB4D9@DM8PR12MB5400.namprd12.prod.outlook.com>
From:   Jason Wang <jasowang@redhat.com>
Date:   Tue, 20 Sep 2022 09:46:07 +0800
Message-ID: <CACGkMEtfLGjHgFV7bgWrjWoFZyiTQMf9oDBGi5L=diQ5mtNRnA@mail.gmail.com>
Subject: Re: [PATCH 3/3] vp_vdpa: support feature provisioning
To:     Eli Cohen <elic@nvidia.com>
Cc:     "mst@redhat.com" <mst@redhat.com>,
        "si-wei.liu@oracle.com" <si-wei.liu@oracle.com>,
        Parav Pandit <parav@nvidia.com>,
        "wuzongyong@linux.alibaba.com" <wuzongyong@linux.alibaba.com>,
        "virtualization@lists.linux-foundation.org" 
        <virtualization@lists.linux-foundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "eperezma@redhat.com" <eperezma@redhat.com>,
        "lingshan.zhu@intel.com" <lingshan.zhu@intel.com>,
        "gdawar@xilinx.com" <gdawar@xilinx.com>,
        "lulu@redhat.com" <lulu@redhat.com>,
        "xieyongji@bytedance.com" <xieyongji@bytedance.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Sep 19, 2022 at 3:05 PM Eli Cohen <elic@nvidia.com> wrote:
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Thursday, 15 September 2022 11:51
> > To: mst@redhat.com; jasowang@redhat.com
> > Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> > <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> > virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> > eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> > lulu@redhat.com; xieyongji@bytedance.com
> > Subject: [PATCH 3/3] vp_vdpa: support feature provisioning
> >
> > This patch allows the device features to be provisioned via
> > netlink. This is done by:
> >
> > 1) validating the provisioned features to be a subset of the parent
> >    features.
> > 2) clearing the features that is not wanted by the userspace
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  drivers/vdpa/virtio_pci/vp_vdpa.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > index 04522077735b..4b28e0c95ba2 100644
> > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/virtio_ring.h>
> >  #include <linux/virtio_pci.h>
> >  #include <linux/virtio_pci_modern.h>
> > +#include <uapi/linux/vdpa.h>
> >
> >  #define VP_VDPA_QUEUE_MAX 256
> >  #define VP_VDPA_DRIVER_NAME "vp_vdpa"
> > @@ -35,6 +36,7 @@ struct vp_vdpa {
> >       struct virtio_pci_modern_device *mdev;
> >       struct vp_vring *vring;
> >       struct vdpa_callback config_cb;
> > +     u64 device_features;
> >       char msix_name[VP_VDPA_NAME_SIZE];
> >       int config_irq;
> >       int queues;
> > @@ -66,9 +68,9 @@ static struct virtio_pci_modern_device
> > *vp_vdpa_to_mdev(struct vp_vdpa *vp_vdpa)
> >
> >  static u64 vp_vdpa_get_device_features(struct vdpa_device *vdpa)
> >  {
> > -     struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
> > +     struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
> >
> > -     return vp_modern_get_features(mdev);
> > +     return vp_vdpa->device_features;
> >  }
>
> Unrelated to this patch. Maybe put in another patch?

Actually not, since we don't want to report the features that are
supported by the device, but the features that are provisioned via
netlink:

> > +     if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > +             if (add_config->device_features & ~device_features)
> > +                     return -EINVAL;
> > +             device_features &= add_config->device_features;
> > +     }
> > +     vp_vdpa->device_features = device_features;

Thanks

>
> >
> >  static int vp_vdpa_set_driver_features(struct vdpa_device *vdpa, u64
> > features)
> > @@ -475,6 +477,7 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev
> > *v_mdev, const char *name,
> >       struct pci_dev *pdev = mdev->pci_dev;
> >       struct device *dev = &pdev->dev;
> >       struct vp_vdpa *vp_vdpa = NULL;
> > +     u64 device_features;
> >       int ret, i;
> >
> >       vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
> > @@ -491,6 +494,14 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev
> > *v_mdev, const char *name,
> >       vp_vdpa->queues = vp_modern_get_num_queues(mdev);
> >       vp_vdpa->mdev = mdev;
> >
> > +     device_features = vp_modern_get_features(mdev);
> > +     if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > +             if (add_config->device_features & ~device_features)
> > +                     return -EINVAL;
> > +             device_features &= add_config->device_features;
> > +     }
> > +     vp_vdpa->device_features = device_features;
> > +
> >       ret = devm_add_action_or_reset(dev, vp_vdpa_free_irq_vectors,
> > pdev);
> >       if (ret) {
> >               dev_err(&pdev->dev,
> > @@ -599,6 +610,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const
> > struct pci_device_id *id)
> >       mgtdev->id_table = mdev_id;
> >       mgtdev->max_supported_vqs =
> > vp_modern_get_num_queues(mdev);
> >       mgtdev->supported_features = vp_modern_get_features(mdev);
> > +     mgtdev->config_attr_mask = (1 << VDPA_ATTR_DEV_FEATURES);
> >       pci_set_master(pdev);
> >       pci_set_drvdata(pdev, vp_vdpa_mgtdev);
> >
> > --
> > 2.25.1
>