diff mbox series

[RFC,15/27] of/fdt: Introduce early_init_dt_add_memory_hyp()

Message ID 20201117181607.1761516-16-qperret@google.com
State Needs Review / ACK
Headers show
Series KVM/arm64: A stage 2 for the host | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Quentin Perret Nov. 17, 2020, 6:15 p.m. UTC
Introduce early_init_dt_add_memory_hyp() to allow KVM to conserve a copy
of the memory regions parsed from DT. This will be needed in the context
of the protected nVHE feature of KVM/arm64 where the code running at EL2
will be cleanly separated from the host kernel during boot, and will
need its own representation of memory.

Signed-off-by: Quentin Perret <qperret@google.com>
---
 drivers/of/fdt.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Rob Herring Nov. 17, 2020, 7:44 p.m. UTC | #1
On Tue, Nov 17, 2020 at 12:16 PM Quentin Perret <qperret@google.com> wrote:
>
> Introduce early_init_dt_add_memory_hyp() to allow KVM to conserve a copy
> of the memory regions parsed from DT. This will be needed in the context
> of the protected nVHE feature of KVM/arm64 where the code running at EL2
> will be cleanly separated from the host kernel during boot, and will
> need its own representation of memory.
>
> Signed-off-by: Quentin Perret <qperret@google.com>
> ---
>  drivers/of/fdt.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 4602e467ca8b..af2b5a09c5b4 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1099,6 +1099,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  #define MAX_MEMBLOCK_ADDR      ((phys_addr_t)~0)
>  #endif
>
> +void __init __weak early_init_dt_add_memory_hyp(u64 base, u64 size)
> +{
> +}
> +
>  void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
>  {
>         const u64 phys_offset = MIN_MEMBLOCK_ADDR;
> @@ -1139,6 +1143,7 @@ void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
>                 base = phys_offset;
>         }
>         memblock_add(base, size);
> +       early_init_dt_add_memory_hyp(base, size);

Can this be done right after we add all the memblocks using the
memblock API? I thought EFI would also need to be handled, but looks
like it just calls early_init_dt_add_memory_arch(). That's odd
especially for ACPI systems...

I don't really like putting what looks like an arm64 only hook here,
but then I don't want an arm64 version of
early_init_dt_add_memory_arch() either. We're almost to the point of
getting rid of the arch specific ones. But I don't have a better
suggestion currently.

Rob
Quentin Perret Nov. 18, 2020, 9:25 a.m. UTC | #2
Hi Rob,

On Tuesday 17 Nov 2020 at 13:44:53 (-0600), Rob Herring wrote:
> Can this be done right after we add all the memblocks using the
> memblock API?

Possibly, but the thing I'm a bit worried about is the way 'no-map'
regions are removed from memblocks early on.

The EL2 object needs to know about these parts of memory too (and in
fact we may be able to enforce the 'no-map' attribute at the host stage
2 level as well). It's also possible we'll need to have portions of the
guests payload preloaded (and verified) by the bootloader into reserved
memory regions, possibly no-map, to make sure the host does not mess
with them in a normal use-case. So, I couldn't find a much better place
than this one but suggestions are very much welcome.

I'll have a go at the memblock stuff to see if I find a way to make it
work from that angle.

> I thought EFI would also need to be handled, but looks
> like it just calls early_init_dt_add_memory_arch(). That's odd
> especially for ACPI systems...
> 
> I don't really like putting what looks like an arm64 only hook here,
> but then I don't want an arm64 version of
> early_init_dt_add_memory_arch() either. We're almost to the point of
> getting rid of the arch specific ones. But I don't have a better
> suggestion currently.

Ack, the ugly truth is that this is likely to remain arm64-specific. I
figured this was simple enough that we might want to consider it,
though.

Thanks,
Quentin
Quentin Perret Nov. 18, 2020, 2:31 p.m. UTC | #3
On Wednesday 18 Nov 2020 at 09:25:47 (+0000), Quentin Perret wrote:
> I'll have a go at the memblock stuff to see if I find a way to make it
> work from that angle.

OK, no luck with the memblock API, but I figured that I can actually
postpone the KVM memory reservation to a later point, after
unflatten_device_tree(), which lets me iterate over the memory nodes
directly rather than having the fdt driver do it for me.

The below seems to boot alright (though I'm not too familiar with
of_address_to_resource() so I may not be using right) and keeps the
whole thing in arch/arm64. Thoughts?

Thanks,
Quentin

---8<---
diff --git a/arch/arm64/kvm/hyp/reserved_mem.c b/arch/arm64/kvm/hyp/reserved_mem.c
index 7da8e2915c1c..cab5ad587a3a 100644
--- a/arch/arm64/kvm/hyp/reserved_mem.c
+++ b/arch/arm64/kvm/hyp/reserved_mem.c
@@ -6,6 +6,7 @@
 
 #include <linux/kvm_host.h>
 #include <linux/memblock.h>
+#include <linux/of_address.h>
 #include <linux/sort.h>
 
 #include <asm/kvm_host.h>
@@ -16,7 +17,7 @@
 phys_addr_t hyp_mem_base;
 phys_addr_t hyp_mem_size;
 
-void __init early_init_dt_add_memory_hyp(u64 base, u64 size)
+static int __init add_hyp_memblock_region(struct resource *rsrc)
 {
 	struct hyp_memblock_region *reg;
 
@@ -24,12 +25,14 @@ void __init early_init_dt_add_memory_hyp(u64 base, u64 size)
 		kvm_nvhe_sym(hyp_memblock_nr) = -1;
 
 	if (kvm_nvhe_sym(hyp_memblock_nr) < 0)
-		return;
+		return -ENOMEM;
 
 	reg = kvm_nvhe_sym(hyp_memory);
-	reg[kvm_nvhe_sym(hyp_memblock_nr)].start = base;
-	reg[kvm_nvhe_sym(hyp_memblock_nr)].end = base + size;
+	reg[kvm_nvhe_sym(hyp_memblock_nr)].start = rsrc->start;
+	reg[kvm_nvhe_sym(hyp_memblock_nr)].end = rsrc->end;
 	kvm_nvhe_sym(hyp_memblock_nr)++;
+
+	return 0;
 }
 
 static int cmp_hyp_memblock(const void *p1, const void *p2)
@@ -52,7 +55,10 @@ void kvm_sort_memblock_regions(void)
 extern bool enable_protected_kvm;
 void __init reserve_kvm_hyp(void)
 {
+	struct device_node *np;
+	struct resource rsrc;
 	u64 nr_pages, prev;
+	int i;
 
 	if (!enable_protected_kvm)
 		return;
@@ -60,8 +66,14 @@ void __init reserve_kvm_hyp(void)
 	if (!is_hyp_mode_available() || is_kernel_in_hyp_mode())
 		return;
 
-	if (kvm_nvhe_sym(hyp_memblock_nr) <= 0)
-		return;
+	for_each_node_by_type(np, "memory") {
+		for (i = 0; !of_address_to_resource(np, i, &rsrc); i++) {
+			if (!add_hyp_memblock_region(&rsrc))
+				continue;
+			kvm_err("Failed to add hyp memblock\n");
+			return;
+		}
+	}
 
 	hyp_mem_size += num_possible_cpus() << PAGE_SHIFT;
 	hyp_mem_size += hyp_s1_pgtable_size();
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index f81da019b677..114f788a4da4 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -391,7 +391,6 @@ void __init arm64_memblock_init(void)
 
 	reserve_elfcorehdr();
 
-	reserve_kvm_hyp();
 
 	high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
 
@@ -423,6 +422,8 @@ void __init bootmem_init(void)
 
 	dma_pernuma_cma_reserve();
 
+	reserve_kvm_hyp();
+
 	/*
 	 * sparse_init() tries to allocate memory from memblock, so must be
 	 * done after the fixed reservations
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index af2b5a09c5b4..4602e467ca8b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1099,10 +1099,6 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 #define MAX_MEMBLOCK_ADDR	((phys_addr_t)~0)
 #endif
 
-void __init __weak early_init_dt_add_memory_hyp(u64 base, u64 size)
-{
-}
-
 void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
 {
 	const u64 phys_offset = MIN_MEMBLOCK_ADDR;
@@ -1143,7 +1139,6 @@ void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
 		base = phys_offset;
 	}
 	memblock_add(base, size);
-	early_init_dt_add_memory_hyp(base, size);
 }
 
 int __init __weak early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size)
diff mbox series

Patch

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 4602e467ca8b..af2b5a09c5b4 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1099,6 +1099,10 @@  int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 #define MAX_MEMBLOCK_ADDR	((phys_addr_t)~0)
 #endif
 
+void __init __weak early_init_dt_add_memory_hyp(u64 base, u64 size)
+{
+}
+
 void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
 {
 	const u64 phys_offset = MIN_MEMBLOCK_ADDR;
@@ -1139,6 +1143,7 @@  void __init __weak early_init_dt_add_memory_arch(u64 base, u64 size)
 		base = phys_offset;
 	}
 	memblock_add(base, size);
+	early_init_dt_add_memory_hyp(base, size);
 }
 
 int __init __weak early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size)