Re: [PATCH v5 5/5] iommufd/vdevice: add TSM request ioctl

["Dan Williams (nvidia)" <djbw@kernel.org>]

Aneesh Kumar K.V wrote:
[..]
> > I think we can wait to move it to its own IOMMU operation unless/until
> > there is a need to set RUN outside of an explicit guest request, right?
> 
> Something like the below? (the diff against this series)
> 
> I have not yet integrated this into the full CCA patchset for testing,
> but I wanted to make sure we are aligned on the UAPI.
[..]
> -static bool iommufd_vdevice_tsm_req_scope_valid(u32 scope)
> +static bool iommufd_vdevice_tsm_req_arch_valid(u32 tvm_arch)
>  {
> -	if (scope > IOMMU_VDEVICE_TSM_REQ_SCOPE_PCI_LAST)
> +	switch (tvm_arch) {
> +	case IOMMU_VDEVICE_TSM_TVM_ARCH_CCA:
> +	case IOMMU_VDEVICE_TSM_TVM_ARCH_SEV:
> +	case IOMMU_VDEVICE_TSM_TVM_ARCH_TDX:

Makes sense for any command that needs tunneling. However, see below, what is
that set, and do we need a IOMMU_VDEVICE_TSM_COMMON when architecture
differentiation is not required?

> +		return true;
> +	default:
>  		return false;
> +	}
> +}
>  
> -	switch (scope) {
> -	case IOMMU_VDEVICE_TSM_REQ_PCI_INFO:
> -	case IOMMU_VDEVICE_TSM_REQ_PCI_STATE_CHANGE:
> -	case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_READ:
> -	case IOMMU_VDEVICE_TSM_REQ_PCI_DEBUG_WRITE:
> +static bool iommufd_vdevice_tsm_req_op_valid(u32 op, u32 tvm_arch)
> +{
> +	switch (op) {
> +	case TSM_REQ_READ_OBJECT:
> +	case TSM_REQ_REGEN_OBJECT:
> +	case TSM_REQ_OBJECT_INFO:

The design goal of the netlink device-evidence interface is to be able
to respond to all shapes of requests for evidence. So netlink caches
objects that the hypercall handler can fill responses from.

It eliminates a class of commands that need tunneling.

> +	case TSM_REQ_VALIDATE_MMIO:
> +	case TSM_REQ_SET_TDI_STATE:

Are these potentially candidates for a IOMMU_VDEVICE_TSM_COMMON? The
handler knows how to do the arch-specific response from the common
iommufd result, or is there TSM-specific payload beyond @tsm_code for
these.

Make it the case that guest_req only needs non-common arch for
operations that are implementation unique, or where the response payload
exceeds what can be conveyed via @tsm_code.

>  		return true;
> +	case TSM_REQ_SEV_ENABLE_DMA:
> +	case TSM_REQ_SEV_DISABLE_DMA:
> +		return tvm_arch == IOMMU_VDEVICE_TSM_TVM_ARCH_SEV;

Right, this appears to be the only case where the command is
implementation unique. The handler can only ask iommufd to take
arch-specific action.