Re: [PATCH v19 2/3] vfio-pci/zdev: Add a device feature for error information

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The UAPI struct `vfio_device_feature_zpci_err` uses C bitfields, which violates UAPI design principles and breaks cross-architecture/cross-compiler compatibility.
--

commit 894c3dcf471b795ab55da889778d75f212c1eee2
Author: Farhan Ali <alifm@linux.ibm.com>

vfio-pci/zdev: Add a device feature for error information

For zPCI devices, platform specific error information is provided by firmware.
This commit adds a new device feature to expose this error information to
userspace to enable recovery.

> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 5de618a3a5eee..83c93193b22de 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h

[ ... ]

> @@ -1534,6 +1534,36 @@ struct vfio_device_feature_dma_buf {
>   */
>  #define VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2  12
>  
> +/**
> + * VFIO_DEVICE_FEATURE_ZPCI_ERROR feature provides PCI error information to
> + * userspace for vfio-pci devices on s390. On s390, PCI error recovery
> + * involves platform firmware and notification to operating systems is done
> + * by architecture specific mechanism. Exposing this information to
> + * userspace allows it to take appropriate actions to handle an
> + * error on the device. The ioctl returns -ENOMSG if there are no pending
> + * PCI errors.
> + */
> +
> +struct vfio_device_feature_zpci_err {
> +	__u32 feature_flags;		/* Indicate future features */
> +	__u32 reserved1;
> +	__u32 fh;			/* function handle */
> +	__u32 fid;			/* function id */
> +	__u64 faddr;                    /* failing address */
> +	__u32 ett		:  4;	/* expected table type */
> +	__u32 mvn		: 12;	/* MSI vector number */
> +	__u32 dmaas		:  8;	/* DMA address space */
> +	__u32 reserved2		:  6;
> +	__u32 q			:  1;	/* event qualifier */
> +	__u32 rw		:  1;	/* read/write */

[Severity: Medium]
Does this UAPI struct violate kernel UAPI design principles by using C
bitfields?

The C standard does not specify the memory layout of bitfields, leaving it
entirely up to the compiler and target endianness. Placing bitfields directly in
a globally exposed UAPI header could mean that generic userspace tools or
hypervisors compiled with different compilers or on architectures with different
endianness might map these fields to the wrong bits when interpreting the
struct.

Could these be replaced with a standard __u32 variable alongside explicit
#define bitmasks and shifts?

> +	__u32 reserved3;
> +	__u16 reserved4;
> +	__u16 pec;			/* PCI event code */
> +	__u8 reserved5[28];		/* Allow for future expansion */
> +};
> +
> +#define VFIO_DEVICE_FEATURE_ZPCI_ERROR 13
> +
>  /* -------- API for Type1 VFIO IOMMU -------- */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615183616.2902-1-alifm@linux.ibm.com?part=2