On 2024-05-09 12:52, Julien Grall wrote:
Hi,

On 09/05/2024 11:39, Alessandro Zucchelli wrote:
In order to comply to MISRA C:2012 Rule 8.4 for ARM asm/mem_access.h in
the case where MEM_ACCESS=n stubs are needed to allow the conditional
compilation of the users of this header.

I think you need to update the commit message given ...


Signed-off-by: Alessandro Zucchelli <alessandro.zucche...@bugseng.com>
---
Changes from v1:
Reverted preprocessor conditional changes to xen/mem_access.h;
added conditional build for xen/mem_access.c;
provided stubs for asm/mem_access.h functions.
---
  xen/arch/arm/Makefile                 | 2 +-
  xen/arch/arm/include/asm/mem_access.h | 9 +++++++++
  2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 7b1350e2ef..45dc29ea53 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_IOREQ_SERVER) += ioreq.o
  obj-y += irq.o
  obj-y += kernel.init.o
  obj-$(CONFIG_LIVEPATCH) += livepatch.o
-obj-y += mem_access.o
+obj-$(CONFIG_MEM_ACCESS) += mem_access.o

... this not only adding stub.

  obj-y += mm.o
  obj-y += monitor.o
  obj-y += p2m.o
diff --git a/xen/arch/arm/include/asm/mem_access.h b/xen/arch/arm/include/asm/mem_access.h
index 35ed0ad154..2f73172e39 100644
--- a/xen/arch/arm/include/asm/mem_access.h
+++ b/xen/arch/arm/include/asm/mem_access.h
@@ -17,6 +17,7 @@
  #ifndef _ASM_ARM_MEM_ACCESS_H
  #define _ASM_ARM_MEM_ACCESS_H
  +#include <xen/types.h>

Can you explain why this is needed?

Without the inclusion of xen/types header NULL would be undefined.

Style: Newline here please.

  static inline
  bool p2m_mem_access_emulate_check(struct vcpu *v,
                                    const struct vm_event_st *rsp)
@@ -35,12 +36,20 @@ 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.
   */
+#ifdef CONFIG_MEM_ACCESS
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,
                                    const struct vcpu *v);
+#else
+static inline bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) {return false;}

Coding style: The line is well over the 80 characters limit. Also we don't tend to provide the implementation of the stub in the single line if it is more than {}. So "{return false;}" should look like:

{
   return false;
}

+
+static inline struct page_info*
+p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
+ const struct vcpu *v) {return NULL;}

Ditto.

+#endif /*CONFIG_MEM_ACCESS*/
  #endif /* _ASM_ARM_MEM_ACCESS_H */
    /*

Thanks for the feedback, I will soon provide the new version of the patch with the requested stylistic changes and a clearer description.
--
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)

Reply via email to