[lkml]   [2018]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
SubjectRE: [PATCH V1 6/6] x86/efi: Introduce EFI_WARN_ON_ILLEGAL_ACCESSES
> How about shortening the name to just 'EFI_WARN_ON_ILLEGAL_ACCESS'?

Makes sense.. I will shorten it.

> > + bool "Warn about illegal memory accesses by firmware"
> > + depends on EFI
> From a distribution p-o-v, I would suggest that we set this CONFIG option only if
> we are in the EXPERT mode, as this need more thrashing with the behaviour we
> see on old, buggy EFI firmwares available on very old x86 machines. So
> something like:
> bool "Warn about illegal memory accesses by firmware" if EXPERT

Agreed. Although the feature is intended to warn about all these buggy firmware's
instead of silently working around (as we are doing presently), it needs more testing.
Hence, as you said, I think it's safe to have it as an EXPERT config option.

> > + help
> > + Enable this debug feature so that the kernel can detect illegal
> > + memory accesses by firmware and issue a warning. Also,
> > + 1. If the illegally accessed region is EFI_BOOT_SERVICES_<CODE/DATA>,
> > + the kernel fixes it up by mapping the requested region.


> > diff --git a/init/main.c b/init/main.c index
> > 3b4ada11ed52..dce0520861a1 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -730,7 +730,8 @@ asmlinkage __visible void __init start_kernel(void)
> > arch_post_acpi_subsys_init();
> > sfi_init_late();
> >
> > - if (efi_enabled(EFI_RUNTIME_SERVICES)) {
> > + if (efi_enabled(EFI_RUNTIME_SERVICES) &&
> Since this is an arch agnostic code leg, do we really want to introduce a generic
> here, without checking for whether we are actually running this on a
> x86 hardware, or alternatively we can consider moving
> CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES out of 'arch/x86/Kconfig' to
> 'arch/Kconfig' so that later it can be used on other archs like ARM64 as well.
> I think the later would be more useful..

Thanks for bringing this up. I see your point. I will refrain from polluting architecture
agnostic code. As we don't need efi_free_boot_services() at all, if EFI_WARN_ON_ILLEGAL_ACCESSES
is enabled, probably making changes to include/linux/efi.h would be better, I guess..

Moving this to arch/Kconfig could be too futuristic.. I guess.. because I am not sure
if this problem exists on ARM machines, if so, probably Ard could suggest something here.

 \ /
  Last update: 2018-08-10 19:34    [W:0.042 / U:55.600 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site