Messages in this thread | | | Date | Thu, 23 Dec 2021 20:15:30 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCH 19/26] x86/tdx: Make pages shared in ioremap() |
| |
On Wed, Dec 22, 2021 at 11:26:59AM -0600, Tom Lendacky wrote: > On 12/14/21 9:02 AM, Kirill A. Shutemov wrote: > > In TDX guests, guest memory is protected from host access. If a guest > > performs I/O, it needs to explicitly share the I/O memory with the host. > > > > Make all ioremap()ed pages that are not backed by normal memory > > (IORES_DESC_NONE or IORES_DESC_RESERVED) mapped as shared. > > > > Since TDX memory encryption support is similar to AMD SEV architecture, > > reuse the infrastructure from AMD SEV code. Introduce CC_ATTR_GUEST_TDX > > to add TDX-specific changes to the AMD SEV/SME memory encryption code. > > > > Add tdx_shared_mask() interface to get the TDX guest shared bitmask. > > > > pgprot_decrypted() is used by drivers (i915, virtio_gpu, vfio). Export > > both pgprot_encrypted() and pgprot_decrypted(). > > > > > --- a/arch/x86/mm/mem_encrypt.c > > +++ b/arch/x86/mm/mem_encrypt.c > > @@ -14,6 +14,33 @@ > > #include <linux/mem_encrypt.h> > > #include <linux/virtio_config.h> > > +#include <asm/tdx.h> > > + > > +/* > > + * Set or unset encryption attribute in vendor agnostic way. > > + */ > > +pgprot_t pgprot_cc_encrypted(pgprot_t prot) > > +{ > > + if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)) > > + return __pgprot(__sme_set(pgprot_val(prot))); > > + else if (cc_platform_has(CC_ATTR_GUEST_TDX)) > > + return __pgprot(pgprot_val(prot) & ~tdx_shared_mask()); > > + > > Hmmm... I believe this breaks SEV guests. __sme_set() uses sme_me_mask which > is used for both SME and SEV. With the current checks, an SEV guest will end > up never setting an encrypted address through this path. Ditto below on the > decrypted path.
Hm, okay. What if I rewrite code like this:
pgprot_t pgprot_cc_encrypted(pgprot_t prot) { if (cc_platform_has(CC_ATTR_GUEST_TDX)) return __pgprot(pgprot_val(prot) & ~tdx_shared_mask()); else return __pgprot(__sme_set(pgprot_val(prot))); }
I believe it should cover all cases, right?
-- Kirill A. Shutemov
| |