Re: [Xen-devel] [RFC 06/22] xen/arm: traps: Check the P2M before injecting a data/instruction abort

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 28 Jul 2016, Julien Grall wrote:
> A data/instruction abort may have occurred if another CPU was playing
> with the stage-2 page table when following the break-before-make
> sequence (see D4.7.1 in ARM DDI 0487A.j). Rather than injecting directly
> the fault to the guest, we need to check whether the mapping exists. If
> it exists, return to the guest to replay the instruction.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/traps.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index b46284c..da56cc0 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2404,6 +2404,7 @@ static void do_trap_instr_abort_guest(struct 
> cpu_user_regs *regs,
>      register_t gva = READ_SYSREG(FAR_EL2);
>      uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
>      paddr_t gpa;
> +    mfn_t mfn;
>  
>      if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
>          gpa = get_faulting_ipa(gva);
> @@ -2417,6 +2418,11 @@ static void do_trap_instr_abort_guest(struct 
> cpu_user_regs *regs,
>           */
>          flush_tlb_local();
>  
> +        /*
> +         * We may not be able to translate because someone is
> +         * playing with the Stage-2 page table of the domain.
> +         * Return to the guest.
> +         */
>          rc = gva_to_ipa(gva, &gpa, GV2M_READ);
>          if ( rc == -EFAULT )
>              return; /* Try again */
> @@ -2437,8 +2443,17 @@ static void do_trap_instr_abort_guest(struct 
> cpu_user_regs *regs,
>          /* Trap was triggered by mem_access, work here is done */
>          if ( !rc )
>              return;
> +        break;
>      }
> -    break;
> +    case FSC_FLT_TRANS:

Please add brackets under the case statement for code style


> +        /*
> +         * The PT walk may have failed because someone was playing
> +         * with the Stage-2 page table. Walk the Stage-2 PT to check
> +         * if the entry exists. If it's the case, return to the guest
> +         */
> +        mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(gpa)), NULL);
> +        if ( !mfn_eq(mfn, INVALID_MFN) )
> +            return;

Just checking, but isn't it possible to get a genuine translation abort
with an mfn != invalid_mfn?


>      }
>  
>      inject_iabt_exception(regs, gva, hsr.len);
> @@ -2455,7 +2470,7 @@ static bool_t try_handle_mmio(struct cpu_user_regs 
> *regs,
>          return 0;
>  
>      /* All the instructions used on emulated MMIO region should be valid */
> -    if ( !dabt.valid )
> +    if ( !info->dabt.valid )
>          return 0;

Spurious change?


>      /*
> @@ -2483,6 +2498,7 @@ static void do_trap_data_abort_guest(struct 
> cpu_user_regs *regs,
>      int rc;
>      mmio_info_t info;
>      uint8_t fsc = hsr.dabt.dfsc & ~FSC_LL_MASK;
> +    mfn_t mfn;
>  
>      info.dabt = dabt;
>  #ifdef CONFIG_ARM_32
> @@ -2496,6 +2512,11 @@ static void do_trap_data_abort_guest(struct 
> cpu_user_regs *regs,
>      else
>      {
>          rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ);
> +        /*
> +         * We may not be able to translate because someone is
> +         * playing with the Stage-2 page table of the domain.
> +         * Return to the guest.
> +         */
>          if ( rc == -EFAULT )
>              return; /* Try again */
>      }
> @@ -2519,11 +2540,26 @@ static void do_trap_data_abort_guest(struct 
> cpu_user_regs *regs,
>          break;
>      }
>      case FSC_FLT_TRANS:
> +        /*
> +         * Attempt first to emulate the MMIO has the data abort will
                                                ^ as

> +         * likely happen an emulated region.
                           ^ on/in

> +         */
>          if ( try_handle_mmio(regs, &info) )
>          {
>              advance_pc(regs, hsr);
>              return;
>          }
> +
> +        /*
> +         * The PT walk may have failed because someone was playing
> +         * with the Stage-2 page table. Walk the Stage-2 PT to check
> +         * if the entry exists. If it's the case, return to the guest
> +         */
> +        mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(info.gpa)), 
> NULL);
> +        if ( !mfn_eq(mfn, INVALID_MFN) )
> +            return;
> +
> +        break;
>      default:
>          gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",
>                  hsr.bits, dabt.dfsc);
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel