Patchwork [for-4.10,1/2] xen/arm: mm: Change the return value of gvirt_to_maddr

login
register
mail settings
Submitter Julien Grall
Date Nov. 15, 2017, 7:34 p.m.
Message ID <20171115193414.7678-2-julien.grall@linaro.org>
Download mbox | patch
Permalink /patch/384711/
State New
Headers show

Comments

Julien Grall - Nov. 15, 2017, 7:34 p.m.
Currently, gvirt_to_maddr return -EFAULT when the translation failed.
It might be useful to return the PAR_EL1 (Physical Address Register)
in such a case to get a better idea of the reason.

So modify the return value to use 0 on sucess or return the PAR on
failure.

The callers are modified to reflect the change of the return value.

Note that with the change in gvirt_to_maddr, ma needs to be initialized
to avoid GCC been confused (i.e value may be unitialized) with the new
construction.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/domain_build.c | 8 ++++----
 xen/arch/arm/kernel.c       | 8 ++++----
 xen/arch/arm/p2m.c          | 6 +++---
 xen/include/asm-arm/mm.h    | 9 +++++++--
 4 files changed, 18 insertions(+), 13 deletions(-)
Stefano Stabellini - Nov. 16, 2017, 1:35 a.m.
On Wed, 15 Nov 2017, Julien Grall wrote:
> Currently, gvirt_to_maddr return -EFAULT when the translation failed.
> It might be useful to return the PAR_EL1 (Physical Address Register)
> in such a case to get a better idea of the reason.
> 
> So modify the return value to use 0 on sucess or return the PAR on
                                          ^ success


> failure.
> 
> The callers are modified to reflect the change of the return value.
> 
> Note that with the change in gvirt_to_maddr, ma needs to be initialized
> to avoid GCC been confused (i.e value may be unitialized) with the new
                                                 ^ uninitialized


> construction.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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

I fixed on commit


> ---
>  xen/arch/arm/domain_build.c | 8 ++++----
>  xen/arch/arm/kernel.c       | 8 ++++----
>  xen/arch/arm/p2m.c          | 6 +++---
>  xen/include/asm-arm/mm.h    | 9 +++++++--
>  4 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index bf29299707..c74f4dd69d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2002,15 +2002,15 @@ static void initrd_load(struct kernel_info *kinfo)
>  
>      for ( offs = 0; offs < len; )
>      {
> -        int rc;
> -        paddr_t s, l, ma;
> +        uint64_t par;
> +        paddr_t s, l, ma = 0;
>          void *dst;
>  
>          s = offs & ~PAGE_MASK;
>          l = min(PAGE_SIZE - s, len);
>  
> -        rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> -        if ( rc )
> +        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> +        if ( par )
>          {
>              panic("Unable to translate guest address");
>              return;
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index c2755a9ab9..a6c6413712 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -167,15 +167,15 @@ static void kernel_zimage_load(struct kernel_info *info)
>             paddr, load_addr, load_addr + len);
>      for ( offs = 0; offs < len; )
>      {
> -        int rc;
> -        paddr_t s, l, ma;
> +        uint64_t par;
> +        paddr_t s, l, ma = 0;
>          void *dst;
>  
>          s = offs & ~PAGE_MASK;
>          l = min(PAGE_SIZE - s, len);
>  
> -        rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> -        if ( rc )
> +        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
> +        if ( par )
>          {
>              panic("Unable to map translate guest address");
>              return;
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 68b488997d..f6b3d8e421 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1414,7 +1414,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>      struct p2m_domain *p2m = p2m_get_hostp2m(d);
>      struct page_info *page = NULL;
>      paddr_t maddr = 0;
> -    int rc;
> +    uint64_t par;
>  
>      /*
>       * XXX: To support a different vCPU, we would need to load the
> @@ -1425,9 +1425,9 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
>  
>      p2m_read_lock(p2m);
>  
> -    rc = gvirt_to_maddr(va, &maddr, flags);
> +    par = gvirt_to_maddr(va, &maddr, flags);
>  
> -    if ( rc )
> +    if ( par )
>          goto err;
>  
>      if ( !mfn_valid(maddr_to_mfn(maddr)) )
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index cd6dfb54b9..ad2f2a43dc 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -266,11 +266,16 @@ static inline void *maddr_to_virt(paddr_t ma)
>  }
>  #endif
>  
> -static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
> +/*
> + * Translate a guest virtual address to a machine address.
> + * Return the fault information if the translation has failed else 0.
> + */
> +static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
> +                                      unsigned int flags)
>  {
>      uint64_t par = gva_to_ma_par(va, flags);
>      if ( par & PAR_F )
> -        return -EFAULT;
> +        return par;
>      *pa = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
>      return 0;
>  }
> -- 
> 2.11.0
>

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index bf29299707..c74f4dd69d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2002,15 +2002,15 @@  static void initrd_load(struct kernel_info *kinfo)
 
     for ( offs = 0; offs < len; )
     {
-        int rc;
-        paddr_t s, l, ma;
+        uint64_t par;
+        paddr_t s, l, ma = 0;
         void *dst;
 
         s = offs & ~PAGE_MASK;
         l = min(PAGE_SIZE - s, len);
 
-        rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
-        if ( rc )
+        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
+        if ( par )
         {
             panic("Unable to translate guest address");
             return;
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index c2755a9ab9..a6c6413712 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -167,15 +167,15 @@  static void kernel_zimage_load(struct kernel_info *info)
            paddr, load_addr, load_addr + len);
     for ( offs = 0; offs < len; )
     {
-        int rc;
-        paddr_t s, l, ma;
+        uint64_t par;
+        paddr_t s, l, ma = 0;
         void *dst;
 
         s = offs & ~PAGE_MASK;
         l = min(PAGE_SIZE - s, len);
 
-        rc = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
-        if ( rc )
+        par = gvirt_to_maddr(load_addr + offs, &ma, GV2M_WRITE);
+        if ( par )
         {
             panic("Unable to map translate guest address");
             return;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 68b488997d..f6b3d8e421 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1414,7 +1414,7 @@  struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     struct page_info *page = NULL;
     paddr_t maddr = 0;
-    int rc;
+    uint64_t par;
 
     /*
      * XXX: To support a different vCPU, we would need to load the
@@ -1425,9 +1425,9 @@  struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
 
     p2m_read_lock(p2m);
 
-    rc = gvirt_to_maddr(va, &maddr, flags);
+    par = gvirt_to_maddr(va, &maddr, flags);
 
-    if ( rc )
+    if ( par )
         goto err;
 
     if ( !mfn_valid(maddr_to_mfn(maddr)) )
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index cd6dfb54b9..ad2f2a43dc 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -266,11 +266,16 @@  static inline void *maddr_to_virt(paddr_t ma)
 }
 #endif
 
-static inline int gvirt_to_maddr(vaddr_t va, paddr_t *pa, unsigned int flags)
+/*
+ * Translate a guest virtual address to a machine address.
+ * Return the fault information if the translation has failed else 0.
+ */
+static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
+                                      unsigned int flags)
 {
     uint64_t par = gva_to_ma_par(va, flags);
     if ( par & PAR_F )
-        return -EFAULT;
+        return par;
     *pa = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
     return 0;
 }