Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. Xen>
  3. Devel
[PATCH v3 5/8] xen/arm: preserve DTB mappings
stefano.stabellini at eu
Dec 18, 2012, 10:46 AM
Post #1 of 5 (765 views)
Permalink
At the moment we destroy the DTB mappings we have in setup_pagetables
and we don't restore them until setup_mm.

Keep the temporary DTB mapping until we create the new ones.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/kernel.h | 2 ++
xen/arch/arm/mm.c | 12 ++++++++++++
xen/arch/arm/setup.c | 1 +
xen/include/asm-arm/mm.h | 2 ++
4 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index 4533568..a179ffb 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -38,4 +38,6 @@ struct kernel_info {
int kernel_prepare(struct kernel_info *info);
void kernel_load(struct kernel_info *info);

+extern char _sdtb[];
+
#endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0d7a163..2410794 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -34,6 +34,7 @@
#include <asm/current.h>
#include <public/memory.h>
#include <xen/sched.h>
+#include "kernel.h"

struct domain *dom_xen, *dom_io;

@@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
/* TLBFLUSH and ISB would be needed here, but wait until we set WXN */

+ /* preserve the DTB mapping a little while longer */
+ pte = mfn_to_xen_entry(((unsigned long) _sdtb + boot_phys_offset) >> PAGE_SHIFT);
+ write_pte(xen_second + second_linear_offset(BOOT_MISC_VIRT_START), pte);
+
/* From now on, no mapping may be both writable and executable. */
WRITE_CP32(READ_CP32(HSCTLR) | SCTLR_WXN, HSCTLR);
/* Flush everything after setting WXN bit. */
flush_xen_text_tlb();
}

+void __init destroy_dtb_mapping(void)
+{
+ /* destroy old DTB mapping */
+ xen_second[second_linear_offset(BOOT_MISC_VIRT_START)].bits = 0;
+ dsb();
+}
+
/* MMU setup for secondary CPUS (which already have paging enabled) */
void __cpuinit mmu_init_secondary_cpu(void)
{
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 2076724..06a878f 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -229,6 +229,7 @@ void __init start_xen(unsigned long boot_phys_offset,

init_xen_time();

+ destroy_dtb_mapping();
setup_mm(atag_paddr, fdt_size);

/* Setup Hyp vector base */
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 4ed5df6..6fa4308 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -139,6 +139,8 @@ extern unsigned long total_pages;

/* Boot-time pagetable setup */
extern void setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr);
+/* Destroy temporary DTB mapping */
+extern void destroy_dtb_mapping(void);
/* MMU setup for seccondary CPUS (which already have paging enabled) */
extern void __cpuinit mmu_init_secondary_cpu(void);
/* Set up the xenheap: up to 1GB of contiguous, always-mapped memory.
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings [ In reply to ]
Ian.Campbell at citrix
Dec 19, 2012, 4:36 AM
Post #2 of 5 (737 views)
Permalink
> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
> /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */
>
> + /* preserve the DTB mapping a little while longer */

Not so much "preserve" as "put back" after this function clobbered it.

I'm not convinced by this effectively open coding of a "stack" of
mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be
used for ephemeral mappings within individual functions or over short
spans of code -- otherwise there is plenty of potential for clashes
which lead to hard to decipher bugs.

Can we not map the boot fdt as and when we need it instead of trying to
preserve a mapping? Or maybe FIXMAP_BOOT_FDT?

> + pte = mfn_to_xen_entry(((unsigned long) _sdtb + boot_phys_offset) >> PAGE_SHIFT);
> + write_pte(xen_second + second_linear_offset(BOOT_MISC_VIRT_START), pte);

This use of _sdtb is only valid if CONFIG_DTB_FILE. You need to
propagate atag_paddr as passed to start_xen.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings [ In reply to ]
stefano.stabellini at eu
Jan 8, 2013, 10:33 AM
Post #3 of 5 (735 views)
Permalink
On Wed, 19 Dec 2012, Ian Campbell wrote:
> > @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> > write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
> > /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */
> >
> > + /* preserve the DTB mapping a little while longer */
>
> Not so much "preserve" as "put back" after this function clobbered it.
>
> I'm not convinced by this effectively open coding of a "stack" of
> mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be
> used for ephemeral mappings within individual functions or over short
> spans of code -- otherwise there is plenty of potential for clashes
> which lead to hard to decipher bugs.
>
> Can we not map the boot fdt as and when we need it instead of trying to
> preserve a mapping?

I don't like this idea because it means that the device initialization
functions in start_xen that are called after setup_pagetables and before
setup_mm, like gic_init, suddenly become position dependent: they cannot
be moved after setup_mm.

> Or maybe FIXMAP_BOOT_FDT?

Assuming that it fits in a single 4k page, that could work. However we
would have a "temporary" fixmap mapping that is only used at boot time
and never again.
At that point maybe it is better to map the DTB somewhere else in
another 2MB slot from head.S, preserve the mapping in setup_pagetables
and remove it at the beginning of setup_mm. Maybe it could be called
BOOT_EARLY_FDT.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings [ In reply to ]
david.vrabel at citrix
Jan 8, 2013, 11:04 AM
Post #4 of 5 (736 views)
Permalink
On 08/01/13 18:33, Stefano Stabellini wrote:
> On Wed, 19 Dec 2012, Ian Campbell wrote:
>>> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>>> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
>>> /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */
>>>
>>> + /* preserve the DTB mapping a little while longer */
>>
>> Not so much "preserve" as "put back" after this function clobbered it.
>>
>> I'm not convinced by this effectively open coding of a "stack" of
>> mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be
>> used for ephemeral mappings within individual functions or over short
>> spans of code -- otherwise there is plenty of potential for clashes
>> which lead to hard to decipher bugs.
>>
>> Can we not map the boot fdt as and when we need it instead of trying to
>> preserve a mapping?
>
> I don't like this idea because it means that the device initialization
> functions in start_xen that are called after setup_pagetables and before
> setup_mm, like gic_init, suddenly become position dependent: they cannot
> be moved after setup_mm.

The mapping of the DTB into BOOT_MISC_VIRT_START was only for use by
device_tree_early_init(). Everything else should be using the final
mapping setup in setup_mm().

I think this means setup_mm() needs to immediately after
setup_pagestables() and before gic_init(), pl011_init() etc.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings [ In reply to ]
stefano.stabellini at eu
Jan 8, 2013, 11:32 AM
Post #5 of 5 (737 views)
Permalink
On Tue, 8 Jan 2013, David Vrabel wrote:
> On 08/01/13 18:33, Stefano Stabellini wrote:
> > On Wed, 19 Dec 2012, Ian Campbell wrote:
> >>> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> >>> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
> >>> /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */
> >>>
> >>> + /* preserve the DTB mapping a little while longer */
> >>
> >> Not so much "preserve" as "put back" after this function clobbered it.
> >>
> >> I'm not convinced by this effectively open coding of a "stack" of
> >> mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be
> >> used for ephemeral mappings within individual functions or over short
> >> spans of code -- otherwise there is plenty of potential for clashes
> >> which lead to hard to decipher bugs.
> >>
> >> Can we not map the boot fdt as and when we need it instead of trying to
> >> preserve a mapping?
> >
> > I don't like this idea because it means that the device initialization
> > functions in start_xen that are called after setup_pagetables and before
> > setup_mm, like gic_init, suddenly become position dependent: they cannot
> > be moved after setup_mm.
>
> The mapping of the DTB into BOOT_MISC_VIRT_START was only for use by
> device_tree_early_init(). Everything else should be using the final
> mapping setup in setup_mm().
>
> I think this means setup_mm() needs to immediately after
> setup_pagestables() and before gic_init(), pl011_init() etc.

That would be a very easy way out of this problem.
I think I am going to go for it.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal