Re: [Xen-devel] [PATCH 1/2] xen/link: Introduce .bss.percpu.page_aligned

[Jan Beulich <JBeulich@xxxxxxxx>]

On 26.07.2019 16:30, Andrew Cooper wrote:
> On 26/07/2019 15:22, Roger Pau Monné wrote:
>> On Fri, Jul 26, 2019 at 02:52:39PM +0100, Andrew Cooper wrote:
>>> Future changes are going to need to page align some percpu data.
>>>
>>> This means that the percpu area needs suiably aligning in the BSS so CPU0 
>>> has
>>> correctly aligned data.  Shuffle the exact link order of items within the 
>>> BSS
>>> to give .bss.percpu.page_aligned appropriate alignment.
>>>
>>> Additionally, introduce DEFINE_PER_CPU_PAGE_ALIGNED()
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: Wei Liu <wl@xxxxxxx>
>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> CC: Julien Grall <julien.grall@xxxxxxx>
>>> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
>>> ---
>>>   xen/arch/arm/xen.lds.S   | 5 +++--
>>>   xen/arch/x86/xen.lds.S   | 5 +++--
>>>   xen/include/xen/percpu.h | 2 ++
>>>   3 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>>> index 12c107f45d..07cbdf2543 100644
>>> --- a/xen/arch/arm/xen.lds.S
>>> +++ b/xen/arch/arm/xen.lds.S
>>> @@ -201,14 +201,15 @@ SECTIONS
>>>          *(.bss.stack_aligned)
>>>          . = ALIGN(PAGE_SIZE);
>>>          *(.bss.page_aligned)
>>> -       *(.bss)
>>> -       . = ALIGN(SMP_CACHE_BYTES);
>> Don't you also need a:
>>
>> . = ALIGN(PAGE_SIZE);
>>
>> here?
> 
> No, (I don't think so).
> 
>> Or is the size of .bss.page_aligned also aligned to page size?
> 
> Every object inside .bss.page_aligned should have suitable (i.e.
> multiple of) size and alignment.  Without this, things will break.

I'm not sure we should have such a requirement: Objects in
.*.page_aligned sections should themselves have PAGE_SIZE alignment
(i.e. there shouldn't be a need to ALIGN() _ahead_ of the section
directive in the script (that is, the one in context above should
actually be redundant). But I'm not sure about demanding their
size to be a multiple of PAGE_SIZE - while C will guarantee this
(and waste space in certain cases), assembly constructs could still
be written such that the trailing unused space can be re-used. Otoh
I agree with your virtual statement of this being a little fragile.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel