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

Farhan Ali
2026-03-26T19:05:36Z

On 3/26/2026 6:03 AM, Niklas Schnelle wrote:
> On Wed, 2026-03-25 at 14:31 +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.
>>
>> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
>> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
>> Signed-off-by: Julian Ruess <julianr@linux.ibm.com>
>> ---
>> drivers/vfio/pci/Kconfig | 2 +
>> drivers/vfio/pci/Makefile | 2 +
>> drivers/vfio/pci/ism/Kconfig | 10 ++
>> drivers/vfio/pci/ism/Makefile | 3 +
>> drivers/vfio/pci/ism/main.c | 408 ++++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 425 insertions(+)
>>
> --- snip ---
>> +
>> +static int ism_vfio_pci_ioctl_get_region_info(struct vfio_device *core_vdev,
>> + struct vfio_region_info *info,
>> + struct vfio_info_cap *caps)
>> +{
>> + struct vfio_pci_core_device *vdev =
>> + container_of(core_vdev, struct vfio_pci_core_device, vdev);
>> + struct pci_dev *pdev = vdev->pdev;
>> +
>> + switch (info->index) {
>> + case VFIO_PCI_CONFIG_REGION_INDEX:
>> + info->offset = ISM_VFIO_PCI_INDEX_TO_OFFSET(info->index);
>> + info->size = pdev->cfg_size;
>> + info->flags = VFIO_REGION_INFO_FLAG_READ |
>> + VFIO_REGION_INFO_FLAG_WRITE;
>> + break;
>> + case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
>> + info->offset = ISM_VFIO_PCI_INDEX_TO_OFFSET(info->index);
>> + info->size = pci_resource_len(pdev, info->index);
>> + if (!info->size) {
>> + info->flags = 0;
>> + break;
>> + }
>> + info->flags = VFIO_REGION_INFO_FLAG_READ |
>> + VFIO_REGION_INFO_FLAG_WRITE;
>> + break;
>> + default:
>> + info->offset = 0;
>> + info->size = 0;
>> + info->flags = 0;
>> + return -EINVAL;
> Thinking more about this, the above default handling actually breaks
> additional regions such as the one added by Farhan for the error
> events. I think this needs to instead call the generic
> vfio_pci_ioctl_get_region_info() for other regions.

Note for error events we are using a VFIO device feature and not a
region. At this point we actually don't have additional regions. I do
agree that if we were to add a zpci specific region we would need to
update here and ism_vfio_pci_rw() to allow region read/write ops. This
ISM driver would also need to support all the possible callbacks in
struct vfio_pci_regops.

Given that we don't have any additional regions yet, I am conflicted if
it makes sense to add all the code to support additional regions without
a good way to test it. Maybe its something we can defer when if we add
additional regions? Thanks

Farhan

>> + }
>> + return 0;
>> +}
> --- snip ---