[PATCH V2 3/3] x86/efi: Use efi_switch_mm() rather than manually twiddling with %cr3

[Sai Praneeth Prakhya]

From: Sai Praneeth 

Use helper function (efi_switch_mm()) to switch to/from efi_mm. We
switch to efi_mm before calling
1. efi_set_virtual_address_map() and
2. Invoking any efi_runtime_service()

Likewise, we need to switch back to previous mm (mm context stolen by
efi_mm) after the above calls return successfully. We can use
efi_switch_mm() helper function only with x86_64 kernel and
"efi=old_map" disabled because, x86_32 and efi=old_map doesn't use
efi_pgd, rather they use swapper_pg_dir.

Signed-off-by: Sai Praneeth Prakhya 
Cc: Lee, Chun-Yi 
Cc: Borislav Petkov 
Cc: Tony Luck 
Cc: Andy Lutomirski 
Cc: Michael S. Tsirkin 
Cc: Ricardo Neri 
Cc: Matt Fleming 
Cc: Ard Biesheuvel 
Cc: Ravi Shankar 
---
 arch/x86/include/asm/efi.h   | 29 ++---
 arch/x86/platform/efi/efi_64.c   | 36 +---
 arch/x86/platform/efi/efi_thunk_64.S |  2 +-
 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 2f77bcefe6b4..23b2137a95e5 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -1,10 +1,14 @@
 #ifndef _ASM_X86_EFI_H
 #define _ASM_X86_EFI_H
 
+#include 
+#include 
+
 #include 
 #include 
 #include 
 #include 
+#include 
 
 /*
  * We map the EFI regions needed for runtime services non-contiguously,
@@ -57,14 +61,13 @@ extern u64 asmlinkage efi_call(void *fp, ...);
 #define efi_call_phys(f, args...)  efi_call((f), args)
 
 /*
- * Scratch space used for switching the pagetable in the EFI stub
+ * struct efi_scratch - Scratch space used while switching to/from efi_mm
+ * @phys_stack: stack used during EFI Mixed Mode
+ * @prev_mm:store/restore stolen mm_struct while switching to/from efi_mm
  */
 struct efi_scratch {
-   u64 r15;
-   u64 prev_cr3;
-   pgd_t   *efi_pgt;
-   booluse_pgd;
-   u64 phys_stack;
+   u64 phys_stack;
+   struct mm_struct*prev_mm;
 } __packed;
 
 #define arch_efi_call_virt_setup() \
@@ -73,11 +76,8 @@ struct efi_scratch {
preempt_disable();  \
__kernel_fpu_begin();   \
\
-   if (efi_scratch.use_pgd) {  \
-   efi_scratch.prev_cr3 = read_cr3();  \
-   write_cr3((unsigned long)efi_scratch.efi_pgt);  \
-   __flush_tlb_all();  \
-   }   \
+   if (!efi_enabled(EFI_OLD_MEMMAP))   \
+   efi_switch_mm(_mm); \
 })
 
 #define arch_efi_call_virt(p, f, args...)  \
@@ -85,10 +85,8 @@ struct efi_scratch {
 
 #define arch_efi_call_virt_teardown()  \
 ({ \
-   if (efi_scratch.use_pgd) {  \
-   write_cr3(efi_scratch.prev_cr3);\
-   __flush_tlb_all();  \
-   }   \
+   if (!efi_enabled(EFI_OLD_MEMMAP))   \
+   efi_switch_mm(efi_scratch.prev_mm); \
\
__kernel_fpu_end(); \
preempt_enable();   \
@@ -130,6 +128,7 @@ extern void __init efi_dump_pagetable(void);
 extern void __init efi_apply_memmap_quirks(void);
 extern int __init efi_reuse_config(u64 tables, int nr_tables);
 extern void efi_delete_dummy_variable(void);
+extern void efi_switch_mm(struct mm_struct *mm);
 
 struct efi_setup_data {
u64 fw_vendor;
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 0bb98c35e178..e0545f56d703 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -80,9 +80,8 @@ pgd_t * __init efi_call_phys_prolog(void)
int n_pgds, i, j;
 
if (!efi_enabled(EFI_OLD_MEMMAP)) {
-   save_pgd = (pgd_t *)read_cr3();
-   write_cr3((unsigned long)efi_scratch.efi_pgt);
-   goto out;
+   efi_switch_mm(_mm);
+   return NULL;
}
 
early_code_mapping_set_exec(1);
@@ -152,8 +151,7 @@ void 

[PATCH V2 1/3] efi: Use efi_mm in x86 as well as ARM

[Sai Praneeth Prakhya]

From: Sai Praneeth 

Presently, only ARM uses mm_struct to manage efi page tables and efi
runtime region mappings. As this is the preferred approach, let's make
this data structure common across architectures. Specially, for
x86, using this data structure improves code maintainability and
readability.

Signed-off-by: Sai Praneeth Prakhya 
Cc: Lee, Chun-Yi 
Cc: Borislav Petkov 
Cc: Tony Luck 
Cc: Andy Lutomirski 
Cc: Michael S. Tsirkin 
Cc: Ricardo Neri 
Cc: Matt Fleming 
Cc: Ard Biesheuvel 
Cc: Ravi Shankar 
---
 drivers/firmware/efi/arm-runtime.c | 9 -
 drivers/firmware/efi/efi.c | 9 +
 include/linux/efi.h| 2 ++
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/efi/arm-runtime.c 
b/drivers/firmware/efi/arm-runtime.c
index 1cc41c3d6315..d6b26534812b 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -31,15 +31,6 @@
 
 extern u64 efi_system_table;
 
-static struct mm_struct efi_mm = {
-   .mm_rb  = RB_ROOT,
-   .mm_users   = ATOMIC_INIT(2),
-   .mm_count   = ATOMIC_INIT(1),
-   .mmap_sem   = __RWSEM_INITIALIZER(efi_mm.mmap_sem),
-   .page_table_lock= __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock),
-   .mmlist = LIST_HEAD_INIT(efi_mm.mmlist),
-};
-
 #ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
 #include 
 
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad3b449..3abbb25602bc 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -55,6 +55,15 @@ struct efi __read_mostly efi = {
 };
 EXPORT_SYMBOL(efi);
 
+struct mm_struct efi_mm = {
+   .mm_rb  = RB_ROOT,
+   .mm_users   = ATOMIC_INIT(2),
+   .mm_count   = ATOMIC_INIT(1),
+   .mmap_sem   = __RWSEM_INITIALIZER(efi_mm.mmap_sem),
+   .page_table_lock= __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock),
+   .mmlist = LIST_HEAD_INIT(efi_mm.mmlist),
+};
+
 static bool disable_runtime;
 static int __init setup_noefi(char *arg)
 {
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 8269bcb8ccf7..d1f261d2ce69 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -927,6 +927,8 @@ extern struct efi {
unsigned long flags;
 } efi;
 
+extern struct mm_struct efi_mm;
+
 static inline int
 efi_guidcmp (efi_guid_t left, efi_guid_t right)
 {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/3] x86/efi: Replace efi_pgd with efi_mm.pgd

[Sai Praneeth Prakhya]

From: Sai Praneeth 

Since the previous patch added support for efi_mm, let's handle efi_pgd
through efi_mm and remove global variable efi_pgd.

Signed-off-by: Sai Praneeth Prakhya 
Cc: Lee, Chun-Yi 
Cc: Borislav Petkov 
Cc: Tony Luck 
Cc: Andy Lutomirski 
Cc: Michael S. Tsirkin 
Cc: Ricardo Neri 
Cc: Matt Fleming 
Cc: Ard Biesheuvel 
Cc: Ravi Shankar 
---
 arch/x86/platform/efi/efi_64.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 8ff1f95627f9..0bb98c35e178 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -187,8 +187,6 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
early_code_mapping_set_exec(0);
 }
 
-static pgd_t *efi_pgd;
-
 /*
  * We need our own copy of the higher levels of the page tables
  * because we want to avoid inserting EFI region mappings (EFI_VA_END
@@ -197,7 +195,7 @@ static pgd_t *efi_pgd;
  */
 int __init efi_alloc_page_tables(void)
 {
-   pgd_t *pgd;
+   pgd_t *pgd, *efi_pgd;
p4d_t *p4d;
pud_t *pud;
gfp_t gfp_mask;
@@ -225,6 +223,8 @@ int __init efi_alloc_page_tables(void)
return -ENOMEM;
}
 
+   efi_mm.pgd = efi_pgd;
+
return 0;
 }
 
@@ -237,6 +237,7 @@ void efi_sync_low_kernel_mappings(void)
pgd_t *pgd_k, *pgd_efi;
p4d_t *p4d_k, *p4d_efi;
pud_t *pud_k, *pud_efi;
+   pgd_t *efi_pgd = efi_mm.pgd;
 
if (efi_enabled(EFI_OLD_MEMMAP))
return;
@@ -330,13 +331,12 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, 
unsigned num_pages)
unsigned long pfn, text;
struct page *page;
unsigned npages;
-   pgd_t *pgd;
+   pgd_t *pgd = efi_mm.pgd;
 
if (efi_enabled(EFI_OLD_MEMMAP))
return 0;
 
-   efi_scratch.efi_pgt = (pgd_t *)__pa(efi_pgd);
-   pgd = efi_pgd;
+   efi_scratch.efi_pgt = (pgd_t *)__pa(pgd);
 
/*
 * It can happen that the physical address of new_memmap lands in memory
@@ -400,7 +400,7 @@ static void __init __map_region(efi_memory_desc_t *md, u64 
va)
 {
unsigned long flags = _PAGE_RW;
unsigned long pfn;
-   pgd_t *pgd = efi_pgd;
+   pgd_t *pgd = efi_mm.pgd;
 
if (!(md->attribute & EFI_MEMORY_WB))
flags |= _PAGE_PCD;
@@ -501,7 +501,7 @@ void __init parse_efi_setup(u64 phys_addr, u32 data_len)
 static int __init efi_update_mappings(efi_memory_desc_t *md, unsigned long pf)
 {
unsigned long pfn;
-   pgd_t *pgd = efi_pgd;
+   pgd_t *pgd = efi_mm.pgd;
int err1, err2;
 
/* Update the 1:1 mapping */
@@ -592,7 +592,7 @@ void __init efi_dump_pagetable(void)
if (efi_enabled(EFI_OLD_MEMMAP))
ptdump_walk_pgd_level(NULL, swapper_pg_dir);
else
-   ptdump_walk_pgd_level(NULL, efi_pgd);
+   ptdump_walk_pgd_level(NULL, efi_mm.pgd);
 #endif
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 0/3] Use mm_struct and switch_mm() instead of manually

[Sai Praneeth Prakhya]

From: Sai Praneeth 

Presently, in x86, to invoke any efi function like
efi_set_virtual_address_map() or any efi_runtime_service() the code path
typically involves read_cr3() (save previous pgd), write_cr3()
(write efi_pgd) and calling efi function. Likewise after returning from
efi function the code path typically involves read_cr3() (save efi_pgd),
write_cr3() (write previous pgd). We do this couple of times in efi
subsystem of Linux kernel, instead we can use helper function
efi_switch_mm() to do this. This improves readability and maintainability.
Also, instead of maintaining a separate struct "efi_scratch" to store/restore
efi_pgd, we can use mm_struct to do this.

I have tested this patch set against LUV (Linux UEFI Validation), so I
think I didn't break any existing configurations. I have tested this
patch set for
1. x86_64,
2. x86_32,
3. Mixed mode
with efi=old_map and for kexec kernel. Please let me know if I have
missed any other configurations.

Changes in V2:
1. Resolve mm_dropping() issue by not mm_dropping()/mm_grabbing() any mm,
as we are not losing/creating any references.

Sai Praneeth (3):
  efi: Use efi_mm in x86 as well as ARM
  x86/efi: Replace efi_pgd with efi_mm.pgd
  x86/efi: Use efi_switch_mm() rather than manually twiddling with %cr3

 arch/x86/include/asm/efi.h   | 29 ++--
 arch/x86/platform/efi/efi_64.c   | 52 
 arch/x86/platform/efi/efi_thunk_64.S |  2 +-
 drivers/firmware/efi/arm-runtime.c   |  9 ---
 drivers/firmware/efi/efi.c   |  9 +++
 include/linux/efi.h  |  2 ++
 6 files changed, 55 insertions(+), 48 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC Part1 PATCH v3 15/17] x86: Add support for changing memory encryption attribute in early boot

[Brijesh Singh]

Hi Boris,


On 8/28/17 5:51 AM, Borislav Petkov wrote:

[..]

> +static int __init early_set_memory_enc_dec(resource_size_t paddr,
>> +   unsigned long size, bool enc)
>> +{
>> +unsigned long vaddr, vaddr_end, vaddr_next;
>> +unsigned long psize, pmask;
>> +int split_page_size_mask;
>> +pte_t *kpte;
>> +int level;
>> +
>> +vaddr = (unsigned long)__va(paddr);
>> +vaddr_next = vaddr;
>> +vaddr_end = vaddr + size;
>> +
>> +/*
>> + * We are going to change the physical page attribute from C=1 to C=0
>> + * or vice versa. Flush the caches to ensure that data is written into
>> + * memory with correct C-bit before we change attribute.
>> + */
>> +clflush_cache_range(__va(paddr), size);
>> +
>> +for (; vaddr < vaddr_end; vaddr = vaddr_next) {
>> +kpte = lookup_address(vaddr, );
>> +if (!kpte || pte_none(*kpte))
>> +return 1;
> Return before flushing TLBs? Perhaps you mean
>
>   ret = 1;
>   goto out;
>
> here and out does
>
>   __flush_tlb_all();
>   return ret;

thanks, good catch. I will fix in next rev.

-Brijesh
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC Part1 PATCH v3 15/17] x86: Add support for changing memory encryption attribute in early boot

[Borislav Petkov]

On Mon, Jul 24, 2017 at 02:07:55PM -0500, Brijesh Singh wrote:
> Some KVM-specific custom MSRs shares the guest physical address with

s/shares/share/

> hypervisor.

"the hypervisor."

> When SEV is active, the shared physical address must be mapped
> with encryption attribute cleared so that both hypervsior and guest can
> access the data.
> 
> Add APIs to change memory encryption attribute in early boot code.
> 
> Signed-off-by: Brijesh Singh 
> ---
>  arch/x86/include/asm/mem_encrypt.h |  17 ++
>  arch/x86/mm/mem_encrypt.c  | 117 
> +
>  2 files changed, 134 insertions(+)

...

> +static int __init early_set_memory_enc_dec(resource_size_t paddr,
> +unsigned long size, bool enc)
> +{
> + unsigned long vaddr, vaddr_end, vaddr_next;
> + unsigned long psize, pmask;
> + int split_page_size_mask;
> + pte_t *kpte;
> + int level;
> +
> + vaddr = (unsigned long)__va(paddr);
> + vaddr_next = vaddr;
> + vaddr_end = vaddr + size;
> +
> + /*
> +  * We are going to change the physical page attribute from C=1 to C=0
> +  * or vice versa. Flush the caches to ensure that data is written into
> +  * memory with correct C-bit before we change attribute.
> +  */
> + clflush_cache_range(__va(paddr), size);
> +
> + for (; vaddr < vaddr_end; vaddr = vaddr_next) {
> + kpte = lookup_address(vaddr, );
> + if (!kpte || pte_none(*kpte))
> + return 1;

Return before flushing TLBs? Perhaps you mean

ret = 1;
goto out;

here and out does

__flush_tlb_all();
return ret;

?

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html