All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] ifcvf: move IRQ request/free to status change handlers
@ 2020-05-12  8:00 Zhu Lingshan
  2020-05-13  4:12 ` Jason Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Zhu Lingshan @ 2020-05-12  8:00 UTC (permalink / raw)
  To: mst, kvm, virtualization, linux-kernel, netdev, jasowang
  Cc: lulu, dan.daly, cunming.liang, Zhu Lingshan

This commit move IRQ request and free operations from probe()
to VIRTIO status change handler to comply with VIRTIO spec.

VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
The device MUST NOT consume buffers or send any used buffer
notifications to the driver before DRIVER_OK.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
changes from V1:
remove ifcvf_stop_datapath() in status == 0 handler, we don't need to do this
twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler (Jason Wang)

 drivers/vdpa/ifcvf/ifcvf_main.c | 120 ++++++++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 47 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index abf6a061..d529ed6 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -28,6 +28,60 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static void ifcvf_free_irq_vectors(void *data)
+{
+	pci_free_irq_vectors(data);
+}
+
+static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues)
+{
+	struct pci_dev *pdev = adapter->pdev;
+	struct ifcvf_hw *vf = &adapter->vf;
+	int i;
+
+
+	for (i = 0; i < queues; i++)
+		devm_free_irq(&pdev->dev, vf->vring[i].irq, &vf->vring[i]);
+
+	ifcvf_free_irq_vectors(pdev);
+}
+
+static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
+{
+	struct pci_dev *pdev = adapter->pdev;
+	struct ifcvf_hw *vf = &adapter->vf;
+	int vector, i, ret, irq;
+
+	ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR,
+				    IFCVF_MAX_INTR, PCI_IRQ_MSIX);
+	if (ret < 0) {
+		IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n");
+		return ret;
+	}
+
+	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
+		snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
+			 pci_name(pdev), i);
+		vector = i + IFCVF_MSI_QUEUE_OFF;
+		irq = pci_irq_vector(pdev, vector);
+		ret = devm_request_irq(&pdev->dev, irq,
+				       ifcvf_intr_handler, 0,
+				       vf->vring[i].msix_name,
+				       &vf->vring[i]);
+		if (ret) {
+			IFCVF_ERR(pdev,
+				  "Failed to request irq for vq %d\n", i);
+			ifcvf_free_irq(adapter, i);
+
+			return ret;
+		}
+
+		vf->vring[i].irq = irq;
+	}
+
+	return 0;
+}
+
 static int ifcvf_start_datapath(void *private)
 {
 	struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
@@ -118,17 +172,34 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
 {
 	struct ifcvf_adapter *adapter;
 	struct ifcvf_hw *vf;
+	u8 status_old;
+	int ret;
 
 	vf  = vdpa_to_vf(vdpa_dev);
 	adapter = dev_get_drvdata(vdpa_dev->dev.parent);
+	status_old = ifcvf_get_status(vf);
 
-	if (status == 0) {
+	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) &&
+	    !(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
 		ifcvf_stop_datapath(adapter);
+		ifcvf_free_irq(adapter, IFCVF_MAX_QUEUE_PAIRS * 2);
+	}
+
+	if (status == 0) {
 		ifcvf_reset_vring(adapter);
 		return;
 	}
 
-	if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+	if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+	    !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) {
+		ret = ifcvf_request_irq(adapter);
+		if (ret) {
+			status = ifcvf_get_status(vf);
+			status |= VIRTIO_CONFIG_S_FAILED;
+			ifcvf_set_status(vf, status);
+			return;
+		}
+
 		if (ifcvf_start_datapath(adapter) < 0)
 			IFCVF_ERR(adapter->pdev,
 				  "Failed to set ifcvf vdpa  status %u\n",
@@ -284,38 +355,6 @@ static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
 	.set_config_cb  = ifcvf_vdpa_set_config_cb,
 };
 
-static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
-{
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
-	int vector, i, ret, irq;
-
-
-	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
-		snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
-			 pci_name(pdev), i);
-		vector = i + IFCVF_MSI_QUEUE_OFF;
-		irq = pci_irq_vector(pdev, vector);
-		ret = devm_request_irq(&pdev->dev, irq,
-				       ifcvf_intr_handler, 0,
-				       vf->vring[i].msix_name,
-				       &vf->vring[i]);
-		if (ret) {
-			IFCVF_ERR(pdev,
-				  "Failed to request irq for vq %d\n", i);
-			return ret;
-		}
-		vf->vring[i].irq = irq;
-	}
-
-	return 0;
-}
-
-static void ifcvf_free_irq_vectors(void *data)
-{
-	pci_free_irq_vectors(data);
-}
-
 static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct device *dev = &pdev->dev;
@@ -349,13 +388,6 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return ret;
 	}
 
-	ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR,
-				    IFCVF_MAX_INTR, PCI_IRQ_MSIX);
-	if (ret < 0) {
-		IFCVF_ERR(pdev, "Failed to alloc irq vectors\n");
-		return ret;
-	}
-
 	ret = devm_add_action_or_reset(dev, ifcvf_free_irq_vectors, pdev);
 	if (ret) {
 		IFCVF_ERR(pdev,
@@ -379,12 +411,6 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	adapter->pdev = pdev;
 	adapter->vdpa.dma_dev = &pdev->dev;
 
-	ret = ifcvf_request_irq(adapter);
-	if (ret) {
-		IFCVF_ERR(pdev, "Failed to request MSI-X irq\n");
-		goto err;
-	}
-
 	ret = ifcvf_init_hw(vf, pdev);
 	if (ret) {
 		IFCVF_ERR(pdev, "Failed to init IFCVF hw\n");
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ifcvf: move IRQ request/free to status change handlers
  2020-05-12  8:00 [PATCH V2] ifcvf: move IRQ request/free to status change handlers Zhu Lingshan
@ 2020-05-13  4:12 ` Jason Wang
       [not found]   ` <5bbe0c21-8638-45e4-04e8-02ad0df44b38@intel.com>
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Wang @ 2020-05-13  4:12 UTC (permalink / raw)
  To: Zhu Lingshan, mst, kvm, virtualization, linux-kernel, netdev
  Cc: lulu, dan.daly, cunming.liang


On 2020/5/12 下午4:00, Zhu Lingshan wrote:
> This commit move IRQ request and free operations from probe()
> to VIRTIO status change handler to comply with VIRTIO spec.
>
> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
> The device MUST NOT consume buffers or send any used buffer
> notifications to the driver before DRIVER_OK.


This comment needs to be checked as I said previously. It's only needed 
if we're sure ifcvf can generate interrupt before DRIVER_OK.


>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
> changes from V1:
> remove ifcvf_stop_datapath() in status == 0 handler, we don't need to do this
> twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler (Jason Wang)


Patch looks good to me, but with this patch ping cannot work on my 
machine. (It works without this patch).

Thanks


>
>   drivers/vdpa/ifcvf/ifcvf_main.c | 120 ++++++++++++++++++++++++----------------
>   1 file changed, 73 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index abf6a061..d529ed6 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -28,6 +28,60 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
>   	return IRQ_HANDLED;
>   }
>   
> +static void ifcvf_free_irq_vectors(void *data)
> +{
> +	pci_free_irq_vectors(data);
> +}
> +
> +static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues)
> +{
> +	struct pci_dev *pdev = adapter->pdev;
> +	struct ifcvf_hw *vf = &adapter->vf;
> +	int i;
> +
> +
> +	for (i = 0; i < queues; i++)
> +		devm_free_irq(&pdev->dev, vf->vring[i].irq, &vf->vring[i]);
> +
> +	ifcvf_free_irq_vectors(pdev);
> +}
> +
> +static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
> +{
> +	struct pci_dev *pdev = adapter->pdev;
> +	struct ifcvf_hw *vf = &adapter->vf;
> +	int vector, i, ret, irq;
> +
> +	ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR,
> +				    IFCVF_MAX_INTR, PCI_IRQ_MSIX);
> +	if (ret < 0) {
> +		IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n");
> +		return ret;
> +	}
> +
> +	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
> +		snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
> +			 pci_name(pdev), i);
> +		vector = i + IFCVF_MSI_QUEUE_OFF;
> +		irq = pci_irq_vector(pdev, vector);
> +		ret = devm_request_irq(&pdev->dev, irq,
> +				       ifcvf_intr_handler, 0,
> +				       vf->vring[i].msix_name,
> +				       &vf->vring[i]);
> +		if (ret) {
> +			IFCVF_ERR(pdev,
> +				  "Failed to request irq for vq %d\n", i);
> +			ifcvf_free_irq(adapter, i);
> +
> +			return ret;
> +		}
> +
> +		vf->vring[i].irq = irq;
> +	}
> +
> +	return 0;
> +}
> +
>   static int ifcvf_start_datapath(void *private)
>   {
>   	struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
> @@ -118,17 +172,34 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>   {
>   	struct ifcvf_adapter *adapter;
>   	struct ifcvf_hw *vf;
> +	u8 status_old;
> +	int ret;
>   
>   	vf  = vdpa_to_vf(vdpa_dev);
>   	adapter = dev_get_drvdata(vdpa_dev->dev.parent);
> +	status_old = ifcvf_get_status(vf);
>   
> -	if (status == 0) {
> +	if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) &&
> +	    !(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>   		ifcvf_stop_datapath(adapter);
> +		ifcvf_free_irq(adapter, IFCVF_MAX_QUEUE_PAIRS * 2);
> +	}
> +
> +	if (status == 0) {
>   		ifcvf_reset_vring(adapter);
>   		return;
>   	}
>   
> -	if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> +	if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> +	    !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) {
> +		ret = ifcvf_request_irq(adapter);
> +		if (ret) {
> +			status = ifcvf_get_status(vf);
> +			status |= VIRTIO_CONFIG_S_FAILED;
> +			ifcvf_set_status(vf, status);
> +			return;
> +		}
> +
>   		if (ifcvf_start_datapath(adapter) < 0)
>   			IFCVF_ERR(adapter->pdev,
>   				  "Failed to set ifcvf vdpa  status %u\n",
> @@ -284,38 +355,6 @@ static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
>   	.set_config_cb  = ifcvf_vdpa_set_config_cb,
>   };
>   
> -static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
> -{
> -	struct pci_dev *pdev = adapter->pdev;
> -	struct ifcvf_hw *vf = &adapter->vf;
> -	int vector, i, ret, irq;
> -
> -
> -	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
> -		snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
> -			 pci_name(pdev), i);
> -		vector = i + IFCVF_MSI_QUEUE_OFF;
> -		irq = pci_irq_vector(pdev, vector);
> -		ret = devm_request_irq(&pdev->dev, irq,
> -				       ifcvf_intr_handler, 0,
> -				       vf->vring[i].msix_name,
> -				       &vf->vring[i]);
> -		if (ret) {
> -			IFCVF_ERR(pdev,
> -				  "Failed to request irq for vq %d\n", i);
> -			return ret;
> -		}
> -		vf->vring[i].irq = irq;
> -	}
> -
> -	return 0;
> -}
> -
> -static void ifcvf_free_irq_vectors(void *data)
> -{
> -	pci_free_irq_vectors(data);
> -}
> -
>   static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -349,13 +388,6 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		return ret;
>   	}
>   
> -	ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR,
> -				    IFCVF_MAX_INTR, PCI_IRQ_MSIX);
> -	if (ret < 0) {
> -		IFCVF_ERR(pdev, "Failed to alloc irq vectors\n");
> -		return ret;
> -	}
> -
>   	ret = devm_add_action_or_reset(dev, ifcvf_free_irq_vectors, pdev);
>   	if (ret) {
>   		IFCVF_ERR(pdev,
> @@ -379,12 +411,6 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	adapter->pdev = pdev;
>   	adapter->vdpa.dma_dev = &pdev->dev;
>   
> -	ret = ifcvf_request_irq(adapter);
> -	if (ret) {
> -		IFCVF_ERR(pdev, "Failed to request MSI-X irq\n");
> -		goto err;
> -	}
> -
>   	ret = ifcvf_init_hw(vf, pdev);
>   	if (ret) {
>   		IFCVF_ERR(pdev, "Failed to init IFCVF hw\n");


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ifcvf: move IRQ request/free to status change handlers
       [not found]   ` <5bbe0c21-8638-45e4-04e8-02ad0df44b38@intel.com>
@ 2020-05-13  5:57       ` Jason Wang
  2020-05-13  7:18     ` Jason Wang
  1 sibling, 0 replies; 7+ messages in thread
From: Jason Wang @ 2020-05-13  5:57 UTC (permalink / raw)
  To: Zhu, Lingshan, mst, kvm, virtualization, linux-kernel, netdev
  Cc: lulu, dan.daly, cunming.liang


On 2020/5/13 下午12:42, Zhu, Lingshan wrote:
>
>
> On 5/13/2020 12:12 PM, Jason Wang wrote:
>>
>> On 2020/5/12 下午4:00, Zhu Lingshan wrote:
>>> This commit move IRQ request and free operations from probe()
>>> to VIRTIO status change handler to comply with VIRTIO spec.
>>>
>>> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
>>> The device MUST NOT consume buffers or send any used buffer
>>> notifications to the driver before DRIVER_OK.
>>
>>
>> This comment needs to be checked as I said previously. It's only 
>> needed if we're sure ifcvf can generate interrupt before DRIVER_OK.
>>
>>
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>> ---
>>> changes from V1:
>>> remove ifcvf_stop_datapath() in status == 0 handler, we don't need 
>>> to do this
>>> twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler 
>>> (Jason Wang)
>>
>>
>> Patch looks good to me, but with this patch ping cannot work on my 
>> machine. (It works without this patch).
>>
>> Thanks
> This is strange, it works on my machines, let's have a check offline.
>
> Thanks,
> BR
> Zhu Lingshan


Note that I tested the patch with vhost-vpda.

Thanks.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ifcvf: move IRQ request/free to status change handlers
@ 2020-05-13  5:57       ` Jason Wang
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Wang @ 2020-05-13  5:57 UTC (permalink / raw)
  To: Zhu, Lingshan, mst, kvm, virtualization, linux-kernel, netdev; +Cc: lulu


On 2020/5/13 下午12:42, Zhu, Lingshan wrote:
>
>
> On 5/13/2020 12:12 PM, Jason Wang wrote:
>>
>> On 2020/5/12 下午4:00, Zhu Lingshan wrote:
>>> This commit move IRQ request and free operations from probe()
>>> to VIRTIO status change handler to comply with VIRTIO spec.
>>>
>>> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
>>> The device MUST NOT consume buffers or send any used buffer
>>> notifications to the driver before DRIVER_OK.
>>
>>
>> This comment needs to be checked as I said previously. It's only 
>> needed if we're sure ifcvf can generate interrupt before DRIVER_OK.
>>
>>
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>> ---
>>> changes from V1:
>>> remove ifcvf_stop_datapath() in status == 0 handler, we don't need 
>>> to do this
>>> twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler 
>>> (Jason Wang)
>>
>>
>> Patch looks good to me, but with this patch ping cannot work on my 
>> machine. (It works without this patch).
>>
>> Thanks
> This is strange, it works on my machines, let's have a check offline.
>
> Thanks,
> BR
> Zhu Lingshan


Note that I tested the patch with vhost-vpda.

Thanks.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ifcvf: move IRQ request/free to status change handlers
       [not found]   ` <5bbe0c21-8638-45e4-04e8-02ad0df44b38@intel.com>
  2020-05-13  5:57       ` Jason Wang
@ 2020-05-13  7:18     ` Jason Wang
  2020-05-19  1:51       ` Cindy Lu
  1 sibling, 1 reply; 7+ messages in thread
From: Jason Wang @ 2020-05-13  7:18 UTC (permalink / raw)
  To: Zhu, Lingshan, mst, kvm, virtualization, linux-kernel, netdev
  Cc: lulu, dan.daly, cunming.liang


On 2020/5/13 下午12:42, Zhu, Lingshan wrote:
>
>
> On 5/13/2020 12:12 PM, Jason Wang wrote:
>>
>> On 2020/5/12 下午4:00, Zhu Lingshan wrote:
>>> This commit move IRQ request and free operations from probe()
>>> to VIRTIO status change handler to comply with VIRTIO spec.
>>>
>>> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
>>> The device MUST NOT consume buffers or send any used buffer
>>> notifications to the driver before DRIVER_OK.
>>
>>
>> This comment needs to be checked as I said previously. It's only 
>> needed if we're sure ifcvf can generate interrupt before DRIVER_OK.
>>
>>
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>> ---
>>> changes from V1:
>>> remove ifcvf_stop_datapath() in status == 0 handler, we don't need 
>>> to do this
>>> twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler 
>>> (Jason Wang)
>>
>>
>> Patch looks good to me, but with this patch ping cannot work on my 
>> machine. (It works without this patch).
>>
>> Thanks
> This is strange, it works on my machines, let's have a check offline.
>
> Thanks,
> BR
> Zhu Lingshan


I give it a try with virito-vpda and a tiny userspace. Either works.

So it could be an issue of qemu codes.

Let's wait for Cindy to test if it really works.

Thanks



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ifcvf: move IRQ request/free to status change handlers
  2020-05-13  7:18     ` Jason Wang
@ 2020-05-19  1:51       ` Cindy Lu
  2020-05-19  5:52         ` Jason Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Cindy Lu @ 2020-05-19  1:51 UTC (permalink / raw)
  To: Jason Wang
  Cc: Zhu, Lingshan, Michael Tsirkin, kvm, virtualization,
	linux-kernel, netdev, dan.daly, Liang, Cunming

Hi ,Jason
It works ok in the latest version of qemu vdpa code , So I think the
patch is ok.
Thanks
Cindy
On Wed, May 13, 2020 at 3:18 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2020/5/13 下午12:42, Zhu, Lingshan wrote:
> >
> >
> > On 5/13/2020 12:12 PM, Jason Wang wrote:
> >>
> >> On 2020/5/12 下午4:00, Zhu Lingshan wrote:
> >>> This commit move IRQ request and free operations from probe()
> >>> to VIRTIO status change handler to comply with VIRTIO spec.
> >>>
> >>> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
> >>> The device MUST NOT consume buffers or send any used buffer
> >>> notifications to the driver before DRIVER_OK.
> >>
> >>
> >> This comment needs to be checked as I said previously. It's only
> >> needed if we're sure ifcvf can generate interrupt before DRIVER_OK.
> >>
> >>
> >>>
> >>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>> ---
> >>> changes from V1:
> >>> remove ifcvf_stop_datapath() in status == 0 handler, we don't need
> >>> to do this
> >>> twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler
> >>> (Jason Wang)
> >>
> >>
> >> Patch looks good to me, but with this patch ping cannot work on my
> >> machine. (It works without this patch).
> >>
> >> Thanks
> > This is strange, it works on my machines, let's have a check offline.
> >
> > Thanks,
> > BR
> > Zhu Lingshan
>
>
> I give it a try with virito-vpda and a tiny userspace. Either works.
>
> So it could be an issue of qemu codes.
>
> Let's wait for Cindy to test if it really works.
>
> Thanks
>
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] ifcvf: move IRQ request/free to status change handlers
  2020-05-19  1:51       ` Cindy Lu
@ 2020-05-19  5:52         ` Jason Wang
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Wang @ 2020-05-19  5:52 UTC (permalink / raw)
  To: Cindy Lu
  Cc: Zhu, Lingshan, Michael Tsirkin, kvm, virtualization,
	linux-kernel, netdev, dan.daly, Liang, Cunming


On 2020/5/19 上午9:51, Cindy Lu wrote:
> Hi ,Jason
> It works ok in the latest version of qemu vdpa code , So I think the
> patch is ok.
> Thanks
> Cindy


Thanks for the testing, (btw, we'd better not do top posting when 
discuss in the community).

So,

Acked-by: Jason Wang <jasowang@redhat.com>



> On Wed, May 13, 2020 at 3:18 PM Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 2020/5/13 下午12:42, Zhu, Lingshan wrote:
>>>
>>> On 5/13/2020 12:12 PM, Jason Wang wrote:
>>>> On 2020/5/12 下午4:00, Zhu Lingshan wrote:
>>>>> This commit move IRQ request and free operations from probe()
>>>>> to VIRTIO status change handler to comply with VIRTIO spec.
>>>>>
>>>>> VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field
>>>>> The device MUST NOT consume buffers or send any used buffer
>>>>> notifications to the driver before DRIVER_OK.
>>>>
>>>> This comment needs to be checked as I said previously. It's only
>>>> needed if we're sure ifcvf can generate interrupt before DRIVER_OK.
>>>>
>>>>
>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>> ---
>>>>> changes from V1:
>>>>> remove ifcvf_stop_datapath() in status == 0 handler, we don't need
>>>>> to do this
>>>>> twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler
>>>>> (Jason Wang)
>>>>
>>>> Patch looks good to me, but with this patch ping cannot work on my
>>>> machine. (It works without this patch).
>>>>
>>>> Thanks
>>> This is strange, it works on my machines, let's have a check offline.
>>>
>>> Thanks,
>>> BR
>>> Zhu Lingshan
>>
>> I give it a try with virito-vpda and a tiny userspace. Either works.
>>
>> So it could be an issue of qemu codes.
>>
>> Let's wait for Cindy to test if it really works.
>>
>> Thanks
>>
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-19  5:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-12  8:00 [PATCH V2] ifcvf: move IRQ request/free to status change handlers Zhu Lingshan
2020-05-13  4:12 ` Jason Wang
     [not found]   ` <5bbe0c21-8638-45e4-04e8-02ad0df44b38@intel.com>
2020-05-13  5:57     ` Jason Wang
2020-05-13  5:57       ` Jason Wang
2020-05-13  7:18     ` Jason Wang
2020-05-19  1:51       ` Cindy Lu
2020-05-19  5:52         ` Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.