Re: [PATCH 1/2] x86/head: check base address alignment

[Jan Beulich <jbeulich@xxxxxxxx>]

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