linux-kernel - Re: [PATCH v4 1/5] efi: Introduce EFI

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: <537A2A28.4010903@citrix.com>
Date:	Mon, 19 May 2014 16:58:32 +0100
From:	David Vrabel <david.vrabel@...rix.com>
To:	Daniel Kiper <daniel.kiper@...cle.com>,
	<linux-efi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<x86@...nel.org>, <xen-devel@...ts.xenproject.org>
CC:	<boris.ostrovsky@...cle.com>, <eshelton@...ox.com>,
	<hpa@...or.com>, <ian.campbell@...rix.com>, <jbeulich@...e.com>,
	<jeremy@...p.org>, <konrad.wilk@...cle.com>,
	<matt.fleming@...el.com>, <mingo@...hat.com>,
	<mjg59@...f.ucam.org>, <stefano.stabellini@...citrix.com>,
	<tglx@...utronix.de>
Subject: Re: [PATCH v4 1/5] efi: Introduce EFI_DIRECT flag

On 16/05/14 21:41, Daniel Kiper wrote:
> Introduce EFI_DIRECT flag. If it is set this means that Linux
> Kernel has direct access to EFI infrastructure. If not then
> kernel runs on EFI platform but it has not direct control
> on EFI stuff. This functionality is used in Xen dom0.

This is backwards.  It should flag should indicate the weird platform
not the standard, default one.

You also need to explain why this is needed rather than presenting a
more complete EFI interface to PV guests.  And you should explain why
each use is necessary.

> +static void __init __iomem *efi_early_ioremap(resource_size_t phys_addr,
> +							unsigned long size)
> +{
> +	if (efi_enabled(EFI_DIRECT))
> +		return early_ioremap(phys_addr, size);
> +
> +	return (__force void __iomem *)phys_addr;
> +}

Er...  Perhaps you mean BUG_ON(!efi_enabled(EFI_DIRECT))? Or return NULL?

> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -893,12 +893,13 @@ extern int __init efi_setup_pcdp_console(char *);
>   * possible, remove EFI-related code altogether.
>   */
>  #define EFI_BOOT		0	/* Were we booted from EFI? */
> -#define EFI_SYSTEM_TABLES	1	/* Can we use EFI system tables? */
> -#define EFI_CONFIG_TABLES	2	/* Can we use EFI config tables? */
> -#define EFI_RUNTIME_SERVICES	3	/* Can we use runtime services? */
> -#define EFI_MEMMAP		4	/* Can we use EFI memory map? */
> -#define EFI_64BIT		5	/* Is the firmware 64-bit? */
> -#define EFI_ARCH_1		6	/* First arch-specific bit */
> +#define EFI_DIRECT		1	/* Can we access EFI directly? */
> +#define EFI_SYSTEM_TABLES	2	/* Can we use EFI system tables? */
> +#define EFI_CONFIG_TABLES	3	/* Can we use EFI config tables? */
> +#define EFI_RUNTIME_SERVICES	4	/* Can we use runtime services? */
> +#define EFI_MEMMAP		5	/* Can we use EFI memory map? */
> +#define EFI_64BIT		6	/* Is the firmware 64-bit? */
> +#define EFI_ARCH_1		7	/* First arch-specific bit */

Unnecessary shuffling of these values.  Why didn't you stick it after
EFI_64BIT?

David
--
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/