Re: [PATCH rfcv2 06/20] host_iommu_device: Define two new capabilities HOST_IOMMU_DEVICE_CAP_[NESTING|FS1GP]

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Mar 06, 2025 at 04:59:39PM +0100, Eric Auger wrote:
> >>> +++ b/include/system/host_iommu_device.h
> >>> @@ -22,10 +22,16 @@
> >>>   *
> >>>   * @hw_caps: host platform IOMMU capabilities (e.g. on IOMMUFD this
> >> represents
> >>>   *           the @out_capabilities value returned from IOMMU_GET_HW_INFO
> >> ioctl)
> >>> + *
> >>> + * @nesting: nesting page table support.
> >>> + *
> >>> + * @fs1gp: first stage(a.k.a, Stage-1) 1GB huge page support.
> >>>   */
> >>>  typedef struct HostIOMMUDeviceCaps {
> >>>      uint32_t type;
> >>>      uint64_t hw_caps;
> >>> +    bool nesting;
> >>> +    bool fs1gp;
> >> this looks quite vtd specific, isn't it? Shouldn't we hide this is a
> >> vendor specific cap struct?
> > Yes? I guess ARM hw could also provide nesting support at least
> > There are some reasons I perfer a flatten struct even if some
> > Elements may be vendor specific.
> > 1. If a vendor doesn't support an capability for other vendor,
> > corresponding element should be zero by default.
> > 2. An element vendor specific may become generic in future
> > and we don't need to update the structure when that happens.
> > 3. vIOMMU calls get_cap() to query if a capability is supported,
> > so a vIOMMU never query a vendor specific capability it doesn't
> > recognize. Even if that happens, zero is returned hinting no support.
> I will let others comment but in general this is frown upon and unions
> are prefered at least.

Yea, it feels odd to me that we stuff vendor specific thing in
the public structure.

It's okay if we want to store in HostIOMMUDeviceCaps the vendor
specific data pointer (opaque), just for convenience.

I think we can have another PCIIOMMUOps op for vendor code to
run iommufd_backend_get_device_info() that returns the hw_caps
for the core code to read.

Or perhaps the vendor code can just return a HWPT directly? If
IOMMU_HW_CAP_DIRTY_TRACKING is set in the hw_caps, the vendor
code can allocate a HWPT for that. And if vendor code detects
the "nesting" cap in vendor struct, then return a nest_parent
HWPT. And returning NULL can let core code allocate a default
HWPT (or just attach the device to IOAS for auto domain/hwpt).

I am also hoping that this can handle a shared S2 nest_parent
HWPT case. Could the core container structure or so store the
HWPT?

Thanks
Nicolin