diff mbox

[v2] arm: Fix CP15 FSR (C5) domain setting

Message ID 1320493322-4012-1-git-send-email-jcd@tribudubois.net
State New
Headers show

Commit Message

Jean-Christophe Dubois Nov. 5, 2011, 11:42 a.m. UTC 
During Xvisor development, it was noted that qemu did not return
the correct domain value in the Cp15 [Data] FSR register (C5).

This patch is a proposal to fix it.

v2:
- fix coding style
- rebase on git.

Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
---
 target-arm/helper.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

Comments

Peter Maydell Nov. 6, 2011, 5:33 p.m. UTC | #1 
On 5 November 2011 11:42, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote:
> During Xvisor development, it was noted that qemu did not return
> the correct domain value in the Cp15 [Data] FSR register (C5).
>
> This patch is a proposal to fix it.
>
> v2:
> - fix coding style
> - rebase on git.
>
> Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

NB: not going to put this into 1.0, it's a bit late and it's not
a critical bug fix IMHO. In particular, well behaved code shouldn't
be relying on this DFSR field; see the ARM ARM B3.9.7:
"From ARMv7, use of the domain field in the DFSR is deprecated. This
field might not be supported in future versions of the ARM architecture.
ARM strongly recommends that new software does not use this field."

-- PMM
Jean-Christophe Dubois Nov. 6, 2011, 5:45 p.m. UTC | #2 
On 06/11/2011 18:33, Peter Maydell wrote:
> On 5 November 2011 11:42, Jean-Christophe DUBOIS<jcd@tribudubois.net>  wrote:
>> During Xvisor development, it was noted that qemu did not return
>> the correct domain value in the Cp15 [Data] FSR register (C5).
>>
>> This patch is a proposal to fix it.
>>
>> v2:
>> - fix coding style
>> - rebase on git.
>>
>> Signed-off-by: Jean-Christophe DUBOIS<jcd@tribudubois.net>
>
> Reviewed-by: Peter Maydell<peter.maydell@linaro.org>
>
> NB: not going to put this into 1.0, it's a bit late and it's not
> a critical bug fix IMHO. In particular, well behaved code shouldn't
> be relying on this DFSR field; see the ARM ARM B3.9.7:
> "From ARMv7, use of the domain field in the DFSR is deprecated. This
> field might not be supported in future versions of the ARM architecture.
> ARM strongly recommends that new software does not use this field."
Yes, same comment from the xvisor side of things. As xvisor targets 
mainly ARMv7 (and MIPS) for now, this was not a real issue.

Nonetheless this is worth fixing at some point in time (but this can be 
post 1.0).

JC
>
>
> -- PMM
>
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 97af4d0..8133087 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -951,13 +951,14 @@  void do_interrupt(CPUARMState *env)
 /* Check section/page access permissions.
    Returns the page protection flags, or zero if the access is not
    permitted.  */
-static inline int check_ap(CPUState *env, int ap, int domain, int access_type,
-                           int is_user)
+static inline int check_ap(CPUState *env, int ap, int domain_prot,
+                           int access_type, int is_user)
 {
   int prot_ro;
 
-  if (domain == 3)
+  if (domain_prot == 3) {
     return PAGE_READ | PAGE_WRITE;
+  }
 
   if (access_type == 1)
       prot_ro = 0;
@@ -1023,6 +1024,7 @@  static int get_phys_addr_v5(CPUState *env, uint32_t address, int access_type,
     int type;
     int ap;
     int domain;
+    int domain_prot;
     uint32_t phys_addr;
 
     /* Pagetable walk.  */
@@ -1030,13 +1032,14 @@  static int get_phys_addr_v5(CPUState *env, uint32_t address, int access_type,
     table = get_level1_table_address(env, address);
     desc = ldl_phys(table);
     type = (desc & 3);
-    domain = (env->cp15.c3 >> ((desc >> 4) & 0x1e)) & 3;
+    domain = (desc >> 5) & 0x0f;
+    domain_prot = (env->cp15.c3 >> (domain * 2)) & 3;
     if (type == 0) {
         /* Section translation fault.  */
         code = 5;
         goto do_fault;
     }
-    if (domain == 0 || domain == 2) {
+    if (domain_prot == 0 || domain_prot == 2) {
         if (type == 2)
             code = 9; /* Section domain fault.  */
         else
@@ -1094,7 +1097,7 @@  static int get_phys_addr_v5(CPUState *env, uint32_t address, int access_type,
         }
         code = 15;
     }
-    *prot = check_ap(env, ap, domain, access_type, is_user);
+    *prot = check_ap(env, ap, domain_prot, access_type, is_user);
     if (!*prot) {
         /* Access permission fault.  */
         goto do_fault;
@@ -1117,6 +1120,7 @@  static int get_phys_addr_v6(CPUState *env, uint32_t address, int access_type,
     int type;
     int ap;
     int domain;
+    int domain_prot;
     uint32_t phys_addr;
 
     /* Pagetable walk.  */
@@ -1134,10 +1138,10 @@  static int get_phys_addr_v6(CPUState *env, uint32_t address, int access_type,
         domain = 0;
     } else {
         /* Section or page.  */
-        domain = (desc >> 4) & 0x1e;
+        domain = (desc >> 5) & 0x0f;
     }
-    domain = (env->cp15.c3 >> domain) & 3;
-    if (domain == 0 || domain == 2) {
+    domain_prot = (env->cp15.c3 >> (domain * 2)) & 3;
+    if (domain_prot == 0 || domain_prot == 2) {
         if (type == 2)
             code = 9; /* Section domain fault.  */
         else
@@ -1182,7 +1186,7 @@  static int get_phys_addr_v6(CPUState *env, uint32_t address, int access_type,
         }
         code = 15;
     }
-    if (domain == 3) {
+    if (domain_prot == 3) {
         *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
     } else {
         if (xn && access_type == 2)
@@ -1194,7 +1198,7 @@  static int get_phys_addr_v6(CPUState *env, uint32_t address, int access_type,
             code = (code == 15) ? 6 : 3;
             goto do_fault;
         }
-        *prot = check_ap(env, ap, domain, access_type, is_user);
+        *prot = check_ap(env, ap, domain_prot, access_type, is_user);
         if (!*prot) {
             /* Access permission fault.  */
             goto do_fault;