|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 01/45] xen: arm: do not panic when failing to translate a guest address
On Wed, 23 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@xxxxxxxxxx>
I think I have already acked this patch
> 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
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |