[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[RFC?] xen/arm: memaccess: Pass struct npfec by reference in p2m_mem_access_check



From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

Today I noticed a "note" when building Xen on Arm64 with
aarch64-poky-linux-gcc (GCC) 9.3.0. It turned out that Andrew Cooper
had alredy reported it before [1]:

mem_access.c: In function 'p2m_mem_access_check':
mem_access.c:227:6: note: parameter passing for argument of type
'const struct npfec' changed in GCC 9.1
  227 | bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla,
                                  const struct npfec npfec)

>From the explanation I understand that nothing bad actually is going
to happen in our case, it is harmless and shown to only draw our
attention that the ABI changed due to bug (with passing bit-fields
by value) fixed in GCC 9.1. This information doesn't mean much for us
as Xen is an embedded project with no external linkage. But, of course,
it would be better to eliminate the note. You can also find related
information about the bug at [2].

So make the note go away by passing bit-fields by reference.

[1] https://www.mail-archive.com/xen-devel@xxxxxxxxxxxxxxxxxxxx/msg87439.html
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88469

Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
---
Compile-tested only.
---
 xen/arch/arm/mem_access.c        | 28 ++++++++++++++--------------
 xen/arch/arm/traps.c             |  2 +-
 xen/include/asm-arm/mem_access.h |  2 +-
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index 3e36202..d21bba7 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -224,7 +224,7 @@ err:
     return page;
 }
 
-bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
+bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec *npfec)
 {
     int rc;
     bool violation;
@@ -248,23 +248,23 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const 
struct npfec npfec)
         violation = false;
         break;
     case XENMEM_access_rw:
-        violation = npfec.insn_fetch;
+        violation = npfec->insn_fetch;
         break;
     case XENMEM_access_wx:
-        violation = npfec.read_access;
+        violation = npfec->read_access;
         break;
     case XENMEM_access_rx:
     case XENMEM_access_rx2rw:
-        violation = npfec.write_access;
+        violation = npfec->write_access;
         break;
     case XENMEM_access_x:
-        violation = npfec.read_access || npfec.write_access;
+        violation = npfec->read_access || npfec->write_access;
         break;
     case XENMEM_access_w:
-        violation = npfec.read_access || npfec.insn_fetch;
+        violation = npfec->read_access || npfec->insn_fetch;
         break;
     case XENMEM_access_r:
-        violation = npfec.write_access || npfec.insn_fetch;
+        violation = npfec->write_access || npfec->insn_fetch;
         break;
     default:
     case XENMEM_access_n:
@@ -277,7 +277,7 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const 
struct npfec npfec)
         return true;
 
     /* First, handle rx2rw and n2rwx conversion automatically. */
-    if ( npfec.write_access && xma == XENMEM_access_rx2rw )
+    if ( npfec->write_access && xma == XENMEM_access_rx2rw )
     {
         rc = p2m_set_mem_access(v->domain, gaddr_to_gfn(gpa), 1,
                                 0, ~0, XENMEM_access_rw, 0);
@@ -324,19 +324,19 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const 
struct npfec npfec)
         /* Send request to mem access subscriber */
         req->u.mem_access.gfn = gpa >> PAGE_SHIFT;
         req->u.mem_access.offset =  gpa & ((1 << PAGE_SHIFT) - 1);
-        if ( npfec.gla_valid )
+        if ( npfec->gla_valid )
         {
             req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
             req->u.mem_access.gla = gla;
 
-            if ( npfec.kind == npfec_kind_with_gla )
+            if ( npfec->kind == npfec_kind_with_gla )
                 req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
-            else if ( npfec.kind == npfec_kind_in_gpt )
+            else if ( npfec->kind == npfec_kind_in_gpt )
                 req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
         }
-        req->u.mem_access.flags |= npfec.read_access    ? MEM_ACCESS_R : 0;
-        req->u.mem_access.flags |= npfec.write_access   ? MEM_ACCESS_W : 0;
-        req->u.mem_access.flags |= npfec.insn_fetch     ? MEM_ACCESS_X : 0;
+        req->u.mem_access.flags |= npfec->read_access    ? MEM_ACCESS_R : 0;
+        req->u.mem_access.flags |= npfec->write_access   ? MEM_ACCESS_W : 0;
+        req->u.mem_access.flags |= npfec->insn_fetch     ? MEM_ACCESS_X : 0;
 
         if ( monitor_traps(v, (xma != XENMEM_access_n2rwx), req) < 0 )
             domain_crash(v->domain);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 219ab3c..4ad1618 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1957,7 +1957,7 @@ static void do_trap_stage2_abort_guest(struct 
cpu_user_regs *regs,
             .kind = xabt.s1ptw ? npfec_kind_in_gpt : npfec_kind_with_gla
         };
 
-        p2m_mem_access_check(gpa, gva, npfec);
+        p2m_mem_access_check(gpa, gva, &npfec);
         /*
          * The only way to get here right now is because of mem_access,
          * thus reinjecting the exception to the guest is never required.
diff --git a/xen/include/asm-arm/mem_access.h b/xen/include/asm-arm/mem_access.h
index 35ed0ad..be43e18 100644
--- a/xen/include/asm-arm/mem_access.h
+++ b/xen/include/asm-arm/mem_access.h
@@ -35,7 +35,7 @@ static inline bool p2m_mem_access_sanity_check(struct domain 
*d)
  * Send mem event based on the access. Boolean return value indicates if trap
  * needs to be injected into guest.
  */
-bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec);
+bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec *npfec);
 
 struct page_info*
 p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
-- 
2.7.4




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.