Re: [PATCH 2/7] PCI/TSM: Add pci_tsm_guest_req() for managing TDIs

[Alexey Kardashevskiy <aik@amd.com>]

On 30/8/25 12:37, dan.j.williams@intel.com wrote:
> Alexey Kardashevskiy wrote:
> [..]
>>>> We have pdev in pci_tdi, pci_tsm and pci_tsm_pf0 (via .base), using
>>>> these in pci_tsm_ops will document better which call is allowed on
>>>> what entity - DSM or TDI. Or may be ditch those back "pdev"
>>>> references?
>>>
>>> Not immediately understanding what change you want here. Do you want
>>> iommufd to track the pci_tdi?
>>
>> I'd like to either:
>>
>> - get rid of pdev back refs in pci_tsm/pci_tdi since we pass pci_dev
>> everywhere as if a pdev from pci_tsm/pci_tdi is used in, say, 1-2
>> places, then it is just cleaner to pass pdev to those places
>> explicitly
> 
> Maybe if we see that that are unused then they are easy to delete later.

It is way easier to do now than later when it grows. I'll dig a bit.


>> oooor
>>
>> - pass pci_tsm/pci_tdi to pci_tsm_ops hooks and use pdev in those when
>> needed, this way it is clearer from the hook prototype what it
>> operates on.
> 
> I think all of the operations need the pdev + extra data so it is always
> operating on the pdev to some extent. I think this is another, get
> "Aneesh and Yilun onboard" case.
>>>>> Out of curiosity (probably could go to the commit log) - for what kind
>>>> of request and on which platform we do not know the response size in
>>>> advance? On AMD, the request and response sizes are fixed.
>>>
>>> I don't know. Given this is to support any possible combination of TSM
>>> and ABI I took inspiration from fwctl which is trying to solve a similar
>>> common transport problem.
>>
>> If guest_req() returns NULL - what is it - error (no response) or
>> success ("request successfully accepted, no response needed")? The PSP
>> returns fw_err (which I pass in my guest_request hook), does this
>> interface suggest that my TSM dev should allocate a sizeof(fw_err)
>> buffer at least, and if there is more - then
>> sizeof(fw_err)+sizeof(response)? I thought TDX does return an error
>> code too, surprised to see it missing here.
> 
> As we talked about on the CCC call it sounds like at least TDX also
> wants to pass an explicit FW response code separate from the response
> buffer, so I will fix this up to not follow fwctl.
> 
> [..]
>>>> What is going to enforce this and how? It is a guest request, ideally
>>>> encrypted, and the host does not really have to know the nature of the
>>>> request (if the guest wants something from the host to do in addition
>>>> to what is it asking the TSM to do - then GHCB is for that). And 3 of
>>>> 4 AMD TIO requests (STATE_CHANGE is a host request and no plan for
>>>> DEBUG) do not fit in any category from the above anyway. imho we do
>>>> not need it at least now.
>>>
>>> While the TSM is in the trust boundary of the TVM, the TSM and the TVM
>>> are not necessarily trusted by the VMM. It has a responsibility to
>>> maintain its own security model especially when marshaling opaque blobs
>>> on behalf of a guest. This scope parameter serves the same purpose as it
>>> does in fwctl to maintain a security model and explicitly control for
>>> requests that are out of scope.
>>>
>>> The enforcement is market and regulatory forces to make solutions are
>>> not bypass security model expectations of the operating system.
>>
>> I get the idea, it just sounds like it should be a mask -
>> READ|WRITE|TDISP_STATE|DEBUG. Which category would MMIO_VALIDATE fall
>> (set "validated" in RMP)? Thanks,
> 
> Curious why is MMIO_VALIDATE separate from other Guest Physical Address
> validation? I think if it needs to be separate from other GPA validation
> then it would be in the PCI_TSM_REQ_STATE_CHANGE scope as it is just
> another expected step in the LOCKED->RUN transition.

For normal memory there is a guest "pvalidate" instruction to set the valid bit in the RMP. For MMIO, there is another bit in RMP which says "pvalidate" will fail on such entry, instead the PSP needs to set it (which is pretty much memcpy for it) as the guest needs assurance from the PSP that the RMP still maps the right thing. Thanks,



-- 
Alexey