On Thu, May 25, 2023 at 5:38 PM Zhu, Lingshan <lingshan.zhu@xxxxxxxxx> wrote:
>
>
>
> On 5/24/2023 4:03 PM, Jason Wang wrote:
> > On Mon, May 8, 2023 at 6:05 PM Zhu Lingshan <lingshan.zhu@xxxxxxxxx> wrote:
> >> This commit synchronize irqs of the virtqueues
> >> and config space in the reset routine.
> >> Thus ifcvf_stop_hw() and reset() are refactored as well.
> >>
> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
> >> ---
> >> drivers/vdpa/ifcvf/ifcvf_base.c | 41 +++++++++++++++++++++--------
> >> drivers/vdpa/ifcvf/ifcvf_base.h | 1 +
> >> drivers/vdpa/ifcvf/ifcvf_main.c | 46 +++++----------------------------
> >> 3 files changed, 38 insertions(+), 50 deletions(-)
> >>
> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> >> index 79e313c5e10e..1f39290baa38 100644
> >> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> >> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> >> @@ -170,12 +170,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
> >>
> >> void ifcvf_reset(struct ifcvf_hw *hw)
> >> {
> >> - hw->config_cb.callback = NULL;
> >> - hw->config_cb.private = NULL;
> >> -
> >> ifcvf_set_status(hw, 0);
> >> - /* flush set_status, make sure VF is stopped, reset */
> >> - ifcvf_get_status(hw);
> >> + while (ifcvf_get_status(hw))
> >> + msleep(1);
> >> }
> >>
> >> u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
> >> @@ -368,20 +365,42 @@ void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready)
> >> vp_iowrite16(ready, &cfg->queue_enable);
> >> }
> >>
> >> -static void ifcvf_hw_disable(struct ifcvf_hw *hw)
> >> +static void ifcvf_reset_vring(struct ifcvf_hw *hw)
> >> {
> >> - u32 i;
> >> + u16 qid;
> >> +
> >> + for (qid = 0; qid < hw->nr_vring; qid++) {
> >> + hw->vring[qid].cb.callback = NULL;
> >> + hw->vring[qid].cb.private = NULL;
> >> + ifcvf_set_vq_vector(hw, qid, VIRTIO_MSI_NO_VECTOR);
> >> + }
> >> +}
> >>
> >> +static void ifcvf_reset_config_handler(struct ifcvf_hw *hw)
> >> +{
> >> + hw->config_cb.callback = NULL;
> >> + hw->config_cb.private = NULL;
> >> ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
> >> - for (i = 0; i < hw->nr_vring; i++) {
> >> - ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
> >> +}
> >> +
> >> +static void ifcvf_synchronize_irq(struct ifcvf_hw *hw)
> >> +{
> >> + u32 nvectors = hw->num_msix_vectors;
> >> + struct pci_dev *pdev = hw->pdev;
> >> + int i, irq;
> >> +
> >> + for (i = 0; i < nvectors; i++) {
> >> + irq = pci_irq_vector(pdev, i);
> >> + if (irq >= 0)
> >> + synchronize_irq(irq);
> >> }
> >> }
> >>
> >> void ifcvf_stop_hw(struct ifcvf_hw *hw)
> >> {
> >> - ifcvf_hw_disable(hw);
> >> - ifcvf_reset(hw);
> >> + ifcvf_synchronize_irq(hw);
> >> + ifcvf_reset_vring(hw);
> >> + ifcvf_reset_config_handler(hw);
> > Nit:
> >
> > So the name of this function is kind of misleading since irq
> > synchronization and virtqueue/config handler are not belong to
> > hardware?
> >
> > Maybe it would be better to call it ifcvf_stop().
> Sure, I will send a V3 with this renaming,
> do you ack patch 1/5?
Yes, I think I've acked to that patch.
Thanks
>
> Thanks
> Zhu Lingshan
> >
> > Thanks
> >
> >> }
> >>
> >> void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid)
> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> >> index d34d3bc0dbf4..7430f80779be 100644
> >> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> >> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> >> @@ -82,6 +82,7 @@ struct ifcvf_hw {
> >> int vqs_reused_irq;
> >> u16 nr_vring;
> >> /* VIRTIO_PCI_CAP_DEVICE_CFG size */
> >> + u32 num_msix_vectors;
> >> u32 cap_dev_config_size;
> >> struct pci_dev *pdev;
> >> };
> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> >> index 968687159e44..3401b9901dd2 100644
> >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> >> @@ -125,6 +125,7 @@ static void ifcvf_free_irq(struct ifcvf_hw *vf)
> >> ifcvf_free_vq_irq(vf);
> >> ifcvf_free_config_irq(vf);
> >> ifcvf_free_irq_vectors(pdev);
> >> + vf->num_msix_vectors = 0;
> >> }
> >>
> >> /* ifcvf MSIX vectors allocator, this helper tries to allocate
> >> @@ -343,36 +344,11 @@ static int ifcvf_request_irq(struct ifcvf_hw *vf)
> >> if (ret)
> >> return ret;
> >>
> >> - return 0;
> >> -}
> >> -
> >> -static int ifcvf_stop_datapath(struct ifcvf_adapter *adapter)
> >> -{
> >> - struct ifcvf_hw *vf = adapter->vf;
> >> - int i;
> >> -
> >> - for (i = 0; i < vf->nr_vring; i++)
> >> - vf->vring[i].cb.callback = NULL;
> >> -
> >> - ifcvf_stop_hw(vf);
> >> + vf->num_msix_vectors = nvectors;
> >>
> >> return 0;
> >> }
> >>
> >> -static void ifcvf_reset_vring(struct ifcvf_adapter *adapter)
> >> -{
> >> - struct ifcvf_hw *vf = adapter->vf;
> >> - int i;
> >> -
> >> - for (i = 0; i < vf->nr_vring; i++) {
> >> - vf->vring[i].last_avail_idx = 0;
> >> - vf->vring[i].cb.callback = NULL;
> >> - vf->vring[i].cb.private = NULL;
> >> - }
> >> -
> >> - ifcvf_reset(vf);
> >> -}
> >> -
> >> static struct ifcvf_adapter *vdpa_to_adapter(struct vdpa_device *vdpa_dev)
> >> {
> >> return container_of(vdpa_dev, struct ifcvf_adapter, vdpa);
> >> @@ -462,23 +438,15 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
> >>
> >> static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
> >> {
> >> - struct ifcvf_adapter *adapter;
> >> - struct ifcvf_hw *vf;
> >> - u8 status_old;
> >> -
> >> - vf = vdpa_to_vf(vdpa_dev);
> >> - adapter = vdpa_to_adapter(vdpa_dev);
> >> - status_old = ifcvf_get_status(vf);
> >> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
> >> + u8 status = ifcvf_get_status(vf);
> >>
> >> - if (status_old == 0)
> >> - return 0;
> >> + ifcvf_stop_hw(vf);
> >>
> >> - if (status_old & VIRTIO_CONFIG_S_DRIVER_OK) {
> >> - ifcvf_stop_datapath(adapter);
> >> + if (status & VIRTIO_CONFIG_S_DRIVER_OK)
> >> ifcvf_free_irq(vf);
> >> - }
> >>
> >> - ifcvf_reset_vring(adapter);
> >> + ifcvf_reset(vf);
> >>
> >> return 0;
> >> }
> >> --
> >> 2.39.1
> >>
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
