Re: [PATCH v5 2/3] vfio/ism: Implement vfio_pci driver for ISM devices

[Niklas Schnelle]

On Tue, 2026-03-17 at 13:58 +0100, Julian Ruess wrote:
> Add a vfio_pci variant driver for the s390-specific Internal Shared
> Memory (ISM) devices used for inter-VM communication.
> 
> This enables the development of vfio-pci-based user space drivers for
> ISM devices.
> 
> On s390, kernel primitives such as ioread() and iowrite() are switched
> over from function handle based PCI load/stores instructions to PCI
> memory-I/O (MIO) loads/stores when these are available and not
> explicitly disabled. Since these instructions cannot be used with ISM
> devices, ensure that classic function handle-based PCI instructions are
> used instead.
> 
> The driver is still required even when MIO instructions are disabled, as
> the ISM device relies on the PCI store block (PCISTB) instruction to
> perform write operations.
> 
> Stores are not fragmented, therefore one ioctl corresponds to exactly
> one PCISTB instruction. User space must ensure to not write more than
> 4096 bytes at once to an ISM BAR which is the maximum payload of the
> PCISTB instruction.
> 
> Signed-off-by: Julian Ruess <julianr@xxxxxxxxxxxxx>
> ---
--- snip ---
> +/*
> + * Use __zpci_load() to bypass automatic use of
> + * PCI MIO instructions which are not supported on ISM devices
> + */
> +#define ISM_READ(size)                                                        \
> +	static int ism_read##size(struct zpci_dev *zdev, int bar,             \
> +				  ssize_t *filled, char __user *buf,          \
> +				  loff_t off)                                 \
> +	{                                                                     \
> +		u64 req, tmp;                                                 \
> +		u##size val;                                                  \
> +		int ret;                                                      \
> +									      \
> +		req = ZPCI_CREATE_REQ(READ_ONCE(zdev->fh), bar, sizeof(val)); \
> +		ret = __zpci_load(&tmp, req, off);                            \
> +		val = (u##size)tmp;                                           \
> +		if (copy_to_user(buf, &val, sizeof(val)))                     \
> +			return -EFAULT;                                       \

This was actually noticed by sashiko.dev but since not everyone knows
that tool yet and it still needs filtering, I'm repeating it here. This
leaks kernel stack memory when __zpci_load() fails since the
copy_to_user() still happens. In v4 on the other hand you had a proper
early return.

> +		*filled = sizeof(val);                                        \
> +		return ret;                                                   \
> +	}
> +
> +ISM_READ(64);
> +ISM_READ(32);
> +ISM_READ(16);
> +ISM_READ(8);
--- snip ---