RE: [PATCH v4 2/5] mshv: Add the HVCALL_GET_PARTITION_PROPERTY_EX hypercall

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx> Sent: Wednesday, October 1, 2025 3:33 PM
> 
> On 9/29/2025 10:55 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx> Sent: Friday, September 26, 2025 9:23 AM
> >>
> >> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@xxxxxxxxxxxxxxxxxxx>
> >>

[snip]

> >> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
> >> index b4067ada02cf..416c0d45b793 100644
> >> --- a/include/hyperv/hvhdk.h
> >> +++ b/include/hyperv/hvhdk.h
> >> @@ -376,6 +376,46 @@ struct hv_input_set_partition_property {
> >>  	u64 property_value;
> >>  } __packed;
> >>
> >> +union hv_partition_property_arg {
> >> +	u64 as_uint64;
> >> +	struct {
> >> +		union {
> >> +			u32 arg;
> >> +			u32 vp_index;
> >> +		};
> >> +		u16 reserved0;
> >> +		u8 reserved1;
> >> +		u8 object_type;
> >> +	} __packed;
> >> +};
> >> +
> >> +struct hv_input_get_partition_property_ex {
> >> +	u64 partition_id;
> >> +	u32 property_code; /* enum hv_partition_property_code */
> >> +	u32 padding;
> >> +	union {
> >> +		union hv_partition_property_arg arg_data;
> >> +		u64 arg;
> >
> > This union, and the "u64 arg" member, seems redundant since
> > union hv_partition_property_arg already has a member "as_uint64".
> >
> > Maybe this is just being copied from the Windows versions of these
> > structures, in which case I realize there are constraints on what you
> > want to change or fix, and you can ignore my comment.
> >
> Yeah, this is just due to it coming from the Windows code. I prefer to
> keep it as close as possible unless it is actually a bug (like missing
> padding which has happened a couple of times).

Makes sense.

> 
> >> +	};
> >> +} __packed;
> >> +
> >> +/*
> >> + * NOTE: Should use hv_input_set_partition_property_ex_header to compute this
> >> + * size, but hv_input_get_partition_property_ex is identical so it suffices
> >> + */
> >> +#define HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE \
> >> +	(HV_HYP_PAGE_SIZE - sizeof(struct hv_input_get_partition_property_ex))
> >> +
> >> +union hv_partition_property_ex {
> >> +	u8 buffer[HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE];
> >
> > It's unclear what this "buffer" field is trying to do, and particularly its size.
> > The comment above references hv_input_set_partition_property_ex_header,
> > which doesn't exist in the Linux code.  And why is the max size of the output
> > buffer reduced by the size of the header of the input to "set partition property"?
> >
> Yes, hv_input_set_partition_property_ex_header doesn't exist here, it's from the
> Windows code (although in CAPs). It's identical to hv_input_get_partition_property_ex.
> I decided to just use hv_input_get_partition_property_ex instead of introducing the
> unused header struct, for now.
> 
> I'm not certain what the buffer is used for, but I suspect it's for data that doesn't
> fit with the other partition property structs. Maybe some raw/unstructured data.
> 
> hv_partition_property_ex has to be that size because it is also used (not yet, in linux)
> for setting properties, and the set hypercall has a header as mentioned above, so the
> entire struct mustn't exceed a page in size - i.e.:
> 
> struct hv_input_set_partition_property_ex {
> 	hv_input_set_partition_property_ex_header header;
> 	hv_partition_property_ex property_value;
> };

OK, that makes sense. Thanks.

Michael