Re: [PATCH v7 27/36] iommu/amd: Allow the AMD IOMMU to work with memory encryption

[Tom Lendacky]

On 6/22/2017 5:56 AM, Borislav Petkov wrote:

On Fri, Jun 16, 2017 at 01:54:59PM -0500, Tom Lendacky 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 
---
  drivers/iommu/amd_iommu.c   |   30 --
  drivers/iommu/amd_iommu_init.c  |   34 --
  drivers/iommu/amd_iommu_proto.h |   10 ++
  drivers/iommu/amd_iommu_types.h |2 +-
  4 files changed, 55 insertions(+), 21 deletions(-)


Reviewed-by: Borislav Petkov 

Btw, I'm assuming the virt_to_phys() difference on SME systems is only
needed in a handful of places. Otherwise, I'd suggest changing the
virt_to_phys() function/macro directly. But I guess most of the places
need the real physical address without the enc bit.


Correct.

Thanks,
Tom





___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v7 19/36] x86/mm: Add support to access boot related data in the clear

[Matt Fleming]

On Fri, 16 Jun, at 01:53:26PM, Tom Lendacky wrote:
> Boot data (such as EFI related data) is not encrypted when the system is
> booted because UEFI/BIOS does not run with SME active. In order to access
> this data properly it needs to be mapped decrypted.
> 
> Update early_memremap() to provide an arch specific routine to modify the
> pagetable protection attributes before they are applied to the new
> mapping. This is used to remove the encryption mask for boot related data.
> 
> Update memremap() to provide an arch specific routine to determine if RAM
> remapping is allowed.  RAM remapping will cause an encrypted mapping to be
> generated. By preventing RAM remapping, ioremap_cache() will be used
> instead, which will provide a decrypted mapping of the boot related data.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/io.h |5 +
>  arch/x86/mm/ioremap.c |  179 
> +
>  include/linux/io.h|2 +
>  kernel/memremap.c |   20 -
>  mm/early_ioremap.c|   18 -
>  5 files changed, 217 insertions(+), 7 deletions(-)

Reviewed-by: Matt Fleming 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v7 18/36] x86/efi: Update EFI pagetable creation to work with SME

[Matt Fleming]

On Fri, 16 Jun, at 01:53:17PM, Tom Lendacky wrote:
> When SME is active, pagetable entries created for EFI need to have the
> encryption mask set as necessary.
> 
> When the new pagetable pages are allocated they are mapped encrypted. So,
> update the efi_pgt value that will be used in cr3 to include the encryption
> mask so that the PGD table can be read successfully. The pagetable mapping
> as well as the kernel are also added to the pagetable mapping as encrypted.
> All other EFI mappings are mapped decrypted (tables, etc.).
> 
> Reviewed-by: Borislav Petkov 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/platform/efi/efi_64.c |   15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
 
Reviewed-by: Matt Fleming 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v7 16/36] efi: Add an EFI table address match function

[Matt Fleming]

On Fri, 16 Jun, at 01:52:53PM, Tom Lendacky wrote:
> Add a function that will determine if a supplied physical address matches
> the address of an EFI table.
> 
> Reviewed-by: Borislav Petkov 
> Signed-off-by: Tom Lendacky 
> ---
>  drivers/firmware/efi/efi.c |   33 +
>  include/linux/efi.h|7 +++
>  2 files changed, 40 insertions(+)

Reviewed-by: Matt Fleming 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v7 27/36] iommu/amd: Allow the AMD IOMMU to work with memory encryption

[Borislav Petkov]

On Fri, Jun 16, 2017 at 01:54:59PM -0500, Tom Lendacky 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 
> ---
>  drivers/iommu/amd_iommu.c   |   30 --
>  drivers/iommu/amd_iommu_init.c  |   34 --
>  drivers/iommu/amd_iommu_proto.h |   10 ++
>  drivers/iommu/amd_iommu_types.h |2 +-
>  4 files changed, 55 insertions(+), 21 deletions(-)

Reviewed-by: Borislav Petkov 

Btw, I'm assuming the virt_to_phys() difference on SME systems is only
needed in a handful of places. Otherwise, I'd suggest changing the
virt_to_phys() function/macro directly. But I guess most of the places
need the real physical address without the enc bit.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v7 17/36] efi: Update efi_mem_type() to return an error rather than 0

[Matt Fleming]

On Fri, 16 Jun, at 01:53:06PM, Tom Lendacky wrote:
> The efi_mem_type() function currently returns a 0, which maps to
> EFI_RESERVED_TYPE, if the function is unable to find a memmap entry for
> the supplied physical address. Returning EFI_RESERVED_TYPE implies that
> a memmap entry exists, when it doesn't.  Instead of returning 0, change
> the function to return a negative error value when no memmap entry is
> found.
> 
> Reviewed-by: Borislav Petkov 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/ia64/kernel/efi.c  |4 ++--
>  arch/x86/platform/efi/efi.c |6 +++---
>  include/linux/efi.h |2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Matt Fleming 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v7 26/36] x86/CPU/AMD: Make the microcode level available earlier in the boot

[Borislav Petkov]

On Fri, Jun 16, 2017 at 01:54:47PM -0500, Tom Lendacky wrote:
> Move the setting of the cpuinfo_x86.microcode field from amd_init() to
> early_amd_init() so that it is available earlier in the boot process. This
> avoids having to read MSR_AMD64_PATCH_LEVEL directly during early boot.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/kernel/cpu/amd.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition

[Xunlei Pang]

On 06/22/2017 at 01:44 AM, Michael Holzheu wrote:
> Am Fri,  9 Jun 2017 10:17:05 +0800
> schrieb Xunlei Pang :
>
>> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
>> is now defined as follows:
>> typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
>> It was changed by the CONFIG_CRASH_CORE feature.
>>
>> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
>> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
>>
>> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related 
>> code under CONFIG_CRASH_CORE")
>> Cc: Dave Young 
>> Cc: Dave Anderson 
>> Cc: Hari Bathini 
>> Cc: Gustavo Luiz Duarte 
>> Signed-off-by: Xunlei Pang 
> Hello Xunlei,
>
> As you already know on s390 we create the ELF header in the new kernel.
> Therefore we don't use the per-cpu buffers for ELF notes to store
> the register state.
>
> For RHEL7 we still store the registers in machine_kexec.c:add_elf_notes().
> Though we also use the ELF header from new kernel ...
>
> We assume your original problem with the "kmem -s" failure
> was caused by the memory overwrite due to the invalid size of the
> "crash_notes" per-cpu buffers.
>
> Therefore your patch looks good for RHEL7 but for upstream we propose the
> patch below.

Hi Michael,

Yes, we already did this way.
Thanks for the confirmation, the patch below looks good to me.

Regards,
Xunlei

> ---
> [PATCH] s390/crash: Remove unused KEXEC_NOTE_BYTES
>
> After commmit 692f66f26a4c19 ("crash: move crashkernel parsing and vmcore
> related code under CONFIG_CRASH_CORE") the KEXEC_NOTE_BYTES macro is not
> used anymore and for s390 we create the ELF header in the new kernel
> anyway. Therefore remove the macro.
>
> Reported-by: Xunlei Pang 
> Reviewed-by: Mikhail Zaslonko 
> Signed-off-by: Michael Holzheu 
> ---
>  arch/s390/include/asm/kexec.h | 18 --
>  include/linux/crash_core.h|  5 +
>  include/linux/kexec.h |  9 -
>  3 files changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
> index 2f924bc30e35..dccf24ee26d3 100644
> --- a/arch/s390/include/asm/kexec.h
> +++ b/arch/s390/include/asm/kexec.h
> @@ -41,24 +41,6 @@
>  /* The native architecture */
>  #define KEXEC_ARCH KEXEC_ARCH_S390
>  
> -/*
> - * Size for s390x ELF notes per CPU
> - *
> - * Seven notes plus zero note at the end: prstatus, fpregset, timer,
> - * tod_cmp, tod_reg, control regs, and prefix
> - */
> -#define KEXEC_NOTE_BYTES \
> - (ALIGN(sizeof(struct elf_note), 4) * 8 + \
> -  ALIGN(sizeof("CORE"), 4) * 7 + \
> -  ALIGN(sizeof(struct elf_prstatus), 4) + \
> -  ALIGN(sizeof(elf_fpregset_t), 4) + \
> -  ALIGN(sizeof(u64), 4) + \
> -  ALIGN(sizeof(u64), 4) + \
> -  ALIGN(sizeof(u32), 4) + \
> -  ALIGN(sizeof(u64) * 16, 4) + \
> -  ALIGN(sizeof(u32), 4) \
> - )
> -
>  /* Provide a dummy definition to avoid build failures. */
>  static inline void crash_setup_regs(struct pt_regs *newregs,
>   struct pt_regs *oldregs) { }
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index 541a197ba4a2..4090a42578a8 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -10,6 +10,11 @@
>  #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
>  #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
>  
> +/*
> + * The per-cpu notes area is a list of notes terminated by a "NULL"
> + * note header.  For kdump, the code in vmcore.c runs in the context
> + * of the second kernel to combine them into one note.
> + */
>  #define CRASH_CORE_NOTE_BYTES   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +  
> \
>CRASH_CORE_NOTE_NAME_BYTES +   \
>CRASH_CORE_NOTE_DESC_BYTES)
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index c9481ebcbc0c..65888418fb69 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -63,15 +63,6 @@
>  #define KEXEC_CORE_NOTE_NAME CRASH_CORE_NOTE_NAME
>  
>  /*
> - * The per-cpu notes area is a list of notes terminated by a "NULL"
> - * note header.  For kdump, the code in vmcore.c runs in the context
> - * of the second kernel to combine them into one note.
> - */
> -#ifndef KEXEC_NOTE_BYTES
> -#define KEXEC_NOTE_BYTES CRASH_CORE_NOTE_BYTES
> -#endif
> -
> -/*
>   * This structure is used to hold the arguments that are used when loading
>   * kernel binaries.
>   */


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec