Re: [Qemu-devel] [PATCH] target-xtensa: fix ITLB/DTLB page protection flags

[Blue Swirl]

Thanks, applied.

On Thu, Dec 13, 2012 at 12:13 AM, Max Filippov <address@hidden> wrote:
> With MMU option xtensa architecture has two TLBs: ITLB and DTLB. ITLB is
> only used for code access, DTLB is only for data. However TLB entries in
> both TLBs have attribute field controlling write and exec access. These
> bits need to be properly masked off depending on TLB type before being
> used as tlb_set_page prot argument. Otherwise the following happens:
>
> (1) ITLB entry for some PFN gets invalidated
> (2) DTLB entry for the same PFN gets updated, attributes allow code
>     execution
> (3) code at the page with that PFN is executed (possible due to step 2),
>     entry for the TB is written into the jump cache
> (4) QEMU TLB entry for the PFN gets replaced with an entry for some
>     other PFN
> (5) code in the TB from step 3 is executed (possible due to jump cache)
>     and it accesses data, for which there's no DTLB entry, causing DTLB
>     miss exception
> (6) re-translation of the TB from step 5 is attempted, but there's no
>     QEMU TLB entry nor xtensa ITLB entry for that PFN, which causes ITLB
>     miss exception at the TB start address
> (7) ITLB miss exception is handled by the guest, but execution is
>     resumed from the beginning of the faulting TB (the point where ITLB
>     miss occured), not from the point where DTLB miss occured, which is
>     wrong.
>
> With that fix the above scenario causes ITLB miss exception (that used
> to be step 7) at step 3, right at the beginning of the TB.
>
> Signed-off-by: Max Filippov <address@hidden>
> Cc: address@hidden
> ---
>  target-xtensa/helper.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
> index 200fb43..bf05575 100644
> --- a/target-xtensa/helper.c
> +++ b/target-xtensa/helper.c
> @@ -522,7 +522,8 @@ static int get_physical_addr_mmu(CPUXtensaState *env, 
> bool update_tlb,
>              INST_FETCH_PRIVILEGE_CAUSE;
>      }
>
> -    *access = mmu_attr_to_access(entry->attr);
> +    *access = mmu_attr_to_access(entry->attr) &
> +        ~(dtlb ? PAGE_EXEC : PAGE_READ | PAGE_WRITE);
>      if (!is_access_granted(*access, is_write)) {
>          return dtlb ?
>              (is_write ?
> --
> 1.7.7.6
>