[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-tab
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-table protection bits |
Date: |
Wed, 23 Sep 2015 09:55:05 -0700 |
On 19 September 2015 at 07:15, Edgar E. Iglesias
<address@hidden> wrote:
> From: "Edgar E. Iglesias" <address@hidden>
>
> Signed-off-by: Edgar E. Iglesias <address@hidden>
> ---
> target-arm/helper.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 33be8c2..6f0ed51 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6008,6 +6008,38 @@ simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx
> mmu_idx, int ap)
> return simple_ap_to_rw_prot_is_user(ap, regime_is_user(env, mmu_idx));
> }
>
> +/* Translate S2 section/page access permissions to protection flags
> + *
> + * @env: CPUARMState
> + * @ap: The 2-bit simple AP (AP[2:1])
I think this should read "The 2-bit stage2 access permissions (S2AP)".
The interpretation of the field differs from the simple-AP bits.
You could helpfully call the argument "s2ap" as well.
> + * @xn: XN (execute-never) bit
> + */
> +static int get_S2prot(CPUARMState *env, int ap, int xn)
> +{
> + int prot_rw;
> +
> + switch (ap) {
> + default:
> + case 0:
> + prot_rw = 0;
> + break;
> + case 1:
> + prot_rw = PAGE_READ | PAGE_EXEC;
> + break;
> + case 2:
> + prot_rw = PAGE_WRITE;
> + break;
> + case 3:
> + prot_rw = PAGE_READ | PAGE_EXEC | PAGE_WRITE;
> + break;
> + }
> +
> + if (xn) {
> + prot_rw &= ~PAGE_EXEC;
> + }
This isn't right -- the XN bit controls executability and the
S2AP bits don't affect it at all. I think you want:
int prot_rw = 0;
if (s2ap & 1) {
prot_rw |= PAGE_READ;
}
if (s2ap & 2) {
prot_rw |= PAGE_WRITE;
}
if (!xn) {
prot_rw |= PAGE_EXEC;
}
> + return prot_rw;
> +}
> +
> /* Translate section/page access permissions to protection flags
> *
> * @env: CPUARMState
> @@ -6617,6 +6649,11 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> /* Extract attributes from the descriptor and merge with table attrs
> */
> attrs = extract64(descriptor, 2, 10)
> | (extract64(descriptor, 52, 12) << 10);
> +
> + if (mmu_idx == ARMMMUIdx_S2NS) {
> + /* The following extractions do not apply to S2. */
I think it would be clearer to say
/* Stage 2 table descriptors do not include any attribute fields. */
> + break;
> + }
and then here put
/* Merge in attributes from table descriptors */
and delete the now-redundant "and merge with table attrs" part from the
comment at the start of this hunk.
> attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
> attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2] */
> /* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
> @@ -6638,11 +6675,16 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> }
>
> ap = extract32(attrs, 4, 2);
> - ns = extract32(attrs, 3, 1);
> xn = extract32(attrs, 12, 1);
> - pxn = extract32(attrs, 11, 1);
>
> - *prot = get_S1prot(env, mmu_idx, va_size == 64, ap, ns, xn, pxn);
> + if (mmu_idx == ARMMMUIdx_S2NS) {
> + ns = true;
> + *prot = get_S2prot(env, ap, xn);
> + } else {
> + ns = extract32(attrs, 3, 1);
> + pxn = extract32(attrs, 11, 1);
> + *prot = get_S1prot(env, mmu_idx, va_size == 64, ap, ns, xn, pxn);
> + }
>
> fault_type = permission_fault;
> if (!(*prot & (1 << access_type))) {
> --
> 1.9.1
>
thanks
-- PMM
- [Qemu-devel] [PATCH RFC 0/8] arm: Steps towards EL2 support round 5, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 1/8] target-arm: Add HPFAR_EL2, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 2/8] target-arm: Add computation of starting level for S2 PTW, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-table protection bits, Edgar E. Iglesias, 2015/09/19
- Re: [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-table protection bits,
Peter Maydell <=
- [Qemu-devel] [PATCH RFC 4/8] target-arm: Avoid inline for get_phys_addr, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 5/8] target-arm: Add ARMMMUFaultInfo, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 6/8] target-arm: Add S2 translation support for S1 PTW, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 7/8] target-arm: Route S2 MMU faults to EL2, Edgar E. Iglesias, 2015/09/19
- [Qemu-devel] [PATCH RFC 8/8] target-arm: Add support for S1 + S2 MMU translations, Edgar E. Iglesias, 2015/09/19
- Re: [Qemu-devel] [PATCH RFC 0/8] arm: Steps towards EL2 support round 5, Edgar E. Iglesias, 2015/09/19
- Re: [Qemu-devel] [PATCH RFC 0/8] arm: Steps towards EL2 support round 5, Peter Maydell, 2015/09/23