[08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

Message ID 1447754231-7772-9-git-send-email-shannon.zhao@linaro.org
State New
Headers show

Commit Message

Shannon Zhao Nov. 17, 2015, 9:57 a.m.
From: Shannon Zhao <shannon.zhao@linaro.org>


Add a new function to parse DT parameters for Xen specific UEFI just
like the way for normal UEFI. Then it could reuse the existing codes.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

---
 drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 5 deletions(-)

-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ard Biesheuvel Nov. 17, 2015, 11:25 a.m. | #1
On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>

>

> Add a new function to parse DT parameters for Xen specific UEFI just

> like the way for normal UEFI. Then it could reuse the existing codes.

>

> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---

>  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----

>  1 file changed, 62 insertions(+), 5 deletions(-)

>

> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c

> index d6144e3..629bd06 100644

> --- a/drivers/firmware/efi/efi.c

> +++ b/drivers/firmware/efi/efi.c

> @@ -24,6 +24,7 @@

>  #include <linux/of_fdt.h>

>  #include <linux/io.h>

>  #include <linux/platform_device.h>

> +#include <xen/xen.h>

>

>  struct efi __read_mostly efi = {

>         .mps        = EFI_INVALID_TABLE_ADDR,

> @@ -488,12 +489,60 @@ static __initdata struct {

>         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)

>  };

>

> +static __initdata struct {

> +       const char name[32];

> +       const char propname[32];

> +       int offset;

> +       int size;

> +} xen_dt_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)

> +};

> +


We discussed (and agreed afair) that dropping the "linux," prefix from
the DT properties was an acceptable change. If we do that, and reuse
the same names in the xen version (i.e., drop the "xen," prefix), we
could make this change a *lot* simpler.

>  struct param_info {

>         int verbose;

>         int found;

>         void *params;

>  };

>

> +static int __init fdt_find_xen_uefi_params(unsigned long node,

> +                                          const char *uname, int depth,

> +                                          void *data)

> +{

> +       struct param_info *info = data;

> +       const void *prop;

> +       void *dest;

> +       u64 val;

> +       int i, len;

> +

> +       if (xen_initial_domain() && (depth != 2 || strcmp(uname, "uefi") != 0))

> +               return 0;

> +

> +       for (i = 0; i < ARRAY_SIZE(xen_dt_params); i++) {

> +               prop = of_get_flat_dt_prop(node, xen_dt_params[i].propname,

> +                                          &len);

> +               if (!prop)

> +                       return 0;

> +               dest = info->params + xen_dt_params[i].offset;

> +               info->found++;

> +

> +               val = of_read_number(prop, len / sizeof(u32));

> +

> +               if (dt_params[i].size == sizeof(u32))

> +                       *(u32 *)dest = val;

> +               else

> +                       *(u64 *)dest = val;

> +

> +               if (info->verbose)

> +                       pr_info("  %s: 0x%0*llx\n", xen_dt_params[i].name,

> +                               xen_dt_params[i].size * 2, val);

> +       }

> +

> +       return 1;

> +}

>  static int __init fdt_find_uefi_params(unsigned long node, const char *uname,

>                                        int depth, void *data)

>  {

> @@ -538,12 +587,20 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)

>         info.found = 0;

>         info.params = params;

>

> -       ret = of_scan_flat_dt(fdt_find_uefi_params, &info);

> -       if (!info.found)

> +       if (xen_initial_domain())

> +               ret = of_scan_flat_dt(fdt_find_xen_uefi_params, &info);

> +       else

> +               ret = of_scan_flat_dt(fdt_find_uefi_params, &info);

> +       if (!info.found) {

>                 pr_info("UEFI not found.\n");

> -       else if (!ret)

> -               pr_err("Can't find '%s' in device tree!\n",

> -                      dt_params[info.found].name);

> +       } else if (!ret) {

> +               if (xen_initial_domain())

> +                       pr_err("Can't find '%s' in device tree!\n",

> +                              xen_dt_params[info.found].name);

> +               else

> +                       pr_err("Can't find '%s' in device tree!\n",

> +                              xen_dt_params[info.found].name);


Wrong array here

> +       }

>

>         return ret;

>  }

> --

> 2.1.0

>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Nov. 17, 2015, 11:37 a.m. | #2
On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:
> On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:

> > From: Shannon Zhao <shannon.zhao@linaro.org>

> >

> > Add a new function to parse DT parameters for Xen specific UEFI just

> > like the way for normal UEFI. Then it could reuse the existing codes.

> >

> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

> > ---

> >  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----

> >  1 file changed, 62 insertions(+), 5 deletions(-)

> >

> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c

> > index d6144e3..629bd06 100644

> > --- a/drivers/firmware/efi/efi.c

> > +++ b/drivers/firmware/efi/efi.c

> > @@ -24,6 +24,7 @@

> >  #include <linux/of_fdt.h>

> >  #include <linux/io.h>

> >  #include <linux/platform_device.h>

> > +#include <xen/xen.h>

> >

> >  struct efi __read_mostly efi = {

> >         .mps        = EFI_INVALID_TABLE_ADDR,

> > @@ -488,12 +489,60 @@ static __initdata struct {

> >         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)

> >  };

> >

> > +static __initdata struct {

> > +       const char name[32];

> > +       const char propname[32];

> > +       int offset;

> > +       int size;

> > +} xen_dt_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)

> > +};

> > +

> 

> We discussed (and agreed afair) that dropping the "linux," prefix from

> the DT properties was an acceptable change. If we do that, and reuse

> the same names in the xen version (i.e., drop the "xen," prefix), we

> could make this change a *lot* simpler.


Regardless of if we drop the "linux," prefix from the existing strings,
I think we need the "xen," prefix here.

The xen EFI interface comes with additional caveats, and we need to
treat it separately.

Unless you're suggesting that /hypervisor/uefi-* is handled differently
to /chosen/uefi-*?

I think I'd still prefer the "xen," prefix regardless.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Nov. 17, 2015, 12:17 p.m. | #3
On 17 November 2015 at 12:37, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:

>> On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:

>> > From: Shannon Zhao <shannon.zhao@linaro.org>

>> >

>> > Add a new function to parse DT parameters for Xen specific UEFI just

>> > like the way for normal UEFI. Then it could reuse the existing codes.

>> >

>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

>> > ---

>> >  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----

>> >  1 file changed, 62 insertions(+), 5 deletions(-)

>> >

>> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c

>> > index d6144e3..629bd06 100644

>> > --- a/drivers/firmware/efi/efi.c

>> > +++ b/drivers/firmware/efi/efi.c

>> > @@ -24,6 +24,7 @@

>> >  #include <linux/of_fdt.h>

>> >  #include <linux/io.h>

>> >  #include <linux/platform_device.h>

>> > +#include <xen/xen.h>

>> >

>> >  struct efi __read_mostly efi = {

>> >         .mps        = EFI_INVALID_TABLE_ADDR,

>> > @@ -488,12 +489,60 @@ static __initdata struct {

>> >         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)

>> >  };

>> >

>> > +static __initdata struct {

>> > +       const char name[32];

>> > +       const char propname[32];

>> > +       int offset;

>> > +       int size;

>> > +} xen_dt_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)

>> > +};

>> > +

>>

>> We discussed (and agreed afair) that dropping the "linux," prefix from

>> the DT properties was an acceptable change. If we do that, and reuse

>> the same names in the xen version (i.e., drop the "xen," prefix), we

>> could make this change a *lot* simpler.

>

> Regardless of if we drop the "linux," prefix from the existing strings,

> I think we need the "xen," prefix here.

>

> The xen EFI interface comes with additional caveats, and we need to

> treat it separately.

>

> Unless you're suggesting that /hypervisor/uefi-* is handled differently

> to /chosen/uefi-*?

>

> I think I'd still prefer the "xen," prefix regardless.

>


Well, we should still be able to reuse more of the existing code
rather than duplicating a bunch of code in fdt_find_xen_uefi_params()
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index d6144e3..629bd06 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -24,6 +24,7 @@ 
 #include <linux/of_fdt.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <xen/xen.h>
 
 struct efi __read_mostly efi = {
 	.mps        = EFI_INVALID_TABLE_ADDR,
@@ -488,12 +489,60 @@  static __initdata struct {
 	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
 };
 
+static __initdata struct {
+	const char name[32];
+	const char propname[32];
+	int offset;
+	int size;
+} xen_dt_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)
+};
+
 struct param_info {
 	int verbose;
 	int found;
 	void *params;
 };
 
+static int __init fdt_find_xen_uefi_params(unsigned long node,
+					   const char *uname, int depth,
+					   void *data)
+{
+	struct param_info *info = data;
+	const void *prop;
+	void *dest;
+	u64 val;
+	int i, len;
+
+	if (xen_initial_domain() && (depth != 2 || strcmp(uname, "uefi") != 0))
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(xen_dt_params); i++) {
+		prop = of_get_flat_dt_prop(node, xen_dt_params[i].propname,
+					   &len);
+		if (!prop)
+			return 0;
+		dest = info->params + xen_dt_params[i].offset;
+		info->found++;
+
+		val = of_read_number(prop, len / sizeof(u32));
+
+		if (dt_params[i].size == sizeof(u32))
+			*(u32 *)dest = val;
+		else
+			*(u64 *)dest = val;
+
+		if (info->verbose)
+			pr_info("  %s: 0x%0*llx\n", xen_dt_params[i].name,
+				xen_dt_params[i].size * 2, val);
+	}
+
+	return 1;
+}
 static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
 				       int depth, void *data)
 {
@@ -538,12 +587,20 @@  int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
 	info.found = 0;
 	info.params = params;
 
-	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
-	if (!info.found)
+	if (xen_initial_domain())
+		ret = of_scan_flat_dt(fdt_find_xen_uefi_params, &info);
+	else
+		ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
+	if (!info.found) {
 		pr_info("UEFI not found.\n");
-	else if (!ret)
-		pr_err("Can't find '%s' in device tree!\n",
-		       dt_params[info.found].name);
+	} else if (!ret) {
+		if (xen_initial_domain())
+			pr_err("Can't find '%s' in device tree!\n",
+			       xen_dt_params[info.found].name);
+		else
+			pr_err("Can't find '%s' in device tree!\n",
+			       xen_dt_params[info.found].name);
+	}
 
 	return ret;
 }