Re: [Qemu-devel] [PATCH v2] tcg-arm: more instruction execution control

2015-01-13 Thread Andrew Jones
On Mon, Jan 12, 2015 at 01:46:47PM +0100, Andrew Jones wrote:
 Cleanup XN/PXN handling in get_phys_addr_lpae, and implement all but
 EL2 support of the following ARMv8 sections
 
   D4.5.1 Memory access control: Access permissions for instruction
  execution
   G4.7.2 Execute-never restrictions on instruction fetching
 
 G4.7.2 matches the ARMv7 section B3.7.2 when long-descriptors are used.
 
 Signed-off-by: Andrew Jones drjo...@redhat.com


While confirming the documentation wasn't wrong (it wasn't),
I see I missed another issue with qemu's instruction execution
control. For AArch64, EL0 can execute code even if it doesn't have
R/W access, i.e. AP[1]=0. To make this fix more clear I've done it
in a separate patch, and then rebased this patch on that. Thus,
please drop this patch, as I'll send a 2-patch patch series now
that replaces it.

drew



[Qemu-devel] [PATCH v2] tcg-arm: more instruction execution control

2015-01-12 Thread Andrew Jones
Cleanup XN/PXN handling in get_phys_addr_lpae, and implement all but
EL2 support of the following ARMv8 sections

  D4.5.1 Memory access control: Access permissions for instruction
 execution
  G4.7.2 Execute-never restrictions on instruction fetching

G4.7.2 matches the ARMv7 section B3.7.2 when long-descriptors are used.

Signed-off-by: Andrew Jones drjo...@redhat.com

---
I didn't test this with secure mode (I didn't even check if that's
currently possible), but I did test all other EL10 XN controls with
both tcg-arm (cortex-a15) and tcg-aarch64 (cortex-a57) by hacking up
some tests with kvm-unit-tests/arm[64]. I also booted Linux (just
up to looking for a rootfs) with both.
---
 target-arm/helper.c | 99 ++---
 1 file changed, 79 insertions(+), 20 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3ef0f1f38eda5..94f7631f0a125 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4516,6 +4516,69 @@ static inline int check_ap(CPUARMState *env, int ap, int 
domain_prot,
   }
 }
 
+/* Check section/page execution permission.
+ *
+ * Returns PAGE_EXEC if execution is permitted, otherwise zero.
+ *
+ * Supports AArch64, AArch32, and v7-LPAE. Doesn't yet support
+ * EL2. Actually, for v7-LPAE, we'd also need to emulate Virt
+ * Extensions to truly have WXN/UWXN. We don't support checking
+ * for that yet, so we just assume we have them.
+ *
+ * @env : CPUARMState
+ * @is_user : 0 for privileged access, 1 for user
+ * @ap  : Access permissions (AP[2:1]) from descriptor
+ * @ns  : (NSTable || NS) from descriptors
+ * @xn  : ([U]XNTable || [U]XN) from descriptors
+ * @pxn : (PXNTable || PXN) from descriptors
+ */
+static int check_xn_lpae(CPUARMState *env, int is_user, int ap,
+ int ns, int xn, int pxn)
+{
+int wxn;
+
+switch (arm_current_el(env)) {
+case 0:
+case 1:
+if (is_user  !(ap  1)) {
+return 0;
+}
+wxn = env-cp15.sctlr_el[1]  SCTLR_WXN;
+if (arm_el_is_aa64(env, 1)) {
+pxn = pxn || ((ap  1)  !(ap  2));
+xn = is_user ? xn : pxn;
+} else {
+int uwxn = env-cp15.sctlr_el[1]  SCTLR_UWXN;
+pxn = pxn || (uwxn  (ap  1)  !(ap  2));
+xn = xn || (!is_user  pxn);
+}
+if (arm_is_secure(env)) {
+xn = xn || (ns  (env-cp15.scr_el3  SCR_SIF));
+}
+break;
+case 2:
+/* TODO actually support EL2 */
+assert(false);
+
+if (arm_el_is_aa64(env, 2)) {
+wxn = env-cp15.sctlr_el[2]  SCTLR_WXN;
+} else {
+/* wxn = HSCTLR.WXN */
+}
+break;
+case 3:
+if (arm_el_is_aa64(env, 3)) {
+wxn = env-cp15.sctlr_el[3]  SCTLR_WXN;
+} else {
+wxn = 0;
+}
+xn = xn || (ns  (env-cp15.scr_el3  SCR_SIF));
+break;
+}
+
+return xn || (!(ap  2)  wxn) ? 0 : PAGE_EXEC;
+}
+
 static bool get_level1_table_address(CPUARMState *env, uint32_t *table,
  uint32_t address)
 {
@@ -4787,7 +4850,7 @@ static int get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
 hwaddr descaddr, descmask;
 uint32_t tableattrs;
 target_ulong page_size;
-uint32_t attrs;
+uint32_t attrs, ap;
 int32_t granule_sz = 9;
 int32_t va_size = 32;
 int32_t tbi = 0;
@@ -4941,7 +5004,7 @@ static int get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
 if (extract32(tableattrs, 2, 1)) {
 attrs = ~(1  4);
 }
-/* Since we're always in the Non-secure state, NSTable is ignored. */
+attrs |= extract32(tableattrs, 4, 1)  3; /* NS */
 break;
 }
 /* Here descaddr is the final physical address, and attributes
@@ -4952,29 +5015,25 @@ static int get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
 /* Access flag */
 goto do_fault;
 }
+
 fault_type = permission_fault;
-if (is_user  !(attrs  (1  4))) {
+ap = extract32(attrs, 4, 2); /* AP[2:1] */
+
+if (is_user  !(ap  1)) {
 /* Unprivileged access not enabled */
 goto do_fault;
 }
-*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
-if ((arm_feature(env, ARM_FEATURE_V8)  is_user  (attrs  (1  12))) ||
-(!arm_feature(env, ARM_FEATURE_V8)  (attrs  (1  12))) ||
-(!is_user  (attrs  (1  11 {
-/* XN/UXN or PXN. Since we only implement EL0/EL1 we unconditionally
- * treat XN/UXN as UXN for v8.
- */
-if (access_type == 2) {
-goto do_fault;
-}
-*prot = ~PAGE_EXEC;
+
+*prot = PAGE_READ;
+*prot |= check_xn_lpae(env, is_user, ap, extract32(attrs, 3, 1),
+   extract32(attrs, 12, 1), extract32(attrs, 11, 1));
+if (!(*prot  PAGE_EXEC)  access_type == 2) {
+goto do_fault;