lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1442589968.28550.18.camel@redhat.com>
Date:	Fri, 18 Sep 2015 11:26:08 -0400
From:	Mark Salter <msalter@...hat.com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Catalin Marinas <Catalin.Marinas@....com>,
	Will Deacon <Will.Deacon@....com>,
	Matt Fleming <matt.fleming@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: Re: [PATCH] arm64: dmi: initialize DMI earlier in boot

On Fri, 2015-09-18 at 15:31 +0100, Mark Rutland wrote:
> On Fri, Sep 18, 2015 at 03:14:26PM +0100, Mark Salter wrote:
> > Currently, DMI initialization takes place in a core initcall. This
> > limits how early in boot the kernel can make DMI-based decisions
> > about firmware/hardware quirks. This patch moves DMI initialization
> > to setup_arch() so that DMI info is available before initcalls run.
> 
> Which firmware/hardware quirks in particular necessitate moving this?

The thing that prompted it for me was my testing of Lorenzo's parking
protocol patch. I have a platform which has a quirky implementation of
that and which has modified firmware to implement it according to the
latest spec. So I need to tell the difference before secondary cores
were brought up. This doesn't really effect upstream in that there is
no need for backwards compatibility because there is no upstream
parking protocol support currently. But it seemed reasonable to post
this patch anyway since having DMI available earlier adds a little
more utility, generally.

> > ---
> >  arch/arm64/include/asm/dmi.h | 19 ++++++++++++++++---
> >  arch/arm64/kernel/efi.c      | 15 ---------------
> >  arch/arm64/kernel/setup.c    |  5 +++++
> >  3 files changed, 21 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/dmi.h b/arch/arm64/include/asm/dmi.h
> > index 69d37d8..e6389fd 100644
> > --- a/arch/arm64/include/asm/dmi.h
> > +++ b/arch/arm64/include/asm/dmi.h
> > @@ -16,16 +16,29 @@
> >  
> >  #include <linux/io.h>
> >  #include <linux/slab.h>
> > +#include <linux/memblock.h>
> 
> Nit: please keep includes ordered (given they were already).

Alphabetically? (some maintainers like them ordered by name length :)

> 
> [...]
> 
> > @@ -413,20 +412,6 @@ static int __init arm64_enable_runtime_services(void)
> >  }
> >  early_initcall(arm64_enable_runtime_services);
> >  
> > -static int __init arm64_dmi_init(void)
> > -{
> > -	/*
> > -	 * On arm64, DMI depends on UEFI, and dmi_scan_machine() needs to
> > -	 * be called early because dmi_id_init(), which is an arch_initcall
> > -	 * itself, depends on dmi_scan_machine() having been called already.
> > -	 */
> > -	dmi_scan_machine();
> > -	if (dmi_available)
> > -		dmi_set_dump_stack_arch_desc();
> > -	return 0;
> > -}
> > -core_initcall(arm64_dmi_init);
> > -
> >  static void efi_set_pgd(struct mm_struct *mm)
> >  {
> >  	if (mm == &init_mm)
> > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> > index dc9eb5f..200c2e9 100644
> > --- a/arch/arm64/kernel/setup.c
> > +++ b/arch/arm64/kernel/setup.c
> > @@ -46,6 +46,7 @@
> >  #include <linux/efi.h>
> >  #include <linux/personality.h>
> >  #include <linux/psci.h>
> > +#include <linux/dmi.h>
> >  
> >  #include <asm/acpi.h>
> >  #include <asm/fixmap.h>
> > @@ -436,6 +437,10 @@ void __init setup_arch(char **cmdline_p)
> >  	relocate_initrd();
> >  	request_standard_resources();
> >  
> > +	dmi_scan_machine();
> > +	if (dmi_available)
> > +		dmi_set_dump_stack_arch_desc();
> 
> We should have a comment as to why this needs to happen so early (much
> like we used to).

The original comment was about why it should be core_initcall rather than
some other initcall, which is why I dropped it altogether.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ