[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] target-arm: apply get_S1prot to get_phys_ad
From: |
Andrew Jones |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] target-arm: apply get_S1prot to get_phys_addr_v6 |
Date: |
Thu, 12 Feb 2015 18:08:27 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Thu, Feb 12, 2015 at 04:05:07PM +0100, Andrew Jones wrote:
> Now that we have get_S1prot, we can apply it to get_phys_addr_v6
> for a minor code cleanup.
Actually, I should point out that this isn't just a cleanup, but
also a fix. See below.
>
> Signed-off-by: Andrew Jones <address@hidden>
> ---
> target-arm/helper.c | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 20e5753bd216d..c41305e7e2bdf 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5064,30 +5064,19 @@ static int get_phys_addr_v6(CPUARMState *env,
> uint32_t address, int access_type,
> }
> code = 15;
> }
> - if (domain_prot == 3) {
> - *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> - } else {
> - bool is_user = regime_is_user(env, mmu_idx);
> -
> - if (pxn && !is_user) {
> - xn = 1;
> - }
> - if (xn && access_type == 2)
> - goto do_fault;
> -
> + if (regime_sctlr(env, mmu_idx) & SCTLR_AFE) {
> /* The simplified model uses AP[0] as an access control bit. */
> - if ((regime_sctlr(env, mmu_idx) & SCTLR_AFE)
> - && (ap & 1) == 0) {
> + if ((ap & 1) == 0) {
> /* Access flag fault. */
> code = (code == 15) ? 6 : 3;
> goto do_fault;
> }
> - *prot = get_rw_prot(env, mmu_idx, is_user, ap, domain_prot);
> - *prot |= *prot && !xn ? PAGE_EXEC : 0;
> - if (!(*prot & (1 << access_type))) {
> - /* Access permission fault. */
> - goto do_fault;
> - }
> + ap >>= 1;
The original code didn't take into account that it may be calling check_ap
with a simple AP, AP[2:1]. The code should have always been similar to
the above, i.e.
if (regime_sctlr(env, mmu_idx) & SCTLR_AFE) {
/* The simplified model uses AP[0] as an access control bit. */
if ((ap & 1) == 0) {
/* Access flag fault. */
code = (code == 15) ? 6 : 3;
goto do_fault;
}
*prot = <handle simple AP somehow>;
} else {
*prot = check_ap(env, mmu_idx, ap, domain_prot, access_type);
}
if (!*prot) {
/* Access permission fault. */
goto do_fault;
}
if (!xn) {
*prot |= PAGE_EXEC;
}
As a simple AP wouldn't be properly translated to protection flags with
check_ap (except for case 6), then I think this should have caused some
problems. Maybe this path just hasn't been tested? I don't see CR_AFE
getting used by Linux, so possibly not.
I should update the commit message to point this fix out. Or, actually,
I should probably add another patch to the series (3/6), which addresses
just this issue, and builds it on patch 2 "...to take simple AP". Peter,
please let me know your preference.
Thanks,
drew
> + }
> + *prot = get_S1prot(env, mmu_idx, false, ap, domain_prot, 0, xn, pxn);
> + if (!(*prot & (1 << access_type))) {
> + /* Access permission fault. */
> + goto do_fault;
> }
> *phys_ptr = phys_addr;
> return 0;
> --
> 1.9.3
>
>
>
- [Qemu-devel] [PATCH 0/5] tcg-arm: LPAE: fix and extend xn control, Andrew Jones, 2015/02/12
- [Qemu-devel] [PATCH 2/5] target-arm: enable get_rw_prot to take simple AP, Andrew Jones, 2015/02/12
- [Qemu-devel] [PATCH 3/5] target-arm: add an is_user param to get_rw_prot, Andrew Jones, 2015/02/12
- [Qemu-devel] [PATCH 5/5] target-arm: apply get_S1prot to get_phys_addr_v6, Andrew Jones, 2015/02/12
- Re: [Qemu-devel] [PATCH 5/5] target-arm: apply get_S1prot to get_phys_addr_v6,
Andrew Jones <=
- [Qemu-devel] [PATCH 1/5] target-arm: convert check_ap to get_rw_prot, Andrew Jones, 2015/02/12
- [Qemu-devel] [PATCH 4/5] target-arm: get_phys_addr_lpae: more xn control, Andrew Jones, 2015/02/12
- Re: [Qemu-devel] [PATCH 0/5] tcg-arm: LPAE: fix and extend xn control, Andrew Jones, 2015/02/24