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-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
