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.) Jan
