@@ -492,26 +492,29 @@ static void __init noinline move_memory(
while ( size )
{
- module_t mod;
+ unsigned int start /* frame */;
+ unsigned int end /* mapsz */;
unsigned int soffs = src & mask;
unsigned int doffs = dst & mask;
unsigned int sz;
void *d, *s;
- mod.mod_start = (src - soffs) >> PAGE_SHIFT;
- mod.mod_end = soffs + size;
- if ( mod.mod_end > blksz )
- mod.mod_end = blksz;
- sz = mod.mod_end - soffs;
- s = bootstrap_map(&mod);
-
- mod.mod_start = (dst - doffs) >> PAGE_SHIFT;
- mod.mod_end = doffs + size;
- if ( mod.mod_end > blksz )
- mod.mod_end = blksz;
- if ( sz > mod.mod_end - doffs )
- sz = mod.mod_end - doffs;
- d = bootstrap_map(&mod);
+ start = (src - soffs) >> PAGE_SHIFT;
+ end = soffs + size;
+ if ( end > blksz )
+ end = blksz;
+ sz = end - soffs;
+ s = bootstrap_map_addr(pfn_to_paddr(start),
+ pfn_to_paddr(start) + end);
+
+ start = (dst - doffs) >> PAGE_SHIFT;
+ end = doffs + size;
+ if ( end > blksz )
+ end = blksz;
+ if ( sz > end - doffs )
+ sz = end - doffs;
+ d = bootstrap_map_addr(pfn_to_paddr(start),
+ pfn_to_paddr(start) + end);
memmove(d + doffs, s + soffs, sz);
@@ -519,7 +522,7 @@ static void __init noinline move_memory(
src += sz;
size -= sz;
- bootstrap_map(NULL);
+ bootstrap_map_addr(0, 0);
}
}
move_memory() is very complicated, and buggy. In order to fix the latter, we have to address the former. Given prior cleanup, bootstrap_map() is now implemented in terms of bootstrap_map_addr(), meaining that it is counterproductive to plumb the mapping through module_t. Delete mod, and introduce two same-sized/named fields. At this point in boot, neither fields have their named purpose, so indicate the purpose in comments. No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Daniel P. Smith <dpsmith@apertussolutions.com> This is broken out both to ease review (I recommend git diff --color-words), and for bisection purposes given how many times I've failed to edit this logic correctly. This does not conflict with anything in Hyperlaunch v7, but it does remove the only callers of bootstrap_map() remaining by the end of the series, allowing for even more cleanup. In terms of bugs. The ones I've spotted so far are: * The blksz calculation depends on the previous thing having been unmapped first. * The calculation halving blksz based on BOOTSTRAP_MAP_BASE is bogus. It might plausibly be a remnent of 32bit Xen with the absence of highmem. * The caller of move_memory() is strictly moving modules forward in memory (dst > src) with a possibility of partial overlap. The while loop maps dst and src, blksz chunks at a time, ascending, with s < d. This results in memmove() doing an unconditional backwards copy. I don't see how this logic could ever have worked for an overlapping move. * The caller of move_memory() already has a good mapping for dst, so we don't even need to split BOOTSTRAP_MAP_LIMIT in half to map both parts. --- xen/arch/x86/setup.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)