Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption

[Tom Lendacky]

On 6/7/2017 9:38 PM, Nick Sarnie wrote:
> On Wed, Jun 7, 2017 at 3:17 PM, Tom Lendacky <thomas.lendacky@xxxxxxx> wrote:
> > The IOMMU is programmed with physical addresses for the various tables
> > and buffers that are used to communicate between the device and the
> > driver. When the driver allocates this memory it is encrypted. In order
> > for the IOMMU to access the memory as encrypted the encryption mask needs
> > to be included in these physical addresses during configuration.
> >
> > The PTE entries created by the IOMMU should also include the encryption
> > mask so that when the device behind the IOMMU performs a DMA, the DMA
> > will be performed to encrypted memory.
> >
> > Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> > ---
> >   arch/x86/include/asm/mem_encrypt.h |    7 +++++++
> >   arch/x86/mm/mem_encrypt.c          |   30 ++++++++++++++++++++++++++++++
> >   drivers/iommu/amd_iommu.c          |   36 +++++++++++++++++++-----------------
> >   drivers/iommu/amd_iommu_init.c     |   18 ++++++++++++------
> >   drivers/iommu/amd_iommu_proto.h    |   10 ++++++++++
> >   drivers/iommu/amd_iommu_types.h    |    2 +-
> >   include/asm-generic/mem_encrypt.h  |    5 +++++
> >   7 files changed, 84 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> > index c7a2525..d86e544 100644
> > --- a/arch/x86/include/asm/mem_encrypt.h
> > +++ b/arch/x86/include/asm/mem_encrypt.h
> > @@ -31,6 +31,8 @@ void __init sme_early_decrypt(resource_size_t paddr,
> >
> >   void __init sme_early_init(void);
> >
> > +bool sme_iommu_supported(void);
> > +
> >   /* Architecture __weak replacement functions */
> >   void __init mem_encrypt_init(void);
> >
> > @@ -62,6 +64,11 @@ static inline void __init sme_early_init(void)
> >   {
> >   }
> >
> > +static inline bool sme_iommu_supported(void)
> > +{
> > +       return true;
> > +}
> > +
> >   #endif /* CONFIG_AMD_MEM_ENCRYPT */
> >
> >   static inline bool sme_active(void)
> > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> > index 5d7c51d..018b58a 100644
> > --- a/arch/x86/mm/mem_encrypt.c
> > +++ b/arch/x86/mm/mem_encrypt.c
> > @@ -197,6 +197,36 @@ void __init sme_early_init(void)
> >                  protection_map[i] = pgprot_encrypted(protection_map[i]);
> >   }
> >
> > +bool sme_iommu_supported(void)
> > +{
> > +       struct cpuinfo_x86 *c = &boot_cpu_data;
> > +
> > +       if (!sme_me_mask || (c->x86 != 0x17))
> > +               return true;
> > +
> > +       /* For Fam17h, a specific level of support is required */
> > +       switch (c->microcode & 0xf000) {
> > +       case 0x0000:
> > +               return false;
> > +       case 0x1000:
> > +               switch (c->microcode & 0x0f00) {
> > +               case 0x0000:
> > +                       return false;
> > +               case 0x0100:
> > +                       if ((c->microcode & 0xff) < 0x26)
> > +                               return false;
> > +                       break;
> > +               case 0x0200:
> > +                       if ((c->microcode & 0xff) < 0x05)
> > +                               return false;
> > +                       break;
> > +               }
> > +               break;
> > +       }
> > +
> > +       return true;
> > +}
> > +
> >   /* Architecture __weak replacement functions */
> >   void __init mem_encrypt_init(void)
> >   {

...

> Hi Tom,
>
> This sounds like a cool feature. I'm trying to test it on my Ryzen
> system, but c->microcode & 0xf000 is evaluating as 0, so IOMMU is not
> being enabled on my system. I'm using the latest microcode for AGESA
> 1.0.0.6, 0x08001126. Is this work reliant on a future microcode
> update, or is there some other issue?

This is my mistake. I moved the check and didn't re-test. At this point
the c->microcode field hasn't been filled in so I'll need to read
MSR_AMD64_PATCH_LEVEL directly in the sme_iommu_supported() function.

Thanks,
Tom

> Thanks,
> Sarnex