On Tue, May 02, 2023 at 01:11:12PM +0200, Jan Beulich wrote: > On 02.05.2023 13:05, Jan Beulich wrote: > > On 02.05.2023 12:51, Roger Pau Monné wrote: > >> On Tue, May 02, 2023 at 12:28:55PM +0200, Jan Beulich wrote: > >>> On 02.05.2023 11:54, Andrew Cooper wrote: > >>>> On 02/05/2023 10:22 am, Roger Pau Monne wrote: > >>>>> @@ -670,6 +674,11 @@ trampoline_setup: > >>>>> cmp %edi, %eax > >>>>> jb 1b > >>>>> > >>>>> + /* Check that the image base is aligned. */ > >>>>> + lea sym_esi(_start), %eax > >>>>> + and $(1 << L2_PAGETABLE_SHIFT) - 1, %eax > >>>>> + jnz not_aligned > >>>> > >>>> You just want to check the value in %esi, which is the base of the Xen > >>>> image. Something like: > >>>> > >>>> mov %esi, %eax > >>>> and ... > >>>> jnz > >>> > >>> Or yet more simply "test $..., %esi" and then "jnz ..."? > >> > >> As replied to Andrew, I would rather keep this inline with the address > >> used to build the PDE, which is sym_esi(_start). > > > > Well, I won't insist, and you've got Andrew's R-b already. > > Actually, one more remark here: While using sym_esi() is more in line > with the actual consumer of the data, the check triggering because of > the transformation yielding a misaligned value (in turn because of a > bug elsewhere) would yield a misleading error message: We might well > have been loaded at a 2Mb-aligned boundary, and instead its internal > logic which would then have been wrong. (I'm sorry, now you'll get to > judge whether keeping the check in line with other code or with the > diagnostic is going to be better. Or split things into a build-time > and a runtime check, as previously suggested.)
What about adding a build time check that XEN_VIRT_START is 2MB aligned, and then just switching to test instead of and, would that be acceptable? I know that using sym_esi(_start) instead of just esi won't change the result of the check if XEN_VIRT_START is aligned, but I would prefer to keep the usage of sym_esi(_start) for consistency with the value used to build the page tables, as I think it's clearer. Thanks, Roger.
