| Message ID | 4ad639bed6b8aa6cee34288a7ce154db21d761b5.1686080337.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/riscv: introduce identity mapping | expand |
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 > >
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
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
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")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/xen.lds.S | 9 +++++++++ 1 file changed, 9 insertions(+)