linux-kernel - Re: [PATCH v5 13/16] asm-generic/hyperv: introduce hv_device

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: <CAK8P3a0m8jEij-RdP1PTcNcJW2+mXQ1dA4=s+JLXhnv+NyFiHw@mail.gmail.com>
Date:   Wed, 3 Feb 2021 14:49:53 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     Wei Liu <wei.liu@...nel.org>
Cc:     Michael Kelley <mikelley@...rosoft.com>,
        Linux on Hyper-V List <linux-hyperv@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        Linux Kernel List <linux-kernel@...r.kernel.org>,
        Vineeth Pillai <viremana@...ux.microsoft.com>,
        Sunil Muthuswamy <sunilmut@...rosoft.com>,
        Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
        "pasha.tatashin@...een.com" <pasha.tatashin@...een.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Arnd Bergmann <arnd@...db.de>,
        "open list:GENERIC INCLUDE/ASM HEADER FILES" 
        <linux-arch@...r.kernel.org>
Subject: Re: [PATCH v5 13/16] asm-generic/hyperv: introduce hv_device_id and
 auxiliary structures

On Wed, Feb 3, 2021 at 2:26 PM Wei Liu <wei.liu@...nel.org> wrote:
> On Tue, Feb 02, 2021 at 05:02:48PM +0000, Wei Liu wrote:
> > On Tue, Jan 26, 2021 at 01:26:52AM +0000, Michael Kelley wrote:
> > > From: Wei Liu <wei.liu@...nel.org> Sent: Wednesday, January 20, 2021 4:01 AM
> > > > +union hv_device_id {
> > > > + u64 as_uint64;
> > > > +
> > > > + struct {
> > > > +         u64 :62;
> > > > +         u64 device_type:2;
> > > > + };
> > >
> > > Are the above 4 lines extraneous junk?
> > > If not, a comment would be helpful.  And we
> > > would normally label the 62 bit field as
> > > "reserved0" or something similar.
> > >
> >
> > No. It is not junk. I got this from a header in tree.
> >
> > I am inclined to just drop this hunk. If that breaks things, I will use
> > "reserved0".
> >
>
> It turns out adding reserved0 is required. Dropping this hunk does not
> work.

Generally speaking, bitfields are not great for specifying binary interfaces,
as the actual bit order can differ by architecture. The normal way we get
around it in the kernel is to use basic integer types and define macros
for bit masks. Ideally, each such field should also be marked with a
particular endianess as __le64 or __be64, in case this is ever used with
an Arm guest running a big-endian kernel.

That said, if you do not care about the specific order of the bits, having
anonymous bitfields for the reserved members is fine, I don't see a
reason to name it as reserved.

      Arnd