[PATCH 2/3] x86/boot: Simplify address calculations in move_memory()

Andrew Cooper posted 3 patches 1 month ago
[PATCH 2/3] x86/boot: Simplify address calculations in move_memory()
Posted by Andrew Cooper 1 month ago
Given that soffs is the offset into the 2M superpage,

  start = (src - soffs) >> PAGE_SIFT

is a complicated expression for the frame address of the containing superpage.
Except, start is converted straight back to a byte address to use, so the
shifting is unnecessary too.

The only done with the mapped pointer is to have soffs added back on for the
memmove() call.  bootstrap_map_addr() passes through the offset, so we can
pass src directly in and simplify the memmove() call too.  For the end mapping
address, this simplifies to just src + sz too.

The same reasoning holds for dst and doffs.

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>
---
 xen/arch/x86/setup.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 46a0b3093c2c..152cd696d6c2 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -492,31 +492,26 @@ static void __init noinline move_memory(
 
     while ( size )
     {
-        unsigned int start /* frame */;
         unsigned int end   /* mapsz */;
         unsigned int soffs = src & mask;
         unsigned int doffs = dst & mask;
         unsigned int sz;
         void *d, *s;
 
-        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);
+        s = bootstrap_map_addr(src, src + sz);
 
-        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);
+        d = bootstrap_map_addr(dst, dst + sz);
 
-        memmove(d + doffs, s + soffs, sz);
+        memmove(d, s, sz);
 
         dst += sz;
         src += sz;
-- 
2.39.5


Re: [PATCH 2/3] x86/boot: Simplify address calculations in move_memory()
Posted by Daniel P. Smith 1 month ago
On 10/22/24 18:39, Andrew Cooper wrote:
> Given that soffs is the offset into the 2M superpage,
> 
>    start = (src - soffs) >> PAGE_SIFT
> 
> is a complicated expression for the frame address of the containing superpage.
> Except, start is converted straight back to a byte address to use, so the
> shifting is unnecessary too.
> 
> The only done with the mapped pointer is to have soffs added back on for the

nit: think you dropped a word here, "The only done".

> memmove() call.  bootstrap_map_addr() passes through the offset, so we can
> pass src directly in and simplify the memmove() call too.  For the end mapping
> address, this simplifies to just src + sz too.
> 
> The same reasoning holds for dst and doffs.
> 
> 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>

After nit,

Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>