Re: [PATCH 1/2] virtio_pci_modern_dev: warn once on invalid status

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Aug 02, 2026 at 05:40:58PM +0000, Abhin Parekadan Jose wrote:
> vp_modern_get_status() returns the raw device_status byte as read
> from the common configuration structure (struct virtio_pci_common_cfg,
> mapped via the VIRTIO_PCI_CAP_COMMON_CFG capability). That byte should
> only ever contain some combination of the status bits defined by the
> virtio spec (bits 0-3, 6-7); bits 4 and 5 are reserved and a
> spec-compliant device must never set them.

Future versions of the spec are likely to use these bits,
I prefer to make the drivers future proof. In fact, things
like kexec mean that we don't know what drove the
device earlier, so we do not know that the previous
driver did not write 0xFF there.



> A value with any other bit
> set means either the device is violating the spec, or the read never
> reached real device state at all -- e.g. because a write of 0x0000 to
> the PCI_COMMAND register (config space offset 4) clears the Memory
> Space Enable bit, causing the device to stop responding to
> memory-mapped register accesses -- effectively simulating an
> unresponsive/removed device without a real hot-unplug. In that case
> the MMIO read returns the bus's synthesized all-ones response instead
> of real device state.
> 
> Add VIRTIO_STATUS_ERROR() to the uapi header to recognize such values,
> and warn once from vp_modern_get_status() when it sees one, so the
> bogus status is visible at its source rather than only showing up as
> confusing behavior in callers.
> 
> Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
> ---
>  drivers/virtio/virtio_pci_modern_dev.c |  8 +++++++-
>  include/uapi/linux/virtio_config.h     | 16 ++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index 413a8c353463..60dd8acf1c28 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -480,8 +480,14 @@ EXPORT_SYMBOL_GPL(vp_modern_generation);
>  u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev)
>  {
>  	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> +	u8 status = vp_ioread8(&cfg->device_status);
>  
> -	return vp_ioread8(&cfg->device_status);
> +	if (VIRTIO_STATUS_ERROR(status)) {
> +		WARN_ONCE(1, "virtio: device returned error status: %#x\n",
> +			  status);
> +	}
> +
> +	return status;
>  }
>  EXPORT_SYMBOL_GPL(vp_modern_get_status);
>  
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 2445f365bce7..6f458914c0ba 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -45,6 +45,22 @@
>  /* We've given up on this device. */
>  #define VIRTIO_CONFIG_S_FAILED		0x80
>  
> +/*
> + * Check if a status value indicates an error
> + * All device_status bits currently defined by the virtio spec (bits
> + * 0,1,2,3,6,7). Bits 4 and 5 (0x10, 0x20) are reserved/undefined -- a
> + * real device must never set them. A status byte with any bit outside
> + * this mask set cannot be a legitimate value: either the device is
> + * violating the spec, or the read never actually reached it (e.g.
> + * PCI_COMMAND memory decode is disabled and this is a synthesized
> + * all-ones bus response instead of real device state).
> + */
> +#define VIRTIO_STATUS_ERROR(val) \
> +	(((u8)(val)) & \
> +	 ~(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | \
> +	   VIRTIO_CONFIG_S_DRIVER_OK | VIRTIO_CONFIG_S_FEATURES_OK | \
> +	   VIRTIO_CONFIG_S_NEEDS_RESET | VIRTIO_CONFIG_S_FAILED))
> +


Not clear what this does in UAPI, even if useful.

>  /*
>   * Virtio feature bits VIRTIO_TRANSPORT_F_START through
>   * VIRTIO_TRANSPORT_F_END are reserved for the transport
> -- 
> 2.51.1