x86/boot: Drop stale comment about zeroing the stack

[PATCH] x86/boot: Drop stale comment about zeroing the stack

Posted by Andrew Cooper 1 year, 3 months ago

This used to be true, but was altered by commit 37786b23b027 ("x86/cet: Remove
writeable mapping of the BSPs shadow stack") which moved cpu0_stack into
.init.bss.stack_aligned.

Fixes: 37786b23b027 ("x86/cet: Remove writeable mapping of the BSPs shadow stack")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/head.S | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 12bbb97f33d4..cfc5a7b47d29 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -652,11 +652,7 @@ trampoline_setup:
         cmpb    $0, sym_esi(efi_platform)
         jnz     1f
 
-        /*
-         * Initialise the BSS.
-         *
-         * !!! WARNING - also zeroes the current stack !!!
-         */
+        /* Initialise the BSS. */
         lea     sym_esi(__bss_start), %edi
         lea     sym_esi(__bss_end), %ecx
         sub     %edi,%ecx

base-commit: a8bf14f6f331d4f428010b4277b67c33f561ed19
-- 
2.39.2

Re: [PATCH] x86/boot: Drop stale comment about zeroing the stack

Posted by Frediano Ziglio 1 year, 2 months ago

On Mon, Sep 16, 2024 at 12:58 PM Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>
> This used to be true, but was altered by commit 37786b23b027 ("x86/cet: Remove
> writeable mapping of the BSPs shadow stack") which moved cpu0_stack into
> .init.bss.stack_aligned.
>
> Fixes: 37786b23b027 ("x86/cet: Remove writeable mapping of the BSPs shadow stack")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
>  xen/arch/x86/boot/head.S | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 12bbb97f33d4..cfc5a7b47d29 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -652,11 +652,7 @@ trampoline_setup:
>          cmpb    $0, sym_esi(efi_platform)
>          jnz     1f
>
> -        /*
> -         * Initialise the BSS.
> -         *
> -         * !!! WARNING - also zeroes the current stack !!!
> -         */
> +        /* Initialise the BSS. */
>          lea     sym_esi(__bss_start), %edi
>          lea     sym_esi(__bss_end), %ecx
>          sub     %edi,%ecx
>
> base-commit: a8bf14f6f331d4f428010b4277b67c33f561ed19

Nice to know!

Acked-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Frediano

Re: [PATCH] x86/boot: Drop stale comment about zeroing the stack

Posted by Jan Beulich 1 year, 2 months ago

On 16.09.2024 15:16, Frediano Ziglio wrote:
> On Mon, Sep 16, 2024 at 12:58 PM Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>>
>> This used to be true, but was altered by commit 37786b23b027 ("x86/cet: Remove
>> writeable mapping of the BSPs shadow stack") which moved cpu0_stack into
>> .init.bss.stack_aligned.
>>
>> Fixes: 37786b23b027 ("x86/cet: Remove writeable mapping of the BSPs shadow stack")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -652,11 +652,7 @@ trampoline_setup:
>>          cmpb    $0, sym_esi(efi_platform)
>>          jnz     1f
>>
>> -        /*
>> -         * Initialise the BSS.
>> -         *
>> -         * !!! WARNING - also zeroes the current stack !!!
>> -         */
>> +        /* Initialise the BSS. */
>>          lea     sym_esi(__bss_start), %edi
>>          lea     sym_esi(__bss_end), %ecx
>>          sub     %edi,%ecx
>>
>> base-commit: a8bf14f6f331d4f428010b4277b67c33f561ed19
> 
> Nice to know!
> 
> Acked-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Frediano, just FYI on the formal aspects here (which we don't always follow to
the letter, but still): If you had offered Reviewed-by:, Andrew would have been
fine to commit with just that. You can't just ack anything you're not
maintainer for (or really: you can, but formally it has no meaning).

Jan

Re: [PATCH] x86/boot: Drop stale comment about zeroing the stack

Posted by Frediano Ziglio 1 year, 2 months ago

On Mon, Sep 23, 2024 at 12:32 PM Jan Beulich <jbeulich@suse.com> wrote:
>
...
> >
> > Acked-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>
> Frediano, just FYI on the formal aspects here (which we don't always follow to
> the letter, but still): If you had offered Reviewed-by:, Andrew would have been
> fine to commit with just that. You can't just ack anything you're not
> maintainer for (or really: you can, but formally it has no meaning).
>
> Jan

It makes sense, thanks

Frediano

Re: [PATCH] x86/boot: Drop stale comment about zeroing the stack

Posted by Andrew Cooper 1 year, 2 months ago

On 16/09/2024 2:16 pm, Frediano Ziglio wrote:
> On Mon, Sep 16, 2024 at 12:58 PM Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>> This used to be true, but was altered by commit 37786b23b027 ("x86/cet: Remove
>> writeable mapping of the BSPs shadow stack") which moved cpu0_stack into
>> .init.bss.stack_aligned.
>>
>> Fixes: 37786b23b027 ("x86/cet: Remove writeable mapping of the BSPs shadow stack")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Frediano Ziglio <frediano.ziglio@cloud.com>
>> ---
>>  xen/arch/x86/boot/head.S | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
>> index 12bbb97f33d4..cfc5a7b47d29 100644
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -652,11 +652,7 @@ trampoline_setup:
>>          cmpb    $0, sym_esi(efi_platform)
>>          jnz     1f
>>
>> -        /*
>> -         * Initialise the BSS.
>> -         *
>> -         * !!! WARNING - also zeroes the current stack !!!
>> -         */
>> +        /* Initialise the BSS. */
>>          lea     sym_esi(__bss_start), %edi
>>          lea     sym_esi(__bss_end), %ecx
>>          sub     %edi,%ecx
>>
>> base-commit: a8bf14f6f331d4f428010b4277b67c33f561ed19
> Nice to know!

Indeed, and it certainly helps one of your rearranging patches.

> Acked-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Thanks.

~Andrew