linux-kernel - Re: [PATCH] firmware/dmi_scan: generalize for use by other archs

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] [day] [month] [year] [list]

Date:	Wed, 27 Nov 2013 13:31:30 +0100
From:	Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	"x86@...nel.org" <x86@...nel.org>,
	"linux-ia64@...r.kernel.org" <linux-ia64@...r.kernel.org>,
	"mingo@...hat.com" <mingo@...hat.com>,
	Tony Luck <tony.luck@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Leif Lindholm <leif.lindholm@...aro.org>,
	Al Stone <al.stone@...aro.org>, Yi Li <yi.li@...aro.org>
Subject: Re: [PATCH] firmware/dmi_scan: generalize for use by other archs

On 27 November 2013 13:22, Grant Likely <grant.likely@...retlab.ca> wrote:
> On Wed, Nov 27, 2013 at 12:12 PM, Grant Likely
> <grant.likely@...retlab.ca> wrote:
>> On Mon, 25 Nov 2013 13:07:23 +0100, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
>>> Hello all,
>>>
>>> Resending this patch to a slightly wider audience.
>>>
>>> The point of this patch is reworking the dmi_scan code slightly so it
>>> can be reused on ARM and arm64.
>>> There are no functional changes for x86 or IA-64, just one open
>>> question, i.e., whether the non-EFI fallback probe should be performed
>>> on IA-64 in the first place.
>>>
>>> If I could get acks for this patch please (if there are no
>>> objections), I will propose it to be merged through the ARM and/or
>>> arm64 trees as part of the complete series to enable SMBIOS.
>>>
>>> On 21 November 2013 12:40, Ard Biesheuvel <ard.biesheuvel@...aro.org> wrote:
>>> > This patch makes a couple of changes to the SMBIOS/DMI scanning
>>> > code so it can be used on other archs (such as ARM and arm64):
>>> > (a) wrap the calls to ioremap()/iounmap(), this allows the use of a
>>> >     flavor of ioremap() more suitable for random unaligned access;
>>> > (b) allow the non-EFI fallback probe into hardcoded physical address
>>> >     0xF0000 to be disabled.
>>> >
>>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>>
>> Looks good.
>>
>> Reviewed-by: Grant Likely <grant.likely@...aro.org>
>
> Actually, I have one thought here...\
>
>>> >         } else {
>>> > -               p = dmi_ioremap(0xF0000, 0x10000);
>>> > +#ifdef DMI_SCAN_MACHINE_NON_EFI_FALLBACK
>>> > +               p = dmi_early_remap(0xF0000, 0x10000);
>>> >                 if (p == NULL)
>>> >                         goto error;
>>> >
>
> This is just ugly. #ifdef blocks inside of code is strongly
> discouraged. Instead, you can change the } else { line to:
>         } else if IS_ENABLED(CONFIG_DMI_NON_EFI_FALLBACK) {
>
> and then add an empty stanza to drivers/firmware/Kconfig:
>
> config DMI_NON_EFI_FALLBACK
>         bool
>
> And finally, make x86 and ia64 select it in arch/{ia64,x86}/Kconfig:
>
> config DMI
>         bool
>         select DMI_NON_EFI_FALLBACK
>         default y
>
> Which eliminates the DMI_SCAN_MACHINE_NON_EFI_FALLBACK #define from
> the header files.
>

I agree that this looks better. My approach was to make as few changes
to the x86 and ia64 code as possible, which included not introducing
new Kconfig symbols, especially as I can't easily test the ia64 build.

Will respin ...

Thanks,
Ard.
--
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/