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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 8 Jun 2017 09:26:38 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Nick Sarnie <commendsarnex@...il.com>
Cc:     linux-arch@...r.kernel.org, linux-efi@...r.kernel.org,
        kvm@...r.kernel.org, linux-doc@...r.kernel.org, x86@...nel.org,
        kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kasan-dev@...glegroups.com, linux-mm@...ck.org,
        iommu@...ts.linux-foundation.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Rik van Riel <riel@...hat.com>,
        Brijesh Singh <brijesh.singh@....com>,
        Toshimitsu Kani <toshi.kani@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Jonathan Corbet <corbet@....net>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Ingo Molnar <mingo@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dave Young <dyoung@...hat.com>,
        Larry Woodman <lwoodman@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with
 memory encryption

On 6/7/2017 9:38 PM, Nick Sarnie wrote:
> On Wed, Jun 7, 2017 at 3:17 PM, Tom Lendacky <thomas.lendacky@....com> 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@....com>
>> ---
>>   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
>