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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9038fb95-f711-235c-8656-1c81e2a18c64@amd.com>
Date:   Wed, 15 Mar 2023 20:25:24 -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 5/7] pds_vdpa: add support for vdpa and
 vdpamgmt interfaces

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:
>>
>> This is the vDPA device support, where we advertise that we can
>> support the virtio queues and deal with the configuration work
>> through the pds_core's adminq.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>> ---
>>   drivers/vdpa/pds/aux_drv.c  |  15 +
>>   drivers/vdpa/pds/aux_drv.h  |   1 +
>>   drivers/vdpa/pds/debugfs.c  | 172 ++++++++++++
>>   drivers/vdpa/pds/debugfs.h  |   8 +
>>   drivers/vdpa/pds/vdpa_dev.c | 545 +++++++++++++++++++++++++++++++++++-
>>   5 files changed, 740 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vdpa/pds/aux_drv.c b/drivers/vdpa/pds/aux_drv.c
>> index 28158d0d98a5..d706f06f7400 100644
>> --- a/drivers/vdpa/pds/aux_drv.c
>> +++ b/drivers/vdpa/pds/aux_drv.c
>> @@ -60,8 +60,21 @@ static int pds_vdpa_probe(struct auxiliary_device *aux_dev,
>>                  goto err_free_mgmt_info;
>>          }
>>
>> +       /* Let vdpa know that we can provide devices */
>> +       err = vdpa_mgmtdev_register(&vdpa_aux->vdpa_mdev);
>> +       if (err) {
>> +               dev_err(dev, "%s: Failed to initialize vdpa_mgmt interface: %pe\n",
>> +                       __func__, ERR_PTR(err));
>> +               goto err_free_virtio;
>> +       }
>> +
>> +       pds_vdpa_debugfs_add_pcidev(vdpa_aux);
>> +       pds_vdpa_debugfs_add_ident(vdpa_aux);
>> +
>>          return 0;
>>
>> +err_free_virtio:
>> +       pds_vdpa_remove_virtio(&vdpa_aux->vd_mdev);
>>   err_free_mgmt_info:
>>          pci_free_irq_vectors(padev->vf->pdev);
>>   err_aux_unreg:
>> @@ -78,11 +91,13 @@ 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;
>>
>> +       vdpa_mgmtdev_unregister(&vdpa_aux->vdpa_mdev);
>>          pds_vdpa_remove_virtio(&vdpa_aux->vd_mdev);
>>          pci_free_irq_vectors(vdpa_aux->padev->vf->pdev);
>>
>>          vdpa_aux->padev->ops->unregister_client(vdpa_aux->padev);
>>
>> +       pds_vdpa_debugfs_del_vdpadev(vdpa_aux);
>>          kfree(vdpa_aux);
>>          auxiliary_set_drvdata(aux_dev, NULL);
>>
>> diff --git a/drivers/vdpa/pds/aux_drv.h b/drivers/vdpa/pds/aux_drv.h
>> index 87ac3c01c476..1ab1ce64da7c 100644
>> --- a/drivers/vdpa/pds/aux_drv.h
>> +++ b/drivers/vdpa/pds/aux_drv.h
>> @@ -11,6 +11,7 @@ struct pds_vdpa_aux {
>>          struct pds_auxiliary_dev *padev;
>>
>>          struct vdpa_mgmt_dev vdpa_mdev;
>> +       struct pds_vdpa_device *pdsv;
>>
>>          struct pds_vdpa_ident ident;
>>
>> diff --git a/drivers/vdpa/pds/debugfs.c b/drivers/vdpa/pds/debugfs.c
>> index aa5e9677fe74..b3ee4f42f3b6 100644
>> --- a/drivers/vdpa/pds/debugfs.c
>> +++ b/drivers/vdpa/pds/debugfs.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/pds/pds_auxbus.h>
>>
>>   #include "aux_drv.h"
>> +#include "vdpa_dev.h"
>>   #include "debugfs.h"
>>
>>   #ifdef CONFIG_DEBUG_FS
>> @@ -26,4 +27,175 @@ void pds_vdpa_debugfs_destroy(void)
>>          dbfs_dir = NULL;
>>   }
>>
>> +#define PRINT_SBIT_NAME(__seq, __f, __name)                     \
>> +       do {                                                    \
>> +               if ((__f) & (__name))                               \
>> +                       seq_printf(__seq, " %s", &#__name[16]); \
>> +       } while (0)
>> +
>> +static void print_status_bits(struct seq_file *seq, u16 status)
>> +{
>> +       seq_puts(seq, "status:");
>> +       PRINT_SBIT_NAME(seq, status, VIRTIO_CONFIG_S_ACKNOWLEDGE);
>> +       PRINT_SBIT_NAME(seq, status, VIRTIO_CONFIG_S_DRIVER);
>> +       PRINT_SBIT_NAME(seq, status, VIRTIO_CONFIG_S_DRIVER_OK);
>> +       PRINT_SBIT_NAME(seq, status, VIRTIO_CONFIG_S_FEATURES_OK);
>> +       PRINT_SBIT_NAME(seq, status, VIRTIO_CONFIG_S_NEEDS_RESET);
>> +       PRINT_SBIT_NAME(seq, status, VIRTIO_CONFIG_S_FAILED);
>> +       seq_puts(seq, "\n");
>> +}
>> +
>> +#define PRINT_FBIT_NAME(__seq, __f, __name)                \
>> +       do {                                               \
>> +               if ((__f) & BIT_ULL(__name))                 \
>> +                       seq_printf(__seq, " %s", #__name); \
>> +       } while (0)
>> +
>> +static void print_feature_bits(struct seq_file *seq, u64 features)
>> +{
>> +       seq_puts(seq, "features:");
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CSUM);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_GUEST_CSUM);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_MTU);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_MAC);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_GUEST_TSO4);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_GUEST_TSO6);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_GUEST_ECN);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_GUEST_UFO);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_HOST_TSO4);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_HOST_TSO6);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_HOST_ECN);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_HOST_UFO);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_MRG_RXBUF);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_STATUS);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CTRL_VQ);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CTRL_RX);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CTRL_VLAN);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CTRL_RX_EXTRA);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_GUEST_ANNOUNCE);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_MQ);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_CTRL_MAC_ADDR);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_HASH_REPORT);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_RSS);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_RSC_EXT);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_STANDBY);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_NET_F_SPEED_DUPLEX);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_NOTIFY_ON_EMPTY);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_ANY_LAYOUT);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_VERSION_1);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_ACCESS_PLATFORM);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_RING_PACKED);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_ORDER_PLATFORM);
>> +       PRINT_FBIT_NAME(seq, features, VIRTIO_F_SR_IOV);
>> +       seq_puts(seq, "\n");
> 
> Should we print the features that are not understood here?

Probably not a bad idea, if we keep this around.  I might end up just 
yanking it out.

> 
>> +}
>> +
>> +void pds_vdpa_debugfs_add_pcidev(struct pds_vdpa_aux *vdpa_aux)
>> +{
>> +       vdpa_aux->dentry = debugfs_create_dir(pci_name(vdpa_aux->padev->vf->pdev), dbfs_dir);
>> +}
>> +
>> +static int identity_show(struct seq_file *seq, void *v)
>> +{
>> +       struct pds_vdpa_aux *vdpa_aux = seq->private;
>> +       struct vdpa_mgmt_dev *mgmt;
>> +
>> +       seq_printf(seq, "aux_dev:            %s\n",
>> +                  dev_name(&vdpa_aux->padev->aux_dev.dev));
>> +
>> +       mgmt = &vdpa_aux->vdpa_mdev;
>> +       seq_printf(seq, "max_vqs:            %d\n", mgmt->max_supported_vqs);
>> +       seq_printf(seq, "config_attr_mask:   %#llx\n", mgmt->config_attr_mask);
>> +       seq_printf(seq, "supported_features: %#llx\n", mgmt->supported_features);
>> +       print_feature_bits(seq, mgmt->supported_features);
>> +
>> +       return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(identity);
>> +
>> +void pds_vdpa_debugfs_add_ident(struct pds_vdpa_aux *vdpa_aux)
>> +{
>> +       debugfs_create_file("identity", 0400, vdpa_aux->dentry,
>> +                           vdpa_aux, &identity_fops);
>> +}
>> +
>> +static int config_show(struct seq_file *seq, void *v)
>> +{
>> +       struct pds_vdpa_device *pdsv = seq->private;
>> +       struct virtio_net_config vc;
>> +
>> +       memcpy_fromio(&vc, pdsv->vdpa_aux->vd_mdev.device,
>> +                     sizeof(struct virtio_net_config));
>> +
>> +       seq_printf(seq, "mac:                  %pM\n", vc.mac);
>> +       seq_printf(seq, "max_virtqueue_pairs:  %d\n",
>> +                  __virtio16_to_cpu(true, vc.max_virtqueue_pairs));
>> +       seq_printf(seq, "mtu:                  %d\n", __virtio16_to_cpu(true, vc.mtu));
>> +       seq_printf(seq, "speed:                %d\n", le32_to_cpu(vc.speed));
>> +       seq_printf(seq, "duplex:               %d\n", vc.duplex);
>> +       seq_printf(seq, "rss_max_key_size:     %d\n", vc.rss_max_key_size);
>> +       seq_printf(seq, "rss_max_indirection_table_length: %d\n",
>> +                  le16_to_cpu(vc.rss_max_indirection_table_length));
>> +       seq_printf(seq, "supported_hash_types: %#x\n",
>> +                  le32_to_cpu(vc.supported_hash_types));
>> +       seq_printf(seq, "vn_status:            %#x\n",
>> +                  __virtio16_to_cpu(true, vc.status));
>> +       print_status_bits(seq, __virtio16_to_cpu(true, vc.status));
>> +
>> +       seq_printf(seq, "req_features:         %#llx\n", pdsv->req_features);
>> +       print_feature_bits(seq, pdsv->req_features);
>> +       seq_printf(seq, "actual_features:      %#llx\n", pdsv->actual_features);
>> +       print_feature_bits(seq, pdsv->actual_features);
>> +       seq_printf(seq, "vdpa_index:           %d\n", pdsv->vdpa_index);
>> +       seq_printf(seq, "num_vqs:              %d\n", pdsv->num_vqs);
>> +
>> +       return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(config);
>> +
>> +static int vq_show(struct seq_file *seq, void *v)
>> +{
>> +       struct pds_vdpa_vq_info *vq = seq->private;
>> +
>> +       seq_printf(seq, "ready:      %d\n", vq->ready);
>> +       seq_printf(seq, "desc_addr:  %#llx\n", vq->desc_addr);
>> +       seq_printf(seq, "avail_addr: %#llx\n", vq->avail_addr);
>> +       seq_printf(seq, "used_addr:  %#llx\n", vq->used_addr);
>> +       seq_printf(seq, "q_len:      %d\n", vq->q_len);
>> +       seq_printf(seq, "qid:        %d\n", vq->qid);
>> +
>> +       seq_printf(seq, "doorbell:   %#llx\n", vq->doorbell);
>> +       seq_printf(seq, "avail_idx:  %d\n", vq->avail_idx);
>> +       seq_printf(seq, "used_idx:   %d\n", vq->used_idx);
>> +       seq_printf(seq, "irq:        %d\n", vq->irq);
>> +       seq_printf(seq, "irq-name:   %s\n", vq->irq_name);
>> +
>> +       seq_printf(seq, "hw_qtype:   %d\n", vq->hw_qtype);
>> +       seq_printf(seq, "hw_qindex:  %d\n", vq->hw_qindex);
>> +
>> +       return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(vq);
>> +
>> +void pds_vdpa_debugfs_add_vdpadev(struct pds_vdpa_aux *vdpa_aux)
>> +{
>> +       int i;
>> +
>> +       debugfs_create_file("config", 0400, vdpa_aux->dentry, vdpa_aux->pdsv, &config_fops);
>> +
>> +       for (i = 0; i < vdpa_aux->pdsv->num_vqs; i++) {
>> +               char name[8];
>> +
>> +               snprintf(name, sizeof(name), "vq%02d", i);
>> +               debugfs_create_file(name, 0400, vdpa_aux->dentry,
>> +                                   &vdpa_aux->pdsv->vqs[i], &vq_fops);
>> +       }
>> +}
>> +
>> +void pds_vdpa_debugfs_del_vdpadev(struct pds_vdpa_aux *vdpa_aux)
>> +{
>> +       debugfs_remove_recursive(vdpa_aux->dentry);
>> +       vdpa_aux->dentry = NULL;
>> +}
>>   #endif /* CONFIG_DEBUG_FS */
>> diff --git a/drivers/vdpa/pds/debugfs.h b/drivers/vdpa/pds/debugfs.h
>> index fff078a869e5..23e8345add0d 100644
>> --- a/drivers/vdpa/pds/debugfs.h
>> +++ b/drivers/vdpa/pds/debugfs.h
>> @@ -10,9 +10,17 @@
>>
>>   void pds_vdpa_debugfs_create(void);
>>   void pds_vdpa_debugfs_destroy(void);
>> +void pds_vdpa_debugfs_add_pcidev(struct pds_vdpa_aux *vdpa_aux);
>> +void pds_vdpa_debugfs_add_ident(struct pds_vdpa_aux *vdpa_aux);
>> +void pds_vdpa_debugfs_add_vdpadev(struct pds_vdpa_aux *vdpa_aux);
>> +void pds_vdpa_debugfs_del_vdpadev(struct pds_vdpa_aux *vdpa_aux);
>>   #else
>>   static inline void pds_vdpa_debugfs_create(void) { }
>>   static inline void pds_vdpa_debugfs_destroy(void) { }
>> +static inline void pds_vdpa_debugfs_add_pcidev(struct pds_vdpa_aux *vdpa_aux) { }
>> +static inline void pds_vdpa_debugfs_add_ident(struct pds_vdpa_aux *vdpa_aux) { }
>> +static inline void pds_vdpa_debugfs_add_vdpadev(struct pds_vdpa_aux *vdpa_aux) { }
>> +static inline void pds_vdpa_debugfs_del_vdpadev(struct pds_vdpa_aux *vdpa_aux) { }
>>   #endif
>>
>>   #endif /* _PDS_VDPA_DEBUGFS_H_ */
>> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
>> index 15d623297203..2e0a5078d379 100644
>> --- a/drivers/vdpa/pds/vdpa_dev.c
>> +++ b/drivers/vdpa/pds/vdpa_dev.c
>> @@ -5,6 +5,7 @@
>>   #include <linux/vdpa.h>
>>   #include <uapi/linux/vdpa.h>
>>   #include <linux/virtio_pci_modern.h>
>> +#include <uapi/linux/virtio_pci.h>
>>
>>   #include <linux/pds/pds_core.h>
>>   #include <linux/pds/pds_adminq.h>
>> @@ -13,7 +14,426 @@
>>
>>   #include "vdpa_dev.h"
>>   #include "aux_drv.h"
>> +#include "cmds.h"
>> +#include "debugfs.h"
>>
>> +static struct pds_vdpa_device *vdpa_to_pdsv(struct vdpa_device *vdpa_dev)
>> +{
>> +       return container_of(vdpa_dev, struct pds_vdpa_device, vdpa_dev);
>> +}
>> +
>> +static int pds_vdpa_set_vq_address(struct vdpa_device *vdpa_dev, u16 qid,
>> +                                  u64 desc_addr, u64 driver_addr, u64 device_addr)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +
>> +       pdsv->vqs[qid].desc_addr = desc_addr;
>> +       pdsv->vqs[qid].avail_addr = driver_addr;
>> +       pdsv->vqs[qid].used_addr = device_addr;
>> +
>> +       return 0;
>> +}
>> +
>> +static void pds_vdpa_set_vq_num(struct vdpa_device *vdpa_dev, u16 qid, u32 num)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +
>> +       pdsv->vqs[qid].q_len = num;
>> +}
>> +
>> +static void pds_vdpa_kick_vq(struct vdpa_device *vdpa_dev, u16 qid)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +
>> +       iowrite16(qid, pdsv->vqs[qid].notify);
>> +}
>> +
>> +static void pds_vdpa_set_vq_cb(struct vdpa_device *vdpa_dev, u16 qid,
>> +                              struct vdpa_callback *cb)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +
>> +       pdsv->vqs[qid].event_cb = *cb;
>> +}
>> +
>> +static irqreturn_t pds_vdpa_isr(int irq, void *data)
>> +{
>> +       struct pds_vdpa_vq_info *vq;
>> +
>> +       vq = data;
>> +       if (vq->event_cb.callback)
>> +               vq->event_cb.callback(vq->event_cb.private);
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static void pds_vdpa_release_irq(struct pds_vdpa_device *pdsv, int qid)
>> +{
>> +       if (pdsv->vqs[qid].irq == VIRTIO_MSI_NO_VECTOR)
>> +               return;
>> +
>> +       free_irq(pdsv->vqs[qid].irq, &pdsv->vqs[qid]);
>> +       pdsv->vqs[qid].irq = VIRTIO_MSI_NO_VECTOR;
>> +}
>> +
>> +static void pds_vdpa_set_vq_ready(struct vdpa_device *vdpa_dev, u16 qid, bool ready)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +       struct pci_dev *pdev = pdsv->vdpa_aux->padev->vf->pdev;
>> +       struct device *dev = &pdsv->vdpa_dev.dev;
>> +       int irq;
>> +       int err;
>> +
>> +       dev_dbg(dev, "%s: qid %d ready %d => %d\n",
>> +               __func__, qid, pdsv->vqs[qid].ready, ready);
>> +       if (ready == pdsv->vqs[qid].ready)
>> +               return;
>> +
>> +       if (ready) {
>> +               irq = pci_irq_vector(pdev, qid);
>> +               snprintf(pdsv->vqs[qid].irq_name, sizeof(pdsv->vqs[qid].irq_name),
>> +                        "vdpa-%s-%d", dev_name(dev), qid);
>> +
>> +               err = request_irq(irq, pds_vdpa_isr, 0,
>> +                                 pdsv->vqs[qid].irq_name, &pdsv->vqs[qid]);
>> +               if (err) {
>> +                       dev_err(dev, "%s: no irq for qid %d: %pe\n",
>> +                               __func__, qid, ERR_PTR(err));
>> +                       return;
>> +               }
>> +               pdsv->vqs[qid].irq = irq;
>> +
>> +               /* Pass vq setup info to DSC */
>> +               err = pds_vdpa_cmd_init_vq(pdsv, qid, &pdsv->vqs[qid]);
>> +               if (err) {
>> +                       pds_vdpa_release_irq(pdsv, qid);
>> +                       ready = false;
>> +               }
>> +       } else {
>> +               err = pds_vdpa_cmd_reset_vq(pdsv, qid);
>> +               if (err)
>> +                       dev_err(dev, "%s: reset_vq failed qid %d: %pe\n",
>> +                               __func__, qid, ERR_PTR(err));
>> +               pds_vdpa_release_irq(pdsv, qid);
>> +       }
>> +
>> +       pdsv->vqs[qid].ready = ready;
>> +}
>> +
>> +static bool pds_vdpa_get_vq_ready(struct vdpa_device *vdpa_dev, u16 qid)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +
>> +       return pdsv->vqs[qid].ready;
>> +}
>> +
>> +static int pds_vdpa_set_vq_state(struct vdpa_device *vdpa_dev, u16 qid,
>> +                                const struct vdpa_vq_state *state)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +       struct pds_auxiliary_dev *padev = pdsv->vdpa_aux->padev;
>> +       struct device *dev = &padev->aux_dev.dev;
>> +       struct pds_vdpa_vq_set_state_cmd cmd = {
>> +               .opcode = PDS_VDPA_CMD_VQ_SET_STATE,
>> +               .vdpa_index = pdsv->vdpa_index,
>> +               .vf_id = cpu_to_le16(pdsv->vdpa_aux->vf_id),
>> +               .qid = cpu_to_le16(qid),
>> +       };
>> +       struct pds_vdpa_comp comp = {0};
>> +       int err;
>> +
>> +       dev_dbg(dev, "%s: qid %d avail %#x\n",
>> +               __func__, qid, state->packed.last_avail_idx);
>> +
>> +       if (pdsv->actual_features & VIRTIO_F_RING_PACKED) {
>> +               cmd.avail = cpu_to_le16(state->packed.last_avail_idx |
>> +                                       (state->packed.last_avail_counter << 15));
>> +               cmd.used = cpu_to_le16(state->packed.last_used_idx |
>> +                                      (state->packed.last_used_counter << 15));
>> +       } else {
>> +               cmd.avail = cpu_to_le16(state->split.avail_index);
>> +               /* state->split does not provide a used_index:
>> +                * the vq will be set to "empty" here, and the vq will read
>> +                * the current used index the next time the vq is kicked.
>> +                */
>> +               cmd.used = cpu_to_le16(state->split.avail_index);
>> +       }
>> +
>> +       err = padev->ops->adminq_cmd(padev,
>> +                                    (union pds_core_adminq_cmd *)&cmd,
>> +                                    sizeof(cmd),
>> +                                    (union pds_core_adminq_comp *)&comp,
>> +                                    0);
> 
> I had one question for adminq command. I think we should use PF
> instead of VF but in __pdsc_adminq_post() I saw:
> 
>          q_info->dest = comp;
>          memcpy(q_info->desc, cmd, sizeof(*cmd));
> 
> So cmd should be fine since it is copied to the q_info->desc which is
> already mapped. But q_info->dest look suspicious, where did it mapped?

The queue descriptors get allocated and mapped as a large single block 
in pdsc_qcq_alloc() with a call to dma_alloc_coherent(), then 
pdsc_q_map() sets up the q_info[].dest pointers.


> 
> Thanks
> 
> 
>> +       if (err)
>> +               dev_err(dev, "Failed to set vq state qid %u, status %d: %pe\n",
>> +                       qid, comp.status, ERR_PTR(err));
>> +
>> +       return err;
>> +}
>> +
>> +static int pds_vdpa_get_vq_state(struct vdpa_device *vdpa_dev, u16 qid,
>> +                                struct vdpa_vq_state *state)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +       struct pds_auxiliary_dev *padev = pdsv->vdpa_aux->padev;
>> +       struct device *dev = &padev->aux_dev.dev;
>> +       struct pds_vdpa_vq_get_state_cmd cmd = {
>> +               .opcode = PDS_VDPA_CMD_VQ_GET_STATE,
>> +               .vdpa_index = pdsv->vdpa_index,
>> +               .vf_id = cpu_to_le16(pdsv->vdpa_aux->vf_id),
>> +               .qid = cpu_to_le16(qid),
>> +       };
>> +       struct pds_vdpa_vq_get_state_comp comp = {0};
>> +       int err;
>> +
>> +       dev_dbg(dev, "%s: qid %d\n", __func__, qid);
>> +
>> +       err = padev->ops->adminq_cmd(padev,
>> +                                    (union pds_core_adminq_cmd *)&cmd,
>> +                                    sizeof(cmd),
>> +                                    (union pds_core_adminq_comp *)&comp,
>> +                                    0);
>> +       if (err) {
>> +               dev_err(dev, "Failed to get vq state qid %u, status %d: %pe\n",
>> +                       qid, comp.status, ERR_PTR(err));
>> +               return err;
>> +       }
>> +
>> +       if (pdsv->actual_features & VIRTIO_F_RING_PACKED) {
>> +               state->packed.last_avail_idx = le16_to_cpu(comp.avail) & 0x7fff;
>> +               state->packed.last_avail_counter = le16_to_cpu(comp.avail) >> 15;
>> +       } else {
>> +               state->split.avail_index = le16_to_cpu(comp.avail);
>> +               /* state->split does not provide a used_index. */
>> +       }
>> +
>> +       return err;
>> +}
>> +
>> +static struct vdpa_notification_area
>> +pds_vdpa_get_vq_notification(struct vdpa_device *vdpa_dev, u16 qid)
>> +{
>> +       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>> +       struct virtio_pci_modern_device *vd_mdev;
>> +       struct vdpa_notification_area area;
>> +
>> +       area.addr = pdsv->vqs[qid].notify_pa;
>> +
>> +       vd_mdev = &pdsv->vdpa_aux->vd_mdev;
>> +       if (!vd_mdev->notify_offset_multiplier)
>> +               area.size = PAGE_SIZE;
> 
> Note that PAGE_SIZE varies among archs, I doubt we should use a fixed size here.

Yeah, good thought, I'll fix that up.

> 
> Others look good.
> 
> Thanks
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ