From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: [PATCH] kvmtrace: fix "Remove use of bit fields in kvm trace structure" patch Date: Thu, 03 Jul 2008 15:30:48 +0200 Message-ID: <486CD488.2000502@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE To: Hollis Blanchard , Avi Kivity , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org Return-path: Received: from mtagate3.uk.ibm.com ([195.212.29.136]:34129 "EHLO mtagate3.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753234AbYGCNbC (ORCPT ); Thu, 3 Jul 2008 09:31:02 -0400 Sender: kvm-owner@vger.kernel.org List-ID: [PATCH] kvmppc: kvmtrace: fix "Remove use of bit fields in kvm trace=20 structure" =46rom: Christian Ehrhardt 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 |=3D TRACE_REC_EVENT_ID(va_arg(*args, u32)); makro: #define TRACE_REC_EVENT_ID (val) \ (0x0fffffff & (val)) is extended to: rec.rec_val |=3D (val) (0x0fffffff & (val))(__builtin_va_arg(*args,u= 32)); 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=20 parameter which is one of the golden macro rules to keep operator precedence=20 independent to what someone passes as argument to your macro. =46urther I found that part of the path added a superfluous assignment = in kvm_trace.c (initialize with =3D0 and directly or something in with |=3D= ). This patch fixes all three issues. As Hollis pointed out yesterday patc= h=20 #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 b= atch 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. Signed-off-by: Christian Ehrhardt --- [diffstat] include/linux/kvm.h | 10 +++++----- virt/kvm/kvm_trace.c | 4 +--- 2 files changed, 6 insertions(+), 8 deletions(-) [diff] diff --git a/include/linux/kvm.h b/include/linux/kvm.h --- a/include/linux/kvm.h +++ b/include/linux/kvm.h @@ -331,12 +331,12 @@ } u; } __attribute__((packed)); =20 -#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)) =20 #define KVMIO 0xAE =20 diff --git a/virt/kvm/kvm_trace.c b/virt/kvm/kvm_trace.c --- a/virt/kvm/kvm_trace.c +++ b/virt/kvm/kvm_trace.c @@ -60,10 +60,8 @@ if (unlikely(kt->trace_state !=3D KVM_TRACE_STATE_RUNNING)) return; =20 - rec.rec_val =3D 0; - =20 /* set event id */ =20 - rec.rec_val |=3D TRACE_REC_EVENT_ID(va_arg(*args, u32)); + rec.rec_val =3D TRACE_REC_EVENT_ID(va_arg(*args, u32)); =20 vcpu =3D va_arg(*args, struct kvm_vcpu *); rec.pid =3D current->tgid; --- Gr=FCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization