Re: [RFC PATCH 07/21] pci/tdisp: Introduce tsm module

[Alexey Kardashevskiy <aik@amd.com>]

On 29/8/24 09:42, Jason Gunthorpe wrote:
> On Wed, Aug 28, 2024 at 01:00:46PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 27/8/24 22:32, Jason Gunthorpe wrote:
>>> On Fri, Aug 23, 2024 at 11:21:21PM +1000, Alexey Kardashevskiy wrote:
>>>> The module responsibilities are:
>>>> 1. detect TEE support in a device and create nodes in the device's sysfs
>>>> entry;
>>>> 2. allow binding a PCI device to a VM for passing it through in a trusted
>>>> manner;
>>>
>>> Binding devices to VMs and managing their lifecycle is the purvue of
>>> VFIO and iommufd, it should not be exposed via weird sysfs calls like
>>> this. You can't build the right security model without being inside
>>> the VFIO context.
>>
>> Is "extend the MAP_DMA uAPI to accept {gmemfd, offset}" enough for the VFIO
>> context, or there is more and I am missing it?
> 
> No, you need to have all the virtual PCI device creation stuff linked
> to a VFIO cdev to prove you have rights to do things to the physical
> device.

The VM-to-VFIOdevice binding is already in the KVM VFIO device, the rest 
is the same old VFIO.

>>> I'm not convinced this should be in some side module - it seems like
>>> this is possibly more logically integrated as part of the iommu..
>>
>> There are two things which the module's sysfs interface tries dealing with:
>>
>> 1) device authentication (by the PSP, contrary to Lukas'es host-based CMA)
>> and PCIe link encryption (PCIe IDE keys only programmable via the PSP);
> 
> So when I look at the spec I think that probably TIO_DEV_* should be
> connected to VFIO, somewhere as vfio/kvm/iommufd ioctls. This needs to
> be coordinated with everyone else because everyone has *some kind* of
> "trusted world create for me a vPCI device in the secure VM" set of
> verbs.
> 
> TIO_TDI is presumably the device authentication stuff?

Not really. In practice:
- TDI is usually a SRIOV VF (and intended for passing through to a VM);
- DEV is always PF#0 of a (possibly multifunction) physical device which 
controls physical link encryption and authentication.

> This is why I picked on tsm_dev_connect_store()..
> >> Besides sysfs, the module provides common "verbs" to be defined by the
>> platform (which is right now a reduced set of the AMD PSP operations but the
>> hope is it can be generalized); and the module also does PCIe DOE bouncing
>> (which is also not uncommon). Part of this exercise is trying to find some
>> common ground (if it is possible), hence routing everything via this module.
> 
> I think there is a seperation between how the internal stuff in the
> kernel works and how/what the uAPIs are.
> 
> General stuff like authenticate/accept/authorize a PCI device needs
> to be pretty cross platform.

So those TIO_DEV_*** are for the PCI layer and nothing there is for 
virtualization and this is what is exposed via sysfs.

TIO_TDI_*** commands are for virtualization and, say, the user cannot 
attach a TDI to a VM by using the sysfs interface (new ioctls are for 
this), the user can only read some shared info about a TDI (interface 
report, status).

> Stuff like creating vPCIs needs to be ioctls linked to KVM/VFIO
> somehow and can have more platform specific components.

Right, I am adding ioctls to the KVM VFIO device.

> I would try to split your topics up more along those lines..

Fair point, my bad. I'll start with a PF-only module and then add TDI 
stuff to it separately.

I wonder if there is enough value to try keeping the TIO_DEV_* and 
TIO_TDI_* API together or having TIO_DEV_* in some PCI module and 
TIO_TDI_* in KVM is a non-confusing way to proceed with this. Adding 
things to the PCI's sysfs from more places bothers me more than this 
frankenmodule. Thanks,


-- 
Alexey