* [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP
@ 2017-03-13 11:43 vijay.kilari
2017-03-13 13:42 ` Jan Beulich
2017-03-14 15:32 ` Julien Grall
0 siblings, 2 replies; 6+ messages in thread
From: vijay.kilari @ 2017-03-13 11:43 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, julien.grall, jbeulich, Vijaya Kumar K
From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
On ARM, virt_to_mfn uses the hardware for address
translation. So if the virtual address is not mapped translation
fault is raised.On ARM, DIRECTMAP_VIRT region is direct mapped.
On ARM with NUMA, While initializing second memory node,
panic is triggered from init_node_heap() when virt_to_mfn()
is called for DIRECTMAP_VIRT region address.
Here the check is made to ensure that MFN less than max MFN mapped.
The max MFN is found by calling virt_to_mfn of DIRECTMAP_VIRT_END
region. Since DIRECMAP_VIRT region is not mapped to any virtual address
on ARM, it fails.
In this patch, instead of calling virt_to_mfn(), arch helper
arch_mfn_below_directmap_max_mfn() is introduced. On ARM this arch helper
will return 1 always and for x86 this helper does virt_to_mfn.
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
xen/common/page_alloc.c | 7 ++-----
xen/include/asm-arm/mm.h | 7 +++++++
xen/include/asm-x86/page.h | 16 ++++++++++++++++
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 42c20cb..85322cd 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -520,9 +520,6 @@ static unsigned long init_node_heap(int node, unsigned long mfn,
unsigned long needed = (sizeof(**_heap) +
sizeof(**avail) * NR_ZONES +
PAGE_SIZE - 1) >> PAGE_SHIFT;
-#ifdef DIRECTMAP_VIRT_END
- unsigned long eva = min(DIRECTMAP_VIRT_END, HYPERVISOR_VIRT_END);
-#endif
int i, j;
if ( !first_node_initialised )
@@ -534,7 +531,7 @@ static unsigned long init_node_heap(int node, unsigned long mfn,
}
#ifdef DIRECTMAP_VIRT_END
else if ( *use_tail && nr >= needed &&
- (mfn + nr) <= (virt_to_mfn(eva - 1) + 1) &&
+ arch_mfn_below_directmap_max_mfn(mfn + nr) &&
(!xenheap_bits ||
!((mfn + nr - 1) >> (xenheap_bits - PAGE_SHIFT))) )
{
@@ -543,7 +540,7 @@ static unsigned long init_node_heap(int node, unsigned long mfn,
PAGE_SIZE - sizeof(**avail) * NR_ZONES;
}
else if ( nr >= needed &&
- (mfn + needed) <= (virt_to_mfn(eva - 1) + 1) &&
+ arch_mfn_below_directmap_max_mfn(mfn + needed) &&
(!xenheap_bits ||
!((mfn + needed - 1) >> (xenheap_bits - PAGE_SHIFT))) )
{
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 60ccbf3..f0c90c2 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -260,6 +260,13 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
#define virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
#define mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
+/*
+ * On ARM DIRECTMAP_VIRT region is directly mapped. Hence return true;
+ */
+static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
+{
+ return 1;
+}
/* Convert between Xen-heap virtual addresses and page-info structures. */
static inline struct page_info *virt_to_page(const void *v)
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index 46faffc..3ea5310 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -18,6 +18,7 @@
#ifndef __ASSEMBLY__
# include <asm/types.h>
# include <xen/lib.h>
+# include <xen/kernel.h>
#endif
#include <asm/x86_64/page.h>
@@ -374,6 +375,21 @@ perms_strictly_increased(uint32_t old_flags, uint32_t new_flags)
return ((of | (of ^ nf)) == nf);
}
+/*
+ * x86 maps DIRECTMAP_VIRT to physical memory. Get the mfn for directmap
+ * memory region.
+ */
+static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
+{
+#ifdef DIRECTMAP_VIRT_END
+ unsigned long eva = min(DIRECTMAP_VIRT_END, HYPERVISOR_VIRT_END);
+
+ return mfn <= (virt_to_mfn(eva - 1) + 1);
+#else
+ return 0;
+#endif
+}
+
#endif /* !__ASSEMBLY__ */
#define PAGE_ALIGN(x) (((x) + PAGE_SIZE - 1) & PAGE_MASK)
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP
2017-03-13 11:43 [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP vijay.kilari
@ 2017-03-13 13:42 ` Jan Beulich
2017-03-14 4:57 ` Vijay Kilari
2017-03-14 15:32 ` Julien Grall
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-03-13 13:42 UTC (permalink / raw)
To: vijay.kilari
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, Vijaya Kumar K, julien.grall, xen-devel
>>> On 13.03.17 at 12:43, <vijay.kilari@gmail.com> wrote:
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -260,6 +260,13 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
> #define virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
> #define mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
>
> +/*
> + * On ARM DIRECTMAP_VIRT region is directly mapped. Hence return true;
> + */
> +static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
> +{
> + return 1;
> +}
bool and true respectively, please (also on the x86 side).
> --- a/xen/include/asm-x86/page.h
> +++ b/xen/include/asm-x86/page.h
> @@ -18,6 +18,7 @@
> #ifndef __ASSEMBLY__
> # include <asm/types.h>
> # include <xen/lib.h>
> +# include <xen/kernel.h>
Why?
> @@ -374,6 +375,21 @@ perms_strictly_increased(uint32_t old_flags, uint32_t new_flags)
> return ((of | (of ^ nf)) == nf);
> }
>
> +/*
> + * x86 maps DIRECTMAP_VIRT to physical memory. Get the mfn for directmap
> + * memory region.
> + */
> +static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
> +{
> +#ifdef DIRECTMAP_VIRT_END
The symbol is always defined on x86 afaics - no need for the #ifdef.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP
2017-03-13 13:42 ` Jan Beulich
@ 2017-03-14 4:57 ` Vijay Kilari
0 siblings, 0 replies; 6+ messages in thread
From: Vijay Kilari @ 2017-03-14 4:57 UTC (permalink / raw)
To: Jan Beulich
Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson, Vijaya Kumar K, Julien Grall,
xen-devel
On Mon, Mar 13, 2017 at 7:12 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 13.03.17 at 12:43, <vijay.kilari@gmail.com> wrote:
>> --- a/xen/include/asm-arm/mm.h
>> +++ b/xen/include/asm-arm/mm.h
>> @@ -260,6 +260,13 @@ static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
>> #define virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
>> #define mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT))
>>
>> +/*
>> + * On ARM DIRECTMAP_VIRT region is directly mapped. Hence return true;
>> + */
>> +static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
>> +{
>> + return 1;
>> +}
>
> bool and true respectively, please (also on the x86 side).
OK
>
>> --- a/xen/include/asm-x86/page.h
>> +++ b/xen/include/asm-x86/page.h
>> @@ -18,6 +18,7 @@
>> #ifndef __ASSEMBLY__
>> # include <asm/types.h>
>> # include <xen/lib.h>
>> +# include <xen/kernel.h>
>
> Why?
Compilation fails saying min() is not defined
>
>> @@ -374,6 +375,21 @@ perms_strictly_increased(uint32_t old_flags, uint32_t new_flags)
>> return ((of | (of ^ nf)) == nf);
>> }
>>
>> +/*
>> + * x86 maps DIRECTMAP_VIRT to physical memory. Get the mfn for directmap
>> + * memory region.
>> + */
>> +static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
>> +{
>> +#ifdef DIRECTMAP_VIRT_END
>
> The symbol is always defined on x86 afaics - no need for the #ifdef.
ok.
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP
2017-03-13 11:43 [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP vijay.kilari
2017-03-13 13:42 ` Jan Beulich
@ 2017-03-14 15:32 ` Julien Grall
2017-03-15 5:11 ` Vijay Kilari
1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2017-03-14 15:32 UTC (permalink / raw)
To: vijay.kilari, xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, jbeulich, Vijaya Kumar K
Hello Vijay,
On 13/03/17 11:43, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> On ARM, virt_to_mfn uses the hardware for address
> translation. So if the virtual address is not mapped translation
> fault is raised.On ARM, DIRECTMAP_VIRT region is direct mapped.
This is not true. As I said before, all the memory will be direct mapped
on ARM64 but not on ARM32.
For ARM32, only xenheap will be direct mapped. So you may want to return
is_xenheap_mfn(...). Or even return false in all the case. Either is
fine by me, but it would need to be explained in the code.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP
2017-03-14 15:32 ` Julien Grall
@ 2017-03-15 5:11 ` Vijay Kilari
2017-03-24 10:54 ` Julien Grall
0 siblings, 1 reply; 6+ messages in thread
From: Vijay Kilari @ 2017-03-15 5:11 UTC (permalink / raw)
To: Julien Grall
Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson, xen-devel, Jan Beulich,
Vijaya Kumar K
On Tue, Mar 14, 2017 at 9:02 PM, Julien Grall <julien.grall@arm.com> wrote:
> Hello Vijay,
>
> On 13/03/17 11:43, vijay.kilari@gmail.com wrote:
>>
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> On ARM, virt_to_mfn uses the hardware for address
>> translation. So if the virtual address is not mapped translation
>> fault is raised.On ARM, DIRECTMAP_VIRT region is direct mapped.
>
>
> This is not true. As I said before, all the memory will be direct mapped on
> ARM64 but not on ARM32.
>
> For ARM32, only xenheap will be direct mapped. So you may want to return
> is_xenheap_mfn(...). Or even return false in all the case. Either is fine by
> me, but it would need to be explained in the code.
Is this ok?.
/*
* On ARM64 DIRECTMAP_VIRT region is directly mapped. Hence return true;
* On ARM only xenheap memory is directly mapped. Hence return false.
*/
static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
{
#ifdef CONFIG_ARM_64
return true;
#else
return false;
#endif
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP
2017-03-15 5:11 ` Vijay Kilari
@ 2017-03-24 10:54 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2017-03-24 10:54 UTC (permalink / raw)
To: Vijay Kilari
Cc: Tim Deegan, Stefano Stabellini, Wei Liu, George Dunlap,
Andrew Cooper, Ian Jackson, xen-devel, Jan Beulich,
Vijaya Kumar K
Hi Vijay,
On 03/15/2017 05:11 AM, Vijay Kilari wrote:
> On Tue, Mar 14, 2017 at 9:02 PM, Julien Grall <julien.grall@arm.com> wrote:
>> Hello Vijay,
>>
>> On 13/03/17 11:43, vijay.kilari@gmail.com wrote:
>>>
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> On ARM, virt_to_mfn uses the hardware for address
>>> translation. So if the virtual address is not mapped translation
>>> fault is raised.On ARM, DIRECTMAP_VIRT region is direct mapped.
>>
>>
>> This is not true. As I said before, all the memory will be direct mapped on
>> ARM64 but not on ARM32.
>>
>> For ARM32, only xenheap will be direct mapped. So you may want to return
>> is_xenheap_mfn(...). Or even return false in all the case. Either is fine by
>> me, but it would need to be explained in the code.
>
> Is this ok?.
>
> /*
> * On ARM64 DIRECTMAP_VIRT region is directly mapped. Hence return true;
> * On ARM only xenheap memory is directly mapped. Hence return false.
> */
> static inline bool_t arch_mfn_below_directmap_max_mfn(unsigned long mfn)
> {
> #ifdef CONFIG_ARM_64
> return true;
> #else
> return false;
> #endif
> }
I would implement the helper in {arm32,arm64}/mm.h and avoid the #ifdef
in the implementation.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-24 10:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 11:43 [PATCH v2] boot allocator: Use arch helper for virt_to_mfn on DIRECTMAP vijay.kilari
2017-03-13 13:42 ` Jan Beulich
2017-03-14 4:57 ` Vijay Kilari
2017-03-14 15:32 ` Julien Grall
2017-03-15 5:11 ` Vijay Kilari
2017-03-24 10:54 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.