Linux Virtualization - Mar 2023 - [PATCH RFC v2 virtio 2/7] pds_vdpa: get vdpa management info

On Thu, Mar 16, 2023 at 11:25?AM Shannon Nelson <shannon.nelson at
amd.com> wrote:
>
> On 3/15/23 12:05 AM, Jason Wang wrote:
> > On Thu, Mar 9, 2023 at 9:31?AM Shannon Nelson <shannon.nelson at
amd.com> wrote:
> >>
> >> Find the vDPA management information from the DSC in order to
> >> advertise it to the vdpa subsystem.
> >>
> >> Signed-off-by: Shannon Nelson <shannon.nelson at amd.com>
> >> ---
> >>   drivers/vdpa/pds/Makefile    |   3 +-
> >>   drivers/vdpa/pds/aux_drv.c   |  13 ++++
> >>   drivers/vdpa/pds/aux_drv.h   |   7 +++
> >>   drivers/vdpa/pds/debugfs.c   |   3 +
> >>   drivers/vdpa/pds/vdpa_dev.c  | 113
+++++++++++++++++++++++++++++++++++
> >>   drivers/vdpa/pds/vdpa_dev.h  |  15 +++++
> >>   include/linux/pds/pds_vdpa.h |  92 ++++++++++++++++++++++++++++
> >>   7 files changed, 245 insertions(+), 1 deletion(-)
> >>   create mode 100644 drivers/vdpa/pds/vdpa_dev.c
> >>   create mode 100644 drivers/vdpa/pds/vdpa_dev.h
> >>
> >> diff --git a/drivers/vdpa/pds/Makefile b/drivers/vdpa/pds/Makefile
> >> index a9cd2f450ae1..13b50394ec64 100644
> >> --- a/drivers/vdpa/pds/Makefile
> >> +++ b/drivers/vdpa/pds/Makefile
> >> @@ -3,6 +3,7 @@
> >>
> >>   obj-$(CONFIG_PDS_VDPA) := pds_vdpa.o
> >>
> >> -pds_vdpa-y := aux_drv.o
> >> +pds_vdpa-y := aux_drv.o \
> >> +             vdpa_dev.o
> >>
> >>   pds_vdpa-$(CONFIG_DEBUG_FS) += debugfs.o
> >> diff --git a/drivers/vdpa/pds/aux_drv.c
b/drivers/vdpa/pds/aux_drv.c
> >> index b3f36170253c..63e40ae68211 100644
> >> --- a/drivers/vdpa/pds/aux_drv.c
> >> +++ b/drivers/vdpa/pds/aux_drv.c
> >> @@ -2,6 +2,8 @@
> >>   /* Copyright(c) 2023 Advanced Micro Devices, Inc */
> >>
> >>   #include <linux/auxiliary_bus.h>
> >> +#include <linux/pci.h>
> >> +#include <linux/vdpa.h>
> >>
> >>   #include <linux/pds/pds_core.h>
> >>   #include <linux/pds/pds_auxbus.h>
> >> @@ -9,6 +11,7 @@
> >>
> >>   #include "aux_drv.h"
> >>   #include "debugfs.h"
> >> +#include "vdpa_dev.h"
> >>
> >>   static const struct auxiliary_device_id pds_vdpa_id_table[] = {
> >>          { .name = PDS_VDPA_DEV_NAME, },
> >> @@ -30,6 +33,7 @@ static int pds_vdpa_probe(struct
auxiliary_device *aux_dev,
> >>                  return -ENOMEM;
> >>
> >>          vdpa_aux->padev = padev;
> >> +       vdpa_aux->vf_id = pci_iov_vf_id(padev->vf->pdev);
> >>          auxiliary_set_drvdata(aux_dev, vdpa_aux);
> >>
> >>          /* Register our PDS client with the pds_core */
> >> @@ -40,8 +44,15 @@ static int pds_vdpa_probe(struct
auxiliary_device *aux_dev,
> >>                  goto err_free_mem;
> >>          }
> >>
> >> +       /* Get device ident info and set up the vdpa_mgmt_dev */
> >> +       err = pds_vdpa_get_mgmt_info(vdpa_aux);
> >> +       if (err)
> >> +               goto err_aux_unreg;
> >> +
> >>          return 0;
> >>
> >> +err_aux_unreg:
> >> +       padev->ops->unregister_client(padev);
> >>   err_free_mem:
> >>          kfree(vdpa_aux);
> >>          auxiliary_set_drvdata(aux_dev, NULL);
> >> @@ -54,6 +65,8 @@ static void pds_vdpa_remove(struct
auxiliary_device *aux_dev)
> >>          struct pds_vdpa_aux *vdpa_aux =
auxiliary_get_drvdata(aux_dev);
> >>          struct device *dev = &aux_dev->dev;
> >>
> >> +       pci_free_irq_vectors(vdpa_aux->padev->vf->pdev);
> >> +
> >>         
vdpa_aux->padev->ops->unregister_client(vdpa_aux->padev);
> >>
> >>          kfree(vdpa_aux);
> >> diff --git a/drivers/vdpa/pds/aux_drv.h
b/drivers/vdpa/pds/aux_drv.h
> >> index 14e465944dfd..94ba7abcaa43 100644
> >> --- a/drivers/vdpa/pds/aux_drv.h
> >> +++ b/drivers/vdpa/pds/aux_drv.h
> >> @@ -10,6 +10,13 @@
> >>   struct pds_vdpa_aux {
> >>          struct pds_auxiliary_dev *padev;
> >>
> >> +       struct vdpa_mgmt_dev vdpa_mdev;
> >> +
> >> +       struct pds_vdpa_ident ident;
> >> +
> >> +       int vf_id;
> >>          struct dentry *dentry;
> >> +
> >> +       int nintrs;
> >>   };
> >>   #endif /* _AUX_DRV_H_ */
> >> diff --git a/drivers/vdpa/pds/debugfs.c
b/drivers/vdpa/pds/debugfs.c
> >> index 3c163dc7b66f..7b7e90fd6578 100644
> >> --- a/drivers/vdpa/pds/debugfs.c
> >> +++ b/drivers/vdpa/pds/debugfs.c
> >> @@ -1,7 +1,10 @@
> >>   // SPDX-License-Identifier: GPL-2.0-only
> >>   /* Copyright(c) 2023 Advanced Micro Devices, Inc */
> >>
> >> +#include <linux/vdpa.h>
> >> +
> >>   #include <linux/pds/pds_core.h>
> >> +#include <linux/pds/pds_vdpa.h>
> >>   #include <linux/pds/pds_auxbus.h>
> >>
> >>   #include "aux_drv.h"
> >> diff --git a/drivers/vdpa/pds/vdpa_dev.c
b/drivers/vdpa/pds/vdpa_dev.c
> >> new file mode 100644
> >> index 000000000000..bd840688503c
> >> --- /dev/null
> >> +++ b/drivers/vdpa/pds/vdpa_dev.c
> >> @@ -0,0 +1,113 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/* Copyright(c) 2023 Advanced Micro Devices, Inc */
> >> +
> >> +#include <linux/pci.h>
> >> +#include <linux/vdpa.h>
> >> +#include <uapi/linux/vdpa.h>
> >> +
> >> +#include <linux/pds/pds_core.h>
> >> +#include <linux/pds/pds_adminq.h>
> >> +#include <linux/pds/pds_auxbus.h>
> >> +#include <linux/pds/pds_vdpa.h>
> >> +
> >> +#include "vdpa_dev.h"
> >> +#include "aux_drv.h"
> >> +
> >> +static struct virtio_device_id pds_vdpa_id_table[] = {
> >> +       {VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID},
> >> +       {0},
> >> +};
> >> +
> >> +static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
char *name,
> >> +                           const struct vdpa_dev_set_config
*add_config)
> >> +{
> >> +       return -EOPNOTSUPP;
> >> +}
> >> +
> >> +static void pds_vdpa_dev_del(struct vdpa_mgmt_dev *mdev,
> >> +                            struct vdpa_device *vdpa_dev)
> >> +{
> >> +}
> >> +
> >> +static const struct vdpa_mgmtdev_ops pds_vdpa_mgmt_dev_ops = {
> >> +       .dev_add = pds_vdpa_dev_add,
> >> +       .dev_del = pds_vdpa_dev_del
> >> +};
> >> +
> >> +int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
> >> +{
> >> +       struct pds_vdpa_ident_cmd ident_cmd = {
> >> +               .opcode = PDS_VDPA_CMD_IDENT,
> >> +               .vf_id = cpu_to_le16(vdpa_aux->vf_id),
> >> +       };
> >> +       struct pds_vdpa_comp ident_comp = {0};
> >> +       struct vdpa_mgmt_dev *mgmt;
> >> +       struct device *pf_dev;
> >> +       struct pci_dev *pdev;
> >> +       dma_addr_t ident_pa;
> >> +       struct device *dev;
> >> +       u16 max_vqs;
> >> +       int err;
> >> +
> >> +       dev = &vdpa_aux->padev->aux_dev.dev;
> >> +       pdev = vdpa_aux->padev->vf->pdev;
> >> +       mgmt = &vdpa_aux->vdpa_mdev;
> >> +
> >> +       /* Get resource info through the PF's adminq.  It is a
block of info,
> >> +        * so we need to map some memory for PF to make available
to the
> >> +        * firmware for writing the data.
> >> +        */
> >
> > It looks to me pds_vdpa_ident is not very large:
> >
> > struct pds_vdpa_ident {
> >          __le64 hw_features;
> >          __le16 max_vqs;
> >          __le16 max_qlen;
> >          __le16 min_qlen;
> > };
> >
> > Any reason it is not packed into some type of the comp structure of
adminq?
>
> Unfortunately, the completion structs are limited to 16 bytes, with 4 up
> front and 1 at the end already spoken for.  I suppose we could shrink
> max_vqs to a single byte and squeeze this into the comp, but then we'd
> have no ability to add to it if needed.  I'd rather leave it as it is
> for now.
Fine.

Thanks
>
> sln
>
> >
> > Others look good.
> >
> > Thanks
> >
>