Re: [PATCH v1 2/2] acpi, x86: Remove encryption mask from ACPI page protection type

[Tom Lendacky]

On 7/27/2017 2:39 AM, Ingo Molnar wrote:
> * Tom Lendacky <thomas.lendacky@xxxxxxx> wrote:
>
> > The function arch_apei_get_mem_attributes() is used to set the page
> > protection type for ACPI physical addresses. When SME is active, the
> > associated protection type needs to not have the encryption mask set
> > since the ACPI tables live in un-encrypted memory. Modify the
> > arch_apei_get_mem_attributes() function to remove the encryption mask
> > when SME is active by returning the PAGE_KERNEL_IO protection type.
> >
> > Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> > ---
> >   arch/x86/include/asm/acpi.h | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> > index 562286f..89df39d 100644
> > --- a/arch/x86/include/asm/acpi.h
> > +++ b/arch/x86/include/asm/acpi.h
> > @@ -34,6 +34,7 @@
> >   
> >   #ifdef CONFIG_ACPI_APEI
> >   # include <asm/pgtable_types.h>
> > +# include <linux/mem_encrypt.h>
> >   #endif
> >   
> >   #ifdef CONFIG_ACPI
> > @@ -164,8 +165,12 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >   	 * anything other than PAGE_KERNEL (some arm64 platforms
> >   	 * require the equivalent of PAGE_KERNEL_NOCACHE), return that
> >   	 * until we know differently.
> > +	 *
> > +	 * When SME is active, the ACPI information will not reside in
> > +	 * in memory in an encrypted state so return a protection attribute
> > +	 * that does not have the encryption bit set.
> >   	 */
> > -	 return PAGE_KERNEL;
> > +	 return sme_active() ? PAGE_KERNEL_IO : PAGE_KERNEL;
>
> 'in in memory'?

Should just be one 'in', will fix.

> Also, this seems a bit ad-hoc to me. What are the rules for what is encrypted and
> what is not?
>
> I presume the main rule is that everything that was written before the kernel
> activates SME, and which the kernel will read later on, is unencrypted -
> everything else is encrypted. Is that correct?

Yes, that is the basic rule.  Anything from BIOS/UEFI is unencrypted
and much of everything else after the kernel encrypts itself will be
encrypted.  Kernel related hings that aren't be encrypted are, for
example, the SWIOTLB bounce buffers, the trampoline area for bringing
up APs, etc.

> How about things like kexec creating a separate mptable for the kexec kernel -
> would that code have to create unencrypted data? See
> e820__memblock_alloc_reserved() and related code.

Yes, that would need to be unencrypted since it will be considered to
be something supplied by BIOS/UEFI (even though kexec builds it). I'm
not familiar with where kexec creates a new mptable. I see the refs to
it in e820.c, but I'm missing something. Can you point me to that?

Thanks,
Tom

> Thanks,
>
> 	Ingo