Ian Campbell
2013-Jan-22 11:38 UTC
[PATCH] 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>
---
xen/arch/arm/guestcopy.c | 30 +++++++++++++++++++----
xen/arch/arm/kernel.c | 14 +++++++++-
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(+), 57 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..c08c230 100644
--- 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 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.9.1
Stefano Stabellini
2013-Jan-22 12:21 UTC
Re: [PATCH] xen: arm: do not panic when failing to translate a guest address
On Tue, 22 Jan 2013, Ian Campbell wrote:> 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>
Ian Campbell
2013-Jan-24 13:01 UTC
Re: [PATCH] xen: arm: do not panic when failing to translate a guest address
On Tue, 2013-01-22 at 12:21 +0000, Stefano Stabellini wrote:> On Tue, 22 Jan 2013, Ian Campbell wrote: > > 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>Applied, thanks.