1. Patchew
  2. Xen
  3. [PATCH v1 0/8] xen/riscv: introduce identity mapping
  4. View patch

[PATCH v1 1/8] xen/riscv: make sure that identity mapping isn't bigger then page size

Oleksii Kurochko posted 8 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH v1 1/8] xen/riscv: make sure that identity mapping isn't bigger then page size
Posted by Oleksii Kurochko 2 years, 8 months ago 
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/xen.lds.S | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 878130f313..74afbaab9b 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -20,6 +20,7 @@ SECTIONS
     . = XEN_VIRT_START;
     _start = .;
     .text : {
+        _idmap_start = .;
         _stext = .;            /* Text section */
         *(.text.header)
 
@@ -35,6 +36,7 @@ SECTIONS
         *(.gnu.warning)
         . = ALIGN(POINTER_ALIGN);
         _etext = .;             /* End of text section */
+        _idmap_end = .;
     } :text
 
     . = ALIGN(PAGE_SIZE);
@@ -174,3 +176,10 @@ ASSERT(!SIZEOF(.got),      ".got non-empty")
 ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
 
 ASSERT(_end - _start <= MB(2), "Xen too large for early-boot assumptions")
+
+/*
+ * We require that Xen is loaded at a page boundary, so this ensures that any
+ * code running on the identity map cannot cross a page boundary.
+ */
+ASSERT(IS_ALIGNED(_idmap_start, PAGE_SIZE), "_idmap_start should be page-aligned")
+ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is larger than a page size")
-- 
2.40.1
Re: [PATCH v1 1/8] xen/riscv: make sure that identity mapping isn't bigger then page size
Posted by Jan Beulich 2 years, 8 months ago 
On 06.06.2023 21:55, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Such commits without description are worrying. This may be okay for
entirely trivial and obvious changes, but that's going to be the
exception.

> --- a/xen/arch/riscv/xen.lds.S
> +++ b/xen/arch/riscv/xen.lds.S
> @@ -20,6 +20,7 @@ SECTIONS
>      . = XEN_VIRT_START;
>      _start = .;
>      .text : {
> +        _idmap_start = .;
>          _stext = .;            /* Text section */
>          *(.text.header)
>  
> @@ -35,6 +36,7 @@ SECTIONS
>          *(.gnu.warning)
>          . = ALIGN(POINTER_ALIGN);
>          _etext = .;             /* End of text section */
> +        _idmap_end = .;
>      } :text

So this covers all of .text. Why is it expected that .text will be (and
remain) ...

> @@ -174,3 +176,10 @@ ASSERT(!SIZEOF(.got),      ".got non-empty")
>  ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
>  
>  ASSERT(_end - _start <= MB(2), "Xen too large for early-boot assumptions")
> +
> +/*
> + * We require that Xen is loaded at a page boundary, so this ensures that any
> + * code running on the identity map cannot cross a page boundary.
> + */
> +ASSERT(IS_ALIGNED(_idmap_start, PAGE_SIZE), "_idmap_start should be page-aligned")
> +ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is larger than a page size")

... less than 4k in size? And why is only .text of interest, but not
other sections?

I find the other assertion a little puzzling too: Isn't that merely
checking that XEN_VIRT_START is page aligned?

Jan
Re: [PATCH v1 1/8] xen/riscv: make sure that identity mapping isn't bigger then page size
Posted by Oleksii 2 years, 8 months ago 
On Mon, 2023-06-12 at 09:09 +0200, Jan Beulich wrote:
> On 06.06.2023 21:55, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Such commits without description are worrying. This may be okay for
> entirely trivial and obvious changes, but that's going to be the
> exception.
> 
> > --- a/xen/arch/riscv/xen.lds.S
> > +++ b/xen/arch/riscv/xen.lds.S
> > @@ -20,6 +20,7 @@ SECTIONS
> >      . = XEN_VIRT_START;
> >      _start = .;
> >      .text : {
> > +        _idmap_start = .;
> >          _stext = .;            /* Text section */
> >          *(.text.header)
> >  
> > @@ -35,6 +36,7 @@ SECTIONS
> >          *(.gnu.warning)
> >          . = ALIGN(POINTER_ALIGN);
> >          _etext = .;             /* End of text section */
> > +        _idmap_end = .;
> >      } :text
> 
> So this covers all of .text. Why is it expected that .text will be
> (and
> remain) ...
> 
> > @@ -174,3 +176,10 @@ ASSERT(!SIZEOF(.got),      ".got non-empty")
> >  ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
> >  
> >  ASSERT(_end - _start <= MB(2), "Xen too large for early-boot
> > assumptions")
> > +
> > +/*
> > + * We require that Xen is loaded at a page boundary, so this
> > ensures that any
> > + * code running on the identity map cannot cross a page boundary.
> > + */
> > +ASSERT(IS_ALIGNED(_idmap_start, PAGE_SIZE), "_idmap_start should
> > be page-aligned")
> > +ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped
> > code is larger than a page size")
> 
> ... less than 4k in size? And why is only .text of interest, but not
> other sections?
An idea was to keep identity mapping as small as possible because
basically identity mapping is needed only for a few instructions.
(probably it will be better to create a separate section and put all
necessary functions there)

Another point was to map the necessary code for switching from 1:1
mapping in one cycle. ( we are using 4K as a page size )

But it looks like PAGE_SIZE isn't enough. I rebased all my patches that
are needed to run Dom0 and compiler complains that _idmap is bigger
than PAGE_SIZE so I probably have to reject this idea ( to map only
PAGE_SIZE ).

Actually not only .text section is needed but also stack should be 1:1
mapped. ( what is done in setup_initial_pagetables() )
> 
> I find the other assertion a little puzzling too: Isn't that merely
> checking that XEN_VIRT_START is page aligned?
Yeah, you are right.

~ Oleksii
Re: [PATCH v1 1/8] xen/riscv: make sure that identity mapping isn't bigger then page size
Posted by Alistair Francis 2 years, 8 months ago 
On Wed, Jun 7, 2023 at 5:55 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  xen/arch/riscv/xen.lds.S | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
> index 878130f313..74afbaab9b 100644
> --- a/xen/arch/riscv/xen.lds.S
> +++ b/xen/arch/riscv/xen.lds.S
> @@ -20,6 +20,7 @@ SECTIONS
>      . = XEN_VIRT_START;
>      _start = .;
>      .text : {
> +        _idmap_start = .;
>          _stext = .;            /* Text section */
>          *(.text.header)
>
> @@ -35,6 +36,7 @@ SECTIONS
>          *(.gnu.warning)
>          . = ALIGN(POINTER_ALIGN);
>          _etext = .;             /* End of text section */
> +        _idmap_end = .;
>      } :text
>
>      . = ALIGN(PAGE_SIZE);
> @@ -174,3 +176,10 @@ ASSERT(!SIZEOF(.got),      ".got non-empty")
>  ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
>
>  ASSERT(_end - _start <= MB(2), "Xen too large for early-boot assumptions")
> +
> +/*
> + * We require that Xen is loaded at a page boundary, so this ensures that any
> + * code running on the identity map cannot cross a page boundary.
> + */
> +ASSERT(IS_ALIGNED(_idmap_start, PAGE_SIZE), "_idmap_start should be page-aligned")
> +ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity mapped code is larger than a page size")
> --
> 2.40.1
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.