On 08/01/2020 11:38, Jan Beulich wrote: >> The purpose of this was to make the handling of l?_bootmap[] as >> consistent as possible between the various environments. The pagetables >> themselves are common, and should be used consistently. > I don't think I can wholeheartedly agree here: l?_bootmap[] are > throw-away page tables (living in .init), and with the non-EFI and > EFI boot paths being so different anyway, them using the available > tables differently is not a big issue imo. This heavy difference of > other aspects was also why back then I decided to be as defensive > towards l2_bootmap[] size changes as possible in code which doesn't > really need it to be multiple pages.
From this description, it suggests that you haven't spotted the rather more subtle bug which will trip up anyone trying to develop in the future. This scheme is incompatible with trying to map a second object (e.g. the trampoline) into the bootmap, because depending on alignment, it may overlap with the PTEs which mapped Xen. There also typically isn't an l3_bootmap[0] => l2_bootmap[0] because of where xen.efi is loaded in memory. > > 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. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
