diff mbox series

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

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

Commit Message

Quentin Perret Jan. 8, 2021, 12: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 Jan. 11, 2021, 2:45 p.m. UTC | #1
On Fri, Jan 8, 2021 at 6:16 AM 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.


What happened to doing this with memblock?

> 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);

>  }

>

>  int __init __weak early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size)

> --

> 2.30.0.284.gd98b1dd5eaa7-goog

>
Quentin Perret Jan. 12, 2021, 9:51 a.m. UTC | #2
On Monday 11 Jan 2021 at 08:45:10 (-0600), Rob Herring wrote:
> On Fri, Jan 8, 2021 at 6:16 AM 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.

> 

> What happened to doing this with memblock?


I gave it a go, but as mentioned in v1, I ran into issues for nomap
regions. I want the hypervisor to know about these memory regions (it's
possible some of those will be given to protected guests for instance)
but these seem to be entirely removed from the memblocks when using DT:

https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1153

EFI appears to do things differently, though, as it 'just' uses
memblock_mark_nomap() instead of actively removing the memblock. And that
means I could actually use the memblock API for EFI, but I'd rather
have a common solution. I tried to understand why things are done
differently but couldn't find an answer and kept things simple and
working for now.

Is there a good reason for not using memblock_mark_nomap() with DT? If
not, I'm happy to try that.

Thanks,
Quentin
Rob Herring Jan. 12, 2021, 2:10 p.m. UTC | #3
On Tue, Jan 12, 2021 at 3:51 AM Quentin Perret <qperret@google.com> wrote:
>

> On Monday 11 Jan 2021 at 08:45:10 (-0600), Rob Herring wrote:

> > On Fri, Jan 8, 2021 at 6:16 AM 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.

> >

> > What happened to doing this with memblock?

>

> I gave it a go, but as mentioned in v1, I ran into issues for nomap

> regions. I want the hypervisor to know about these memory regions (it's

> possible some of those will be given to protected guests for instance)

> but these seem to be entirely removed from the memblocks when using DT:

>

> https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1153

>

> EFI appears to do things differently, though, as it 'just' uses

> memblock_mark_nomap() instead of actively removing the memblock. And that

> means I could actually use the memblock API for EFI, but I'd rather

> have a common solution. I tried to understand why things are done

> differently but couldn't find an answer and kept things simple and

> working for now.

>

> Is there a good reason for not using memblock_mark_nomap() with DT? If

> not, I'm happy to try that.


There were 2 patches to do that, but it never got resolved. See here[1].

Rob

[1] https://lore.kernel.org/linux-devicetree/?q=s%3Ano-map
Quentin Perret Jan. 12, 2021, 2:26 p.m. UTC | #4
On Tuesday 12 Jan 2021 at 08:10:47 (-0600), Rob Herring wrote:
> On Tue, Jan 12, 2021 at 3:51 AM Quentin Perret <qperret@google.com> wrote:

> >

> > On Monday 11 Jan 2021 at 08:45:10 (-0600), Rob Herring wrote:

> > > On Fri, Jan 8, 2021 at 6:16 AM 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.

> > >

> > > What happened to doing this with memblock?

> >

> > I gave it a go, but as mentioned in v1, I ran into issues for nomap

> > regions. I want the hypervisor to know about these memory regions (it's

> > possible some of those will be given to protected guests for instance)

> > but these seem to be entirely removed from the memblocks when using DT:

> >

> > https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1153

> >

> > EFI appears to do things differently, though, as it 'just' uses

> > memblock_mark_nomap() instead of actively removing the memblock. And that

> > means I could actually use the memblock API for EFI, but I'd rather

> > have a common solution. I tried to understand why things are done

> > differently but couldn't find an answer and kept things simple and

> > working for now.

> >

> > Is there a good reason for not using memblock_mark_nomap() with DT? If

> > not, I'm happy to try that.

> 

> There were 2 patches to do that, but it never got resolved. See here[1].


Thanks. So the DT stuff predates the introduction of memblock_mark_nomap,
that's why...

By reading the discussions, [1] still looks a sensible patch on its own,
independently from the issue Nicolas tried to solve. Any reason for not
applying it?

I'll try to rework my series on top and see how that goes.

Thanks,
Quentin

[1] https://lore.kernel.org/linux-devicetree/1562920284-18638-1-git-send-email-karahmed@amazon.de/
Rob Herring Jan. 12, 2021, 3:53 p.m. UTC | #5
On Tue, Jan 12, 2021 at 8:26 AM Quentin Perret <qperret@google.com> wrote:
>

> On Tuesday 12 Jan 2021 at 08:10:47 (-0600), Rob Herring wrote:

> > On Tue, Jan 12, 2021 at 3:51 AM Quentin Perret <qperret@google.com> wrote:

> > >

> > > On Monday 11 Jan 2021 at 08:45:10 (-0600), Rob Herring wrote:

> > > > On Fri, Jan 8, 2021 at 6:16 AM 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.

> > > >

> > > > What happened to doing this with memblock?

> > >

> > > I gave it a go, but as mentioned in v1, I ran into issues for nomap

> > > regions. I want the hypervisor to know about these memory regions (it's

> > > possible some of those will be given to protected guests for instance)

> > > but these seem to be entirely removed from the memblocks when using DT:

> > >

> > > https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1153

> > >

> > > EFI appears to do things differently, though, as it 'just' uses

> > > memblock_mark_nomap() instead of actively removing the memblock. And that

> > > means I could actually use the memblock API for EFI, but I'd rather

> > > have a common solution. I tried to understand why things are done

> > > differently but couldn't find an answer and kept things simple and

> > > working for now.

> > >

> > > Is there a good reason for not using memblock_mark_nomap() with DT? If

> > > not, I'm happy to try that.

> >

> > There were 2 patches to do that, but it never got resolved. See here[1].

>

> Thanks. So the DT stuff predates the introduction of memblock_mark_nomap,

> that's why...

>

> By reading the discussions, [1] still looks a sensible patch on its own,

> independently from the issue Nicolas tried to solve. Any reason for not

> applying it?


As I mentioned in the thread, same patch with 2 different reasons. So
I just wanted a better commit message covering both.

Rob
Quentin Perret Jan. 12, 2021, 4:15 p.m. UTC | #6
On Tuesday 12 Jan 2021 at 09:53:36 (-0600), Rob Herring wrote:
> On Tue, Jan 12, 2021 at 8:26 AM Quentin Perret <qperret@google.com> wrote:

> >

> > On Tuesday 12 Jan 2021 at 08:10:47 (-0600), Rob Herring wrote:

> > > On Tue, Jan 12, 2021 at 3:51 AM Quentin Perret <qperret@google.com> wrote:

> > > >

> > > > On Monday 11 Jan 2021 at 08:45:10 (-0600), Rob Herring wrote:

> > > > > On Fri, Jan 8, 2021 at 6:16 AM 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.

> > > > >

> > > > > What happened to doing this with memblock?

> > > >

> > > > I gave it a go, but as mentioned in v1, I ran into issues for nomap

> > > > regions. I want the hypervisor to know about these memory regions (it's

> > > > possible some of those will be given to protected guests for instance)

> > > > but these seem to be entirely removed from the memblocks when using DT:

> > > >

> > > > https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1153

> > > >

> > > > EFI appears to do things differently, though, as it 'just' uses

> > > > memblock_mark_nomap() instead of actively removing the memblock. And that

> > > > means I could actually use the memblock API for EFI, but I'd rather

> > > > have a common solution. I tried to understand why things are done

> > > > differently but couldn't find an answer and kept things simple and

> > > > working for now.

> > > >

> > > > Is there a good reason for not using memblock_mark_nomap() with DT? If

> > > > not, I'm happy to try that.

> > >

> > > There were 2 patches to do that, but it never got resolved. See here[1].

> >

> > Thanks. So the DT stuff predates the introduction of memblock_mark_nomap,

> > that's why...

> >

> > By reading the discussions, [1] still looks a sensible patch on its own,

> > independently from the issue Nicolas tried to solve. Any reason for not

> > applying it?

> 

> As I mentioned in the thread, same patch with 2 different reasons. So

> I just wanted a better commit message covering both.


Sorry if I'm being thick, but I'm not seeing it. How are they the same?
IIUC, as per Nicolas' last reply, using memblock_mark_nomap() does not
solve his issue with a broken DT. These 2 patches address two completely
separate issues no?

Thanks,
Quentin
Rob Herring Jan. 12, 2021, 4:45 p.m. UTC | #7
On Tue, Jan 12, 2021 at 10:15 AM Quentin Perret <qperret@google.com> wrote:
>

> On Tuesday 12 Jan 2021 at 09:53:36 (-0600), Rob Herring wrote:

> > On Tue, Jan 12, 2021 at 8:26 AM Quentin Perret <qperret@google.com> wrote:

> > >

> > > On Tuesday 12 Jan 2021 at 08:10:47 (-0600), Rob Herring wrote:

> > > > On Tue, Jan 12, 2021 at 3:51 AM Quentin Perret <qperret@google.com> wrote:

> > > > >

> > > > > On Monday 11 Jan 2021 at 08:45:10 (-0600), Rob Herring wrote:

> > > > > > On Fri, Jan 8, 2021 at 6:16 AM 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.

> > > > > >

> > > > > > What happened to doing this with memblock?

> > > > >

> > > > > I gave it a go, but as mentioned in v1, I ran into issues for nomap

> > > > > regions. I want the hypervisor to know about these memory regions (it's

> > > > > possible some of those will be given to protected guests for instance)

> > > > > but these seem to be entirely removed from the memblocks when using DT:

> > > > >

> > > > > https://elixir.bootlin.com/linux/latest/source/drivers/of/fdt.c#L1153

> > > > >

> > > > > EFI appears to do things differently, though, as it 'just' uses

> > > > > memblock_mark_nomap() instead of actively removing the memblock. And that

> > > > > means I could actually use the memblock API for EFI, but I'd rather

> > > > > have a common solution. I tried to understand why things are done

> > > > > differently but couldn't find an answer and kept things simple and

> > > > > working for now.

> > > > >

> > > > > Is there a good reason for not using memblock_mark_nomap() with DT? If

> > > > > not, I'm happy to try that.

> > > >

> > > > There were 2 patches to do that, but it never got resolved. See here[1].

> > >

> > > Thanks. So the DT stuff predates the introduction of memblock_mark_nomap,

> > > that's why...

> > >

> > > By reading the discussions, [1] still looks a sensible patch on its own,

> > > independently from the issue Nicolas tried to solve. Any reason for not

> > > applying it?

> >

> > As I mentioned in the thread, same patch with 2 different reasons. So

> > I just wanted a better commit message covering both.

>

> Sorry if I'm being thick, but I'm not seeing it. How are they the same?

> IIUC, as per Nicolas' last reply, using memblock_mark_nomap() does not

> solve his issue with a broken DT. These 2 patches address two completely

> separate issues no?


Umm, yes you are right. But both are dealing with nomap. So someone
needs to sort out what the right thing to do here is. No one cared
enough to follow up in a year and a half.

Rob
Quentin Perret Jan. 12, 2021, 4:50 p.m. UTC | #8
On Tuesday 12 Jan 2021 at 10:45:56 (-0600), Rob Herring wrote:
> Umm, yes you are right. But both are dealing with nomap. So someone

> needs to sort out what the right thing to do here is. No one cared

> enough to follow up in a year and a half.


Fair enough, happy to do that. I'll send a small series with these two
patches independently from this series which may take a while to land.

Thanks,
Quentin
Quentin Perret Jan. 15, 2021, 11:49 a.m. UTC | #9
On Tuesday 12 Jan 2021 at 16:50:12 (+0000), Quentin Perret wrote:
> On Tuesday 12 Jan 2021 at 10:45:56 (-0600), Rob Herring wrote:

> > Umm, yes you are right. But both are dealing with nomap. So someone

> > needs to sort out what the right thing to do here is. No one cared

> > enough to follow up in a year and a half.

> 

> Fair enough, happy to do that. I'll send a small series with these two

> patches independently from this series which may take a while to land.


Now sent:

https://lore.kernel.org/lkml/20210115114544.1830068-1-qperret@google.com/

Thanks,
Quentin
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)