diff mbox series

[v5,11/13] Revert "xen/arm: Remove unused BOOT_RELOC_VIRT_START"

Message ID 20240102095138.17933-12-carlo.nonato@minervasys.tech (mailing list archive)
State Superseded
Headers show
Series Arm cache coloring | expand

Commit Message

Carlo Nonato Jan. 2, 2024, 9:51 a.m. UTC 
This reverts commit 0c18fb76323bfb13615b6f13c98767face2d8097 (not clean).

This is not a clean revert since the rework of the memory layout, but it is
sufficiently similar to a clean one.
The only difference is that the BOOT_RELOC_VIRT_START must match the new
layout.

Cache coloring support for Xen needs to relocate Xen code and data in a new
colored physical space. The BOOT_RELOC_VIRT_START will be used as the virtual
base address for a temporary mapping to this new space.

Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech>
---
 xen/arch/arm/include/asm/mmu/layout.h | 2 ++
 xen/arch/arm/mmu/setup.c              | 1 +
 2 files changed, 3 insertions(+)

Comments

Julien Grall Jan. 5, 2024, 6:20 p.m. UTC | #1 
Hi Carlo,

On 02/01/2024 09:51, Carlo Nonato wrote:
> This reverts commit 0c18fb76323bfb13615b6f13c98767face2d8097 (not clean).
> 
> This is not a clean revert since the rework of the memory layout, but it is
> sufficiently similar to a clean one.
> The only difference is that the BOOT_RELOC_VIRT_START must match the new
> layout.
> 
> Cache coloring support for Xen needs to relocate Xen code and data in a new
> colored physical space. The BOOT_RELOC_VIRT_START will be used as the virtual
> base address for a temporary mapping to this new space.
> 
> Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
> Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech>
> ---
>   xen/arch/arm/include/asm/mmu/layout.h | 2 ++
>   xen/arch/arm/mmu/setup.c              | 1 +
>   2 files changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/mmu/layout.h b/xen/arch/arm/include/asm/mmu/layout.h
> index eac7eef885..30031f74d9 100644
> --- a/xen/arch/arm/include/asm/mmu/layout.h
> +++ b/xen/arch/arm/include/asm/mmu/layout.h
> @@ -74,6 +74,8 @@
>   #define BOOT_FDT_VIRT_START     (FIXMAP_VIRT_START + FIXMAP_VIRT_SIZE)
>   #define BOOT_FDT_VIRT_SIZE      _AT(vaddr_t, MB(4))
>   
> +#define BOOT_RELOC_VIRT_START   (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)

This new addition wants to be documented in the layout comment in a few 
lines above. Also, the area you are using is 2MB whereas Xen can now be 
up to 8MB.

Secondly, you want to add a BOOTRELOC_VIRT_SIZE with a check in 
build_assertions() making sure that this is at least as big as 
XEN_VIRT_SIZE.

Overall, I am not sure this is really a revert at this point. The idea 
is the same, but you are defining BOOT_FDT_VIRT_START differently.

To me it feels like it belong to the first patch where you will use it. 
And that would be ok to mention in the commit message that the idea was 
borrowed from a previously reverted commit.

> +
>   #ifdef CONFIG_LIVEPATCH
>   #define LIVEPATCH_VMAP_START    (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
>   #define LIVEPATCH_VMAP_SIZE    _AT(vaddr_t, MB(2))
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index d5264e51bc..37b6d230ad 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -69,6 +69,7 @@ static void __init __maybe_unused build_assertions(void)
>       /* 2MB aligned regions */
>       BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
>       BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
> +    BUILD_BUG_ON(BOOT_RELOC_VIRT_START & ~SECOND_MASK);
>       /* 1GB aligned regions */
>   #ifdef CONFIG_ARM_32
>       BUILD_BUG_ON(XENHEAP_VIRT_START & ~FIRST_MASK);
Carlo Nonato Jan. 8, 2024, 11:28 a.m. UTC | #2 
Hi Julien,

On Fri, Jan 5, 2024 at 7:20 PM Julien Grall <julien@xen.org> wrote:
>
> Hi Carlo,
>
> On 02/01/2024 09:51, Carlo Nonato wrote:
> > This reverts commit 0c18fb76323bfb13615b6f13c98767face2d8097 (not clean).
> >
> > This is not a clean revert since the rework of the memory layout, but it is
> > sufficiently similar to a clean one.
> > The only difference is that the BOOT_RELOC_VIRT_START must match the new
> > layout.
> >
> > Cache coloring support for Xen needs to relocate Xen code and data in a new
> > colored physical space. The BOOT_RELOC_VIRT_START will be used as the virtual
> > base address for a temporary mapping to this new space.
> >
> > Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
> > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech>
> > ---
> >   xen/arch/arm/include/asm/mmu/layout.h | 2 ++
> >   xen/arch/arm/mmu/setup.c              | 1 +
> >   2 files changed, 3 insertions(+)
> >
> > diff --git a/xen/arch/arm/include/asm/mmu/layout.h b/xen/arch/arm/include/asm/mmu/layout.h
> > index eac7eef885..30031f74d9 100644
> > --- a/xen/arch/arm/include/asm/mmu/layout.h
> > +++ b/xen/arch/arm/include/asm/mmu/layout.h
> > @@ -74,6 +74,8 @@
> >   #define BOOT_FDT_VIRT_START     (FIXMAP_VIRT_START + FIXMAP_VIRT_SIZE)
> >   #define BOOT_FDT_VIRT_SIZE      _AT(vaddr_t, MB(4))
> >
> > +#define BOOT_RELOC_VIRT_START   (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
>
> This new addition wants to be documented in the layout comment in a few
> lines above. Also, the area you are using is 2MB whereas Xen can now be
> up to 8MB.
>
> Secondly, you want to add a BOOTRELOC_VIRT_SIZE with a check in
> build_assertions() making sure that this is at least as big as
> XEN_VIRT_SIZE.
>
> Overall, I am not sure this is really a revert at this point. The idea
> is the same, but you are defining BOOT_FDT_VIRT_START differently.
>
> To me it feels like it belong to the first patch where you will use it.
> And that would be ok to mention in the commit message that the idea was
> borrowed from a previously reverted commit.

Ok, nice.

> > +
> >   #ifdef CONFIG_LIVEPATCH
> >   #define LIVEPATCH_VMAP_START    (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
> >   #define LIVEPATCH_VMAP_SIZE    _AT(vaddr_t, MB(2))
> > diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> > index d5264e51bc..37b6d230ad 100644
> > --- a/xen/arch/arm/mmu/setup.c
> > +++ b/xen/arch/arm/mmu/setup.c
> > @@ -69,6 +69,7 @@ static void __init __maybe_unused build_assertions(void)
> >       /* 2MB aligned regions */
> >       BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
> >       BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
> > +    BUILD_BUG_ON(BOOT_RELOC_VIRT_START & ~SECOND_MASK);
> >       /* 1GB aligned regions */
> >   #ifdef CONFIG_ARM_32
> >       BUILD_BUG_ON(XENHEAP_VIRT_START & ~FIRST_MASK);
>
> --
> Julien Grall

Thanks.
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/mmu/layout.h b/xen/arch/arm/include/asm/mmu/layout.h
index eac7eef885..30031f74d9 100644
--- a/xen/arch/arm/include/asm/mmu/layout.h
+++ b/xen/arch/arm/include/asm/mmu/layout.h
@@ -74,6 +74,8 @@ 
 #define BOOT_FDT_VIRT_START     (FIXMAP_VIRT_START + FIXMAP_VIRT_SIZE)
 #define BOOT_FDT_VIRT_SIZE      _AT(vaddr_t, MB(4))
 
+#define BOOT_RELOC_VIRT_START   (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
+
 #ifdef CONFIG_LIVEPATCH
 #define LIVEPATCH_VMAP_START    (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
 #define LIVEPATCH_VMAP_SIZE    _AT(vaddr_t, MB(2))
diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
index d5264e51bc..37b6d230ad 100644
--- a/xen/arch/arm/mmu/setup.c
+++ b/xen/arch/arm/mmu/setup.c
@@ -69,6 +69,7 @@  static void __init __maybe_unused build_assertions(void)
     /* 2MB aligned regions */
     BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
     BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
+    BUILD_BUG_ON(BOOT_RELOC_VIRT_START & ~SECOND_MASK);
     /* 1GB aligned regions */
 #ifdef CONFIG_ARM_32
     BUILD_BUG_ON(XENHEAP_VIRT_START & ~FIRST_MASK);