Re: [PATCH v5 5/9] mshv: Import data structs around device passthru from hyperv headers

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:
- [High] The fields in `union hv_device_domain_id` are ordered incorrectly,=
 resulting in a flawed bitfield layout that contradicts the domain ID const=
ants.
- [High] `struct hv_input_create_device_domain` is exactly 28 bytes in size=
, violating the MS C implicit padding expectations for 8-byte aligned struc=
tures.
--

--- Patch [5]: [PATCH v5 5/9] mshv: Import data structs around device passt=
hru from hyperv headers ---
commit 9932d93213b193c633772c7ddbee866c39282647
Author: Mukesh R <[email protected]>

mshv: Import data structs around device passthru from hyperv headers

Copy/import from Hyper-V public headers, definitions and declarations that
are related to creating domains in the hypervisor, attaching devices to
them, etc.

> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index b4cb2fa26e9bc..90497635c3cbd 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
[ ... ]
> +/* ID for stage 2 default domain and NULL domain */
> +#define HV_DEVICE_DOMAIN_ID_S2_DEFAULT 0
> +#define HV_DEVICE_DOMAIN_ID_S2_NULL    0xFFFFFFFFULL
> +
> +union hv_device_domain_id {
> +	u64 as_uint64;
> +	struct {
> +		u32 type : 4;
> +		u32 reserved : 28;
> +		u32 id;
> +	};
> +} __packed;

[Severity: High]
Are the fields in this struct ordered correctly for the ABI?

In little-endian C bitfields, the first member occupies the lowest bits. By
placing type and reserved before id, they will occupy the lower 32 bits
while id occupies the upper 32 bits.

This seems to contradict the constant HV_DEVICE_DOMAIN_ID_S2_NULL defined j=
ust
above this union, which expects the id (0xFFFFFFFF) in the lower 32 bits and
type (0) in the upper 32 bits.

Will this cause the hypervisor to receive swapped values and fail domain
attachment?

[ ... ]
> +struct hv_input_create_device_domain {	/* HV_INPUT_CREATE_DEVICE_DOMAIN =
*/
> +	struct hv_input_device_domain device_domain;
> +	union hv_create_device_domain_flags create_device_domain_flags;
> +} __packed;

[Severity: High]
Does this struct need explicit padding at the end to match the hypervisor's
32-byte size expectation?

Because of the __packed attribute, the compiler won't add the 4-byte implic=
it
trailing padding required to bring the struct to its natural 32-byte size
(due to the 8-byte alignment of device_domain).

If standard hypercall initialization clears the input using:
memset(in, 0, sizeof(*in));

the sizeof will be 28, leaving the last 4 bytes of the 32-byte block
uninitialized on the shared hypercall page.=20

If the hypervisor validates these padding bytes to be zero, could this fail
with HV_STATUS_INVALID_PARAMETER if there is leftover garbage from a
previous hypercall?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731223427.2554=
[email protected]?part=3D5