Message ID | 1431893048-5214-23-git-send-email-parth.dixit@linaro.org |
---|---|
State | New |
Headers | show |
+shannon On 2 June 2015 at 22:57, Julien Grall <julien.grall@citrix.com> wrote: > Hi Parth, > > On 17/05/15 21:03, Parth Dixit wrote: >> Create a DT for DOM0 for ACPI-case only. >> DT contains minmal required informations such as > > s/minmal/minimal/ > >> DOM0 bootargs, initrd, efi description table >> and address of uefi memory table. >> Add placeholder for tables to be marked as >> reserved in efi table. This is requird for > > s/requird/required/ > >> DT function's signature. > > Well, you can modify later the prototype. It's usually better to define > where it's used because we can understand why you need this how you will > use it. > > Also, a link the description of the minimal DT in the Linux doc would > have been nice. > >> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org> >> Signed-off-by: Parth Dixit <parth.dixit@linaro.org> >> --- >> xen/arch/arm/domain_build.c | 75 ++++++++++++++++++++++++++++++++++++++++++++- >> xen/include/asm-arm/acpi.h | 10 ++++++ >> 2 files changed, 84 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 1e545fe..c830702 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -9,6 +9,7 @@ >> #include <asm/regs.h> >> #include <xen/errno.h> >> #include <xen/device_tree.h> >> +#include <xen/acpi.h> >> #include <xen/libfdt/libfdt.h> >> #include <xen/guest_access.h> >> #include <xen/iocap.h> >> @@ -18,6 +19,7 @@ >> #include <asm/psci.h> >> #include <asm/setup.h> >> #include <asm/cpufeature.h> >> +#include <asm/acpi.h> > > Not necessary, xen/acpi.h already includes asm/acpi.h > >> >> #include <asm/gic.h> >> #include <xen/irq.h> >> @@ -61,6 +63,11 @@ custom_param("dom0_mem", parse_dom0_mem); >> */ >> #define DOM0_FDT_EXTRA_SIZE (128 + sizeof(struct fdt_reserve_entry)) >> >> +#ifdef CONFIG_ACPI >> +/* Reserve DOM0 FDT size in ACPI case only */ >> +#define DOM0_FDT_MIN_SIZE 4096 > > This can be moved within the big #ifdef CONFIG_ACPI below. > Also please rename the define to show that it's ACPI specific. > >> +#endif >> + >> struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0) >> { >> if ( opt_dom0_max_vcpus == 0 ) >> @@ -1211,7 +1218,68 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, >> >> return res; >> } > > Newline. > >> +#ifdef CONFIG_ACPI >> +/* >> + * Prepare a minimal DTB for DOM0 which contains >> + * bootargs, initrd, memory information, >> + * EFI table. >> + */ >> +static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct membank tbl_add[]) >> +{ >> + int new_size; >> + int ret; >> + >> + DPRINT("Prepare a min DTB for DOM0\n"); >> + >> + /* Allocate min size for DT */ >> + new_size = DOM0_FDT_MIN_SIZE; >> + kinfo->fdt = xmalloc_bytes(DOM0_FDT_MIN_SIZE); > > Why not using new_size here? It would be less error-prone. > >> + >> + if ( kinfo->fdt == NULL ) >> + return -ENOMEM; >> + >> + /* Create a new empty DT for DOM0 */ >> + ret = fdt_create(kinfo->fdt, new_size); >> + if ( ret < 0 ) >> + goto err; >> + >> + ret = fdt_finish_reservemap(kinfo->fdt); >> + if ( ret < 0 ) >> + goto err; >> + >> + ret = fdt_begin_node(kinfo->fdt, "/"); >> + if ( ret < 0 ) >> + goto err; >> + >> + ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2); >> + if ( ret ) >> + return ret; >> >> + ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1); >> + if ( ret ) >> + return ret; >> + >> + ret = fdt_end_node(kinfo->fdt); >> + if ( ret < 0 ) >> + goto err; >> + >> + ret = fdt_finish(kinfo->fdt); >> + if ( ret < 0 ) >> + goto err; >> + >> + return 0; >> + >> + err: >> + printk("Device tree generation failed (%d).\n", ret); >> + xfree(kinfo->fdt); >> + return -EINVAL; >> +} >> +#else >> +static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct membank tbl_add[]) >> +{ > > Please add a comment and a BUG_ON to make clear this should never be called. > >> + return -EINVAL; >> +} >> +#endif >> static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) >> { >> const void *fdt; >> @@ -1370,6 +1438,7 @@ int construct_dom0(struct domain *d) >> struct kernel_info kinfo = {}; >> struct vcpu *saved_current; >> int rc, i, cpu; >> + struct membank tbl_add[TBL_MMAX] = {}; >> >> struct vcpu *v = d->vcpu[0]; >> struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs; >> @@ -1403,7 +1472,11 @@ int construct_dom0(struct domain *d) >> >> allocate_memory(d, &kinfo); >> >> - rc = prepare_dtb(d, &kinfo); >> + if (acpi_disabled) > > if ( ... ) > >> + rc = prepare_dtb(d, &kinfo); >> + else >> + rc = create_acpi_dtb(d, &kinfo, tbl_add); >> + >> if ( rc < 0 ) >> return rc; >> >> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h >> index 482cc5b..2df9ae0 100644 >> --- a/xen/include/asm-arm/acpi.h >> +++ b/xen/include/asm-arm/acpi.h >> @@ -50,6 +50,16 @@ static inline void disable_acpi(void) >> acpi_disabled = 1; >> } >> >> +/* Tables marked as reserved in efi table */ >> +enum EFI_MEM_RES{ >> + TBL_STAO, >> + TBL_XENV, >> + TBL_XSDT, >> + TBL_EFIT, >> + TBL_MMAP, >> + TBL_MMAX, >> +}; >> + > > This doesn't belong to this patch ... > >> #define ACPI_GTDT_INTR_MASK ( ACPI_GTDT_INTERRUPT_MODE | ACPI_GTDT_INTERRUPT_POLARITY ) >> >> /** >> > > Regards, > > -- > Julien Grall
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 1e545fe..c830702 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -9,6 +9,7 @@ #include <asm/regs.h> #include <xen/errno.h> #include <xen/device_tree.h> +#include <xen/acpi.h> #include <xen/libfdt/libfdt.h> #include <xen/guest_access.h> #include <xen/iocap.h> @@ -18,6 +19,7 @@ #include <asm/psci.h> #include <asm/setup.h> #include <asm/cpufeature.h> +#include <asm/acpi.h> #include <asm/gic.h> #include <xen/irq.h> @@ -61,6 +63,11 @@ custom_param("dom0_mem", parse_dom0_mem); */ #define DOM0_FDT_EXTRA_SIZE (128 + sizeof(struct fdt_reserve_entry)) +#ifdef CONFIG_ACPI +/* Reserve DOM0 FDT size in ACPI case only */ +#define DOM0_FDT_MIN_SIZE 4096 +#endif + struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0) { if ( opt_dom0_max_vcpus == 0 ) @@ -1211,7 +1218,68 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, return res; } +#ifdef CONFIG_ACPI +/* + * Prepare a minimal DTB for DOM0 which contains + * bootargs, initrd, memory information, + * EFI table. + */ +static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct membank tbl_add[]) +{ + int new_size; + int ret; + + DPRINT("Prepare a min DTB for DOM0\n"); + + /* Allocate min size for DT */ + new_size = DOM0_FDT_MIN_SIZE; + kinfo->fdt = xmalloc_bytes(DOM0_FDT_MIN_SIZE); + + if ( kinfo->fdt == NULL ) + return -ENOMEM; + + /* Create a new empty DT for DOM0 */ + ret = fdt_create(kinfo->fdt, new_size); + if ( ret < 0 ) + goto err; + + ret = fdt_finish_reservemap(kinfo->fdt); + if ( ret < 0 ) + goto err; + + ret = fdt_begin_node(kinfo->fdt, "/"); + if ( ret < 0 ) + goto err; + + ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2); + if ( ret ) + return ret; + ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1); + if ( ret ) + return ret; + + ret = fdt_end_node(kinfo->fdt); + if ( ret < 0 ) + goto err; + + ret = fdt_finish(kinfo->fdt); + if ( ret < 0 ) + goto err; + + return 0; + + err: + printk("Device tree generation failed (%d).\n", ret); + xfree(kinfo->fdt); + return -EINVAL; +} +#else +static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct membank tbl_add[]) +{ + return -EINVAL; +} +#endif static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) { const void *fdt; @@ -1370,6 +1438,7 @@ int construct_dom0(struct domain *d) struct kernel_info kinfo = {}; struct vcpu *saved_current; int rc, i, cpu; + struct membank tbl_add[TBL_MMAX] = {}; struct vcpu *v = d->vcpu[0]; struct cpu_user_regs *regs = &v->arch.cpu_info->guest_cpu_user_regs; @@ -1403,7 +1472,11 @@ int construct_dom0(struct domain *d) allocate_memory(d, &kinfo); - rc = prepare_dtb(d, &kinfo); + if (acpi_disabled) + rc = prepare_dtb(d, &kinfo); + else + rc = create_acpi_dtb(d, &kinfo, tbl_add); + if ( rc < 0 ) return rc; diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h index 482cc5b..2df9ae0 100644 --- a/xen/include/asm-arm/acpi.h +++ b/xen/include/asm-arm/acpi.h @@ -50,6 +50,16 @@ static inline void disable_acpi(void) acpi_disabled = 1; } +/* Tables marked as reserved in efi table */ +enum EFI_MEM_RES{ + TBL_STAO, + TBL_XENV, + TBL_XSDT, + TBL_EFIT, + TBL_MMAP, + TBL_MMAX, +}; + #define ACPI_GTDT_INTR_MASK ( ACPI_GTDT_INTERRUPT_MODE | ACPI_GTDT_INTERRUPT_POLARITY ) /**