Re: [Xen-devel] [PATCH 4/6] x86/boot: Clean up l?_bootmap[] construction

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2020 18:09, Andrew Cooper wrote:
> On 08/01/2020 16:55, Jan Beulich wrote:
>> On 08.01.2020 17:15, Andrew Cooper wrote:
>>> On 08/01/2020 11:38, Jan Beulich wrote:
>>>> As said - I'm going to try to not stand in the way of you re-
>>>> arranging this, but
>>>> - the new code should not break silently when (in particular)
>>>>   l2_bootmap[] changes
>>> What practical changes do you think could be done here?  I can't spot
>>> any which would be helpful.
>>>
>>> A BUILD_BUG_ON() doesn't work.  The most likely case for something going
>>> wrong here is an edit to x86_64.S and no equivalent edit to page.h,
>>> which a BUILD_BUG_ON() wouldn't spot.  head.S similarly has no useful
>>> protections which could be added.
>> Well, the fundamental assumption is that the .S files and the
>> C declaration of l?_bootmap[] are kept in sync. No BUILD_BUG_ON()
>> can cover a mistake made there. But rather than using the literal
>> 4 as you did, an ARRAY_SIZE() construct should be usable to either
>> replace it, or amend it with a BUILD_BUG_ON().
> 
> You are aware that ARRAY_SIZE(l2_bootmap) is 2048 and
> ARRAY_SIZE(l3_bootmap) is 512, neither of which would be correct here?

Yes, I am (which is why I added "construct"). Dividing by
L<n>_PAGETABLE_ENTRIES would be one option. In particular for
l2_bootmap declaring at as [4][L2_PAGETABLE_ENTRIES] would be
another.

Jan

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