[Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
When EFI is used, don't use EFI's GetTime() to get the time, because it is broken on many platforms. From Linux commit 7efe665903d0 ("rtc: Disable EFI rtc for x86"): "Disable it explicitly for x86 so that we don't give users false hope that this driver will work - it won't, and your machine is likely to crash." Signed-off-by: Ross Lagerwall --- xen/arch/x86/time.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 30d52c4..28895c2 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -679,20 +679,28 @@ static void __get_cmos_time(struct rtc_time *rtc) rtc->year += 100; } +/* EFI's GetTime() is frequently broken so don't use it by default. */ +#undef USE_EFI_GET_TIME + static unsigned long get_cmos_time(void) { -unsigned long res, flags; +#ifdef USE_EFI_GET_TIME +unsigned long res; +#endif +unsigned long flags; struct rtc_time rtc; unsigned int seconds = 60; static bool_t __read_mostly cmos_rtc_probe; boolean_param("cmos-rtc-probe", cmos_rtc_probe); +#ifdef USE_EFI_GET_TIME if ( efi_enabled ) { res = efi_get_time(); if ( res ) return res; } +#endif if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) ) cmos_rtc_probe = 0; -- 2.4.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
On 01/12/15 16:57, Ross Lagerwall wrote: > When EFI is used, don't use EFI's GetTime() to get the time, because it > is broken on many platforms. [...] > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -679,20 +679,28 @@ static void __get_cmos_time(struct rtc_time *rtc) > rtc->year += 100; > } > > +/* EFI's GetTime() is frequently broken so don't use it by default. */ > +#undef USE_EFI_GET_TIME > + > static unsigned long get_cmos_time(void) > { > -unsigned long res, flags; > +#ifdef USE_EFI_GET_TIME > +unsigned long res; > +#endif You could move this res into the if ( efi_enabled ) below. > +unsigned long flags; > struct rtc_time rtc; > unsigned int seconds = 60; > static bool_t __read_mostly cmos_rtc_probe; > boolean_param("cmos-rtc-probe", cmos_rtc_probe); > > +#ifdef USE_EFI_GET_TIME > if ( efi_enabled ) > { > res = efi_get_time(); > if ( res ) > return res; > } > +#endif David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
>>> On 01.12.15 at 17:57, wrote: > When EFI is used, don't use EFI's GetTime() to get the time, because it > is broken on many platforms. From Linux commit 7efe665903d0 ("rtc: > Disable EFI rtc for x86"): > "Disable it explicitly for x86 so that we don't give users false > hope that this driver will work - it won't, and your machine is likely > to crash." > > Signed-off-by: Ross Lagerwall NAK, since being conceptually wrong (and both of my systems work fine). Vendors should get their firmware fixed, and by not using runtime service functions we would give them even less reason to do so. Until then we have "efi=no-rs". Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
On 01/12/15 17:26, Jan Beulich wrote: On 01.12.15 at 17:57, wrote: >> When EFI is used, don't use EFI's GetTime() to get the time, because it >> is broken on many platforms. From Linux commit 7efe665903d0 ("rtc: >> Disable EFI rtc for x86"): >> "Disable it explicitly for x86 so that we don't give users false >> hope that this driver will work - it won't, and your machine is likely >> to crash." >> >> Signed-off-by: Ross Lagerwall > NAK, since being conceptually wrong (and both of my systems work > fine). Vendors should get their firmware fixed, and by not using > runtime service functions we would give them even less reason to > do so. Until then we have "efi=no-rs". This is completely unreasonable. It is not conceptually wrong. GetTime() is very well known completely broken, especially after ExitBootServices(), to the point that every other EFI implementation (including windows) completely avoids it. The fact that your two systems don't crash immediately is curious, but they are not a representative of systems in general. Not a single broadwell or skylake platform I have access to boots in EFI mode if GetTime() is used (which include 4 different manufactures). Vendors will not fix their firmware. Disabling all runtime services is not a reasonable alternative. This is a firmware bug just like many others and needs to be worked around by default like others. Anything else is actively damaging to the Xen community. People just get frustrated when it doesn't work (especially if the problem has been identify and a fix rejected upstream) and will move elsewhere instead. Any situation where a command line override is required to make Xen boot is a bug in Xen and should be fixed. This is why we have __init time quirks. it doesn't matter if we have some truly horrendous workarounds; Xen needs to be able to boot by default wherever possible. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
>>> On 01.12.15 at 20:26, wrote: > On 01/12/15 17:26, Jan Beulich wrote: > On 01.12.15 at 17:57, wrote: >>> When EFI is used, don't use EFI's GetTime() to get the time, because it >>> is broken on many platforms. From Linux commit 7efe665903d0 ("rtc: >>> Disable EFI rtc for x86"): >>> "Disable it explicitly for x86 so that we don't give users false >>> hope that this driver will work - it won't, and your machine is likely >>> to crash." >>> >>> Signed-off-by: Ross Lagerwall >> NAK, since being conceptually wrong (and both of my systems work >> fine). Vendors should get their firmware fixed, and by not using >> runtime service functions we would give them even less reason to >> do so. Until then we have "efi=no-rs". > > This is completely unreasonable. > > It is not conceptually wrong. I'm sorry, but no. Otherwise you mean to state that specifications don't even need to be written, since if people don't play by them, workarounds will get implemented anyway. > GetTime() is very well known completely > broken, especially after ExitBootServices(), to the point that every > other EFI implementation (including windows) completely avoids it. I'm not sure about the order of things. I started working with EFI on IA64, where using runtime services is a must. Windows started using EFI on IA64 too, so I'm pretty convinced they used GetTime() there. Whether they didn't on x86 because x86 implementations were broken, or whether x86 implementations end up broken because Windows never used those functions is simply an unknown. > The fact that your two systems don't crash immediately is curious, but > they are not a representative of systems in general. Not a single > broadwell or skylake platform I have access to boots in EFI mode if > GetTime() is used (which include 4 different manufactures). The original x86 implementation we used almost 15 years back (on top of systems not even supporting EFI, since there simply were none back then) didn't have this problem either. > Vendors will not fix their firmware. Disabling all runtime services is > not a reasonable alternative. Then we should see about adding support for "efi=no-time". > This is a firmware bug just like many others and needs to be worked > around by default like others. Except that you propose to work around it even when there is nothing to be worked around. I wouldn't mind switching on the workaround for known broken systems. > Anything else is actively damaging to the Xen community. People just > get frustrated when it doesn't work (especially if the problem has been > identify and a fix rejected upstream) and will move elsewhere instead. I'd leave specifying (or switching on by source patching) default workarounds to distros then. They know what their customers need, or what systems they expect their stuff to be run on. > Any situation where a command line override is required to make Xen boot > is a bug in Xen and should be fixed. This is why we have __init time > quirks. it doesn't matter if we have some truly horrendous workarounds; > Xen needs to be able to boot by default wherever possible. But requiring command line overrides for other things that are considered to not work by default is fine? I'm thinking of XenServer's universal disabling of XSAVE here... Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
On Wed, 2015-12-02 at 02:33 -0700, Jan Beulich wrote: > > > > On 01.12.15 at 20:26, wrote: > > On 01/12/15 17:26, Jan Beulich wrote: > > > > > > On 01.12.15 at 17:57, wrote: > > > > When EFI is used, don't use EFI's GetTime() to get the time, > > > > because it > > > > is broken on many platforms. From Linux commit 7efe665903d0 ("rtc: > > > > Disable EFI rtc for x86"): > > > > "Disable it explicitly for x86 so that we don't give users false > > > > hope that this driver will work - it won't, and your machine is > > > > likely > > > > to crash." > > > > > > > > Signed-off-by: Ross Lagerwall > > > NAK, since being conceptually wrong (and both of my systems work > > > fine). Vendors should get their firmware fixed, and by not using > > > runtime service functions we would give them even less reason to > > > do so. Until then we have "efi=no-rs". > > > > This is completely unreasonable. > > > > It is not conceptually wrong. > > I'm sorry, but no. Otherwise you mean to state that specifications > don't even need to be written, since if people don't play by them, > workarounds will get implemented anyway. In an ideal world such specifications would indeed be worth the paper they are written on. But the reality is that firmware implementations are often complete rubbish and the Xen project has nowhere near the leverage needed to actually get this fixed, even Linux lacks such leverage AFAICT. In reality only Microsoft (and perhaps to a lesser extent Apple) have any sort of ability to force things in this way (even so it is alleged in this thread that even Windows avoids the GetTime call as too broken in the field as well). Given we have no leverage in this it seems to me that punishing our users by ensuring that Xen won't work on the majority of EFI systems (based on anecdotal evidence from both Andrew and yourself) is counter productive. All it means is that at best we will get a continuous stream of bug reports from such users, or worse those users will just silently disappear and use something else which Just Works on their hardware and tell their friends that Xen is broken on modern hardware. There is only the smallest chance they will actually report a bug to their firmware vendor and even if they do there is basically an infinitesimal chance that the vendor will care in the slightest given that Windows presumably boots and works on that system. Realistically about all we can do is support efforts such as http://biosbits.org/ https://wiki.ubuntu.com/Kernel/Reference/fwts https://01.org/linux-uefi-validation and hope that they gain sufficient momentum, we certainly cannot effect any kind of change in the x86 firmware world directly ourselves by holding basic Xen functionality to ransom. > > GetTime() is very well known completely > > broken, especially after ExitBootServices(), to the point that every > > other EFI implementation (including windows) completely avoids it. > > I'm not sure about the order of things. I started working with EFI on > IA64, where using runtime services is a must. Windows started > using EFI on IA64 too, so I'm pretty convinced they used GetTime() > there. Whether they didn't on x86 because x86 implementations > were broken, or whether x86 implementations end up broken > because Windows never used those functions is simply an unknown. I don't think it is relevant to use why x86 UEFI implementations are broken in this way, just that it seems that the reality is that they mostly are. > > Vendors will not fix their firmware. Disabling all runtime services is > > not a reasonable alternative. > > Then we should see about adding support for "efi=no-time". And based on what I'm reading in this thread about the reliability of the time RS in the field it seems to me we should make it the default (on x86 at least) and provide efi=time to opt in. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
On Tue, Dec 08, 2015 at 11:02:16AM +, Ian Campbell wrote: > On Wed, 2015-12-02 at 02:33 -0700, Jan Beulich wrote: > > > > > On 01.12.15 at 20:26, wrote: > > > On 01/12/15 17:26, Jan Beulich wrote: > > > > > > > On 01.12.15 at 17:57, wrote: > > > > > When EFI is used, don't use EFI's GetTime() to get the time, > > > > > because it > > > > > is broken on many platforms. From Linux commit 7efe665903d0 ("rtc: > > > > > Disable EFI rtc for x86"): > > > > > "Disable it explicitly for x86 so that we don't give users false > > > > > hope that this driver will work - it won't, and your machine is > > > > > likely > > > > > to crash." > > > > > > > > > > Signed-off-by: Ross Lagerwall > > > > NAK, since being conceptually wrong (and both of my systems work > > > > fine). Vendors should get their firmware fixed, and by not using > > > > runtime service functions we would give them even less reason to > > > > do so. Until then we have "efi=no-rs". > > > > > > This is completely unreasonable. > > > > > > It is not conceptually wrong. > > > > I'm sorry, but no. Otherwise you mean to state that specifications > > don't even need to be written, since if people don't play by them, > > workarounds will get implemented anyway. > > In an ideal world such specifications would indeed be worth the paper they > are written on. But the reality is that firmware implementations are often > complete rubbish and the Xen project has nowhere near the leverage needed > to actually get this fixed, even Linux lacks such leverage AFAICT. > > In reality only Microsoft (and perhaps to a lesser extent Apple) have any > sort of ability to force things in this way (even so it is alleged in this > thread that even Windows avoids the GetTime call as too broken in the field > as well). We could confirm this right? We can run OVMF and instrument the 'GetTime' and see what Windows does? > > Given we have no leverage in this it seems to me that punishing our users > by ensuring that Xen won't work on the majority of EFI systems (based on > anecdotal evidence from both Andrew and yourself) is counter productive. > All it means is that at best we will get a continuous stream of bug reports > from such users, or worse those users will just silently disappear and use > something else which Just Works on their hardware and tell their friends > that Xen is broken on modern hardware. > > There is only the smallest chance they will actually report a bug to their > firmware vendor and even if they do there is basically an infinitesimal > chance that the vendor will care in the slightest given that Windows > presumably boots and works on that system. Plus I have had no luck actually finding a way to report this. Take for example my Lenovo hardware which has EFI holes everywhere.. > > Realistically about all we can do is support efforts such as > http://biosbits.org/ > https://wiki.ubuntu.com/Kernel/Reference/fwts > https://01.org/linux-uefi-validation > and hope that they gain sufficient momentum, we certainly cannot effect any > kind of change in the x86 firmware world directly ourselves by holding > basic Xen functionality to ransom. > > > > GetTime() is very well known completely > > > broken, especially after ExitBootServices(), to the point that every > > > other EFI implementation (including windows) completely avoids it. > > > > I'm not sure about the order of things. I started working with EFI on > > IA64, where using runtime services is a must. Windows started > > using EFI on IA64 too, so I'm pretty convinced they used GetTime() > > there. Whether they didn't on x86 because x86 implementations > > because Windows never used those functions is simply an unknown. > > I don't think it is relevant to use why x86 UEFI implementations are broken > in this way, just that it seems that the reality is that they mostly are. At least on Lenovo equipment they seem to assume a lot of things after ExitBootServicers - especially as we don't follow the same mechanism of switching to virtual addresses. (And Lenovo EFI code will blindly use virtual addresses after ExitBootServices which is why hack allows me to run Xen on Lenovo equipment). > > > > Vendors will not fix their firmware. Disabling all runtime services is > > > not a reasonable alternative. > > > > Then we should see about adding support for "efi=no-time". > > And based on what I'm reading in this thread about the reliability of the > time RS in the field it seems to me we should make it the default (on x86 > at least) and provide efi=time to opt in. > > Ian. > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
>>> On 14.12.15 at 17:53, wrote: > On Tue, Dec 08, 2015 at 11:02:16AM +, Ian Campbell wrote: >> In reality only Microsoft (and perhaps to a lesser extent Apple) have any >> sort of ability to force things in this way (even so it is alleged in this >> thread that even Windows avoids the GetTime call as too broken in the field >> as well). > > We could confirm this right? We can run OVMF and instrument the 'GetTime' > and > see what Windows does? I have no doubt MS doesn't use it. I believe though that it's broken in many case because they don't, not that they don't because it's broken. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
>>> On 08.12.15 at 12:02, wrote: > On Wed, 2015-12-02 at 02:33 -0700, Jan Beulich wrote: >> Then we should see about adding support for "efi=no-time". > > And based on what I'm reading in this thread about the reliability of the > time RS in the field it seems to me we should make it the default (on x86 > at least) and provide efi=time to opt in. Which would get us to actively violate at least early versions of the spec (the current one doesn't appear to be as strict anymore). I'm fine making it easy to work around the issue, but I'm against us doing the wrong thing by default. Apart from that, Ross, the patch you provided breaks on systems without CMOS clock (e.g. so called legacy free ones). This definitely can't be a compile time thing. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/time: Don't use EFI's GetTime call by default
>>> On 15.12.15 at 15:50, wrote: > On Tue, Dec 15, 2015 at 12:19:45AM -0700, Jan Beulich wrote: >> >>> On 08.12.15 at 12:02, wrote: >> > On Wed, 2015-12-02 at 02:33 -0700, Jan Beulich wrote: >> >> Then we should see about adding support for "efi=no-time". >> > >> > And based on what I'm reading in this thread about the reliability of the >> > time RS in the field it seems to me we should make it the default (on x86 >> > at least) and provide efi=time to opt in. >> >> Which would get us to actively violate at least early versions of the >> spec (the current one doesn't appear to be as strict anymore). I'm >> fine making it easy to work around the issue, but I'm against us doing >> the wrong thing by default. > > Are you OK with DMI quirks? Sure, why not? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel