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

Niklas Schnelle
2026-03-18T14:53:16Z

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@linux.ibm.com>
> ---
--- 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 ---