Re: [Xen-devel] [PATCH v2] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Stefano Stabellini]

On Fri, 13 May 2016, Matt Fleming wrote:
> (Including more folks, quoting entire patch)
> 
> On Thu, 12 May, at 08:19:54PM, Shannon Zhao wrote:
> > From: Shannon Zhao 
> > 
> > The EFI DT parameters for bare metal are located under /chosen node,
> > while for Xen Dom0 they are located under /hyperviosr/uefi node. These
> > parameters under /chosen and /hyperviosr/uefi are not expected to appear
> > at the same time.
> > Parse these EFI parameters and initialize EFI like the way for bare
> > metal except the runtime services because the runtime services for Xen
> > Dom0 are available through hypercalls and they are always enabled. So it
> > sets the EFI_RUNTIME_SERVICES flag if it finds /hyperviosr/uefi node and
> > bails out in arm_enable_runtime_services() when EFI_RUNTIME_SERVICES
> > flag is set already.
> > 
> > CC: Matt Fleming 
> > Signed-off-by: Shannon Zhao 
> > ---
> > v2: rebase it on top of EFI and Xen next branch, add some comments to
> > explain the codes. 
> > ---
> >  arch/arm/xen/enlighten.c   | 22 +++
> >  drivers/firmware/efi/arm-runtime.c |  5 +++
> >  drivers/firmware/efi/efi.c | 81 
> > ++
> >  3 files changed, 92 insertions(+), 16 deletions(-)
>  
> This looks good to me. Thanks for all the work Shannon.
> 
> Stefan, OK, let's figure out how to deal with this.
> 
> What I've done is merge xen/linux-next into the 'efi/xen-arm' topic
> branch and applied this patch here,
> 
>   
> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
> 
> Yes, the merge includes some Xen patches that are not actually needed
> for any of the EFI patches, but Linus has complained about picking
> random commits of a tree to merge. At least this way this is all the
> Xen linux-next material.
> 
> If you're OK with this, and assuming no one else has any preferred
> methods, I'll send out a pull request to tip over the weekend.

I am OK with it. Thanks!


> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 13e3e9f..d4f36eb 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -15,7 +15,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -261,6 +263,19 @@ static int __init fdt_find_hyper_node(unsigned long 
> > node, const char *uname,
> > !strncmp(hyper_node.prefix, s, strlen(hyper_node.prefix)))
> > hyper_node.version = s + strlen(hyper_node.prefix);
> >  
> > +   /*
> > +* Check if Xen supports EFI by checking whether there is the
> > +* "/hypervisor/uefi" node in DT. If so, runtime services are available
> > +* through proxy functions (e.g. in case of Xen dom0 EFI implementation
> > +* they call special hypercall which executes relevant EFI functions)
> > +* and that is why they are always enabled.
> > +*/
> > +   if (IS_ENABLED(CONFIG_XEN_EFI)) {
> > +   if ((of_get_flat_dt_subnode_by_name(node, "uefi") > 0) &&
> > +   !efi_runtime_disabled())
> > +   set_bit(EFI_RUNTIME_SERVICES, );
> > +   }
> > +
> > return 0;
> >  }
> >  
> > @@ -352,6 +367,13 @@ static int __init xen_guest_init(void)
> > return -ENODEV;
> > }
> >  
> > +   /*
> > +* The fdt parsing codes have set EFI_RUNTIME_SERVICES if Xen EFI
> > +* parameters are found. Force enable runtime services.
> > +*/
> > +   if (efi_enabled(EFI_RUNTIME_SERVICES))
> > +   xen_efi_runtime_setup();
> > +
> > shared_info_page = (struct shared_info *)get_zeroed_page(GFP_KERNEL);
> >  
> > if (!shared_info_page) {
> > diff --git a/drivers/firmware/efi/arm-runtime.c 
> > b/drivers/firmware/efi/arm-runtime.c
> > index 17ccf0a..c394b81 100644
> > --- a/drivers/firmware/efi/arm-runtime.c
> > +++ b/drivers/firmware/efi/arm-runtime.c
> > @@ -107,6 +107,11 @@ static int __init arm_enable_runtime_services(void)
> > return 0;
> > }
> >  
> > +   if (efi_enabled(EFI_RUNTIME_SERVICES)) {
> > +   pr_info("EFI runtime services access via paravirt.\n");
> > +   return 0;
> > +   }
> > +
> > pr_info("Remapping and enabling EFI services.\n");
> >  
> > mapsize = efi.memmap.map_end - efi.memmap.map;
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index 05509f3..1c6f9dd 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -472,12 +472,14 @@ device_initcall(efi_load_efivars);
> > FIELD_SIZEOF(struct efi_fdt_params, field) \
> > }
> >  
> > -static __initdata struct {
> > +struct params {
> > const char name[32];
> > const char propname[32];
> > int offset;
> > int size;
> > -} dt_params[] = {
> > +};
> > +
> > +static __initdata struct params fdt_params[] = {
> > UEFI_PARAM("System Table", 

Re: [Xen-devel] [PATCH v2] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Matt Fleming]

(Including more folks, quoting entire patch)

On Thu, 12 May, at 08:19:54PM, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> The EFI DT parameters for bare metal are located under /chosen node,
> while for Xen Dom0 they are located under /hyperviosr/uefi node. These
> parameters under /chosen and /hyperviosr/uefi are not expected to appear
> at the same time.
> Parse these EFI parameters and initialize EFI like the way for bare
> metal except the runtime services because the runtime services for Xen
> Dom0 are available through hypercalls and they are always enabled. So it
> sets the EFI_RUNTIME_SERVICES flag if it finds /hyperviosr/uefi node and
> bails out in arm_enable_runtime_services() when EFI_RUNTIME_SERVICES
> flag is set already.
> 
> CC: Matt Fleming 
> Signed-off-by: Shannon Zhao 
> ---
> v2: rebase it on top of EFI and Xen next branch, add some comments to
> explain the codes. 
> ---
>  arch/arm/xen/enlighten.c   | 22 +++
>  drivers/firmware/efi/arm-runtime.c |  5 +++
>  drivers/firmware/efi/efi.c | 81 
> ++
>  3 files changed, 92 insertions(+), 16 deletions(-)
 
This looks good to me. Thanks for all the work Shannon.

Stefan, OK, let's figure out how to deal with this.

What I've done is merge xen/linux-next into the 'efi/xen-arm' topic
branch and applied this patch here,

  
https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen

Yes, the merge includes some Xen patches that are not actually needed
for any of the EFI patches, but Linus has complained about picking
random commits of a tree to merge. At least this way this is all the
Xen linux-next material.

If you're OK with this, and assuming no one else has any preferred
methods, I'll send out a pull request to tip over the weekend.

> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 13e3e9f..d4f36eb 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -15,7 +15,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -261,6 +263,19 @@ static int __init fdt_find_hyper_node(unsigned long 
> node, const char *uname,
>   !strncmp(hyper_node.prefix, s, strlen(hyper_node.prefix)))
>   hyper_node.version = s + strlen(hyper_node.prefix);
>  
> + /*
> +  * Check if Xen supports EFI by checking whether there is the
> +  * "/hypervisor/uefi" node in DT. If so, runtime services are available
> +  * through proxy functions (e.g. in case of Xen dom0 EFI implementation
> +  * they call special hypercall which executes relevant EFI functions)
> +  * and that is why they are always enabled.
> +  */
> + if (IS_ENABLED(CONFIG_XEN_EFI)) {
> + if ((of_get_flat_dt_subnode_by_name(node, "uefi") > 0) &&
> + !efi_runtime_disabled())
> + set_bit(EFI_RUNTIME_SERVICES, );
> + }
> +
>   return 0;
>  }
>  
> @@ -352,6 +367,13 @@ static int __init xen_guest_init(void)
>   return -ENODEV;
>   }
>  
> + /*
> +  * The fdt parsing codes have set EFI_RUNTIME_SERVICES if Xen EFI
> +  * parameters are found. Force enable runtime services.
> +  */
> + if (efi_enabled(EFI_RUNTIME_SERVICES))
> + xen_efi_runtime_setup();
> +
>   shared_info_page = (struct shared_info *)get_zeroed_page(GFP_KERNEL);
>  
>   if (!shared_info_page) {
> diff --git a/drivers/firmware/efi/arm-runtime.c 
> b/drivers/firmware/efi/arm-runtime.c
> index 17ccf0a..c394b81 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -107,6 +107,11 @@ static int __init arm_enable_runtime_services(void)
>   return 0;
>   }
>  
> + if (efi_enabled(EFI_RUNTIME_SERVICES)) {
> + pr_info("EFI runtime services access via paravirt.\n");
> + return 0;
> + }
> +
>   pr_info("Remapping and enabling EFI services.\n");
>  
>   mapsize = efi.memmap.map_end - efi.memmap.map;
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 05509f3..1c6f9dd 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -472,12 +472,14 @@ device_initcall(efi_load_efivars);
>   FIELD_SIZEOF(struct efi_fdt_params, field) \
>   }
>  
> -static __initdata struct {
> +struct params {
>   const char name[32];
>   const char propname[32];
>   int offset;
>   int size;
> -} dt_params[] = {
> +};
> +
> +static __initdata struct params fdt_params[] = {
>   UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
>   UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
>   UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> @@ -485,44 +487,91 @@ static __initdata struct {
>   UEFI_PARAM("MemMap Desc. 

[Xen-devel] [PATCH v2] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Shannon Zhao]

From: Shannon Zhao 

The EFI DT parameters for bare metal are located under /chosen node,
while for Xen Dom0 they are located under /hyperviosr/uefi node. These
parameters under /chosen and /hyperviosr/uefi are not expected to appear
at the same time.
Parse these EFI parameters and initialize EFI like the way for bare
metal except the runtime services because the runtime services for Xen
Dom0 are available through hypercalls and they are always enabled. So it
sets the EFI_RUNTIME_SERVICES flag if it finds /hyperviosr/uefi node and
bails out in arm_enable_runtime_services() when EFI_RUNTIME_SERVICES
flag is set already.

CC: Matt Fleming 
Signed-off-by: Shannon Zhao 
---
v2: rebase it on top of EFI and Xen next branch, add some comments to
explain the codes. 
---
 arch/arm/xen/enlighten.c   | 22 +++
 drivers/firmware/efi/arm-runtime.c |  5 +++
 drivers/firmware/efi/efi.c | 81 ++
 3 files changed, 92 insertions(+), 16 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 13e3e9f..d4f36eb 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -15,7 +15,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -261,6 +263,19 @@ static int __init fdt_find_hyper_node(unsigned long node, 
const char *uname,
!strncmp(hyper_node.prefix, s, strlen(hyper_node.prefix)))
hyper_node.version = s + strlen(hyper_node.prefix);
 
+   /*
+* Check if Xen supports EFI by checking whether there is the
+* "/hypervisor/uefi" node in DT. If so, runtime services are available
+* through proxy functions (e.g. in case of Xen dom0 EFI implementation
+* they call special hypercall which executes relevant EFI functions)
+* and that is why they are always enabled.
+*/
+   if (IS_ENABLED(CONFIG_XEN_EFI)) {
+   if ((of_get_flat_dt_subnode_by_name(node, "uefi") > 0) &&
+   !efi_runtime_disabled())
+   set_bit(EFI_RUNTIME_SERVICES, );
+   }
+
return 0;
 }
 
@@ -352,6 +367,13 @@ static int __init xen_guest_init(void)
return -ENODEV;
}
 
+   /*
+* The fdt parsing codes have set EFI_RUNTIME_SERVICES if Xen EFI
+* parameters are found. Force enable runtime services.
+*/
+   if (efi_enabled(EFI_RUNTIME_SERVICES))
+   xen_efi_runtime_setup();
+
shared_info_page = (struct shared_info *)get_zeroed_page(GFP_KERNEL);
 
if (!shared_info_page) {
diff --git a/drivers/firmware/efi/arm-runtime.c 
b/drivers/firmware/efi/arm-runtime.c
index 17ccf0a..c394b81 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -107,6 +107,11 @@ static int __init arm_enable_runtime_services(void)
return 0;
}
 
+   if (efi_enabled(EFI_RUNTIME_SERVICES)) {
+   pr_info("EFI runtime services access via paravirt.\n");
+   return 0;
+   }
+
pr_info("Remapping and enabling EFI services.\n");
 
mapsize = efi.memmap.map_end - efi.memmap.map;
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 05509f3..1c6f9dd 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -472,12 +472,14 @@ device_initcall(efi_load_efivars);
FIELD_SIZEOF(struct efi_fdt_params, field) \
}
 
-static __initdata struct {
+struct params {
const char name[32];
const char propname[32];
int offset;
int size;
-} dt_params[] = {
+};
+
+static __initdata struct params fdt_params[] = {
UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
@@ -485,44 +487,91 @@ static __initdata struct {
UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
 };
 
+static __initdata struct params xen_fdt_params[] = {
+   UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
+   UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
+   UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
+   UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
+   UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
+};
+
+#define EFI_FDT_PARAMS_SIZEARRAY_SIZE(fdt_params)
+
+static __initdata struct {
+   const char *uname;
+   const char *subnode;
+   struct params *params;
+} dt_params[] = {
+   { "hypervisor", "uefi", xen_fdt_params },
+   { "chosen", NULL, fdt_params },
+};
+
 struct param_info {
int found;
void *params;
+   const char *missing;
 };
 
-static int __init