Re: [Xen-devel] [PATCH v4 1/5] efi: Introduce EFI_DIRECT flag

[David Vrabel <david.vrabel@xxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel