On Fri, Mar 6, 2015 at 10:24 PM, Tamas K Lengyel <tkleng...@sec.in.tum.de>
wrote:
> Add missing structure definition for iabt and update the trap handling
> mechanism to only inject the exception if the mem_access checker
> decides to do so.
>
> Signed-off-by: Tamas K Lengyel <tkleng...@sec.in.tum.de>
> Acked-by: Ian Campbell <ian.campb...@citrix.com>
> Reviewed-by: Julien Grall <julien.gr...@linaro.org>
> ---
> v10: - Minor comment fix for describing s1ptw.
> v8: - Revert to arch specific p2m_mem_access_check.
> - Retire iabt_fsc enum and use FSC_FLT instead.
> - Complete the struct definition of hsr_iabt.
> v7: - Use the new common mem_access_check.
> v6: - Make npfec a const.
> v4: - Don't mark instruction fetch violation as read violation.
> - Use new struct npfec to pass violation info.
> v2: - Add definition for instruction abort instruction fetch status codes
> (enum iabt_ifsc) and only call p2m_mem_access_check for traps
> triggered
> for permission violations.
> ---
> xen/arch/arm/traps.c | 31 +++++++++++++++++++++++++++++--
> xen/include/asm-arm/processor.h | 11 +++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index f5aa647..0670145 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1961,8 +1961,35 @@ done:
> static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
> union hsr hsr)
> {
> - register_t addr = READ_SYSREG(FAR_EL2);
> - inject_iabt_exception(regs, addr, hsr.len);
> + struct hsr_iabt iabt = hsr.iabt;
> + int rc;
> + paddr_t gpa;
> + register_t gva = READ_SYSREG(FAR_EL2);
>
>
So I have a question here. The following call to gva_to_ipa will use the
MMU to translate the gva as if it was a data-read access. However, we got
here because of an instruction fetch access. I did a quick check and (at
least some) ARM CPUs have split-TLBs. So technically using gva_to_ipa here
could get us an IPA that wasn't the actual address if the guest pagetable
has since been updated and the TLBs primed. Should the TLB be flushed here
just to be sure we have an accurate translation?
> + rc = gva_to_ipa(gva, &gpa);
> + if ( -EFAULT == rc )
> + return;
> +
> + switch ( iabt.ifsc & 0x3f )
> + {
> + case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
> + {
> + const struct npfec npfec = {
> + .insn_fetch = 1,
> + .gla_valid = 1,
> + .kind = iabt.s1ptw ? npfec_kind_in_gpt : npfec_kind_with_gla
> + };
> +
> + rc = p2m_mem_access_check(gpa, gva, npfec);
> +
> + /* Trap was triggered by mem_access, work here is done */
> + if ( !rc )
> + return;
> + }
> + break;
> + }
> +
> + inject_iabt_exception(regs, gva, hsr.len);
> }
>
> static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
> diff --git a/xen/include/asm-arm/processor.h
> b/xen/include/asm-arm/processor.h
> index cf7ab7c..db07fdd 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -438,6 +438,17 @@ union hsr {
> } sysreg; /* HSR_EC_SYSREG */
> #endif
>
> + struct hsr_iabt {
> + unsigned long ifsc:6; /* Instruction fault status code */
> + unsigned long res0:1;
> + unsigned long s1ptw:1; /* Stage 2 fault during stage 1
> translation */
> + unsigned long res1:1;
> + unsigned long eat:1; /* External abort type */
> + unsigned long res2:15;
> + unsigned long len:1; /* Instruction length */
> + unsigned long ec:6; /* Exception Class */
> + } iabt; /* HSR_EC_INSTR_ABORT_* */
> +
> struct hsr_dabt {
> unsigned long dfsc:6; /* Data Fault Status Code */
> unsigned long write:1; /* Write / not Read */
> --
> 2.1.4
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
