[Xen-devel] [PATCH 1/3] x86/EFI: don't apply relocations to l{2, 3}_bootmap
Other than claimed in commit 2ce5963727's ("x86: construct the
{l2,l3}_bootmap at compile time") the initialization of the two page
tables doesn't take care of everything without furher adjustment: The
compile time initialization obviously requires base relocations, and
those get processed after efi_arch_memory_setup(). Hence without
additional care the correctly initialized values may then get wrongly
"adjusted" again. Except the two table from being subject to base
relocation.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -47,11 +47,23 @@ static void __init efi_arch_relocate_ima
for ( base_relocs = __base_relocs_start; base_relocs < __base_relocs_end; )
{
-unsigned int i, n;
+unsigned int i = 0, n;
n = (base_relocs->size - sizeof(*base_relocs)) /
sizeof(*base_relocs->entries);
-for ( i = 0; i < n; ++i )
+
+/*
+ * Relevant l{2,3}_bootmap entries get initialized explicitly in
+ * efi_arch_memory_setup(), so we must not apply relocations there.
+ * l2_identmap's first slot, otoh, should be handled normally, as
+ * efi_arch_memory_setup() won't touch it (xen_phys_start should
+ * never be zero).
+ */
+if ( xen_phys_start + base_relocs->rva == (unsigned long)l3_bootmap ||
+ xen_phys_start + base_relocs->rva == (unsigned long)l2_bootmap )
+i = n;
+
+for ( ; i < n; ++i )
{
unsigned long addr = xen_phys_start + base_relocs->rva +
(base_relocs->entries[i] & 0xfff);
x86/EFI: don't apply relocations to l{2,3}_bootmap
Other than claimed in commit 2ce5963727's ("x86: construct the
{l2,l3}_bootmap at compile time") the initialization of the two page
tables doesn't take care of everything without furher adjustment: The
compile time initialization obviously requires base relocations, and
those get processed after efi_arch_memory_setup(). Hence without
additional care the correctly initialized values may then get wrongly
"adjusted" again. Except the two table from being subject to base
relocation.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -47,11 +47,23 @@ static void __init efi_arch_relocate_ima
for ( base_relocs = __base_relocs_start; base_relocs < __base_relocs_end; )
{
-unsigned int i, n;
+unsigned int i = 0, n;
n = (base_relocs->size - sizeof(*base_relocs)) /
sizeof(*base_relocs->entries);
-for ( i = 0; i < n; ++i )
+
+/*
+ * Relevant l{2,3}_bootmap entries get initialized explicitly in
+ * efi_arch_memory_setup(), so we must not apply relocations there.
+ * l2_identmap's first slot, otoh, should be handled normally, as
+ * efi_arch_memory_setup() won't touch it (xen_phys_start should
+ * never be zero).
+ */
+if ( xen_phys_start + base_relocs->rva == (unsigned long)l3_bootmap ||
+ xen_phys_start + base_relocs->rva == (unsigned long)l2_bootmap )
+i = n;
+
+for ( ; i < n; ++i )
{
unsigned long addr = xen_phys_start + base_relocs->rva +
(base_relocs->entries[i] & 0xfff);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/EFI: don't apply relocations to l{2, 3}_bootmap
On 19/08/16 08:50, Jan Beulich wrote:
> Other than claimed in commit 2ce5963727's ("x86: construct the
> {l2,l3}_bootmap at compile time") the initialization of the two page
> tables doesn't take care of everything without furher adjustment: The
> compile time initialization obviously requires base relocations, and
> those get processed after efi_arch_memory_setup(). Hence without
> additional care the correctly initialized values may then get wrongly
> "adjusted" again. Except the two table from being subject to base
> relocation.
Do you mean Exempt? "Except the two tables" doesn't parse.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -47,11 +47,23 @@ static void __init efi_arch_relocate_ima
>
> for ( base_relocs = __base_relocs_start; base_relocs <
> __base_relocs_end; )
> {
> -unsigned int i, n;
> +unsigned int i = 0, n;
>
> n = (base_relocs->size - sizeof(*base_relocs)) /
> sizeof(*base_relocs->entries);
> -for ( i = 0; i < n; ++i )
> +
> +/*
> + * Relevant l{2,3}_bootmap entries get initialized explicitly in
> + * efi_arch_memory_setup(), so we must not apply relocations there.
> + * l2_identmap's first slot, otoh, should be handled normally, as
And l3 surely?
Reviewed-by: Andrew Cooper
> + * efi_arch_memory_setup() won't touch it (xen_phys_start should
> + * never be zero).
> + */
> +if ( xen_phys_start + base_relocs->rva == (unsigned long)l3_bootmap
> ||
> + xen_phys_start + base_relocs->rva == (unsigned long)l2_bootmap )
> +i = n;
> +
> +for ( ; i < n; ++i )
> {
> unsigned long addr = xen_phys_start + base_relocs->rva +
> (base_relocs->entries[i] & 0xfff);
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] x86/EFI: don't apply relocations to l{2, 3}_bootmap
>>> On 19.08.16 at 12:34, wrote:
> On 19/08/16 08:50, Jan Beulich wrote:
>> Other than claimed in commit 2ce5963727's ("x86: construct the
>> {l2,l3}_bootmap at compile time") the initialization of the two page
>> tables doesn't take care of everything without furher adjustment: The
>> compile time initialization obviously requires base relocations, and
>> those get processed after efi_arch_memory_setup(). Hence without
>> additional care the correctly initialized values may then get wrongly
>> "adjusted" again. Except the two table from being subject to base
>> relocation.
>
> Do you mean Exempt? "Except the two tables" doesn't parse.
Hmm, my dictionary tells me that "except" can be used as a verb,
and it also tells me that I rather don't mean "exempt".
>> Signed-off-by: Jan Beulich
>>
>> --- a/xen/arch/x86/efi/efi-boot.h
>> +++ b/xen/arch/x86/efi/efi-boot.h
>> @@ -47,11 +47,23 @@ static void __init efi_arch_relocate_ima
>>
>> for ( base_relocs = __base_relocs_start; base_relocs <
>> __base_relocs_end; )
>> {
>> -unsigned int i, n;
>> +unsigned int i = 0, n;
>>
>> n = (base_relocs->size - sizeof(*base_relocs)) /
>> sizeof(*base_relocs->entries);
>> -for ( i = 0; i < n; ++i )
>> +
>> +/*
>> + * Relevant l{2,3}_bootmap entries get initialized explicitly in
>> + * efi_arch_memory_setup(), so we must not apply relocations there.
>> + * l2_identmap's first slot, otoh, should be handled normally, as
>
> And l3 surely?
Generally yes, but the comment refers to efi_arch_memory_setup(),
which only touches l2_identmap[]. I.e. I'm trying to answer the
reasonably to raise question why it doesn't need special casing here
despite that function touching it.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
