| Message ID | 1396616219-32219-3-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Ian, On 04/04/2014 01:56 PM, Ian Campbell wrote: > This reverts commit 6c21cb36e263de2db8716b477157a5b6cd531e1e. > > The Linux = issue which this works around was fixed in v3.13 via f52bb722547f > "ARM: mm: Correct virt_to_phys patching for 64 bit physical addresses". > > This is the second attempt to revert this. Now that we have fixed > allocate_memory_11 to allocate accessible memory on 32-bit this is safe to do. > This is not quite a straight revert since we need to ensure that for 32-bit > domain 0 we do not allocate dom0's memory above 4GB where the domain cannot > access it without paging (which is disabled at start of day) and LPAE (which > the kernel may not support) enabled. When multiple banks will be supported, I guess it will be safe to allocate above 32bits (if dom0 has more than 4G) of RAM. Anyway, it's not the goal of this patch :). > xen/arch/arm/domain_build.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 4d6b26b..e7cc2c9 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -69,19 +69,21 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo) > { > paddr_t start; > paddr_t size; > - struct page_info *pg = NULL; > + struct page_info *pg; > unsigned int order = get_order_from_bytes(dom0_mem); > int res; > paddr_t spfn; > - unsigned int bits; > > - for ( bits = PAGE_SHIFT + 1; bits < PADDR_BITS; bits++ ) > + if ( is_32bit_domain(d) ) > { > - pg = alloc_domheap_pages(d, order, MEMF_bits(bits)); > - if ( pg != NULL ) > - break; > + printk("32 bit domain\n"); [..] > + printk("64 bit domain\n"); Can you be more explicit on both of theses messages? Nothing specify we are allocate memory before them. Regards,
On Fri, 2014-04-04 at 15:48 +0100, Julien Grall wrote: > Hi Ian, > > On 04/04/2014 01:56 PM, Ian Campbell wrote: > > This reverts commit 6c21cb36e263de2db8716b477157a5b6cd531e1e. > > > > The Linux = issue which this works around was fixed in v3.13 via f52bb722547f > > "ARM: mm: Correct virt_to_phys patching for 64 bit physical addresses". > > > > This is the second attempt to revert this. Now that we have fixed > > allocate_memory_11 to allocate accessible memory on 32-bit this is safe to do. > > This is not quite a straight revert since we need to ensure that for 32-bit > > domain 0 we do not allocate dom0's memory above 4GB where the domain cannot > > access it without paging (which is disabled at start of day) and LPAE (which > > the kernel may not support) enabled. > > When multiple banks will be supported, I guess it will be safe to > allocate above 32bits (if dom0 has more than 4G) of RAM. Yes. We should aim for as much of dom0's memory as possible to be below 4GB, but if the user asked for more than that we may as well put it above 4GB and hope that the kernel supports LPAE. > Anyway, it's not the goal of this patch :). Yes. > > xen/arch/arm/domain_build.c | 16 +++++++++------- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index 4d6b26b..e7cc2c9 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -69,19 +69,21 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo) > > { > > paddr_t start; > > paddr_t size; > > - struct page_info *pg = NULL; > > + struct page_info *pg; > > unsigned int order = get_order_from_bytes(dom0_mem); > > int res; > > paddr_t spfn; > > - unsigned int bits; > > > > - for ( bits = PAGE_SHIFT + 1; bits < PADDR_BITS; bits++ ) > > + if ( is_32bit_domain(d) ) > > { > > - pg = alloc_domheap_pages(d, order, MEMF_bits(bits)); > > - if ( pg != NULL ) > > - break; > > + printk("32 bit domain\n"); > > [..] > > > + printk("64 bit domain\n"); > > Can you be more explicit on both of theses messages? Nothing specify we > are allocate memory before them. ops, those were debugging which I shuld have removed.
On 04/04/2014 03:52 PM, Ian Campbell wrote: > On Fri, 2014-04-04 at 15:48 +0100, Julien Grall wrote: >> Can you be more explicit on both of theses messages? Nothing specify we >> are allocate memory before them. > > ops, those were debugging which I shuld have removed. With this change: Acked-by: Julien Grall <julien.grall@linaro.org>
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 4d6b26b..e7cc2c9 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -69,19 +69,21 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo) { paddr_t start; paddr_t size; - struct page_info *pg = NULL; + struct page_info *pg; unsigned int order = get_order_from_bytes(dom0_mem); int res; paddr_t spfn; - unsigned int bits; - for ( bits = PAGE_SHIFT + 1; bits < PADDR_BITS; bits++ ) + if ( is_32bit_domain(d) ) { - pg = alloc_domheap_pages(d, order, MEMF_bits(bits)); - if ( pg != NULL ) - break; + printk("32 bit domain\n"); + pg = alloc_domheap_pages(d, order, MEMF_bits(32)); + } + else + { + printk("64 bit domain\n"); + pg = alloc_domheap_pages(d, order, 0); } - if ( !pg ) panic("Failed to allocate contiguous memory for dom0");
This reverts commit 6c21cb36e263de2db8716b477157a5b6cd531e1e. The Linux = issue which this works around was fixed in v3.13 via f52bb722547f "ARM: mm: Correct virt_to_phys patching for 64 bit physical addresses". This is the second attempt to revert this. Now that we have fixed allocate_memory_11 to allocate accessible memory on 32-bit this is safe to do. This is not quite a straight revert since we need to ensure that for 32-bit domain 0 we do not allocate dom0's memory above 4GB where the domain cannot access it without paging (which is disabled at start of day) and LPAE (which the kernel may not support) enabled. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/domain_build.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)