Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Sun, 2012-11-04 at 17:44 +, Corentin Chary wrote: > On Sun, Nov 4, 2012 at 5:35 PM, Matt Fleming wrote: > > From: Matt Fleming > > > > We've started getting reports of users seeing Machine Check Exceptions > > when booting their Samsung laptops in UEFI mode, > > > > https://bugzilla.kernel.org/show_bug.cgi?id=47121 > > > > This module seems to be the culprit as it's grovelling around in the > > 0xf region which has no mapping in either the e820 or EFI memory > > maps on the affected machines. > > > > Reported-by: Alessandro Crismani > > Reported-by: Mikhail Bakhterev > > Tested-by: Patrick H > > Cc: H. Peter Anvin > > Cc: Matthew Garrett > > Cc: Corentin Chary > > Cc: sta...@vger.kernel.org > > Signed-off-by: Matt Fleming > > --- > > drivers/platform/x86/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index c86bae8..0fe5200 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -717,7 +717,7 @@ config XO15_EBOOK > > > > config SAMSUNG_LAPTOP > > tristate "Samsung Laptop driver" > > - depends on X86 > > + depends on X86 && !EFI > > depends on RFKILL || RFKILL = n > > depends on BACKLIGHT_CLASS_DEVICE > > select LEDS_CLASS > > -- > > 1.7.11.7 > > > > Acked-by: Corentin Chary > > Matthew could you merge this in your tree ? Thanks. > > Matt, any idea if SABI doesn't exist on these machines, or is just > mapped somewhere else ? > By any luck, could Samsung tell you how to safely detect SABI aware machines ? I don't know whether or not SABI exists on these machines and I don't have one of them to try and figure that out, sorry. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Sun, 2012-11-04 at 19:37 +, Alan Cox wrote: > > Acked-by: Corentin Chary > > This is totally bogus and prevents users build a kernel which can work in > either mode. As such its a regression. > > Do the detection check at runtime. If it was booted via EFI then don't > grovel in places you shouldn't. Indeed its possible EFI should reserve > those memory regions ? The kernel would have to reserve the gaps in the memory mappings since there is no mapping in the EFI memory map for 0xf. There is no support for that currently AFAIK. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Mon, 2012-11-05 at 12:07 +, Alan Cox wrote: > > There is the 'efi_enabled' variable, but it doesn't strictly mean > > "this_is_a_uefi_system()", it actually means "Do we have EFI runtime > > services?". The whole thing is a bit of a mess and I'm planning on > > cleaning it up this week. > > > As far as I can understand it we should be reserving those areas on a > UEFI booted machine and just marking them as busy so that they are not > available to any drivers to go peering into. Yes, that would be the best way to resolve this going forward. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Mon, 2012-11-05 at 11:37 +0100, Greg KH wrote: > On Sun, Nov 04, 2012 at 05:35:06PM +, Matt Fleming wrote: > > From: Matt Fleming > > > > We've started getting reports of users seeing Machine Check Exceptions > > when booting their Samsung laptops in UEFI mode, > > > > https://bugzilla.kernel.org/show_bug.cgi?id=47121 > > > > This module seems to be the culprit as it's grovelling around in the > > 0xf region which has no mapping in either the e820 or EFI memory > > maps on the affected machines. > > So, does this mean that if we try to ioremap a memory location in the > kernel, like this driver is, it will not fail, but, when accessing the > memory, bad things happen? Yes. I'm not sure exactly what sequence of memory accesses is causing the MCEs because the above bug report states that the MCEs were frequent but intermittent and I don't have any hardware to reproduce this on. > That's not good, shouldn't the call to ioremap_nocache() have failed > originally? That sounds like a core EFI/platform bug here, and one that > you might run into other places. I think it's an x86 bug. AFAIK it's entirely possible to ioremap arbitrary memory addresses whether or not the firmware/BIOS knows about them (by which I mean, includes a mapping in the e820 map). It's just that traditionally the firmware didn't throw MCEs when you peeked at some part of the address map. The < 1MB region of the x86 address map also fills me with visions of voodoo. Jacob Shin and Yinghai Lu have been working on some patches that avoid mapping the holes (the address regions for which there is no entry in the e820 memmap) and I think that could be used to address this problem. > Shouldn't fixing that be the real fix? Absolutely, and it was always my intention to fix this properly so that I didn't have the same bug reported to me over and over again. However, initially I was more concerned about writing a fix that was good enough and small enough to be marked for stable. Messing with the kernel memory map, particularly for EFI systems, has been fraught with peril in the past. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
> There is the 'efi_enabled' variable, but it doesn't strictly mean > "this_is_a_uefi_system()", it actually means "Do we have EFI runtime > services?". The whole thing is a bit of a mess and I'm planning on > cleaning it up this week. As far as I can understand it we should be reserving those areas on a UEFI booted machine and just marking them as busy so that they are not available to any drivers to go peering into. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Mon, 2012-11-05 at 11:30 +0100, Greg KH wrote: > Odds are, the windows driver just isn't even loaded on the newer > machines, as ACPI works just fine for this. But, we don't have the > option of shipping custom systems for different laptops like Samsung > does, so we have to probe for this somehow. > > Initally we were looking at the DMI strings for specific laptop models, > but that got annoying as we had to keep adding new models. So we now > just check the memory locations for all Samsung laptops, which was > working fine. > > What is the problem if we try to access this memory on UEFI machines? > What is the error that is caused? Machine Check Exceptions are generated. > Is there any "this_is_a_uefi_system()" type call drivers can make to > just opt-out if that call is true? There is the 'efi_enabled' variable, but it doesn't strictly mean "this_is_a_uefi_system()", it actually means "Do we have EFI runtime services?". The whole thing is a bit of a mess and I'm planning on cleaning it up this week. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Sun, Nov 04, 2012 at 05:35:06PM +, Matt Fleming wrote: > From: Matt Fleming > > We've started getting reports of users seeing Machine Check Exceptions > when booting their Samsung laptops in UEFI mode, > > https://bugzilla.kernel.org/show_bug.cgi?id=47121 > > This module seems to be the culprit as it's grovelling around in the > 0xf region which has no mapping in either the e820 or EFI memory > maps on the affected machines. So, does this mean that if we try to ioremap a memory location in the kernel, like this driver is, it will not fail, but, when accessing the memory, bad things happen? That's not good, shouldn't the call to ioremap_nocache() have failed originally? That sounds like a core EFI/platform bug here, and one that you might run into other places. Shouldn't fixing that be the real fix? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Mon, Nov 05, 2012 at 09:12:01AM +, Corentin Chary wrote: > On Sun, Nov 4, 2012 at 7:37 PM, Alan Cox wrote: > >> Acked-by: Corentin Chary > > > > This is totally bogus and prevents users build a kernel which can work in > > either mode. As such its a regression. > > Arg.. Sorry for that, I didn't realized that CONFIG_EFI=y was not > something rare these days. > > > Do the detection check at runtime. If it was booted via EFI then don't > > grovel in places you shouldn't. Indeed its possible EFI should reserve > > those memory regions ? > > I wonder how the windows driver works in this case.. Maybe they use > something completly different, and the SABI interface is still there > because nobody removed/disabled it ? In this case it's probably not a > good idea to use it on these machines since the implementation is > likely to be completly broken. Odds are, the windows driver just isn't even loaded on the newer machines, as ACPI works just fine for this. But, we don't have the option of shipping custom systems for different laptops like Samsung does, so we have to probe for this somehow. Initally we were looking at the DMI strings for specific laptop models, but that got annoying as we had to keep adding new models. So we now just check the memory locations for all Samsung laptops, which was working fine. What is the problem if we try to access this memory on UEFI machines? What is the error that is caused? Is there any "this_is_a_uefi_system()" type call drivers can make to just opt-out if that call is true? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Sun, Nov 4, 2012 at 7:37 PM, Alan Cox wrote: >> Acked-by: Corentin Chary > > This is totally bogus and prevents users build a kernel which can work in > either mode. As such its a regression. Arg.. Sorry for that, I didn't realized that CONFIG_EFI=y was not something rare these days. > Do the detection check at runtime. If it was booted via EFI then don't > grovel in places you shouldn't. Indeed its possible EFI should reserve > those memory regions ? I wonder how the windows driver works in this case.. Maybe they use something completly different, and the SABI interface is still there because nobody removed/disabled it ? In this case it's probably not a good idea to use it on these machines since the implementation is likely to be completly broken. -- Corentin Chary http://xf.iksaif.net -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Sun, 2012-11-04 at 09:47 -0800, Jonathan Nieder wrote: > Matt Fleming wrote: > > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -717,7 +717,7 @@ config XO15_EBOOK > > > > config SAMSUNG_LAPTOP > > tristate "Samsung Laptop driver" > > - depends on X86 > > + depends on X86 && !EFI > > That means distros would just not get the samsung-laptop driver. > Is there a runtime check that could be used instead? Well, the closest thing we have at the moment is the 'efi_enabled' variable, but that doesn't actually mean "We were booted from EFI?", it means "Do we have EFI runtime services?", and that's not a broad enough check for this case. We don't have access to the EFI runtime services when a 64-bit kernel is booted from 32-bit EFI firmware or vice-versa. Notably the chromebooks use this scheme. And seeing as Samsung make chromebooks, I'm not convinced we won't hit that case. But yeah, you've got a valid point. Clearly we need a way to check this at runtime. I'll repsin this patch. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
> Acked-by: Corentin Chary This is totally bogus and prevents users build a kernel which can work in either mode. As such its a regression. Do the detection check at runtime. If it was booted via EFI then don't grovel in places you shouldn't. Indeed its possible EFI should reserve those memory regions ? Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
Matt Fleming wrote: > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -717,7 +717,7 @@ config XO15_EBOOK > > config SAMSUNG_LAPTOP > tristate "Samsung Laptop driver" > - depends on X86 > + depends on X86 && !EFI That means distros would just not get the samsung-laptop driver. Is there a runtime check that could be used instead? Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] samsung-laptop: Disable if CONFIG_EFI=y
On Sun, Nov 4, 2012 at 5:35 PM, Matt Fleming wrote: > From: Matt Fleming > > We've started getting reports of users seeing Machine Check Exceptions > when booting their Samsung laptops in UEFI mode, > > https://bugzilla.kernel.org/show_bug.cgi?id=47121 > > This module seems to be the culprit as it's grovelling around in the > 0xf region which has no mapping in either the e820 or EFI memory > maps on the affected machines. > > Reported-by: Alessandro Crismani > Reported-by: Mikhail Bakhterev > Tested-by: Patrick H > Cc: H. Peter Anvin > Cc: Matthew Garrett > Cc: Corentin Chary > Cc: sta...@vger.kernel.org > Signed-off-by: Matt Fleming > --- > drivers/platform/x86/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index c86bae8..0fe5200 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -717,7 +717,7 @@ config XO15_EBOOK > > config SAMSUNG_LAPTOP > tristate "Samsung Laptop driver" > - depends on X86 > + depends on X86 && !EFI > depends on RFKILL || RFKILL = n > depends on BACKLIGHT_CLASS_DEVICE > select LEDS_CLASS > -- > 1.7.11.7 > Acked-by: Corentin Chary Matthew could you merge this in your tree ? Thanks. Matt, any idea if SABI doesn't exist on these machines, or is just mapped somewhere else ? By any luck, could Samsung tell you how to safely detect SABI aware machines ? -- Corentin Chary http://xf.iksaif.net -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] samsung-laptop: Disable if CONFIG_EFI=y
From: Matt Fleming We've started getting reports of users seeing Machine Check Exceptions when booting their Samsung laptops in UEFI mode, https://bugzilla.kernel.org/show_bug.cgi?id=47121 This module seems to be the culprit as it's grovelling around in the 0xf region which has no mapping in either the e820 or EFI memory maps on the affected machines. Reported-by: Alessandro Crismani Reported-by: Mikhail Bakhterev Tested-by: Patrick H Cc: H. Peter Anvin Cc: Matthew Garrett Cc: Corentin Chary Cc: sta...@vger.kernel.org Signed-off-by: Matt Fleming --- drivers/platform/x86/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index c86bae8..0fe5200 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -717,7 +717,7 @@ config XO15_EBOOK config SAMSUNG_LAPTOP tristate "Samsung Laptop driver" - depends on X86 + depends on X86 && !EFI depends on RFKILL || RFKILL = n depends on BACKLIGHT_CLASS_DEVICE select LEDS_CLASS -- 1.7.11.7 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
