# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1360259288 0
# Node ID 7feb83e3f79a3094e3002560c896a9ee97f1e907
# Parent 6c1b12c884b4521a940e079c8dfebc5d8e88d2e9
x86/setup: don''t relocate the VGA hole.
Copying the contents of the VGA hole is at best pointless and at worst
dangerous. Booting Xen on Xen, it causes a very long delay as each
byte is referred to qemu.
Since we were already discarding the first 1MB of the relocated area,
just avoid copying it in the first place.
Reported-by: Jon Ludlam <jonathan.ludlam@eu.citrix.com>
Signed-off-by: Tim Deegan <tim@xen.org>
diff -r 6c1b12c884b4 -r 7feb83e3f79a xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c Tue Feb 05 15:47:41 2013 +0000
+++ b/xen/arch/x86/setup.c Thu Feb 07 17:48:08 2013 +0000
@@ -826,7 +826,6 @@ void __init __start_xen(unsigned long mb
l3_pgentry_t *pl3e;
l2_pgentry_t *pl2e;
int i, j, k;
- void *dst;
/* Select relocation address. */
e = end - reloc_size;
@@ -840,10 +839,8 @@ void __init __start_xen(unsigned long mb
* data until after we have switched to the relocated pagetables!
*/
barrier();
- dst = move_memory(e, 0, (unsigned long)&_end - XEN_VIRT_START,
1);
-
- /* Poison low 1MB to detect stray pointers to physical 0-1MB. */
- memset(dst, 0x55, 1U << 20);
+ move_memory(e + (1u << 20), (1u << 20),
+ ((unsigned long)&_end - XEN_VIRT_START) - (1u
<< 20), 1);
/* Walk initial pagetables, relocating page directory entries. */
pl4e = __va(__pa(idle_pg_table));
> @@ -840,10 +839,8 @@ void __init __start_xen(unsigned long mb > * data until after we have switched to the relocated pagetables! > */ > barrier(); > - dst = move_memory(e, 0, (unsigned long)&_end - XEN_VIRT_START, 1); > - > - /* Poison low 1MB to detect stray pointers to physical 0-1MB. */ > - memset(dst, 0x55, 1U << 20);Is this poisoning not still desirable?> + move_memory(e + (1u << 20), (1u << 20), > + ((unsigned long)&_end - XEN_VIRT_START) - (1u << 20), 1); >
On 07/02/2013 20:20, "Ian Campbell" <ian.campbell@citrix.com> wrote:>> @@ -840,10 +839,8 @@ void __init __start_xen(unsigned long mb >> * data until after we have switched to the relocated >> pagetables! >> */ >> barrier(); >> - dst = move_memory(e, 0, (unsigned long)&_end - XEN_VIRT_START, >> 1); >> - >> - /* Poison low 1MB to detect stray pointers to physical 0-1MB. */ >> - memset(dst, 0x55, 1U << 20); > > Is this poisoning not still desirable?If we don''t copy that low 1MB I don''t see any reason to poison it at the destination.>> + move_memory(e + (1u << 20), (1u << 20), >> + ((unsigned long)&_end - XEN_VIRT_START) - (1u << 20), >> 1);1MB here is actually something of a magic number. We could make use of _start instead, eg: off = (ulong)&_start - XEN_VIRT_START; move_memory(e + off, off, (ulong)&_end - (ulong)&_start, 1); This would be my preference. -- Keir> > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Thu, 2013-02-07 at 20:38 +0000, Keir Fraser wrote:> On 07/02/2013 20:20, "Ian Campbell" <ian.campbell@citrix.com> wrote: > > >> @@ -840,10 +839,8 @@ void __init __start_xen(unsigned long mb > >> * data until after we have switched to the relocated > >> pagetables! > >> */ > >> barrier(); > >> - dst = move_memory(e, 0, (unsigned long)&_end - XEN_VIRT_START, > >> 1); > >> - > >> - /* Poison low 1MB to detect stray pointers to physical 0-1MB. */ > >> - memset(dst, 0x55, 1U << 20); > > > > Is this poisoning not still desirable? > > If we don''t copy that low 1MB I don''t see any reason to poison it at the > destination.The poisoning was to protect against stray pointers out of the low 1MB region? I assumed it was against pointers into it... Ian.
On 07/02/2013 21:29, "Ian Campbell" <ian.campbell@citrix.com> wrote:> On Thu, 2013-02-07 at 20:38 +0000, Keir Fraser wrote: >> On 07/02/2013 20:20, "Ian Campbell" <ian.campbell@citrix.com> wrote: >> >>>> @@ -840,10 +839,8 @@ void __init __start_xen(unsigned long mb >>>> * data until after we have switched to the relocated >>>> pagetables! >>>> */ >>>> barrier(); >>>> - dst = move_memory(e, 0, (unsigned long)&_end - XEN_VIRT_START, >>>> 1); >>>> - >>>> - /* Poison low 1MB to detect stray pointers to physical 0-1MB. >>>> */ >>>> - memset(dst, 0x55, 1U << 20); >>> >>> Is this poisoning not still desirable? >> >> If we don''t copy that low 1MB I don''t see any reason to poison it at the >> destination. > > The poisoning was to protect against stray pointers out of the low 1MB > region? I assumed it was against pointers into it...I mean any stray pointers into that region are only going to see garbage at the relocated highmem address anyway, as contents of 0-1MB are no longer being copied. So deliberate poisoning seems even more paranoid than it already was (it was already really quite paranoid). -- Keir> Ian. >
On Thu, 2013-02-07 at 21:38 +0000, Keir Fraser wrote:> > The poisoning was to protect against stray pointers out of the low 1MB > > region? I assumed it was against pointers into it... > > I mean any stray pointers into that region are only going to see garbage at > the relocated highmem address anyway, as contents of 0-1MB are no longer > being copied. So deliberate poisoning seems even more paranoid than it > already was (it was already really quite paranoid).Right, I assumed the idea was that you could recognise the 0x55...55 as being something more specific than random garbage. It is indeed quite paranoid ;-) Ian.