[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] x86/head: check base address alignment
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Tue, 2 May 2023 15:02:48 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=WHooOrtZ455hB4epaiuSklactcjPB2cz/QE9dt8MMoM=; b=bqx9oLbUw770c78j8DOGlmy16Dd/pAICUV9YKU06py1+loQLua4rrmTtfqi8GbRxUxwpuJCZ9kc38emPlzsBXyCMx9hpQVO4rQfRUQEEpWaYIr/lztSiGjB075ZYoC7zFMTzjnU/zJ8CoPRagzJHxqPK1cCTpJyPgaPONtZ/L/NtAZo90zChzjTKq33NtuiI0FquL958wRHbg7K90GMefCsB2+U8TIzMSk46L5HhrzP3+P8pI5/n+l67CB9vMWyI3+1eBkvgB64l2ly254DSatg7o0VibYvjOOCtJ97Pq1fnX5dVYGK5bXccSlah/uKb1KgGpfBOob0tzwpxTdsdWQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EdJA6QaxrgoHJnqycfxY331CoBQSLDPAmChLgX05HhhBiKZ05E0U6CUwAcA9fHO9kMKUtBkPrNE7J9InYr5ojKiI8xp77OL1pDmaCe3LbSqDRPQ0SE/EnojlH23Nplhpr5EOaXW2A/0Jdfzz1ckfqBy8PvIQ2iNgAXOE8RRFvxdGuGoh6ulELC4MZIkYgF4T9w/SaAo6OZC39li4eKBkjseEOzvGPfUCWfQq4nP2UP/UZVB58DQcMpVVReWWU/1akvzgJwmIVrgaOM+tU/478tnRWyUnC01uteEtqkRhOGWfQP0PD7VUtb0eERTdb72dykf0Imtx2sHQT6vkrbZh/A==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Tue, 02 May 2023 13:03:14 +0000
- Ironport-data: A9a23:ibHLBaBbp/QqzxVW/w7iw5YqxClBgxIJ4kV8jS/XYbTApGsg3zJRz 2IXXD3TPvzfamf2co0iaNnjpE9Tu5OHzNFnQQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8nk/nOHuGmYAL9EngZbRd+Tys8gg5Ulec8g4p56fC0GArIs t7pyyHlEAbNNwVcbyRFuspvlDs15K6p4G5A5ARkDRx2lAS2e0c9Xcp3yZ6ZdxMUcqEMdsamS uDKyq2O/2+x13/B3fv8z94X2mVTKlLjFVDmZkh+AsBOsTAbzsAG6Y4pNeJ0VKtio27hc+ada jl6ncfYpQ8BZsUgkQmGOvVSO3kW0aZuoNcrLZUj2CA6IoKvn3bEmp1T4E8K0YIwo9pLI0Fn8 uwieBMqYhPYtsnn7amdc7w57igjBJGD0II3nFhFlW2cJ9B2BJfJTuPN+MNS2yo2ioZWB/HCa sEFaD1pKhPdfxlIPVRRA5U79AuqriCnL3sE9xTK+exrswA/zyQouFTpGMDSddGQA91cg26Tp 37c/nS/CRYfXDCa4WPdrS/93rKVzEsXXqoyKq3gz8BR2mer/TQKIUQyS36xjKmQ3xvWt9V3b hZ8FjAVhao4+VGvT9L9dwalu3PCtRkZM/JPF8Uq5QfLzbDbiy6JC25BQjNfZdgOsM4tWSdsx lKPh8nuBzFkrPuSU331y1uPhTa7OCxQJ2lSYyYBFVcB+4O7/NF1iQ/TRNF+FqLzlsfyBTz73 zGNqm45mqkXiskIka68+Dgrng6Rm3QAdSZtji2/Y45vxloRiFKND2Bw1WXm0A==
- Ironport-hdrordr: A9a23:hjQZhaBsMXwhdGblHela55DYdb4zR+YMi2TDt3oddfWaSKylfq GV7ZImPHrP4gr5N0tOpTntAse9qDbnhPxICOoqTNCftWvdyQiVxehZhOOP/9SjIVyaygc078 xdmsNFebnN5DZB7PoT4GODYqkdKNvsytHXuQ8JpU0dPD2DaMtbnndE4h7wKDwOeOHfb6BJaa Z14KB81kKdUEVSVOuXLF8fUdPOotXa/aiWHSLvV3YcmXKzZSrD0s+BLySl
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
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.
|
