From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hollis Blanchard Subject: Re: [PATCH 1 of 3] Remove use of bit fields in kvm trace structure Date: Fri, 20 Jun 2008 11:59:28 -0500 Message-ID: <1213981168.5618.5.camel@localhost.localdomain> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, "Liu, Eric E" , Jerone Young To: Avi Kivity Return-path: Received: from e2.ny.us.ibm.com ([32.97.182.142]:46675 "EHLO e2.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752542AbYFTQ7c (ORCPT ); Fri, 20 Jun 2008 12:59:32 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Slightly unconventional coding style, but Acked-by: Hollis Blanchard Eric, as I mentioned previously, bitfields cannot be used in portable binary formats. Avi, would you apply please? =EF=BB=BF --=20 Hollis Blanchard IBM Linux Technology Center On Thu, 2008-06-19 at 23:19 -0500, Jerone Young wrote: > 2 files changed, 21 insertions(+), 11 deletions(-) > include/linux/kvm.h | 10 +++++++--- > virt/kvm/kvm_trace.c | 22 ++++++++++++++-------- >=20 >=20 > This patch fixes kvmtrace use on big endian systems. When using bit f= ields the compiler will lay data out in the wrong order expected when l= aid down into a file. This fixes it by using one variable instead of us= ing bit fields. >=20 > Signed-off-by: Jerone Young >=20 > diff --git a/include/linux/kvm.h b/include/linux/kvm.h > --- a/include/linux/kvm.h > +++ b/include/linux/kvm.h > @@ -311,9 +311,13 @@ struct kvm_s390_interrupt { >=20 > /* This structure represents a single trace buffer record. */ > struct kvm_trace_rec { > - __u32 event:28; > - __u32 extra_u32:3; > - __u32 cycle_in:1; > + /* variable rec_val > + * is split into: > + * bits 0 - 27 -> event id > + * bits 28 -30 -> number of extra data args of size u32 > + * bits 31 -> binary indicator for if tsc is in record > + */ > + __u32 rec_val; > __u32 pid; > __u32 vcpu_id; > union { > 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 > @@ -54,12 +54,15 @@ static void kvm_add_trace(void *probe_pr > struct kvm_trace *kt =3D kvm_trace; > struct kvm_trace_rec rec; > struct kvm_vcpu *vcpu; > - int i, extra, size; > + int i, size; > + u32 extra; >=20 > if (unlikely(kt->trace_state !=3D KVM_TRACE_STATE_RUNNING)) > return; > +=09 > + /* set event id */=09 > + rec.rec_val =3D 0x0fffffff & va_arg(*args, u32); >=20 > - rec.event =3D va_arg(*args, u32); > vcpu =3D va_arg(*args, struct kvm_vcpu *); > rec.pid =3D current->tgid; > rec.vcpu_id =3D vcpu->vcpu_id; > @@ -67,21 +70,24 @@ static void kvm_add_trace(void *probe_pr > extra =3D va_arg(*args, u32); > WARN_ON(!(extra <=3D KVM_TRC_EXTRA_MAX)); > extra =3D min_t(u32, extra, KVM_TRC_EXTRA_MAX); > - rec.extra_u32 =3D extra; >=20 > - rec.cycle_in =3D p->cycle_in; > + /* set inidicator for tcs record */ > + rec.rec_val |=3D 0x80000000 & (p->cycle_in << 31); > +=09 > + /* set extra data num */ > + rec.rec_val |=3D 0x70000000 & (extra << 28); >=20 > - if (rec.cycle_in) { > + if (p->cycle_in) { > rec.u.cycle.cycle_u64 =3D get_cycles(); >=20 > - for (i =3D 0; i < rec.extra_u32; i++) > + for (i =3D 0; i < extra; i++) > rec.u.cycle.extra_u32[i] =3D va_arg(*args, u32); > } else { > - for (i =3D 0; i < rec.extra_u32; i++) > + for (i =3D 0; i < extra; i++) > rec.u.nocycle.extra_u32[i] =3D va_arg(*args, u32); > } >=20 > - size =3D calc_rec_size(rec.cycle_in, rec.extra_u32 * sizeof(u32)); > + size =3D calc_rec_size(p->cycle_in, extra * sizeof(u32)); > relay_write(kt->rchan, &rec, size); > } >=20 > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html