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 [off-list ref] wrote:

On Tue, Feb 02, 2021 at 05:02:48PM +0000, Wei Liu wrote:

quoted

On Tue, Jan 26, 2021 at 01:26:52AM +0000, Michael Kelley wrote:

quoted

From: Wei Liu <wei.liu@kernel.org> Sent: Wednesday, January 20, 2021 4:01 AM

quoted

+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