| Message ID | 0802fad2743526da4fe49f0225e14161464f192e.1690582001.git.sanastasio@raptorengineering.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/ppc: Add early Radix MMU support | expand |
On 29.07.2023 00:21, Shawn Anastasio wrote: > Introduce a small assembly loop in `start` to copy the kernel to > physical address 0 before continuing. This ensures that the physical > address lines up with XEN_VIRT_START (0xc000000000000000) and allows us > to identity map the kernel when the MMU is set up in the next patch. So PPC guarantees there's always a reasonable amount of memory at 0, and that's available for use? > --- a/xen/arch/ppc/ppc64/head.S > +++ b/xen/arch/ppc/ppc64/head.S > @@ -18,6 +18,33 @@ ENTRY(start) > addis %r2, %r12, .TOC.-1b@ha > addi %r2, %r2, .TOC.-1b@l > > + /* > + * Copy Xen to physical address zero and jump to XEN_VIRT_START > + * (0xc000000000000000). This works because the hardware will ignore the top > + * four address bits when the MMU is off. > + */ > + LOAD_REG_ADDR(%r1, start) I think you really mean _start here (which is missing from the linker script), not start. See also Andrew's recent related RISC-V change. > + LOAD_IMM64(%r12, XEN_VIRT_START) > + > + /* If we're at the correct address, skip copy */ > + cmpld %r1, %r12 > + beq .L_correct_address Can this ever be the case, especially with the MMU-off behavior you describe in the comment above? Wouldn't you need to ignore the top four bits in the comparison? > + /* Copy bytes until _end */ > + LOAD_REG_ADDR(%r11, _end) > + addi %r1, %r1, -8 > + li %r13, -8 > +.L_copy_xen: > + ldu %r10, 8(%r1) > + stdu %r10, 8(%r13) > + cmpld %r1, %r11 > + blt .L_copy_xen > + > + /* Jump to XEN_VIRT_START */ > + mtctr %r12 > + bctr > +.L_correct_address: Can the two regions potentially overlap? Looking at the ELF header it's not clear to me what guarantees there are that this can't happen. Jan
On 7/31/23 10:46 AM, Jan Beulich wrote: > On 29.07.2023 00:21, Shawn Anastasio wrote: >> Introduce a small assembly loop in `start` to copy the kernel to >> physical address 0 before continuing. This ensures that the physical >> address lines up with XEN_VIRT_START (0xc000000000000000) and allows us >> to identity map the kernel when the MMU is set up in the next patch. > > So PPC guarantees there's always a reasonable amount of memory at 0, > and that's available for use? Both Linux and FreeBSD rely on this being the case, so it's essentially a de facto standard, though I'm not aware of any specification that guarantees it. >> --- a/xen/arch/ppc/ppc64/head.S >> +++ b/xen/arch/ppc/ppc64/head.S >> @@ -18,6 +18,33 @@ ENTRY(start) >> addis %r2, %r12, .TOC.-1b@ha >> addi %r2, %r2, .TOC.-1b@l >> >> + /* >> + * Copy Xen to physical address zero and jump to XEN_VIRT_START >> + * (0xc000000000000000). This works because the hardware will ignore the top >> + * four address bits when the MMU is off. >> + */ >> + LOAD_REG_ADDR(%r1, start) > > I think you really mean _start here (which is missing from the linker > script), The PIC patch series fixes the missing _start definition in the linker script. In the cover letter of v2 I'll add a clear note that this series is based on that one. > not start. See also Andrew's recent related RISC-V change. Good point. In practice this worked because the `start` function was the first thing in the first section of the linker script, but of course using _start here is more correct. > >> + LOAD_IMM64(%r12, XEN_VIRT_START) >> + >> + /* If we're at the correct address, skip copy */ >> + cmpld %r1, %r12 >> + beq .L_correct_address > > Can this ever be the case, especially with the MMU-off behavior you > describe in the comment above? Wouldn't you need to ignore the top > four bits in the comparison? It will always be the case after the code jumps to XEN_VIRT_START after the copy takes place. I could have it jump past the copy loop entirely, but then I'd need to duplicate the TOC setup. >> + /* Copy bytes until _end */ >> + LOAD_REG_ADDR(%r11, _end) >> + addi %r1, %r1, -8 >> + li %r13, -8 >> +.L_copy_xen: >> + ldu %r10, 8(%r1) >> + stdu %r10, 8(%r13) >> + cmpld %r1, %r11 >> + blt .L_copy_xen >> + >> + /* Jump to XEN_VIRT_START */ >> + mtctr %r12 >> + bctr >> +.L_correct_address: > > Can the two regions potentially overlap? Looking at the ELF header > it's not clear to me what guarantees there are that this can't > happen. As I understand it, any bootloader that placed the kernel at a low enough address for this to be an issue wouldn't be able to boot Linux or FreeBSD, so in practice it's a safe bet that this won't be the case. > Jan Thanks, Shawn
On 01.08.2023 01:37, Shawn Anastasio wrote: > On 7/31/23 10:46 AM, Jan Beulich wrote: >> On 29.07.2023 00:21, Shawn Anastasio wrote: >>> + /* If we're at the correct address, skip copy */ >>> + cmpld %r1, %r12 >>> + beq .L_correct_address >> >> Can this ever be the case, especially with the MMU-off behavior you >> describe in the comment above? Wouldn't you need to ignore the top >> four bits in the comparison? > > It will always be the case after the code jumps to XEN_VIRT_START after > the copy takes place. Well, of course. > I could have it jump past the copy loop entirely, > but then I'd need to duplicate the TOC setup. I don't think I understand this part of your reply: .L_correct_address _is_ past the copy loop. >>> + /* Copy bytes until _end */ >>> + LOAD_REG_ADDR(%r11, _end) >>> + addi %r1, %r1, -8 >>> + li %r13, -8 >>> +.L_copy_xen: >>> + ldu %r10, 8(%r1) >>> + stdu %r10, 8(%r13) >>> + cmpld %r1, %r11 >>> + blt .L_copy_xen >>> + >>> + /* Jump to XEN_VIRT_START */ >>> + mtctr %r12 >>> + bctr >>> +.L_correct_address: >> >> Can the two regions potentially overlap? Looking at the ELF header >> it's not clear to me what guarantees there are that this can't >> happen. > > As I understand it, any bootloader that placed the kernel at a low > enough address for this to be an issue wouldn't be able to boot Linux or > FreeBSD, so in practice it's a safe bet that this won't be the case. Fair enough then. Jan
On 8/1/23 1:08 AM, Jan Beulich wrote: > On 01.08.2023 01:37, Shawn Anastasio wrote: >> On 7/31/23 10:46 AM, Jan Beulich wrote: >>> On 29.07.2023 00:21, Shawn Anastasio wrote: >>>> + /* If we're at the correct address, skip copy */ >>>> + cmpld %r1, %r12 >>>> + beq .L_correct_address >>> >>> Can this ever be the case, especially with the MMU-off behavior you >>> describe in the comment above? Wouldn't you need to ignore the top >>> four bits in the comparison? >> >> It will always be the case after the code jumps to XEN_VIRT_START after >> the copy takes place. > > Well, of course. > >> I could have it jump past the copy loop entirely, >> but then I'd need to duplicate the TOC setup. > > I don't think I understand this part of your reply: .L_correct_address > _is_ past the copy loop. Sorry, let me elaborate. I meant that I could have the end of the copy loop (the mtctr + btctr preceeding .L_correct_address) jump to (XEN_VIRT_START + .L_correct_address) as opposed to XEN_VIRT_START so that the address comparison you originally commented on wouldn't be hit again. This would mean adding another TOC setup block at .L_correct_address, though, since we'd be skipping over the one at the beginning of the routine and the TOC needs to be reconfigured after the relocation. >>>> + /* Copy bytes until _end */ >>>> + LOAD_REG_ADDR(%r11, _end) >>>> + addi %r1, %r1, -8 >>>> + li %r13, -8 >>>> +.L_copy_xen: >>>> + ldu %r10, 8(%r1) >>>> + stdu %r10, 8(%r13) >>>> + cmpld %r1, %r11 >>>> + blt .L_copy_xen >>>> + >>>> + /* Jump to XEN_VIRT_START */ >>>> + mtctr %r12 >>>> + bctr >>>> +.L_correct_address: >>> >>> Can the two regions potentially overlap? Looking at the ELF header >>> it's not clear to me what guarantees there are that this can't >>> happen. >> >> As I understand it, any bootloader that placed the kernel at a low >> enough address for this to be an issue wouldn't be able to boot Linux or >> FreeBSD, so in practice it's a safe bet that this won't be the case. > > Fair enough then. > > Jan Thanks, Shawn
diff --git a/xen/arch/ppc/ppc64/head.S b/xen/arch/ppc/ppc64/head.S index 5ac2dad2ee..beff8257fa 100644 --- a/xen/arch/ppc/ppc64/head.S +++ b/xen/arch/ppc/ppc64/head.S @@ -18,6 +18,33 @@ ENTRY(start) addis %r2, %r12, .TOC.-1b@ha addi %r2, %r2, .TOC.-1b@l + /* + * Copy Xen to physical address zero and jump to XEN_VIRT_START + * (0xc000000000000000). This works because the hardware will ignore the top + * four address bits when the MMU is off. + */ + LOAD_REG_ADDR(%r1, start) + LOAD_IMM64(%r12, XEN_VIRT_START) + + /* If we're at the correct address, skip copy */ + cmpld %r1, %r12 + beq .L_correct_address + + /* Copy bytes until _end */ + LOAD_REG_ADDR(%r11, _end) + addi %r1, %r1, -8 + li %r13, -8 +.L_copy_xen: + ldu %r10, 8(%r1) + stdu %r10, 8(%r13) + cmpld %r1, %r11 + blt .L_copy_xen + + /* Jump to XEN_VIRT_START */ + mtctr %r12 + bctr +.L_correct_address: + /* set up the initial stack */ LOAD_REG_ADDR(%r1, cpu0_boot_stack) li %r11, 0
Introduce a small assembly loop in `start` to copy the kernel to physical address 0 before continuing. This ensures that the physical address lines up with XEN_VIRT_START (0xc000000000000000) and allows us to identity map the kernel when the MMU is set up in the next patch. We are also able to start execution at XEN_VIRT_START after the copy since hardware will ignore the top 4 address bits when operating in Real Mode (MMU off). Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> --- xen/arch/ppc/ppc64/head.S | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)