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]

Date:	Mon, 19 May 2014 23:02:55 +0200
From:	Daniel Kiper <daniel.kiper@...cle.com>
To:	David Vrabel <david.vrabel@...rix.com>
Cc:	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
	x86@...nel.org, xen-devel@...ts.xenproject.org,
	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 Mon, May 19, 2014 at 04:58:32PM +0100, David Vrabel wrote:
> 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.

Do you wish EFI_NO_DIRECT? I understand your idea but I would like to avoid this
name because it is quite difficult to read e.g. !efi_enabled(EFI_NO_DIRECT).
You must think a bit to know what is going on. However, maybe you have a better idea?

> 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.

OK.

> > +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?

It is correct. As I said earlier: in case of !efi_enabled(EFI_DIRECT) some
structures are created artificially and they live in virtual address space.
So that is why they should not be mapped.

> > --- 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?

I was going to have EFI_DIRECT close to EFI_BOOT which is quite generic
and platform independent name like EFI_BOOT. However, I do not insist
on having it in that place.

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