diff mbox series

[Xen-devel,07/24] xen/arm: mm: Clean-up mfn_to_xen_entry

Message ID 20170613161323.25196-8-julien.grall@arm.com
State Accepted
Commit 7ddc55f2e3ecab095a99221971b56e1fbf85d803
Headers show
Series xen/arm: Extend the usage of typesafe MFN | expand

Commit Message

Julien Grall June 13, 2017, 4:13 p.m. UTC
The physical address is computed from the machine frame number, so
checking if the physical address is page aligned is pointless.

Furthermore, directly assigned the MFN to the corresponding field in the
entry rather than converting to a physical address and orring the value.
It will avoid to rely on the field position and make the code clearer.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Stefano Stabellini June 15, 2017, 10:38 p.m. UTC | #1
On Tue, 13 Jun 2017, Julien Grall wrote:
> The physical address is computed from the machine frame number, so
> checking if the physical address is page aligned is pointless.
> 
> Furthermore, directly assigned the MFN to the corresponding field in the
> entry rather than converting to a physical address and orring the value.
> It will avoid to rely on the field position and make the code clearer.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/mm.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 6f63e4315a..d164ed2eda 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -261,7 +261,6 @@ void dump_hyp_walk(vaddr_t addr)
>   */
>  static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>  {
> -    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
>      lpae_t e = (lpae_t) {
>          .pt = {
>              .valid = 1,           /* Mappings are present */
> @@ -316,11 +315,9 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
>          break;
>      }
>  
> -    ASSERT(!(pa & ~PAGE_MASK));
> -    ASSERT(!(pa & ~PADDR_MASK));
> +    ASSERT(!(pfn_to_paddr(mfn) & ~PADDR_MASK));
>  
> -    /* XXX shifts */
> -    e.bits |= pa;
> +    e.pt.base = mfn;
>  
>      return e;
>  }
> -- 
> 2.11.0
>
diff mbox series

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6f63e4315a..d164ed2eda 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -261,7 +261,6 @@  void dump_hyp_walk(vaddr_t addr)
  */
 static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
 {
-    paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
     lpae_t e = (lpae_t) {
         .pt = {
             .valid = 1,           /* Mappings are present */
@@ -316,11 +315,9 @@  static inline lpae_t mfn_to_xen_entry(unsigned long mfn, unsigned attr)
         break;
     }
 
-    ASSERT(!(pa & ~PAGE_MASK));
-    ASSERT(!(pa & ~PADDR_MASK));
+    ASSERT(!(pfn_to_paddr(mfn) & ~PADDR_MASK));
 
-    /* XXX shifts */
-    e.bits |= pa;
+    e.pt.base = mfn;
 
     return e;
 }