* [PATCH 1/3] virtio: update reset callback to return status
@ 2021-04-07 12:09 Max Gurtovoy
2021-04-07 12:09 ` [PATCH 2/3] virito_pci: add timeout to reset device operation Max Gurtovoy
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Max Gurtovoy @ 2021-04-07 12:09 UTC (permalink / raw)
To: mst, kvm, virtualization, jasowang; +Cc: oren, nitzanc, Max Gurtovoy
The reset device operation, usually is an operation that might fail from
various reasons. For example, the controller might be in a bad state and
can't answer to any request. Usually, the paravirt SW based virtio
devices always succeed in reset operation but this is not the case for
HW based virtio devices.
This commit is also a preparation for adding a timeout mechanism for
resetting virtio devices.
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
drivers/remoteproc/remoteproc_virtio.c | 3 ++-
drivers/virtio/virtio.c | 22 +++++++++++++++-------
drivers/virtio/virtio_mmio.c | 3 ++-
drivers/virtio/virtio_pci_legacy.c | 3 ++-
drivers/virtio/virtio_pci_modern.c | 3 ++-
drivers/virtio/virtio_vdpa.c | 3 ++-
include/linux/virtio_config.h | 5 +++--
7 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 0cc617f76068..432f39288c15 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -191,7 +191,7 @@ static void rproc_virtio_set_status(struct virtio_device *vdev, u8 status)
dev_dbg(&vdev->dev, "status: %d\n", status);
}
-static void rproc_virtio_reset(struct virtio_device *vdev)
+static int rproc_virtio_reset(struct virtio_device *vdev)
{
struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
struct fw_rsc_vdev *rsc;
@@ -200,6 +200,7 @@ static void rproc_virtio_reset(struct virtio_device *vdev)
rsc->status = 0;
dev_dbg(&vdev->dev, "reset !\n");
+ return 0;
}
/* provide the vdev features as retrieved from the firmware */
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 4b15c00c0a0a..ddbfd5b5f3bd 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -338,7 +338,7 @@ int register_virtio_device(struct virtio_device *dev)
/* Assign a unique device index and hence name. */
err = ida_simple_get(&virtio_index_ida, 0, 0, GFP_KERNEL);
if (err < 0)
- goto out;
+ goto out_err;
dev->index = err;
dev_set_name(&dev->dev, "virtio%u", dev->index);
@@ -349,7 +349,9 @@ int register_virtio_device(struct virtio_device *dev)
/* We always start by resetting the device, in case a previous
* driver messed it up. This also tests that code path a little. */
- dev->config->reset(dev);
+ err = dev->config->reset(dev);
+ if (err)
+ goto out_ida;
/* Acknowledge that we've seen the device. */
virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
@@ -362,10 +364,14 @@ int register_virtio_device(struct virtio_device *dev)
*/
err = device_add(&dev->dev);
if (err)
- ida_simple_remove(&virtio_index_ida, dev->index);
-out:
- if (err)
- virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ goto out_ida;
+
+ return 0;
+
+out_ida:
+ ida_simple_remove(&virtio_index_ida, dev->index);
+out_err:
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
}
EXPORT_SYMBOL_GPL(register_virtio_device);
@@ -408,7 +414,9 @@ int virtio_device_restore(struct virtio_device *dev)
/* We always start by resetting the device, in case a previous
* driver messed it up. */
- dev->config->reset(dev);
+ ret = dev->config->reset(dev);
+ if (ret)
+ goto err;
/* Acknowledge that we've seen the device. */
virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 56128b9c46eb..12b8f048c48d 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -256,12 +256,13 @@ static void vm_set_status(struct virtio_device *vdev, u8 status)
writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
}
-static void vm_reset(struct virtio_device *vdev)
+static int vm_reset(struct virtio_device *vdev)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
/* 0 status means a reset. */
writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
+ return 0;
}
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index d62e9835aeec..ff4d9506aa65 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -89,7 +89,7 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
iowrite8(status, vp_dev->ioaddr + VIRTIO_PCI_STATUS);
}
-static void vp_reset(struct virtio_device *vdev)
+static int vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
/* 0 status means a reset. */
@@ -99,6 +99,7 @@ static void vp_reset(struct virtio_device *vdev)
ioread8(vp_dev->ioaddr + VIRTIO_PCI_STATUS);
/* Flush pending VQ/configuration callbacks. */
vp_synchronize_vectors(vdev);
+ return 0;
}
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index fbd4ebc00eb6..cc3412a96a17 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -158,7 +158,7 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
vp_modern_set_status(&vp_dev->mdev, status);
}
-static void vp_reset(struct virtio_device *vdev)
+static int vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
@@ -174,6 +174,7 @@ static void vp_reset(struct virtio_device *vdev)
msleep(1);
/* Flush pending VQ/configuration callbacks. */
vp_synchronize_vectors(vdev);
+ return 0;
}
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index e28acf482e0c..da76190f7b07 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -97,11 +97,12 @@ static void virtio_vdpa_set_status(struct virtio_device *vdev, u8 status)
return ops->set_status(vdpa, status);
}
-static void virtio_vdpa_reset(struct virtio_device *vdev)
+static int virtio_vdpa_reset(struct virtio_device *vdev)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
vdpa_reset(vdpa);
+ return 0;
}
static bool virtio_vdpa_notify(struct virtqueue *vq)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 8519b3ae5d52..d2b0f1699a75 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -44,9 +44,10 @@ struct virtio_shm_region {
* status: the new status byte
* @reset: reset the device
* vdev: the virtio device
- * After this, status and feature negotiation must be done again
+ * Upon success, status and feature negotiation must be done again
* Device must not be reset from its vq/config callbacks, or in
* parallel with being added/removed.
+ * Returns 0 on success or error status.
* @find_vqs: find virtqueues and instantiate them.
* vdev: the virtio_device
* nvqs: the number of virtqueues to find
@@ -82,7 +83,7 @@ struct virtio_config_ops {
u32 (*generation)(struct virtio_device *vdev);
u8 (*get_status)(struct virtio_device *vdev);
void (*set_status)(struct virtio_device *vdev, u8 status);
- void (*reset)(struct virtio_device *vdev);
+ int (*reset)(struct virtio_device *vdev);
int (*find_vqs)(struct virtio_device *, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
const char * const names[], const bool *ctx,
--
2.25.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/3] virito_pci: add timeout to reset device operation
2021-04-07 12:09 [PATCH 1/3] virtio: update reset callback to return status Max Gurtovoy
@ 2021-04-07 12:09 ` Max Gurtovoy
2021-04-07 13:45 ` Michael S. Tsirkin
2021-04-07 12:09 ` [PATCH 3/3] virtio_pci: add module param for reset_timeout Max Gurtovoy
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Max Gurtovoy @ 2021-04-07 12:09 UTC (permalink / raw)
To: mst, kvm, virtualization, jasowang; +Cc: oren, nitzanc, Max Gurtovoy
According to the spec after writing 0 to device_status, the driver MUST
wait for a read of device_status to return 0 before reinitializing the
device. In case we have a device that won't return 0, the reset
operation will loop forever and cause the host/vm to stuck. Set timeout
for 3 minutes before giving up on the device.
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
drivers/virtio/virtio_pci_modern.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index cc3412a96a17..dcee616e8d21 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+ unsigned long timeout = jiffies + msecs_to_jiffies(180000);
/* 0 status means a reset. */
vp_modern_set_status(mdev, 0);
@@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
* device_status to return 0 before reinitializing the device.
* This will flush out the status write, and flush in device writes,
* including MSI-X interrupts, if any.
+ * Set a timeout before giving up on the device.
*/
- while (vp_modern_get_status(mdev))
+ while (vp_modern_get_status(mdev)) {
+ if (time_after(jiffies, timeout)) {
+ dev_err(&vdev->dev, "virtio: device not ready. "
+ "Aborting. Try again later\n");
+ return -EAGAIN;
+ }
msleep(1);
+ }
/* Flush pending VQ/configuration callbacks. */
vp_synchronize_vectors(vdev);
return 0;
--
2.25.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] virito_pci: add timeout to reset device operation
2021-04-07 12:09 ` [PATCH 2/3] virito_pci: add timeout to reset device operation Max Gurtovoy
@ 2021-04-07 13:45 ` Michael S. Tsirkin
0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2021-04-07 13:45 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: kvm, virtualization, jasowang, oren, nitzanc
On Wed, Apr 07, 2021 at 12:09:23PM +0000, Max Gurtovoy wrote:
> According to the spec after writing 0 to device_status, the driver MUST
> wait for a read of device_status to return 0 before reinitializing the
> device. In case we have a device that won't return 0, the reset
> operation will loop forever and cause the host/vm to stuck. Set timeout
> for 3 minutes before giving up on the device.
>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> drivers/virtio/virtio_pci_modern.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index cc3412a96a17..dcee616e8d21 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> + unsigned long timeout = jiffies + msecs_to_jiffies(180000);
>
> /* 0 status means a reset. */
> vp_modern_set_status(mdev, 0);
> @@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
> * device_status to return 0 before reinitializing the device.
> * This will flush out the status write, and flush in device writes,
> * including MSI-X interrupts, if any.
> + * Set a timeout before giving up on the device.
> */
> - while (vp_modern_get_status(mdev))
> + while (vp_modern_get_status(mdev)) {
> + if (time_after(jiffies, timeout)) {
> + dev_err(&vdev->dev, "virtio: device not ready. "
> + "Aborting. Try again later\n");
> + return -EAGAIN;
> + }
> msleep(1);
> + }
> /* Flush pending VQ/configuration callbacks. */
> vp_synchronize_vectors(vdev);
> return 0;
Problem is everyone just ignores the return code from reset.
Timing out like that has a chance to cause a lot of trouble
if the device remains active - we need to make reset robust.
What exactly is going on with the device that
get status never returns 0? E.g. maybe it's in a state
where it's returning all 1's because it's wedged permanently -
using that would be better...
> --
> 2.25.4
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] virito_pci: add timeout to reset device operation
@ 2021-04-07 13:45 ` Michael S. Tsirkin
0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2021-04-07 13:45 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: nitzanc, oren, kvm, virtualization
On Wed, Apr 07, 2021 at 12:09:23PM +0000, Max Gurtovoy wrote:
> According to the spec after writing 0 to device_status, the driver MUST
> wait for a read of device_status to return 0 before reinitializing the
> device. In case we have a device that won't return 0, the reset
> operation will loop forever and cause the host/vm to stuck. Set timeout
> for 3 minutes before giving up on the device.
>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> drivers/virtio/virtio_pci_modern.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index cc3412a96a17..dcee616e8d21 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> + unsigned long timeout = jiffies + msecs_to_jiffies(180000);
>
> /* 0 status means a reset. */
> vp_modern_set_status(mdev, 0);
> @@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
> * device_status to return 0 before reinitializing the device.
> * This will flush out the status write, and flush in device writes,
> * including MSI-X interrupts, if any.
> + * Set a timeout before giving up on the device.
> */
> - while (vp_modern_get_status(mdev))
> + while (vp_modern_get_status(mdev)) {
> + if (time_after(jiffies, timeout)) {
> + dev_err(&vdev->dev, "virtio: device not ready. "
> + "Aborting. Try again later\n");
> + return -EAGAIN;
> + }
> msleep(1);
> + }
> /* Flush pending VQ/configuration callbacks. */
> vp_synchronize_vectors(vdev);
> return 0;
Problem is everyone just ignores the return code from reset.
Timing out like that has a chance to cause a lot of trouble
if the device remains active - we need to make reset robust.
What exactly is going on with the device that
get status never returns 0? E.g. maybe it's in a state
where it's returning all 1's because it's wedged permanently -
using that would be better...
> --
> 2.25.4
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] virito_pci: add timeout to reset device operation
2021-04-07 13:45 ` Michael S. Tsirkin
(?)
@ 2021-04-07 14:06 ` Max Gurtovoy
-1 siblings, 0 replies; 12+ messages in thread
From: Max Gurtovoy @ 2021-04-07 14:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, virtualization, jasowang, oren, nitzanc
On 4/7/2021 4:45 PM, Michael S. Tsirkin wrote:
> On Wed, Apr 07, 2021 at 12:09:23PM +0000, Max Gurtovoy wrote:
>> According to the spec after writing 0 to device_status, the driver MUST
>> wait for a read of device_status to return 0 before reinitializing the
>> device. In case we have a device that won't return 0, the reset
>> operation will loop forever and cause the host/vm to stuck. Set timeout
>> for 3 minutes before giving up on the device.
>>
>> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> ---
>> drivers/virtio/virtio_pci_modern.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
>> index cc3412a96a17..dcee616e8d21 100644
>> --- a/drivers/virtio/virtio_pci_modern.c
>> +++ b/drivers/virtio/virtio_pci_modern.c
>> @@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
>> {
>> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>> struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
>> + unsigned long timeout = jiffies + msecs_to_jiffies(180000);
>>
>> /* 0 status means a reset. */
>> vp_modern_set_status(mdev, 0);
>> @@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
>> * device_status to return 0 before reinitializing the device.
>> * This will flush out the status write, and flush in device writes,
>> * including MSI-X interrupts, if any.
>> + * Set a timeout before giving up on the device.
>> */
>> - while (vp_modern_get_status(mdev))
>> + while (vp_modern_get_status(mdev)) {
>> + if (time_after(jiffies, timeout)) {
>> + dev_err(&vdev->dev, "virtio: device not ready. "
>> + "Aborting. Try again later\n");
>> + return -EAGAIN;
>> + }
>> msleep(1);
>> + }
>> /* Flush pending VQ/configuration callbacks. */
>> vp_synchronize_vectors(vdev);
>> return 0;
> Problem is everyone just ignores the return code from reset.
> Timing out like that has a chance to cause a lot of trouble
> if the device remains active - we need to make reset robust.
But in commit 1/3 I added a code that doesn't ignore the reset return code.
>
> What exactly is going on with the device that
> get status never returns 0? E.g. maybe it's in a state
> where it's returning all 1's because it's wedged permanently -
> using that would be better...
In HW devices you might have situations that the controller is in bad
state (maybe bad FW) but still can be seen under the PCI bus.
As long as the device is not returning 0, this is legal. But in today's
code, it will cause the kernel to be in endless while loop because of
one bad device (that might recover later).
If we have 10 devices, and the first will stuck, all the others will
wait forever to be probed.
By Virtio spec, setting FAILED is allowed in case "..driver didn’t like
the device for some reason, or even a fatal error during device operation."
For example, in the NVMe spec there is TO (timeout) register that "is
the worst case time that host software shall wait for CSTS.RDY to
transition from: ..." and the driver wait for this time until it
understands that the device is not ready to operate.
I tried to add similar logic to virtio.
>
>
>
>> --
>> 2.25.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] virtio_pci: add module param for reset_timeout
2021-04-07 12:09 [PATCH 1/3] virtio: update reset callback to return status Max Gurtovoy
2021-04-07 12:09 ` [PATCH 2/3] virito_pci: add timeout to reset device operation Max Gurtovoy
@ 2021-04-07 12:09 ` Max Gurtovoy
2021-04-07 13:44 ` Cornelia Huck
2021-04-07 17:51 ` kernel test robot
3 siblings, 0 replies; 12+ messages in thread
From: Max Gurtovoy @ 2021-04-07 12:09 UTC (permalink / raw)
To: mst, kvm, virtualization, jasowang; +Cc: oren, nitzanc, Max Gurtovoy
Enable the user to set the time for waiting for successful reset by the
virtio controller. Set the default to 180 seconds.
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
drivers/virtio/virtio_pci_common.c | 5 +++++
drivers/virtio/virtio_pci_common.h | 2 ++
drivers/virtio/virtio_pci_modern.c | 3 ++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 222d630c41fc..3a4c57839ed8 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -24,6 +24,11 @@ MODULE_PARM_DESC(force_legacy,
"Force legacy mode for transitional virtio 1 devices");
#endif
+unsigned int reset_timeout = 180;
+module_param_named(reset_timeout, reset_timeout, uint, 0644);
+MODULE_PARM_DESC(reset_timeout,
+ "timeout in seconds for reset virtio device operation");
+
/* wait for pending irq handlers */
void vp_synchronize_vectors(struct virtio_device *vdev)
{
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index beec047a8f8d..4760cdf74961 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -29,6 +29,8 @@
#include <linux/highmem.h>
#include <linux/spinlock.h>
+extern unsigned int reset_timeout;
+
struct virtio_pci_vq_info {
/* the actual virtqueue */
struct virtqueue *vq;
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index dcee616e8d21..811fc1719d8c 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -162,7 +162,8 @@ static int vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
- unsigned long timeout = jiffies + msecs_to_jiffies(180000);
+ unsigned long timeout = jiffies +
+ msecs_to_jiffies(reset_timeout * 1000);
/* 0 status means a reset. */
vp_modern_set_status(mdev, 0);
--
2.25.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] virtio: update reset callback to return status
2021-04-07 12:09 [PATCH 1/3] virtio: update reset callback to return status Max Gurtovoy
@ 2021-04-07 13:44 ` Cornelia Huck
2021-04-07 12:09 ` [PATCH 3/3] virtio_pci: add module param for reset_timeout Max Gurtovoy
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2021-04-07 13:44 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: mst, kvm, virtualization, jasowang, oren, nitzanc
On Wed, 7 Apr 2021 12:09:22 +0000
Max Gurtovoy <mgurtovoy@nvidia.com> wrote:
> The reset device operation, usually is an operation that might fail from
> various reasons. For example, the controller might be in a bad state and
> can't answer to any request. Usually, the paravirt SW based virtio
> devices always succeed in reset operation but this is not the case for
> HW based virtio devices.
>
> This commit is also a preparation for adding a timeout mechanism for
> resetting virtio devices.
>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> drivers/remoteproc/remoteproc_virtio.c | 3 ++-
> drivers/virtio/virtio.c | 22 +++++++++++++++-------
> drivers/virtio/virtio_mmio.c | 3 ++-
> drivers/virtio/virtio_pci_legacy.c | 3 ++-
> drivers/virtio/virtio_pci_modern.c | 3 ++-
> drivers/virtio/virtio_vdpa.c | 3 ++-
> include/linux/virtio_config.h | 5 +++--
> 7 files changed, 28 insertions(+), 14 deletions(-)
You missed drivers/s390/virtio/virtio_ccw.c.
virtio_ccw_reset() should probably return -ENOMEM on allocation failure
and forward the return code of ccw_io_helper().
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] virtio: update reset callback to return status
@ 2021-04-07 13:44 ` Cornelia Huck
0 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2021-04-07 13:44 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: kvm, mst, virtualization, nitzanc, oren
On Wed, 7 Apr 2021 12:09:22 +0000
Max Gurtovoy <mgurtovoy@nvidia.com> wrote:
> The reset device operation, usually is an operation that might fail from
> various reasons. For example, the controller might be in a bad state and
> can't answer to any request. Usually, the paravirt SW based virtio
> devices always succeed in reset operation but this is not the case for
> HW based virtio devices.
>
> This commit is also a preparation for adding a timeout mechanism for
> resetting virtio devices.
>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> drivers/remoteproc/remoteproc_virtio.c | 3 ++-
> drivers/virtio/virtio.c | 22 +++++++++++++++-------
> drivers/virtio/virtio_mmio.c | 3 ++-
> drivers/virtio/virtio_pci_legacy.c | 3 ++-
> drivers/virtio/virtio_pci_modern.c | 3 ++-
> drivers/virtio/virtio_vdpa.c | 3 ++-
> include/linux/virtio_config.h | 5 +++--
> 7 files changed, 28 insertions(+), 14 deletions(-)
You missed drivers/s390/virtio/virtio_ccw.c.
virtio_ccw_reset() should probably return -ENOMEM on allocation failure
and forward the return code of ccw_io_helper().
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] virtio: update reset callback to return status
2021-04-07 13:44 ` Cornelia Huck
(?)
@ 2021-04-07 14:35 ` Max Gurtovoy
-1 siblings, 0 replies; 12+ messages in thread
From: Max Gurtovoy @ 2021-04-07 14:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: mst, kvm, virtualization, jasowang, oren, nitzanc
On 4/7/2021 4:44 PM, Cornelia Huck wrote:
> On Wed, 7 Apr 2021 12:09:22 +0000
> Max Gurtovoy <mgurtovoy@nvidia.com> wrote:
>
>> The reset device operation, usually is an operation that might fail from
>> various reasons. For example, the controller might be in a bad state and
>> can't answer to any request. Usually, the paravirt SW based virtio
>> devices always succeed in reset operation but this is not the case for
>> HW based virtio devices.
>>
>> This commit is also a preparation for adding a timeout mechanism for
>> resetting virtio devices.
>>
>> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> ---
>> drivers/remoteproc/remoteproc_virtio.c | 3 ++-
>> drivers/virtio/virtio.c | 22 +++++++++++++++-------
>> drivers/virtio/virtio_mmio.c | 3 ++-
>> drivers/virtio/virtio_pci_legacy.c | 3 ++-
>> drivers/virtio/virtio_pci_modern.c | 3 ++-
>> drivers/virtio/virtio_vdpa.c | 3 ++-
>> include/linux/virtio_config.h | 5 +++--
>> 7 files changed, 28 insertions(+), 14 deletions(-)
> You missed drivers/s390/virtio/virtio_ccw.c.
>
> virtio_ccw_reset() should probably return -ENOMEM on allocation failure
> and forward the return code of ccw_io_helper().
thanks, I found another 1-2 places that I missed. I'll update in v2.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] virtio: update reset callback to return status
2021-04-07 12:09 [PATCH 1/3] virtio: update reset callback to return status Max Gurtovoy
2021-04-07 12:09 ` [PATCH 2/3] virito_pci: add timeout to reset device operation Max Gurtovoy
@ 2021-04-07 17:51 ` kernel test robot
2021-04-07 13:44 ` Cornelia Huck
2021-04-07 17:51 ` kernel test robot
3 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-04-07 17:51 UTC (permalink / raw)
To: Max Gurtovoy, mst, kvm, virtualization, jasowang
Cc: kbuild-all, clang-built-linux, oren, nitzanc, Max Gurtovoy
[-- Attachment #1: Type: text/plain, Size: 10757 bytes --]
Hi Max,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linus/master v5.12-rc6 next-20210407]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Max-Gurtovoy/virtio-update-reset-callback-to-return-status/20210407-201026
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: s390-randconfig-r032-20210407 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c060945b23a1c54d4b2a053ff4b093a2277b303d)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/74d4580725028f138a4713e8594f9068e9c83805
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Max-Gurtovoy/virtio-update-reset-callback-to-return-status/20210407-201026
git checkout 74d4580725028f138a4713e8594f9068e9c83805
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/s390/virtio/virtio_ccw.c:1016:11: error: incompatible function pointer types initializing 'int (*)(struct virtio_device *)' with an expression of type 'void (struct virtio_device *)' [-Werror,-Wincompatible-function-pointer-types]
.reset = virtio_ccw_reset,
^~~~~~~~~~~~~~~~
20 warnings and 1 error generated.
vim +1016 drivers/s390/virtio/virtio_ccw.c
971bedca26e037 drivers/s390/virtio/virtio_ccw.c Cornelia Huck 2019-01-21 1008
0db1dba5dfaf70 drivers/s390/virtio/virtio_ccw.c Bhumika Goyal 2017-01-14 1009 static const struct virtio_config_ops virtio_ccw_config_ops = {
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1010 .get_features = virtio_ccw_get_features,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1011 .finalize_features = virtio_ccw_finalize_features,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1012 .get = virtio_ccw_get_config,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1013 .set = virtio_ccw_set_config,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1014 .get_status = virtio_ccw_get_status,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1015 .set_status = virtio_ccw_set_status,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 @1016 .reset = virtio_ccw_reset,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1017 .find_vqs = virtio_ccw_find_vqs,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1018 .del_vqs = virtio_ccw_del_vqs,
971bedca26e037 drivers/s390/virtio/virtio_ccw.c Cornelia Huck 2019-01-21 1019 .bus_name = virtio_ccw_bus_name,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1020 };
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1021
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1022
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16026 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] virtio: update reset callback to return status
@ 2021-04-07 17:51 ` kernel test robot
0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-04-07 17:51 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10917 bytes --]
Hi Max,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linus/master v5.12-rc6 next-20210407]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Max-Gurtovoy/virtio-update-reset-callback-to-return-status/20210407-201026
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: s390-randconfig-r032-20210407 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c060945b23a1c54d4b2a053ff4b093a2277b303d)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/74d4580725028f138a4713e8594f9068e9c83805
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Max-Gurtovoy/virtio-update-reset-callback-to-return-status/20210407-201026
git checkout 74d4580725028f138a4713e8594f9068e9c83805
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/s390/virtio/virtio_ccw.c:1016:11: error: incompatible function pointer types initializing 'int (*)(struct virtio_device *)' with an expression of type 'void (struct virtio_device *)' [-Werror,-Wincompatible-function-pointer-types]
.reset = virtio_ccw_reset,
^~~~~~~~~~~~~~~~
20 warnings and 1 error generated.
vim +1016 drivers/s390/virtio/virtio_ccw.c
971bedca26e037 drivers/s390/virtio/virtio_ccw.c Cornelia Huck 2019-01-21 1008
0db1dba5dfaf70 drivers/s390/virtio/virtio_ccw.c Bhumika Goyal 2017-01-14 1009 static const struct virtio_config_ops virtio_ccw_config_ops = {
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1010 .get_features = virtio_ccw_get_features,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1011 .finalize_features = virtio_ccw_finalize_features,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1012 .get = virtio_ccw_get_config,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1013 .set = virtio_ccw_set_config,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1014 .get_status = virtio_ccw_get_status,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1015 .set_status = virtio_ccw_set_status,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 @1016 .reset = virtio_ccw_reset,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1017 .find_vqs = virtio_ccw_find_vqs,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1018 .del_vqs = virtio_ccw_del_vqs,
971bedca26e037 drivers/s390/virtio/virtio_ccw.c Cornelia Huck 2019-01-21 1019 .bus_name = virtio_ccw_bus_name,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1020 };
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1021
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1022
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 16026 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] virtio: update reset callback to return status
@ 2021-04-07 17:51 ` kernel test robot
0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-04-07 17:51 UTC (permalink / raw)
To: Max Gurtovoy, mst, kvm, virtualization, jasowang
Cc: Max Gurtovoy, clang-built-linux, nitzanc, kbuild-all, oren
[-- Attachment #1: Type: text/plain, Size: 10757 bytes --]
Hi Max,
I love your patch! Yet something to improve:
[auto build test ERROR on vhost/linux-next]
[also build test ERROR on linus/master v5.12-rc6 next-20210407]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Max-Gurtovoy/virtio-update-reset-callback-to-return-status/20210407-201026
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: s390-randconfig-r032-20210407 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c060945b23a1c54d4b2a053ff4b093a2277b303d)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/74d4580725028f138a4713e8594f9068e9c83805
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Max-Gurtovoy/virtio-update-reset-callback-to-return-status/20210407-201026
git checkout 74d4580725028f138a4713e8594f9068e9c83805
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from drivers/s390/virtio/virtio_ccw.c:12:
In file included from include/linux/memblock.h:14:
In file included from arch/s390/include/asm/dma.h:5:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/s390/virtio/virtio_ccw.c:1016:11: error: incompatible function pointer types initializing 'int (*)(struct virtio_device *)' with an expression of type 'void (struct virtio_device *)' [-Werror,-Wincompatible-function-pointer-types]
.reset = virtio_ccw_reset,
^~~~~~~~~~~~~~~~
20 warnings and 1 error generated.
vim +1016 drivers/s390/virtio/virtio_ccw.c
971bedca26e037 drivers/s390/virtio/virtio_ccw.c Cornelia Huck 2019-01-21 1008
0db1dba5dfaf70 drivers/s390/virtio/virtio_ccw.c Bhumika Goyal 2017-01-14 1009 static const struct virtio_config_ops virtio_ccw_config_ops = {
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1010 .get_features = virtio_ccw_get_features,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1011 .finalize_features = virtio_ccw_finalize_features,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1012 .get = virtio_ccw_get_config,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1013 .set = virtio_ccw_set_config,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1014 .get_status = virtio_ccw_get_status,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1015 .set_status = virtio_ccw_set_status,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 @1016 .reset = virtio_ccw_reset,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1017 .find_vqs = virtio_ccw_find_vqs,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1018 .del_vqs = virtio_ccw_del_vqs,
971bedca26e037 drivers/s390/virtio/virtio_ccw.c Cornelia Huck 2019-01-21 1019 .bus_name = virtio_ccw_bus_name,
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1020 };
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1021
7e64e0597fd67c drivers/s390/kvm/virtio_ccw.c Cornelia Huck 2012-12-14 1022
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16026 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-04-07 17:52 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 12:09 [PATCH 1/3] virtio: update reset callback to return status Max Gurtovoy
2021-04-07 12:09 ` [PATCH 2/3] virito_pci: add timeout to reset device operation Max Gurtovoy
2021-04-07 13:45 ` Michael S. Tsirkin
2021-04-07 13:45 ` Michael S. Tsirkin
2021-04-07 14:06 ` Max Gurtovoy
2021-04-07 12:09 ` [PATCH 3/3] virtio_pci: add module param for reset_timeout Max Gurtovoy
2021-04-07 13:44 ` [PATCH 1/3] virtio: update reset callback to return status Cornelia Huck
2021-04-07 13:44 ` Cornelia Huck
2021-04-07 14:35 ` Max Gurtovoy
2021-04-07 17:51 ` kernel test robot
2021-04-07 17:51 ` kernel test robot
2021-04-07 17:51 ` kernel test robot
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.