[PATCH 01/45] xen: arm: do not panic when failing to translate a guest address

* [PATCH 01/45] xen: arm: do not panic when failing to translate a guest address
  2013-01-23 15:56 [PATCH 00/45] initial arm v8 (64-bit) support Ian Campbell
@ 2013-01-23 15:56 ` Ian Campbell
  2013-01-23 18:50   ` Stefano Stabellini
  2013-01-23 15:56 ` [PATCH 02/45] xen: arm32: Don't bother with the bootloader provided ARM-Linux machine type Ian Campbell
                   ` (44 subsequent siblings)
  45 siblings, 1 reply; 133+ messages in thread
From: Ian Campbell @ 2013-01-23 15:56 UTC (permalink / raw)
  To: xen-devel; +Cc: tim, stefano.stabellini, Ian Campbell

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>
---
 xen/arch/arm/guestcopy.c        |   30 +++++++++++++++++++----
 xen/arch/arm/kernel.c           |   15 +++++++++--
 xen/arch/arm/traps.c            |   49 ++++++++++++++++----------------------
 xen/include/asm-arm/mm.h        |    9 ++++--
 xen/include/asm-arm/page.h      |   29 +++++++++-------------
 xen/include/asm-arm/processor.h |    2 +-
 6 files changed, 76 insertions(+), 58 deletions(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index d9eb7ac..5504e19 100644
--- 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 2f7a9ff..143b9f6 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -76,12 +76,21 @@ 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 8afcf0c..0a1c483 100644
--- 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 146507e..4175e4d 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -191,10 +191,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 d89261e..067345e 100644
--- 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>
 
@@ -362,13 +363,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);
@@ -378,15 +379,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);
@@ -396,14 +389,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 e0c0beb..0c94f6b 100644
--- 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);
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 133+ messages in thread