kvmtrace: fix "Remove use of bit fields in kvm trace structure" patch

[PATCH] kvmppc: kvmtrace: fix "Remove use of bit fields in kvm trace structure"

From: Christian Ehrhardt <[email protected]>

The current patch kvm_trace_nobitfields.diff in the kvmppc.hg-dev repo doesn't compile and has some minor flaws.
This patch is known to the kvm list as "[PATCH] [v2] Remove use of bit fields in kvm trace structure" from Jerone.

Problem:
code:
rec.rec_val |= TRACE_REC_EVENT_ID(va_arg(*args, u32));
makro:

   #define TRACE_REC_EVENT_ID (val) \
                (0x0fffffff & (val))

is extended to:
rec.rec_val |= (val) (0x0fffffff & (val))(__builtin_va_arg(*args,u32));

that is failing with:
kvm_trace.c: In function 'kvm_add_trace':
kvm_trace.c:66: error: 'val' undeclared (first use in this function) kvm_trace.c:66: error: (Each undeclared identifier is reported only once kvm_trace.c:66: error: for each function it appears in.)

Obviously caused by the blank between macro definition and (val) which makes it a non-parameter macro.

Additionally the two macro definitions below don't put () around the parameter
which is one of the golden macro rules to keep operator precedence independent
to what someone passes as argument to your macro.

Further I found that part of the path added a superfluous assignment in kvm_trace.c (initialize with =0 and directly or something in with |=).

This patch fixes all three issues. As Hollis pointed out yesterday patch #1 of
the initial series of 3 patches should already be applied according to your (Avi) response while it is not seen on kvm-commits yet.
He offered to collect those patches in his patch queue and send you a batch soon.
That said you can consider this patch FYI (to collect comments now) and wait until our consolidated batch arrives including all fixes at once.

---

[diffstat]

 include/linux/kvm.h  |   10 +++++-----
 virt/kvm/kvm_trace.c |    4 +---
 2 files changed, 6 insertions(+), 8 deletions(-)

[diff]

@@ -331,12 +331,12 @@
     } u;
 } __attribute__((packed));
 
-#define TRACE_REC_EVENT_ID (val) \
+#define TRACE_REC_EVENT_ID(val) \
         (0x0fffffff & (val))
-#define TRACE_REC_NUM_DATA_ARGS (val) \
-        (0x70000000 & (val << 28))
-#define TRACE_REC_TCS (val) \
-        (0x80000000 & (val << 31))
+#define TRACE_REC_NUM_DATA_ARGS(val) \
+        (0x70000000 & ((val) << 28))
+#define TRACE_REC_TCS(val) \
+        (0x80000000 & ((val) << 31))
 
 #define KVMIO 0xAE
 

@@ -60,10 +60,8 @@
     if (unlikely(kt->trace_state != KVM_TRACE_STATE_RUNNING))
         return;
 
-    rec.rec_val = 0;
-   
     /* set event id */   
-    rec.rec_val    |= TRACE_REC_EVENT_ID(va_arg(*args, u32));
+    rec.rec_val    = TRACE_REC_EVENT_ID(va_arg(*args, u32));
 
     vcpu        = va_arg(*args, struct kvm_vcpu *);
     rec.pid        = current->tgid;

---

Grüsse / regards,
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization