Re: [PATCH v2 13/16] ARM: XEN: Set EFI_PARAVIRT if Xen supports EFI

From: Mark Rutland
Date: Mon Jan 18 2016 - 05:58:02 EST


On Fri, Jan 15, 2016 at 02:55:26PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>
> Check if there is "uefi" node in the DT. If so, set EFI_PARAVIRT flag.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> ---
> arch/arm/xen/enlighten.c | 23 +++++++++++++++++++++++
> arch/arm64/kernel/efi.c | 5 +++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 5d0fe68..485e117 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -31,6 +31,7 @@
> #include <linux/timekeeping.h>
> #include <linux/timekeeper_internal.h>
> #include <linux/acpi.h>
> +#include <linux/efi.h>
>
> #include <linux/mm.h>
>
> @@ -248,6 +249,19 @@ static int __init fdt_find_xen_node(unsigned long node, const char *uname,
> return 0;
> }
>
> +static int __init fdt_find_uefi_node(unsigned long node, const char *uname,
> + int depth, void *data)
> +{
> + bool *found = data;
> +
> + if (depth != 2 || strcmp(uname, "uefi") != 0)
> + return 0;
> +
> + *found = true;
> +
> + return 1;
> +}

I don't like this. What if we had to add a uefi node in the !Xen case
for some reason?

You want to look for /hypervisor/uefi, specifically when the hypervisor
compatible contains "xen,xen".

It would be better to find the "/hypervisor" node, checking for the
compatible string, then walk within that in the Xen-specific init
routine.

> +
> /*
> * see Documentation/devicetree/bindings/arm/xen.txt for the
> * documentation of the Xen Device Tree format.
> @@ -255,6 +269,8 @@ static int __init fdt_find_xen_node(unsigned long node, const char *uname,
> #define GRANT_TABLE_PHYSADDR 0
> void __init xen_early_init(void)
> {
> + bool uefi_found = false;
> +
> of_scan_flat_dt(fdt_find_xen_node, NULL);
> if (!xen_node.found) {
> pr_debug("No Xen support\n");
> @@ -279,6 +295,13 @@ void __init xen_early_init(void)
>
> if (!console_set_on_cmdline && !xen_initial_domain())
> add_preferred_console("hvc", 0, NULL);
> +
> + if (IS_ENABLED(CONFIG_XEN_EFI)) {
> + /* Check if Xen support UEFI */
> + of_scan_flat_dt(fdt_find_uefi_node, &uefi_found);
> + if (uefi_found)
> + set_bit(EFI_PARAVIRT, &efi.flags);
> + }
> }

This alone is insufficient given that we haven't parsed the rest of the
/hypervisor/uefi properties. Is the kernel resilient such that this
patch alone will not result in a panic?

I think it would be best for this to be in the same patch as the rest of
the hypervisor UEFI property parsing, being unified with that.

Mark.

> static int __init xen_guest_init(void)
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 4eeb171..16c6b72 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -288,6 +288,11 @@ static int __init arm64_enable_runtime_services(void)
> return 0;
> }
>
> + if (efi_enabled(EFI_PARAVIRT)) {
> + pr_info("EFI runtime services access via paravirt.\n");
> + return -1;
> + }
> +
> pr_info("Remapping and enabling EFI services.\n");
>
> mapsize = memmap.map_end - memmap.map;
> --
> 2.0.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>