| 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: <ad9ab1f3-43ff-a73d-0a62-50565aa5196f@amd.com>
Date: Wed, 15 Mar 2023 20:25:05 -0700
From: Shannon Nelson <shannon.nelson@....com>
To: Jason Wang <jasowang@...hat.com>
Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org,
brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
kuba@...nel.org, drivers@...sando.io
Subject: Re: [PATCH RFC v2 virtio 2/7] pds_vdpa: get vdpa management info
On 3/15/23 12:05 AM, Jason Wang wrote:
> On Thu, Mar 9, 2023 at 9:31 AM Shannon Nelson <shannon.nelson@....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@....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.
sln
>
> Others look good.
>
> Thanks
>
Powered by blists - more mailing lists