xen: arm: do not panic when failing to translate a guest address

The gva_to_{par,ipa} functions currently panic if the guest address
cannot be translated. Often the input comes from the guest so
panicing the host is a bit harsh!

Change the API of those functions to allow them to return a failure
and plumb it through where it is used.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index d9eb7ac55897ecc0ed3bd594e458727c011d8fc8..5504e197e2934a8283bb964bfcdf369419209f4e 100644 (file)
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -12,10 +12,16 @@ unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len)
 
     while ( len )
     {
-        paddr_t g = gvirt_to_maddr((uint32_t) to);
-        void *p = map_domain_page(g>>PAGE_SHIFT);
+        int rc;
+        paddr_t g;
+        void *p;
         unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
 
+        rc = gvirt_to_maddr((uint32_t) to, &g);
+        if ( rc )
+            return rc;
+
+        p = map_domain_page(g>>PAGE_SHIFT);
         p += offset;
         memcpy(p, from, size);
 
@@ -36,10 +42,16 @@ unsigned long raw_clear_guest(void *to, unsigned len)
 
     while ( len )
     {
-        paddr_t g = gvirt_to_maddr((uint32_t) to);
-        void *p = map_domain_page(g>>PAGE_SHIFT);
+        int rc;
+        paddr_t g;
+        void *p;
         unsigned size = min(len, (unsigned)PAGE_SIZE - offset);
 
+        rc = gvirt_to_maddr((uint32_t) to, &g);
+        if ( rc )
+            return rc;
+
+        p = map_domain_page(g>>PAGE_SHIFT);
         p += offset;
         memset(p, 0x00, size);
 
@@ -56,10 +68,16 @@ unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned le
 {
     while ( len )
     {
-        paddr_t g = gvirt_to_maddr((uint32_t) from & PAGE_MASK);
-        void *p = map_domain_page(g>>PAGE_SHIFT);
+        int rc;
+        paddr_t g;
+        void *p;
         unsigned size = min(len, (unsigned)(PAGE_SIZE - ((unsigned)from & (~PAGE_MASK))));
 
+        rc = gvirt_to_maddr((uint32_t) from & PAGE_MASK, &g);
+        if ( rc )
+            return rc;
+
+        p = map_domain_page(g>>PAGE_SHIFT);
         p += ((unsigned long)from & (~PAGE_MASK));
 
         memcpy(to, p, size);

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 2f7a9ff2a5fbc95eaa276c9d2e09541ff3329535..c08c2307403dc03c6a43d47734b0dd826e4f82ef 100644 (file)
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -76,12 +76,22 @@ static void kernel_zimage_load(struct kernel_info *info)
            paddr, load_addr, load_addr + len);
     for ( offs = 0; offs < len; )
     {
-        paddr_t s, l, ma = gvirt_to_maddr(load_addr + offs);
-        void *dst = map_domain_page(ma>>PAGE_SHIFT);
+        int rc;
+        paddr_t s, l, ma;
+        void *dst;
 
         s = offs & ~PAGE_MASK;
         l = min(PAGE_SIZE - s, len);
 
+        rc = gvirt_to_maddr(load_addr + offs, &ma);
+        if ( rc )
+        {
+            panic("\nUnable to map translate guest address\n");
+            return;
+        }
+
+        dst = map_domain_page(ma>>PAGE_SHIFT);
+
         copy_from_paddr(dst + s, paddr + offs, l, attr);
 
         unmap_domain_page(dst);

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8afcf0c4ab93e771a7af6d9016a6b2c20877cc5b..0a1c48343f40780dcc204432b00fd86db495900c 100644 (file)
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -200,32 +200,22 @@ static const char *fsc_level_str(int level)
     }
 }
 
-void panic_PAR(uint64_t par, const char *when)
+void panic_PAR(uint64_t par)
 {
-    if ( par & PAR_F )
-    {
-        const char *msg;
-        int level = -1;
-        int stage = par & PAR_STAGE2 ? 2 : 1;
-        int second_in_first = !!(par & PAR_STAGE21);
-
-        msg = decode_fsc( (par&PAR_FSC_MASK) >> PAR_FSC_SHIFT, &level);
-
-        printk("PAR: %010"PRIx64": %s stage %d%s%s\n",
-               par, msg,
-               stage,
-               second_in_first ? " during second stage lookup" : "",
-               fsc_level_str(level));
-    }
-    else
-    {
-        printk("PAR: %010"PRIx64": paddr:%010"PRIx64
-               " attr %"PRIx64" sh %"PRIx64" %s\n",
-               par, par & PADDR_MASK, par >> PAR_MAIR_SHIFT,
-               (par & PAR_SH_MASK) >> PAR_SH_SHIFT,
-               (par & PAR_NS) ? "Non-Secure" : "Secure");
-    }
-    panic("Error during %s-to-physical address translation\n", when);
+    const char *msg;
+    int level = -1;
+    int stage = par & PAR_STAGE2 ? 2 : 1;
+    int second_in_first = !!(par & PAR_STAGE21);
+
+    msg = decode_fsc( (par&PAR_FSC_MASK) >> PAR_FSC_SHIFT, &level);
+
+    printk("PAR: %010"PRIx64": %s stage %d%s%s\n",
+           par, msg,
+           stage,
+           second_in_first ? " during second stage lookup" : "",
+           fsc_level_str(level));
+
+    panic("Error during Hypervisor-to-physical address translation\n");
 }
 
 struct reg_ctxt {
@@ -721,7 +711,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
                                      struct hsr_dabt dabt)
 {
     const char *msg;
-    int level = -1;
+    int rc, level = -1;
     mmio_info_t info;
 
     info.dabt = dabt;
@@ -730,7 +720,9 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
     if (dabt.s1ptw)
         goto bad_data_abort;
 
-    info.gpa = gva_to_ipa(info.gva);
+    rc = gva_to_ipa(info.gva, &info.gpa);
+    if ( rc == -EFAULT )
+        goto bad_data_abort;
 
     if (handle_mmio(&info))
     {
@@ -742,6 +734,7 @@ bad_data_abort:
 
     msg = decode_fsc( dabt.dfsc, &level);
 
+    /* XXX inject a suitable fault into the guest */
     printk("Guest data abort: %s%s%s\n"
            "    gva=%"PRIx32"\n",
            msg, dabt.s1ptw ? " S2 during S1" : "",
@@ -761,7 +754,7 @@ bad_data_abort:
     else
         dump_guest_s1_walk(current->domain, info.gva);
     show_execution_state(regs);
-    panic("Unhandled guest data abort\n");
+    domain_crash_synchronous();
 }
 
 asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 67eb7d2595a0f31198d0fedaff9f4ce90c6935c1..db6818cf5f68351704a6f2d099d59c9c879b271a 100644 (file)
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -192,10 +192,13 @@ static inline void *maddr_to_virt(paddr_t ma)
     return (void *)(unsigned long) ma + XENHEAP_VIRT_START;
 }
 
-static inline paddr_t gvirt_to_maddr(uint32_t va)
+static inline int gvirt_to_maddr(uint32_t va, paddr_t *pa)
 {
-    uint64_t par = gva_to_par(va);
-    return (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
+    uint64_t par = gva_to_ma_par(va);
+    if ( par & PAR_F )
+        return -EFAULT;
+    *pa = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
+    return 0;
 }
 
 /* Convert between Xen-heap virtual addresses and machine addresses. */

diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 090d93f46176ad6586630981fd642059f1cdaedd..9acd0af9a1b612a9f65094081b37f7f7217362a3 100644 (file)
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -2,6 +2,7 @@
 #define __ARM_PAGE_H__
 
 #include <xen/config.h>
+#include <xen/errno.h>
 #include <public/xen.h>
 #include <asm/processor.h>
 
@@ -366,13 +367,13 @@ static inline uint64_t va_to_par(uint32_t va)
     if ( par & PAR_F )
     {
         dump_hyp_walk(va);
-        panic_PAR(par, "Hypervisor");
+        panic_PAR(par);
     }
     return par;
 }
 
 /* Ask the MMU to translate a Guest VA for us */
-static inline uint64_t __gva_to_par(uint32_t va)
+static inline uint64_t gva_to_ma_par(uint32_t va)
 {
     uint64_t par, tmp;
     tmp = READ_CP64(PAR);
@@ -382,15 +383,7 @@ static inline uint64_t __gva_to_par(uint32_t va)
     WRITE_CP64(tmp, PAR);
     return par;
 }
-static inline uint64_t gva_to_par(uint32_t va)
-{
-    uint64_t par = __gva_to_par(va);
-    /* It is not OK to call this with an invalid VA */
-    /* XXX harsh for a guest address... */
-    if ( par & PAR_F ) panic_PAR(par, "Guest");
-    return par;
-}
-static inline uint64_t __gva_to_ipa(uint32_t va)
+static inline uint64_t gva_to_ipa_par(uint32_t va)
 {
     uint64_t par, tmp;
     tmp = READ_CP64(PAR);
@@ -400,14 +393,16 @@ static inline uint64_t __gva_to_ipa(uint32_t va)
     WRITE_CP64(tmp, PAR);
     return par;
 }
-static inline uint64_t gva_to_ipa(uint32_t va)
+
+static inline int gva_to_ipa(uint32_t va, paddr_t *paddr)
 {
-    uint64_t par = __gva_to_ipa(va);
-    /* It is not OK to call this with an invalid VA */
-    /* XXX harsh for a guest address... */
-    if ( par & PAR_F ) panic_PAR(par, "Guest");
-    return (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
+    uint64_t par = gva_to_ipa_par(va);
+    if ( par & PAR_F )
+        return -EFAULT;
+    *paddr = (par & PADDR_MASK & PAGE_MASK) | ((unsigned long) va & ~PAGE_MASK);
+    return 0;
 }
+
 /* Bits in the PAR returned by va_to_par */
 #define PAR_FAULT 0x1
 

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index e0c0bebf6a16d42817fcde876f1d4c46eda6609b..0c94f6b886fb4313dde215b9598279c7f2b8efa9 100644 (file)
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -231,7 +231,7 @@ union hsr {
 #ifndef __ASSEMBLY__
 extern uint32_t hyp_traps_vector[8];
 
-void panic_PAR(uint64_t par, const char *when);
+void panic_PAR(uint64_t par);
 
 void show_execution_state(struct cpu_user_regs *regs);
 void show_registers(struct cpu_user_regs *regs);