xen/common/efi/boot.c | 58 +++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 19 deletions(-)
Although code is compiled with -fpic option data is not position
independent. This causes data pointer to become invalid if
code is not relocated properly which is what happens for
efi_multiboot2 which is called by multiboot entry code.
Code tested adding
PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
in efi_multiboot2 before calling efi_arch_edd (this function
can potentially call PrintErrMesg).
Before the patch (XenServer installation on Qemu, xen replaced
with vanilla xen.gz):
Booting `XenServer (Serial)'Booting `XenServer (Serial)'
Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
RSI - FFFF82D040467CE8, RDI - 0000000000000000
R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
R14 - 000000007EA33328, R15 - 000000007EA332D8
DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
GS - 0000000000000030, SS - 0000000000000030
CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
CR4 - 0000000000000668, CR8 - 0000000000000000
DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
FXSAVE_STATE - 000000007FF0BDE0
!!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
After the patch:
Booting `XenServer (Serial)'Booting `XenServer (Serial)'
Test message: Buffer too small
BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
This partially rollback commit 00d5d5ce23e6.
Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
xen/common/efi/boot.c | 58 +++++++++++++++++++++++++++++--------------
1 file changed, 39 insertions(+), 19 deletions(-)
---
Changes since v1:
- added "Fixes:" tag;
- fixed cast style change.
Changes since v2:
- wrap long line.
Changes since v3:
- fixed "Fixes:" tag.
Changes since v4:
- use switch instead of tables.
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index efbec00af9..143b5681ba 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -287,33 +287,53 @@ static bool __init match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2)
/* generic routine for printing error messages */
static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
{
- static const CHAR16* const ErrCodeToStr[] __initconstrel = {
- [~EFI_ERROR_MASK & EFI_NOT_FOUND] = L"Not found",
- [~EFI_ERROR_MASK & EFI_NO_MEDIA] = L"The device has no media",
- [~EFI_ERROR_MASK & EFI_MEDIA_CHANGED] = L"Media changed",
- [~EFI_ERROR_MASK & EFI_DEVICE_ERROR] = L"Device error",
- [~EFI_ERROR_MASK & EFI_VOLUME_CORRUPTED] = L"Volume corrupted",
- [~EFI_ERROR_MASK & EFI_ACCESS_DENIED] = L"Access denied",
- [~EFI_ERROR_MASK & EFI_OUT_OF_RESOURCES] = L"Out of resources",
- [~EFI_ERROR_MASK & EFI_VOLUME_FULL] = L"Volume is full",
- [~EFI_ERROR_MASK & EFI_SECURITY_VIOLATION] = L"Security violation",
- [~EFI_ERROR_MASK & EFI_CRC_ERROR] = L"CRC error",
- [~EFI_ERROR_MASK & EFI_COMPROMISED_DATA] = L"Compromised data",
- [~EFI_ERROR_MASK & EFI_BUFFER_TOO_SMALL] = L"Buffer too small",
- };
- EFI_STATUS ErrIdx = ErrCode & ~EFI_ERROR_MASK;
-
StdOut = StdErr;
PrintErr(mesg);
PrintErr(L": ");
- if( (ErrIdx < ARRAY_SIZE(ErrCodeToStr)) && ErrCodeToStr[ErrIdx] )
- mesg = ErrCodeToStr[ErrIdx];
- else
+ switch (ErrCode)
{
+ case EFI_NOT_FOUND:
+ mesg = L"Not found";
+ break;
+ case EFI_NO_MEDIA:
+ mesg = L"The device has no media";
+ break;
+ case EFI_MEDIA_CHANGED:
+ mesg = L"Media changed";
+ break;
+ case EFI_DEVICE_ERROR:
+ mesg = L"Device error";
+ break;
+ case EFI_VOLUME_CORRUPTED:
+ mesg = L"Volume corrupted";
+ break;
+ case EFI_ACCESS_DENIED:
+ mesg = L"Access denied";
+ break;
+ case EFI_OUT_OF_RESOURCES:
+ mesg = L"Out of resources";
+ break;
+ case EFI_VOLUME_FULL:
+ mesg = L"Volume is full";
+ break;
+ case EFI_SECURITY_VIOLATION:
+ mesg = L"Security violation";
+ break;
+ case EFI_CRC_ERROR:
+ mesg = L"CRC error";
+ break;
+ case EFI_COMPROMISED_DATA:
+ mesg = L"Compromised data";
+ break;
+ case EFI_BUFFER_TOO_SMALL:
+ mesg = L"Buffer too small";
+ break;
+ default:
PrintErr(L"ErrCode: ");
DisplayUint(ErrCode, 0);
mesg = NULL;
+ break;
}
blexit(mesg);
}
--
2.34.1On 22/01/2025 10:14 am, Frediano Ziglio wrote:
> Although code is compiled with -fpic option data is not position
> independent.
This doesn't parse. ITYM "Although the code is compiled with -fpic,
pointers in data are not position independent."
> This causes data pointer to become invalid if
> code is not relocated properly which is what happens for
> efi_multiboot2 which is called by multiboot entry code.
"efi_multiboot2()" to highlight that you're naming a function.
>
> Code tested adding
> PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
> in efi_multiboot2 before calling efi_arch_edd (this function
> can potentially call PrintErrMesg).
>
> Before the patch (XenServer installation on Qemu, xen replaced
> with vanilla xen.gz):
> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
> ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
> RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
> RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
> RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
> RSI - FFFF82D040467CE8, RDI - 0000000000000000
> R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
> R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
> R14 - 000000007EA33328, R15 - 000000007EA332D8
> DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
> GS - 0000000000000030, SS - 0000000000000030
> CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
> CR4 - 0000000000000668, CR8 - 0000000000000000
> DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
> IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
> FXSAVE_STATE - 000000007FF0BDE0
> !!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
>
> After the patch:
> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> Test message: Buffer too small
> BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
>
> This partially rollback commit 00d5d5ce23e6.
>
> Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
00d5d5ce23e6 needs to be a second fixes tag then too.
In hindsight, it was a naive fix. But, by reverting this part of it,
you're reintroducing a build breakage on Clang.
I wrote that construct because it is what Clang-3.8 generated, albeit
with the proper __initconstrel attribute to make the
SPEICAL_DATA_SECTIONS check be happy.
Anyway, see:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=1105ab42a84bc11c62597005f78ccad2434fbd66
This needs to be paired with an unconditional -fno-jump-tables covering
any TU which the MB2+EFI path executes at the wrong address.
~Andrew
On Tue, Feb 4, 2025 at 4:20 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 22/01/2025 10:14 am, Frediano Ziglio wrote:
> > Although code is compiled with -fpic option data is not position
> > independent.
>
> This doesn't parse. ITYM "Although the code is compiled with -fpic,
> pointers in data are not position independent."
>
> > This causes data pointer to become invalid if
> > code is not relocated properly which is what happens for
> > efi_multiboot2 which is called by multiboot entry code.
>
> "efi_multiboot2()" to highlight that you're naming a function.
>
Should I post a new version or are you fixing the grammar on commit ?
> >
> > Code tested adding
> > PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
> > in efi_multiboot2 before calling efi_arch_edd (this function
> > can potentially call PrintErrMesg).
> >
> > Before the patch (XenServer installation on Qemu, xen replaced
> > with vanilla xen.gz):
> > Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> > Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
> > ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
> > RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
> > RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
> > RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
> > RSI - FFFF82D040467CE8, RDI - 0000000000000000
> > R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
> > R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
> > R14 - 000000007EA33328, R15 - 000000007EA332D8
> > DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
> > GS - 0000000000000030, SS - 0000000000000030
> > CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
> > CR4 - 0000000000000668, CR8 - 0000000000000000
> > DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> > DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> > GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
> > IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
> > FXSAVE_STATE - 000000007FF0BDE0
> > !!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
> >
> > After the patch:
> > Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> > Test message: Buffer too small
> > BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> > BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> >
> > This partially rollback commit 00d5d5ce23e6.
> >
> > Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>
> 00d5d5ce23e6 needs to be a second fixes tag then too.
>
> In hindsight, it was a naive fix. But, by reverting this part of it,
> you're reintroducing a build breakage on Clang.
>
> I wrote that construct because it is what Clang-3.8 generated, albeit
> with the proper __initconstrel attribute to make the
> SPEICAL_DATA_SECTIONS check be happy.
>
> Anyway, see:
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=1105ab42a84bc11c62597005f78ccad2434fbd66
>
> This needs to be paired with an unconditional -fno-jump-tables covering
> any TU which the MB2+EFI path executes at the wrong address.
>
Newer compiler versions seem to generate position independent jump
tables, but it's not guaranteed. So, yes, it would be better to add
that option.
> ~Andrew
Frediano
On 22.01.2025 11:14, Frediano Ziglio wrote:
> Although code is compiled with -fpic option data is not position
> independent. This causes data pointer to become invalid if
> code is not relocated properly which is what happens for
> efi_multiboot2 which is called by multiboot entry code.
>
> Code tested adding
> PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
> in efi_multiboot2 before calling efi_arch_edd (this function
> can potentially call PrintErrMesg).
>
> Before the patch (XenServer installation on Qemu, xen replaced
> with vanilla xen.gz):
> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
> ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
> RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
> RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
> RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
> RSI - FFFF82D040467CE8, RDI - 0000000000000000
> R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
> R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
> R14 - 000000007EA33328, R15 - 000000007EA332D8
> DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
> GS - 0000000000000030, SS - 0000000000000030
> CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
> CR4 - 0000000000000668, CR8 - 0000000000000000
> DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
> IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
> FXSAVE_STATE - 000000007FF0BDE0
> !!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
>
> After the patch:
> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> Test message: Buffer too small
> BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
>
> This partially rollback commit 00d5d5ce23e6.
>
> Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
I expect we want this in before the release. Oleksii? Maintainers?
Jan
On 2/4/25 2:07 PM, Jan Beulich wrote:
> On 22.01.2025 11:14, Frediano Ziglio wrote:
>> Although code is compiled with -fpic option data is not position
>> independent. This causes data pointer to become invalid if
>> code is not relocated properly which is what happens for
>> efi_multiboot2 which is called by multiboot entry code.
>>
>> Code tested adding
>> PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
>> in efi_multiboot2 before calling efi_arch_edd (this function
>> can potentially call PrintErrMesg).
>>
>> Before the patch (XenServer installation on Qemu, xen replaced
>> with vanilla xen.gz):
>> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
>> Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
>> ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
>> RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
>> RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
>> RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
>> RSI - FFFF82D040467CE8, RDI - 0000000000000000
>> R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
>> R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
>> R14 - 000000007EA33328, R15 - 000000007EA332D8
>> DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
>> GS - 0000000000000030, SS - 0000000000000030
>> CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
>> CR4 - 0000000000000668, CR8 - 0000000000000000
>> DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
>> DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
>> GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
>> IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
>> FXSAVE_STATE - 000000007FF0BDE0
>> !!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
>>
>> After the patch:
>> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
>> Test message: Buffer too small
>> BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
>> BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
>>
>> This partially rollback commit 00d5d5ce23e6.
>>
>> Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
>> Signed-off-by: Frediano Ziglio<frediano.ziglio@cloud.com>
> I expect we want this in before the release. Oleksii? Maintainers?
Interesting it is a fix for a ~3 years old bug ( if to look at when9180f5365524 is introduced ) so it seems it happens not often. Anyway, I
agree that we want this fix before the release: R-Acked-by: Oleksii
Kurochko <oleksii.kurochko@gmail.com> Thanks. ~ Oleksii
>
> Jan
On Wed, Feb 5, 2025 at 10:24 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
>
> On 2/4/25 2:07 PM, Jan Beulich wrote:
>
> On 22.01.2025 11:14, Frediano Ziglio wrote:
>
> Although code is compiled with -fpic option data is not position
> independent. This causes data pointer to become invalid if
> code is not relocated properly which is what happens for
> efi_multiboot2 which is called by multiboot entry code.
>
> Code tested adding
> PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
> in efi_multiboot2 before calling efi_arch_edd (this function
> can potentially call PrintErrMesg).
>
> Before the patch (XenServer installation on Qemu, xen replaced
> with vanilla xen.gz):
> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
> ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
> RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
> RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
> RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
> RSI - FFFF82D040467CE8, RDI - 0000000000000000
> R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
> R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
> R14 - 000000007EA33328, R15 - 000000007EA332D8
> DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
> GS - 0000000000000030, SS - 0000000000000030
> CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
> CR4 - 0000000000000668, CR8 - 0000000000000000
> DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
> IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
> FXSAVE_STATE - 000000007FF0BDE0
> !!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
>
> After the patch:
> Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> Test message: Buffer too small
> BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
>
> This partially rollback commit 00d5d5ce23e6.
>
> Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>
> I expect we want this in before the release. Oleksii? Maintainers?
>
> Interesting it is a fix for a ~3 years old bug ( if to look at when 9180f5365524 is introduced ) so it seems it happens not often.
I would say it's quite normal for booting messages, usually we expect
them to work and not get errors, we are in a pretty "stable" state.
The problem happens when there are some strange combinations of
firmware bugs or behavior resulting in errors. There was a bug some
time ago during the boot phase where a message would be helpful
instead of a crash, but it exercised a different error path.
> Anyway, I agree that we want this fix before the release:
> R-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> Thanks.
>
> ~ Oleksii
>
> Jan
ping
On Wed, Feb 5, 2025 at 11:40 AM Frediano Ziglio
<frediano.ziglio@cloud.com> wrote:
>
> On Wed, Feb 5, 2025 at 10:24 AM Oleksii Kurochko
> <oleksii.kurochko@gmail.com> wrote:
> >
> >
> > On 2/4/25 2:07 PM, Jan Beulich wrote:
> >
> > On 22.01.2025 11:14, Frediano Ziglio wrote:
> >
> > Although code is compiled with -fpic option data is not position
> > independent. This causes data pointer to become invalid if
> > code is not relocated properly which is what happens for
> > efi_multiboot2 which is called by multiboot entry code.
> >
> > Code tested adding
> > PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
> > in efi_multiboot2 before calling efi_arch_edd (this function
> > can potentially call PrintErrMesg).
> >
> > Before the patch (XenServer installation on Qemu, xen replaced
> > with vanilla xen.gz):
> > Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> > Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
> > ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
> > RIP - 000000007EE21E9A, CS - 0000000000000038, RFLAGS - 0000000000210246
> > RAX - 000000007FF0C1B5, RCX - 0000000000000050, RDX - 0000000000000010
> > RBX - 0000000000000000, RSP - 000000007FF0C180, RBP - 000000007FF0C210
> > RSI - FFFF82D040467CE8, RDI - 0000000000000000
> > R8 - 000000007FF0C1C8, R9 - 000000007FF0C1C0, R10 - 0000000000000000
> > R11 - 0000000000001020, R12 - FFFF82D040467CE8, R13 - 000000007FF0C1B8
> > R14 - 000000007EA33328, R15 - 000000007EA332D8
> > DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
> > GS - 0000000000000030, SS - 0000000000000030
> > CR0 - 0000000080010033, CR2 - FFFF82D040467CE8, CR3 - 000000007FC01000
> > CR4 - 0000000000000668, CR8 - 0000000000000000
> > DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> > DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> > GDTR - 000000007F9DB000 0000000000000047, LDTR - 0000000000000000
> > IDTR - 000000007F48E018 0000000000000FFF, TR - 0000000000000000
> > FXSAVE_STATE - 000000007FF0BDE0
> > !!!! Find image based on IP(0x7EE21E9A) (No PDB) (ImageBase=000000007EE20000, EntryPoint=000000007EE23935) !!!!
> >
> > After the patch:
> > Booting `XenServer (Serial)'Booting `XenServer (Serial)'
> > Test message: Buffer too small
> > BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> > BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
> >
> > This partially rollback commit 00d5d5ce23e6.
> >
> > Fixes: 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms")
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> >
> > I expect we want this in before the release. Oleksii? Maintainers?
> >
> > Interesting it is a fix for a ~3 years old bug ( if to look at when 9180f5365524 is introduced ) so it seems it happens not often.
>
> I would say it's quite normal for booting messages, usually we expect
> them to work and not get errors, we are in a pretty "stable" state.
> The problem happens when there are some strange combinations of
> firmware bugs or behavior resulting in errors. There was a bug some
> time ago during the boot phase where a message would be helpful
> instead of a crash, but it exercised a different error path.
>
> > Anyway, I agree that we want this fix before the release:
> > R-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> >
> > Thanks.
> >
> > ~ Oleksii
> >
> > Jan
On 17/02/2025 1:55 pm, Frediano Ziglio wrote: > ping Ping what? You have no maintainer ack, an outstanding bug raised against this version, and a suggestion on how to address it. I'd really like to see this in 4.20 too, but this needs a v6 before it's going to progress any further. ~Andrew
Hello Frediano, On 2/17/25 3:45 PM, Andrew Cooper wrote: > On 17/02/2025 1:55 pm, Frediano Ziglio wrote: >> ping > Ping what? > > You have no maintainer ack, an outstanding bug raised against this > version, and a suggestion on how to address it. > > I'd really like to see this in 4.20 too, but this needs a v6 before it's > going to progress any further. As I mentioned in one of my previous replies to this patch, I'm giving Release-Acked with the expectation that a proper maintainer's acknowledgment will be obtained. ~ Oleksii
© 2016 - 2025 Red Hat, Inc.