[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 12:51:59 +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=ePrgMFJN8aSP6o4ya3j7L9UOUOtT16JJ5FwlOhlpZ7c=; b=OZzBgj/2IdZ5jMNftSBn896BZPXYYHwnz0GPYv78lPgXaI0MEud+uloeeCZIoAn+HJF/W+Dzkr+lwEk+axtpcHd++KAcyHMbyCw3+LYWeRKrrcPpF1Jnp8ZR51KcoPGFAFpiabwI7lOXpAzC+4IrAXKCbyNr7zvoJcCxU3PzWVcijpjAyToQawfpeKi1RduIyBHTLlfmaiyRGxJr3QiRJ0ts/oFnzCaGaHKiu3429Qumn0X8TGcFLKJyi4fewfzvubPGx1iatJYklo1Kp+moRSkBqbKtd1A4leVN2PPEpW+ONTNvAKoC/0sHjZjx3TA946uuFCn5MZfVLqfZ4xn3MA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mm4isgJGn7SLOuiPepVD+Pu7uF+Tx0/YFG3RJUEOajKfq7zqH1ERJag3JJNNu5X/u6XfR+T90fnm2Vij7r9sc6DdZq8xB89dm4dnATbAnVF7QMymPbE99yPRixV34/jdeK0yj7igy0Rqz/oPylH4TN6v6rpIrMf1Bd6UvPS2T5KlrFj2Oyk4Y28HjE+17QUEu1yBcDA4dBHNh72FMuhPHRcHtr7dRoFM0BRnpnkNraxVU6X0O/c++bV+42k345Gkxst2tu6YyxeQg5HSdt8K3bpShqMaAlawEL/VOxdTmHAGoj4hV7i9mqCvY/tnyMQH4FVCaLmjo4XOeOb3w+UsJw==
- 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 10:52:36 +0000
- Ironport-data: A9a23:+0GAsKJQRSMe+wXlFE+R95QlxSXFcZb7ZxGr2PjKsXjdYENSgmdUx 2cYWW+EOv6LNmWnKopyOt6z808G6MSAm4NmHFdlqX01Q3x08seUXt7xwmUcnc+xBpaaEB84t ZV2hv3odp1coqr0/0/1WlTZhSAgk/rOHvykU7Ss1hlZHWdMUD0mhQ9oh9k3i4tphcnRKw6Ws Jb5rta31GWNglaYCUpJrfPSwP9TlK6q4mhA4wRkPaojUGL2zBH5MrpOfcldEFOgKmVkNrbSb /rOyri/4lTY838FYj9yuu+mGqGiaue60Tmm0hK6aYD76vRxjnVaPpIAHOgdcS9qZwChxLid/ jnvWauYEm/FNoWU8AgUvoIx/ytWZcWq85efSZSzXFD6I+QrvBIAzt03ZHzaM7H09c4pO2tE0 9ZBIQo3c0+qrcuE5rW5Ss9j05FLwMnDZOvzu1lG5BSAVbMKZM6GRK/Ho9hFwD03m8ZCW+7EY NYUYiZuaxKGZABTPlAQC9Q1m+LAanvXKmUE7g7K4/dnpTGLl2Sd05C0WDbRUsaNSshP2F6Ru 0rN/njjAwFcP9uaodaA2iv02LSWwHynCer+EpWq3K42klGo21ApViA6DGSDr8KLiEeHDoc3x 0s8v3BGQbIJ3E6hQ8T5Xha4iGWZpRNaUN1Ve8Ul7Cmdx6yS5ByWbkAUQzgEZNE4ucseQT0xy kTPj97vHSZosrCeVTSa7Lj8kN+pES0cLGtHaSpaSwIAuoHnuNtq1kmJSct/GqmoiNGzASv33 z2BsCk5gfMUkNIP0KK4u1vAhlpAu6T0c+L83S2PNkrN0++zTNfNi1CAgbQD0ct9EQ==
- Ironport-hdrordr: A9a23:GHNFDa6fsrOPXBzd/wPXwNvXdLJyesId70hD6qkRc203TiX2ra qTdZgguCMc6wxwZJhDo7690cC7KBu2yXcS2+Us1NyZPTUO1lHGEGmnhrGSpgEJ3EbFh4xg6Z s=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
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:
> >> Ensure that the base address is 2M aligned, or else the page table
> >> entries created would be corrupt as reserved bits on the PDE end up
> >> set.
> >>
> >> We have found a broken firmware where the loader would end up loading
> >> Xen at a non 2M aligned region, and that caused a very difficult to
> >> debug triple fault.
> >
> > It's probably worth saying that in this case, the OEM has fixed their
> > firmware.
>
> I'm curious: What firmware loads Xen directly? I thought there was
> always a boot loader involved (except for xen.efi of course).
This was a result of a bug in firmware plus a bug in grub, there's
also one pending change for grub, see:
https://lists.gnu.org/archive/html/grub-devel/2023-04/msg00157.html
The firmware would return error for some calls to Boot Services
allocate_pages method, and that triggered a bug in grub that resulted
in the memory allocated for Xen not being aligned as requested.
> I'm further a little puzzled by this talking about alignment and not
> xen.efi: xen.gz only specifies alignment for MB2 afaik. For MB1 all
> it does specify is the physical address (2Mb) that it wants to be
> loaded at. So maybe MB2 wants mentioning here as well, for clarity?
"We have found a broken firmware where grub2 would end up loading Xen
at a non 2M aligned region when using the multiboot2 protocol, and
that caused a very difficult to debug triple fault."
Would that be better?
> >> @@ -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).
Thanks, Roger.
|
