Re: [Xen-devel] [PATCH] xen/arm64: Don't zero BSS when booting using EFI

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.02.17 at 15:35, <julien.grall@xxxxxxx> wrote:
> On 03/02/17 14:31, Jan Beulich wrote:
>>>>> On 03.02.17 at 15:24, <julien.grall@xxxxxxx> wrote:
>>> On 03/02/17 07:41, Jan Beulich wrote:
>>>>>>> On 02.02.17 at 20:25, <julien.grall@xxxxxxx> wrote:
>>>>> @@ -261,6 +265,8 @@ GLOBAL(init_secondary)
>>>>>          sub   x20, x19, x0           /* x20 := phys-offset */
>>>>>
>>>>>          mov   x22, #1                /* x22 := is_secondary_cpu */
>>>>> +        /* Skip zero BSS on secondary CPUs to avoid nasty surprises. */
>>>>> +        mov   x26, #1                /* X26 := skip_zero_bss */
>>>>>
>>>>>  common_start:
>>>>>          mov   x24, #0                /* x24 := CPU ID. Initialy zero 
>>>>> until we
>>>>> @@ -314,8 +320,8 @@ common_start:
>>>>>
>>>>>  el2:    PRINT("- Xen starting at EL2 -\r\n")
>>>>>
>>>>> -        /* Zero BSS On the boot CPU to avoid nasty surprises */
>>>>> -        cbnz  x22, skip_bss
>>>>> +        /* Zero BSS only when requested to avoid nasty surprises. */
>>>>> +        cbnz  x26, skip_bss
>>>>
>>>> Comparing the original comment here with both this and the
>>>> earlier hunk, I think the intended meaning is lost. Zeroing the
>>>> BSS on secondary CPUs is certainly a bug, not a nasty surprise.
>>>> What I think the original comment is meaning to say is "the
>>>> BSS should have been zeroed already, but let's better not rely
>>>> on that".
>>>
>>> This is not the original meaning.
>>
>> Are you sure the comment wasn't just copied from x86 code?
> 
> Maybe. Regardless that I think the code holds on ARM in both case.
> 
> It is a nasty surprises when zero BSS on secondary CPU or EFI because we 
> loose all the information.
> 
>>> On non-EFI setup BSS will not be
>>> zeroed before hand as the loader of Xen does not know the size of BSS.
>>
>> So I would recommend correcting the comments at once to reflect
>> what they are now supposed to mean.
> 
> IHMO, the comment is valid. See why above.

Well, I disagree, but you're the maintainer, and I merely wanted
to point out a possible lack of clarity.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel