linux-kernel - RE: [PATCH v2 1/4] x86/hyper-v: move synic/stimer control structures definitions to hyperv-tlfs.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY4PR21MB0773225F986528897624BAA6D7D00@CY4PR21MB0773.namprd21.prod.outlook.com>
Date:   Tue, 27 Nov 2018 15:52:51 +0000
From:   Michael Kelley <mikelley@...rosoft.com>
To:     vkuznets <vkuznets@...hat.com>, Roman Kagan <rkagan@...tuozzo.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: RE: [PATCH v2 1/4] x86/hyper-v: move synic/stimer control structures
 definitions to hyperv-tlfs.h

From: Vitaly Kuznetsov <vkuznets@...hat.com> Tuesday, November 27, 2018 5:11 AM

> > I personally tend to prefer masks over bitfields, so I'd rather do the
> > consolidation in the opposite direction: use the definitions in
> > hyperv-tlfs.h and replace those unions/bitfields elsewhere.  (I vaguely
> > remember posting such a patchset a couple of years ago but I lacked the
> > motivation to complete it).
> 
> Are there any known advantages of using masks over bitfields or the
> resulting binary code is the same? Assuming there are no I'd personally
> prefer bitfields - not sure why but to me e.g.
>  (stimer->config.enabled && !stimer->config.direct_mode)
>  looks nicer than
>  (stimer->config & HV_STIMER_ENABLED && !(stimer->config & HV_STIMER_DIRECT))
> 
> + there's no need to do shifts, e.g.
> 
>  vector = stimer->config.apic_vector
> 
>  looks cleaner that
> 
>  vector = (stimer->config & HV_STIMER_APICVECTOR_MASK) >>
> HV_STIMER_APICVECTOR_SHIFT
> 
> ... and we already use a few in hyperv-tlfs.h. I'm, however, flexible :-)
> 
> K. Y., Michael, Haiyang, Stephen - would you prefer to get rid of all
> bitfields from hyperv-tlfs.h? If so I can work on a patchset. As to this
> series, my understanding is that Paolo already queued it so in any case
> this is going to be a separate effort (unless there are strong
> objections of course).
> 

I prefer to keep the bit fields.  I concur think it makes the code more
readable.   Even if there is a modest performance benefit, at least
within a Linux guest most of the manipulation of the fields is during
initialization when performance doesn't matter.  But I can't speak to
KVM's implementation of the hypervisor side.

Michael