On 9/20/19 10:02 AM, Cornelia Huck wrote: > On Thu, 19 Sep 2019 16:55:57 -0400 > Matthew Rosato <mjrosato@xxxxxxxxxxxxx> wrote: > >> On 9/19/19 11:20 AM, Cornelia Huck wrote: >>> On Fri, 6 Sep 2019 20:13:50 -0400 >>> Matthew Rosato <mjrosato@xxxxxxxxxxxxx> wrote: >>> >>>> From: Pierre Morel <pmorel@xxxxxxxxxxxxx> >>>> >>>> We define a new device region in vfio.h to be able to >>>> get the ZPCI CLP information by reading this region from >>>> userland. >>>> >>>> We create a new file, vfio_zdev.h to define the structure >>>> of the new region we defined in vfio.h >>>> >>>> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx> >>>> Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx> >>>> --- >>>> include/uapi/linux/vfio.h | 1 + >>>> include/uapi/linux/vfio_zdev.h | 35 +++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 36 insertions(+) >>>> create mode 100644 include/uapi/linux/vfio_zdev.h > >>>> diff --git a/include/uapi/linux/vfio_zdev.h b/include/uapi/linux/vfio_zdev.h >>>> new file mode 100644 >>>> index 0000000..55e0d6d >>>> --- /dev/null >>>> +++ b/include/uapi/linux/vfio_zdev.h >>>> @@ -0,0 +1,35 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ >>>> +/* >>>> + * Region definition for ZPCI devices >>>> + * >>>> + * Copyright IBM Corp. 2019 >>>> + * >>>> + * Author(s): Pierre Morel <pmorel@xxxxxxxxxxxxx> >>>> + */ >>>> + >>>> +#ifndef _VFIO_ZDEV_H_ >>>> +#define _VFIO_ZDEV_H_ >>>> + >>>> +#include <linux/types.h> >>>> + >>>> +/** >>>> + * struct vfio_region_zpci_info - ZPCI information. >>> >>> Hm... probably should also get some more explanation. E.g. is that >>> derived from a hardware structure? >>> >> >> The structure itself is not mapped 1:1 to a hardware structure, but it >> does serve as a collection of information that was derived from other >> hardware structures. >> >> "Used for passing hardware feature information about a zpci device >> between the host and guest" ? > > "zPCI specific hardware feature information for a device"? > > Are we reasonably sure that this is complete for now? I'm not sure if > expanding this structure would work; adding another should always be > possible, though (if a bit annoying). > I think trying to make the structure expandable would be best... If we allow arbitrary-sized reads of the info, and only add new fields onto the end it should be OK, no? (older qemu doesn't get the info it doesn't ask for / understand).... But I guess that's not compatible with having util_str[] size being defined dynamically. Another caveat would be if CLP_UTIL_STR_LEN were to grow in size in the future, and assuming util_str[] was no longer at the end of the structure, I guess the additional data would have to end up in a util_str2[CLP_UTIL_STR_LEN_NEW-CLP_UTIL_STR_LEN_OLD]... To explain what I mean, something like: struct vfio_region_zpci_info { <..> __u8 util_str[CLP_UTIL_STR_LEN_OLD]; /* END OF V1 */ __u8 foo; /* END OF V2 */ __u8 util_str2[CLP_UTIL_STR_LEN_NEW-CLP_UTIL_STR_LEN_OLD]; /* END OF V3 */ } __packed; >> >>>> + * >>>> + */ >>>> +struct vfio_region_zpci_info { >>>> + __u64 dasm; >>>> + __u64 start_dma; >>>> + __u64 end_dma; >>>> + __u64 msi_addr; >>>> + __u64 flags; >>>> + __u16 pchid; >>>> + __u16 mui; >>>> + __u16 noi; >>>> + __u16 maxstbl; >>>> + __u8 version; >>>> + __u8 gid; >>>> +#define VFIO_PCI_ZDEV_FLAGS_REFRESH 1 >>>> + __u8 util_str[]; >>>> +} __packed; >>>> + >>>> +#endif >>> >>> >> >