| Message ID | 20240913075907.34460-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/time: improvements to wallclock logic | expand |
On 13/09/2024 8:59 am, Roger Pau Monne wrote: > The EFI_GET_TIME implementation is well known to be broken for many firmware > implementations, for Xen the result on such implementations are: > > ----[ Xen-4.19-unstable x86_64 debug=y Tainted: C ]---- > CPU: 0 > RIP: e008:[<0000000062ccfa70>] 0000000062ccfa70 > [...] > Xen call trace: > [<0000000062ccfa70>] R 0000000062ccfa70 > [<00000000732e9a3f>] S 00000000732e9a3f > [<ffff82d04034f34f>] F arch/x86/time.c#get_cmos_time+0x1b3/0x26e > [<ffff82d04045926f>] F init_xen_time+0x28/0xa4 > [<ffff82d040454bc4>] F __start_xen+0x1ee7/0x2578 > [<ffff82d040203334>] F __high_start+0x94/0xa0 > > Pagetable walk from 0000000062ccfa70: > L4[0x000] = 000000207ef1c063 ffffffffffffffff > L3[0x001] = 000000005d6c0063 ffffffffffffffff > L2[0x116] = 8000000062c001e3 ffffffffffffffff (PSE) > > **************************************** > Panic on CPU 0: > FATAL PAGE FAULT > [error_code=0011] > Faulting linear address: 0000000062ccfa70 > **************************************** > > Swap the preference to default to CMOS first, and EFI later, in an attempt to > use EFI_GET_TIME as a last resort option only. Note that Linux for example > doesn't allow calling the get_time method, and instead provides a dummy handler > that unconditionally returns EFI_UNSUPPORTED on x86-64. > > Such change in the preferences requires some re-arranging of the function > logic, so that panic messages with workaround suggestions are suitably printed. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> This brings us better-in-line with the rest of the world on the matter. ~Andrew
On Fri, Sep 13, 2024 at 09:59:07AM +0200, Roger Pau Monne wrote: > The EFI_GET_TIME implementation is well known to be broken for many firmware > implementations, for Xen the result on such implementations are: > > ----[ Xen-4.19-unstable x86_64 debug=y Tainted: C ]---- > CPU: 0 > RIP: e008:[<0000000062ccfa70>] 0000000062ccfa70 > [...] > Xen call trace: > [<0000000062ccfa70>] R 0000000062ccfa70 > [<00000000732e9a3f>] S 00000000732e9a3f > [<ffff82d04034f34f>] F arch/x86/time.c#get_cmos_time+0x1b3/0x26e > [<ffff82d04045926f>] F init_xen_time+0x28/0xa4 > [<ffff82d040454bc4>] F __start_xen+0x1ee7/0x2578 > [<ffff82d040203334>] F __high_start+0x94/0xa0 > > Pagetable walk from 0000000062ccfa70: > L4[0x000] = 000000207ef1c063 ffffffffffffffff > L3[0x001] = 000000005d6c0063 ffffffffffffffff > L2[0x116] = 8000000062c001e3 ffffffffffffffff (PSE) > > **************************************** > Panic on CPU 0: > FATAL PAGE FAULT > [error_code=0011] > Faulting linear address: 0000000062ccfa70 > **************************************** > > Swap the preference to default to CMOS first, and EFI later, in an attempt to > use EFI_GET_TIME as a last resort option only. Note that Linux for example > doesn't allow calling the get_time method, and instead provides a dummy handler > that unconditionally returns EFI_UNSUPPORTED on x86-64. > > Such change in the preferences requires some re-arranging of the function > logic, so that panic messages with workaround suggestions are suitably printed. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Since this changes behavior for running on EFI, Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > Changes since v2: > - Updated to match previous changes. > --- > xen/arch/x86/time.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c > index e4751684951e..b86e4d58b40c 100644 > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -1592,14 +1592,14 @@ static void __init probe_wallclock(void) > wallclock_source = WALLCLOCK_XEN; > return; > } > - if ( efi_enabled(EFI_RS) && efi_get_time() ) > + if ( cmos_rtc_probe() ) > { > - wallclock_source = WALLCLOCK_EFI; > + wallclock_source = WALLCLOCK_CMOS; > return; > } > - if ( cmos_rtc_probe() ) > + if ( efi_enabled(EFI_RS) && efi_get_time() ) > { > - wallclock_source = WALLCLOCK_CMOS; > + wallclock_source = WALLCLOCK_EFI; > return; > } > > -- > 2.46.0 >
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index e4751684951e..b86e4d58b40c 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -1592,14 +1592,14 @@ static void __init probe_wallclock(void) wallclock_source = WALLCLOCK_XEN; return; } - if ( efi_enabled(EFI_RS) && efi_get_time() ) + if ( cmos_rtc_probe() ) { - wallclock_source = WALLCLOCK_EFI; + wallclock_source = WALLCLOCK_CMOS; return; } - if ( cmos_rtc_probe() ) + if ( efi_enabled(EFI_RS) && efi_get_time() ) { - wallclock_source = WALLCLOCK_CMOS; + wallclock_source = WALLCLOCK_EFI; return; }
The EFI_GET_TIME implementation is well known to be broken for many firmware implementations, for Xen the result on such implementations are: ----[ Xen-4.19-unstable x86_64 debug=y Tainted: C ]---- CPU: 0 RIP: e008:[<0000000062ccfa70>] 0000000062ccfa70 [...] Xen call trace: [<0000000062ccfa70>] R 0000000062ccfa70 [<00000000732e9a3f>] S 00000000732e9a3f [<ffff82d04034f34f>] F arch/x86/time.c#get_cmos_time+0x1b3/0x26e [<ffff82d04045926f>] F init_xen_time+0x28/0xa4 [<ffff82d040454bc4>] F __start_xen+0x1ee7/0x2578 [<ffff82d040203334>] F __high_start+0x94/0xa0 Pagetable walk from 0000000062ccfa70: L4[0x000] = 000000207ef1c063 ffffffffffffffff L3[0x001] = 000000005d6c0063 ffffffffffffffff L2[0x116] = 8000000062c001e3 ffffffffffffffff (PSE) **************************************** Panic on CPU 0: FATAL PAGE FAULT [error_code=0011] Faulting linear address: 0000000062ccfa70 **************************************** Swap the preference to default to CMOS first, and EFI later, in an attempt to use EFI_GET_TIME as a last resort option only. Note that Linux for example doesn't allow calling the get_time method, and instead provides a dummy handler that unconditionally returns EFI_UNSUPPORTED on x86-64. Such change in the preferences requires some re-arranging of the function logic, so that panic messages with workaround suggestions are suitably printed. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v2: - Updated to match previous changes. --- xen/arch/x86/time.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)