Message ID | 1456658360-16080-16-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sun, 28 Feb 2016, Shannon Zhao wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > Create a few EFI memory descriptors to tell Dom0 the RAM region > information, ACPI table regions and EFI tables reserved resions. > > Cc: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > --- > v4: use a single descriptor for new created tables > --- > xen/arch/arm/domain_build.c | 2 ++ > xen/common/efi/boot.c | 38 ++++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/setup.h | 5 +++++ > 3 files changed, 45 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 09f9770..1ec6271 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo) > acpi_map_other_tables(d); > acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table, > tbl_add); > + acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len, > + d->arch.efi_acpi_table, &kinfo->mem, tbl_add); > > return 0; > } > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > index 238c5fd..b8d7409 100644 > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, > tbl_add[TBL_EFIT].start = table_addr; > tbl_add[TBL_EFIT].size = table_size; > } > + > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, > + void *efi_acpi_table, > + const struct meminfo *mem, > + struct membank tbl_add[]) This function probably belongs to xen/arch/arm/domain_build.c or arch/arm/efi > +{ > + EFI_MEMORY_DESCRIPTOR *memory_map; > + int i, offset; > + u8 *base_ptr; > + > + tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP); > + tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR) > + * (mem->nr_banks + acpi_mem.nr_banks + 1); I don't like the fact that we are getting the number of memory banks from a parameter and the number of acpi banks from a global variable. Can we pass in acpi_mem.nr_banks too? > + base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP); > + memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr); > + > + offset = 0; > + for( i = 0; i < mem->nr_banks; i++, offset++ ) > + { > + memory_map[offset].Type = EfiConventionalMemory; > + memory_map[offset].PhysicalStart = mem->bank[i].start; > + memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size); > + memory_map[offset].Attribute = EFI_MEMORY_WB; > + } > + > + for( i = 0; i < acpi_mem.nr_banks; i++, offset++ ) > + { > + memory_map[offset].Type = EfiACPIReclaimMemory; > + memory_map[offset].PhysicalStart = acpi_mem.bank[i].start; > + memory_map[offset].NumberOfPages = PFN_UP(acpi_mem.bank[i].size); > + memory_map[offset].Attribute = EFI_MEMORY_WB; > + } > + > + memory_map[offset].Type = EfiACPIReclaimMemory; > + memory_map[offset].PhysicalStart = paddr; > + memory_map[offset].NumberOfPages = PFN_UP(size); > + memory_map[offset].Attribute = EFI_MEMORY_WB; > +} > #endif > > #ifndef CONFIG_ARM /* TODO - runtime service support */ > diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h > index 2d65796..af5a038 100644 > --- a/xen/include/asm-arm/setup.h > +++ b/xen/include/asm-arm/setup.h > @@ -56,6 +56,11 @@ int estimate_efi_size(int mem_nr_banks); > void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, > struct membank tbl_add[]); > > +void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, > + void *efi_acpi_table, > + const struct meminfo *mem, > + struct membank tbl_add[]); > + > int construct_dom0(struct domain *d); > > void discard_initial_modules(void);
On 2016/2/29 22:37, Stefano Stabellini wrote: > On Sun, 28 Feb 2016, Shannon Zhao wrote: >> > From: Shannon Zhao <shannon.zhao@linaro.org> >> > >> > Create a few EFI memory descriptors to tell Dom0 the RAM region >> > information, ACPI table regions and EFI tables reserved resions. >> > >> > Cc: Jan Beulich <jbeulich@suse.com> >> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >> > --- >> > v4: use a single descriptor for new created tables >> > --- >> > xen/arch/arm/domain_build.c | 2 ++ >> > xen/common/efi/boot.c | 38 ++++++++++++++++++++++++++++++++++++++ >> > xen/include/asm-arm/setup.h | 5 +++++ >> > 3 files changed, 45 insertions(+) >> > >> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> > index 09f9770..1ec6271 100644 >> > --- a/xen/arch/arm/domain_build.c >> > +++ b/xen/arch/arm/domain_build.c >> > @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo) >> > acpi_map_other_tables(d); >> > acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table, >> > tbl_add); >> > + acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len, >> > + d->arch.efi_acpi_table, &kinfo->mem, tbl_add); >> > >> > return 0; >> > } >> > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c >> > index 238c5fd..b8d7409 100644 >> > --- a/xen/common/efi/boot.c >> > +++ b/xen/common/efi/boot.c >> > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, >> > tbl_add[TBL_EFIT].start = table_addr; >> > tbl_add[TBL_EFIT].size = table_size; >> > } >> > + >> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, >> > + void *efi_acpi_table, >> > + const struct meminfo *mem, >> > + struct membank tbl_add[]) > This function probably belongs to xen/arch/arm/domain_build.c or > arch/arm/efi > As said before at previous patch set, placing these functions in common/efi/boot.c is to use the EFI_MEMORY_DESCRIPTOR etc. See http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00097.html But I agree that it cloud be moved to a new file(e.g. dom0-efi.c) under arch/arm/efi. > >> > +{ >> > + EFI_MEMORY_DESCRIPTOR *memory_map; >> > + int i, offset; >> > + u8 *base_ptr; >> > + >> > + tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP); >> > + tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR) >> > + * (mem->nr_banks + acpi_mem.nr_banks + 1); > I don't like the fact that we are getting the number of memory banks > from a parameter and the number of acpi banks from a global variable. > Can we pass in acpi_mem.nr_banks too? > While acpi_mem is defined in efi-boot.h, the caller in domain_build.c of acpi_create_efi_mmap_table can't access acpi_mem since if we include efi-boot.h in domain_build.c there are some compiling errors like above. Thanks,
On Tue, 1 Mar 2016, Shannon Zhao wrote: > On 2016/2/29 22:37, Stefano Stabellini wrote: > > On Sun, 28 Feb 2016, Shannon Zhao wrote: > >> > From: Shannon Zhao <shannon.zhao@linaro.org> > >> > > >> > Create a few EFI memory descriptors to tell Dom0 the RAM region > >> > information, ACPI table regions and EFI tables reserved resions. > >> > > >> > Cc: Jan Beulich <jbeulich@suse.com> > >> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > >> > --- > >> > v4: use a single descriptor for new created tables > >> > --- > >> > xen/arch/arm/domain_build.c | 2 ++ > >> > xen/common/efi/boot.c | 38 ++++++++++++++++++++++++++++++++++++++ > >> > xen/include/asm-arm/setup.h | 5 +++++ > >> > 3 files changed, 45 insertions(+) > >> > > >> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > >> > index 09f9770..1ec6271 100644 > >> > --- a/xen/arch/arm/domain_build.c > >> > +++ b/xen/arch/arm/domain_build.c > >> > @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo) > >> > acpi_map_other_tables(d); > >> > acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table, > >> > tbl_add); > >> > + acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len, > >> > + d->arch.efi_acpi_table, &kinfo->mem, tbl_add); > >> > > >> > return 0; > >> > } > >> > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > >> > index 238c5fd..b8d7409 100644 > >> > --- a/xen/common/efi/boot.c > >> > +++ b/xen/common/efi/boot.c > >> > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, > >> > tbl_add[TBL_EFIT].start = table_addr; > >> > tbl_add[TBL_EFIT].size = table_size; > >> > } > >> > + > >> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, > >> > + void *efi_acpi_table, > >> > + const struct meminfo *mem, > >> > + struct membank tbl_add[]) > > This function probably belongs to xen/arch/arm/domain_build.c or > > arch/arm/efi > > > As said before at previous patch set, placing these functions in > common/efi/boot.c is to use the EFI_MEMORY_DESCRIPTOR etc. > > See > http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00097.html Sorry Shannon, I forgot about it. > But I agree that it cloud be moved to a new file(e.g. dom0-efi.c) under > arch/arm/efi. Given the limitations, it would be acceptable to have the function here, but given that this is about acpi and only useful on ARM, adding the function to a file under arch/arm/efi would be even better. > >> > +{ > >> > + EFI_MEMORY_DESCRIPTOR *memory_map; > >> > + int i, offset; > >> > + u8 *base_ptr; > >> > + > >> > + tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP); > >> > + tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR) > >> > + * (mem->nr_banks + acpi_mem.nr_banks + 1); > > I don't like the fact that we are getting the number of memory banks > > from a parameter and the number of acpi banks from a global variable. > > Can we pass in acpi_mem.nr_banks too? > > > While acpi_mem is defined in efi-boot.h, the caller in domain_build.c of > acpi_create_efi_mmap_table can't access acpi_mem since if we include > efi-boot.h in domain_build.c there are some compiling errors like above. All right, it is not very nice, but I cannot think of a better way of doing it right now.
>>> On 01.03.16 at 15:57, <stefano.stabellini@eu.citrix.com> wrote: > On Tue, 1 Mar 2016, Shannon Zhao wrote: >> On 2016/2/29 22:37, Stefano Stabellini wrote: >> > On Sun, 28 Feb 2016, Shannon Zhao wrote: >> >> > --- a/xen/common/efi/boot.c >> >> > +++ b/xen/common/efi/boot.c >> >> > @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, >> >> > tbl_add[TBL_EFIT].start = table_addr; >> >> > tbl_add[TBL_EFIT].size = table_size; >> >> > } >> >> > + >> >> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, >> >> > + void *efi_acpi_table, >> >> > + const struct meminfo *mem, >> >> > + struct membank tbl_add[]) >> > This function probably belongs to xen/arch/arm/domain_build.c or >> > arch/arm/efi >> > >> As said before at previous patch set, placing these functions in >> common/efi/boot.c is to use the EFI_MEMORY_DESCRIPTOR etc. >> >> See >> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00097.html > > Sorry Shannon, I forgot about it. > > >> But I agree that it cloud be moved to a new file(e.g. dom0-efi.c) under >> arch/arm/efi. > > Given the limitations, it would be acceptable to have the function here, Afaic, having it in xen/common/efi/ is a no-go (we be acceptable only it we were really just taking of one relatively small function, but more gets added here by other patches), hence ... > but given that this is about acpi and only useful on ARM, adding the > function to a file under arch/arm/efi would be even better. ... this seems the only viable alternative. Jan
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 09f9770..1ec6271 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo) acpi_map_other_tables(d); acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table, tbl_add); + acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len, + d->arch.efi_acpi_table, &kinfo->mem, tbl_add); return 0; } diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 238c5fd..b8d7409 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1233,6 +1233,44 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, tbl_add[TBL_EFIT].start = table_addr; tbl_add[TBL_EFIT].size = table_size; } + +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, + void *efi_acpi_table, + const struct meminfo *mem, + struct membank tbl_add[]) +{ + EFI_MEMORY_DESCRIPTOR *memory_map; + int i, offset; + u8 *base_ptr; + + tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP); + tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR) + * (mem->nr_banks + acpi_mem.nr_banks + 1); + base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP); + memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr); + + offset = 0; + for( i = 0; i < mem->nr_banks; i++, offset++ ) + { + memory_map[offset].Type = EfiConventionalMemory; + memory_map[offset].PhysicalStart = mem->bank[i].start; + memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size); + memory_map[offset].Attribute = EFI_MEMORY_WB; + } + + for( i = 0; i < acpi_mem.nr_banks; i++, offset++ ) + { + memory_map[offset].Type = EfiACPIReclaimMemory; + memory_map[offset].PhysicalStart = acpi_mem.bank[i].start; + memory_map[offset].NumberOfPages = PFN_UP(acpi_mem.bank[i].size); + memory_map[offset].Attribute = EFI_MEMORY_WB; + } + + memory_map[offset].Type = EfiACPIReclaimMemory; + memory_map[offset].PhysicalStart = paddr; + memory_map[offset].NumberOfPages = PFN_UP(size); + memory_map[offset].Attribute = EFI_MEMORY_WB; +} #endif #ifndef CONFIG_ARM /* TODO - runtime service support */ diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index 2d65796..af5a038 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -56,6 +56,11 @@ int estimate_efi_size(int mem_nr_banks); void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table, struct membank tbl_add[]); +void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size, + void *efi_acpi_table, + const struct meminfo *mem, + struct membank tbl_add[]); + int construct_dom0(struct domain *d); void discard_initial_modules(void);