Re: [PATCH v3 19/37] vfio/spapr: Introduce spapr backend and target interface

[Cédric Le Goater]

On 10/26/23 12:30, Zhenzhong Duan wrote:
> Introduce an empry spapr backend which will hold spapr specific
> content, currently only prereg_listener and hostwin_list.
>
> Also introduce and instantiate a spapr specific target interface,
> currently only has add/del_window callbacks.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-common.h         | 8 ++++++++
>   include/hw/vfio/vfio-container-base.h | 2 ++
>   hw/vfio/spapr.c                       | 8 ++++++++
>   3 files changed, 18 insertions(+)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 938f75e70c..a74e60e677 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -169,6 +169,14 @@ VFIOAddressSpace *vfio_get_address_space(AddressSpace *as);
>   void vfio_put_address_space(VFIOAddressSpace *space);
>   
>   /* SPAPR specific */
> +typedef struct VFIOIOMMUSpaprOps {
> +    int (*add_window)(VFIOContainerBase *bcontainer,
> +                      MemoryRegionSection *section,
> +                      Error **errp);
> +    void (*del_window)(VFIOContainerBase *bcontainer,
> +                       MemoryRegionSection *section);
> +} VFIOIOMMUSpaprOps;

I don't think we need a new structure type.

If possible, these callbacks should be under VFIOIOMMUOps and only
defined in the VFIOIOMMUOps struct for spapr. This struct would a
copy of vfio_legacy_ops plus the two add/del_window  handlers.

Thanks,

C.


> +
>   int vfio_container_add_section_window(VFIOContainer *container,
>                                         MemoryRegionSection *section,
>                                         Error **errp);
> diff --git a/include/hw/vfio/vfio-container-base.h 
> b/include/hw/vfio/vfio-container-base.h
> index 2ffafb0d58..1e1854d24f 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -31,6 +31,7 @@
>   
>   typedef struct VFIODevice VFIODevice;
>   typedef struct VFIOIOMMUOps VFIOIOMMUOps;
> +typedef struct VFIOIOMMUSpaprOps VFIOIOMMUSpaprOps;
>   
>   typedef struct {
>       unsigned long *bitmap;
> @@ -49,6 +50,7 @@ typedef struct VFIOAddressSpace {
>    */
>   typedef struct VFIOContainerBase {
>       const VFIOIOMMUOps *ops;
> +    const VFIOIOMMUSpaprOps *spapr_ops;
>       VFIOAddressSpace *space;
>       MemoryListener listener;
>       Error *error;
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 5786377317..3739004151 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -24,6 +24,10 @@
>   #include "qapi/error.h"
>   #include "trace.h"
>   
> +typedef struct VFIOSpaprContainer {
> +    VFIOContainer container;
> +} VFIOSpaprContainer;
> +
>   static bool vfio_prereg_listener_skipped_section(MemoryRegionSection *section)
>   {
>       if (memory_region_is_iommu(section->mr)) {
> @@ -384,6 +388,8 @@ void vfio_container_del_section_window(VFIOContainer 
> *container,
>       }
>   }
>   
> +const VFIOIOMMUSpaprOps vfio_iommu_spapr_ops;
> +
>   bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>   {
>       VFIOContainerBase *bcontainer = &container->bcontainer;
> @@ -447,6 +453,8 @@ bool vfio_spapr_container_init(VFIOContainer *container, 
> Error **errp)
>                             0x1000);
>       }
>   
> +    bcontainer->spapr_ops = &vfio_iommu_spapr_ops;
> +
>   listener_unregister_exit:
>       if (v2) {
>           memory_listener_unregister(&container->prereg_listener);