diff mbox

[7/7] efi_rtc: probe function error out in case no efi runtime enabled

Message ID 1408007953-15151-1-git-send-email-dyoung@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Young Aug. 14, 2014, 9:19 a.m. UTC
efi rtc depends on efi runtime services, so if efi runtime services are not
usable it should error out.

Without this patch rtc-efi will panic with 'noefi' boot

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 drivers/rtc/rtc-efi.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Mark Salter Sept. 22, 2014, 4:33 p.m. UTC | #1
On Thu, 2014-08-14 at 17:19 +0800, Dave Young wrote:
> efi rtc depends on efi runtime services, so if efi runtime services are not
> usable it should error out.
> 
> Without this patch rtc-efi will panic with 'noefi' boot

Is this still needed after commit da167ad7638759:

  rtc: ia64: allow other architectures to use EFI RTC

That should keep efi_rtc_probe from being called if runtime services
are not available.

> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  drivers/rtc/rtc-efi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index c4c3843..ca2487b 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev)
>  {
>  	struct rtc_device *rtc;
>  
> +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> +		return -ENODEV;
> +
>  	rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops,
>  					THIS_MODULE);
>  	if (IS_ERR(rtc))
Dave Young Sept. 23, 2014, 7:24 a.m. UTC | #2
Hi, Mark

On 09/22/14 at 12:33pm, Mark Salter wrote:
> On Thu, 2014-08-14 at 17:19 +0800, Dave Young wrote:
> > efi rtc depends on efi runtime services, so if efi runtime services are not
> > usable it should error out.
> > 
> > Without this patch rtc-efi will panic with 'noefi' boot
> 
> Is this still needed after commit da167ad7638759:
> 
>   rtc: ia64: allow other architectures to use EFI RTC
> 
> That should keep efi_rtc_probe from being called if runtime services
> are not available.

If that's the case then this patch should be unnecessary, I have no machine
to test it currently though.

> 
> > 
> > Signed-off-by: Dave Young <dyoung@redhat.com>
> > ---
> >  drivers/rtc/rtc-efi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> > index c4c3843..ca2487b 100644
> > --- a/drivers/rtc/rtc-efi.c
> > +++ b/drivers/rtc/rtc-efi.c
> > @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev)
> >  {
> >  	struct rtc_device *rtc;
> >  
> > +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > +		return -ENODEV;
> > +
> >  	rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops,
> >  					THIS_MODULE);
> >  	if (IS_ERR(rtc))
> 
>
Matt Fleming Sept. 25, 2014, 9:18 p.m. UTC | #3
On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote:
> 
> Is this still needed after commit da167ad7638759:
> 
>   rtc: ia64: allow other architectures to use EFI RTC

Umm... we should really look at disabling this for x86, since the EFI
time functions don't work on a bunch of machines I've got here.

And it looks like someone enabled this driver and triggered an oops on
their 32-bit EFI tablet,

  https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21
Mark Salter Sept. 25, 2014, 9:38 p.m. UTC | #4
On Thu, 2014-09-25 at 22:18 +0100, Matt Fleming wrote:
> On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote:
> > 
> > Is this still needed after commit da167ad7638759:
> > 
> >   rtc: ia64: allow other architectures to use EFI RTC
> 
> Umm... we should really look at disabling this for x86, since the EFI
> time functions don't work on a bunch of machines I've got here.
> 
> And it looks like someone enabled this driver and triggered an oops on
> their 32-bit EFI tablet,
> 
>   https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21

Previously, it depended on IA64. If it's not worth the trouble for
x86 then I'd say have it depend on !X86 also. Newer EFI-supporting
architectures are unlikely to have the same legacy issues as x86
so I don't think it'll end up being an ever growing list.
Matt Fleming Sept. 28, 2014, 4:41 p.m. UTC | #5
On Tue, 23 Sep, at 03:24:26PM, Dave Young wrote:
> 
> If that's the case then this patch should be unnecessary, I have no
> machine to test it currently though.

Thanks, I've dropped this from the v3.18 queue.
Matt Fleming Oct. 3, 2014, 12:11 p.m. UTC | #6
(Argh, this was sat in my outbox for over a week)

On Thu, 25 Sep, at 05:38:50PM, Mark Salter wrote:
> On Thu, 2014-09-25 at 22:18 +0100, Matt Fleming wrote:
> > On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote:
> > > 
> > > Is this still needed after commit da167ad7638759:
> > > 
> > >   rtc: ia64: allow other architectures to use EFI RTC
> > 
> > Umm... we should really look at disabling this for x86, since the EFI
> > time functions don't work on a bunch of machines I've got here.
> > 
> > And it looks like someone enabled this driver and triggered an oops on
> > their 32-bit EFI tablet,
> > 
> >   https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21
> 
> Previously, it depended on IA64. If it's not worth the trouble for
> x86 then I'd say have it depend on !X86 also. Newer EFI-supporting
> architectures are unlikely to have the same legacy issues as x86
> so I don't think it'll end up being an ever growing list.
 
Yeah that's what I was thinking. For x86, it's the age old "Windows
doesn't use it, so it was never validated" issue.

I'll send an update.
diff mbox

Patch

diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index c4c3843..ca2487b 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -191,6 +191,9 @@  static int __init efi_rtc_probe(struct platform_device *dev)
 {
 	struct rtc_device *rtc;
 
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+		return -ENODEV;
+
 	rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops,
 					THIS_MODULE);
 	if (IS_ERR(rtc))