| Message ID | 1447754231-7772-9-git-send-email-shannon.zhao@linaro.org |
|---|---|
| State | New |
| Headers | show |
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
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
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
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; }