[PATCH 2/3] xen/ppc: Relocate kernel to physical address 0 on boot

Shawn Anastasio posted 3 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH 2/3] xen/ppc: Relocate kernel to physical address 0 on boot
Posted by Shawn Anastasio 2 years, 6 months ago
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(+)

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
-- 
2.30.2
Re: [PATCH 2/3] xen/ppc: Relocate kernel to physical address 0 on boot
Posted by Jan Beulich 2 years, 6 months ago
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
Re: [PATCH 2/3] xen/ppc: Relocate kernel to physical address 0 on boot
Posted by Shawn Anastasio 2 years, 6 months ago
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
Re: [PATCH 2/3] xen/ppc: Relocate kernel to physical address 0 on boot
Posted by Jan Beulich 2 years, 6 months ago
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
Re: [PATCH 2/3] xen/ppc: Relocate kernel to physical address 0 on boot
Posted by Shawn Anastasio 2 years, 6 months ago
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