From: Borislav Petkov <bp@xxxxxxxxx> Sent: Wednesday, January 25, 2023 6:56 AM
>
> On Sat, Jan 21, 2023 at 04:10:23AM +0000, Michael Kelley (LINUX) wrote:
[snip]
>
> > But in any case, the whole point of cc_platform_has() is to provide a level of
> > abstraction from the hardware registers, and it's fully safe to use on every x86
> > bare-metal system or VM. And while I don't anticipate it now, maybe there's
> > some future scheme where a paravisor-like entity could be used with Intel
> > TDX. It seems like using a cc_platform_has() abstraction is better than directly
> > accessing the MSR.
>
> That's fine but we're talking about this particular implementation and that is
> vTOM-like with the address space split. If TDX does address space split later,
> we can accomodate it too. (Although I think they are not interested in this).
>
> And if you really want to use cc_platform_has(), we could do
>
> cc_platform_has(CC_ADDRESS_SPACE_SPLIT_ON_A_PARAVISOR)
>
> or something with a better name.
I do think it makes sense to use the cc_platform_has() abstraction. It's
then a question of agreeing on how to name the attribute. We've
discussed various approaches in different versions of this patch series:
v1 & v2: CC_ATTR_HAS_PARAVISOR
v3: CC_ATTR_EMULATED_IOAPIC
v4 & v5: CC_ATTR_ACCESS_IOAPIC_ENCRYPTED
I could do:
1. CC_ATTR_PARAVISOR_SPLIT_ADDRESS_SPACE, which is similar to
what I had for v1 & v2. At the time, somebody commented that
this might be a bit too general.
2. Keep CC_ATTR_ACCESS_IOAPIC_ENCRYPTED and add
CC_ATTR_ACCESS_TPM_ENCRYPTED, which would decouple them
3. CC_ATTR_ACCESS_IOAPIC_AND_TPM_ENCRYPTED, which is very
narrow and specific.
I have weak preference for #1 above, but I could go with any of them.
What's your preference?
> > My resolution of the TPM driver issue is admittedly a work-around. I think
> > of it as temporary in anticipation of future implementations of PCIe TDISP
> > hardware, which allows PCI devices to DMA directly into guest encrypted
> > memory.
>
> Yap, that sounds real nice.
>
> > TDISP also places the device's BAR values in an encrypted portion
> > of the GPA space. Assuming TDISP hardware comes along in the next couple
> > of years, Linux will need a robust way to deal with a mix of PCI devices
> > being in unencrypted and encrypted GPA space. I don't know how a
> > specific device will be mapped correctly, but I hope it can happen in the
> > generic PCI code, and not by modifying each device driver.
>
> I guess those devices would advertize that capability somehow so that code can
> query it and act accordingly.
>
> > It's probably premature to build that robust mechanism now, but when it comes,
> > my work-around would be replaced.
>
> It would be replaced if it doesn't have any users. By the looks of it, it'll
> soon grow others and then good luck removing it.
>
> > With all that in mind, I don't want to modify the TPM driver to special-case
> > its MMIO space being encrypted. FWIW, the TPM driver today uses
> > devm_ioremap_resource() to do the mapping, which defaults to mapping
> > decrypted except for the exceptions implemented in __ioremap_caller().
> > There's no devm_* option for specifying encrypted.
>
> You mean, it is hard to add a DEVM_IOREMAP_ENCRYPTED type which will have
> __devm_ioremap() call ioremap_encrypted()?
>
> Or define a IORESOURCE_ENCRYPTED and pass it through the ioresource flags?
>
> Why is that TPM driver so precious that it can be touched and the arch code
> would have to accept hacks?
>
> > Handling decrypted vs. encrypted in the driver would require extending the
> > driver API to provide an "encrypted" option, and that seems like going in the
> > wrong long-term direction.
>
> Sorry, I can't follow here.
>
For v6 of the patch series, I've coded devm_ioremap_resource_enc() to call
__devm_ioremap(), which then calls ioremap_encrypted(). I've updated the
TPM driver to use cc_platform_has() with whatever attribute name we agree
on to decide between devm_ioremap_resource_enc() and
devm_ioremap_resource().
If this approach is OK with the TPM driver maintainers, I'm good with it.
More robust handling of a mix of encrypted and decrypted devices can get
sorted out later.
Michael
