RE: [PATCH v6 03/10] arm64: hyperv: Add hypercall and register access functions

[Michael Kelley]

From: Hillf Danton <hdanton@xxxxxxxx> Sent: Tuesday, March 17, 2020 8:12 PM
> 
> On Sat, 14 Mar 2020 08:35:12 -0700 Michael Kelley wrote:
> > +/*
> > + * Get the value of a single VP register.  One version
> > + * returns just 64 bits and another returns the full 128 bits.
> > + * The two versions are separate to avoid complicating the
> > + * calling sequence for the more frequently used 64 bit version.
> > + */
> > +
> > +/*
> > + * Input and output memory allocation sizes are rounded up to a power
> > + * of 2 so kmalloc() will guarantee alignment. In turn, the alignment
> > + * ensures that the allocations don't cross a page boundary, which is
> 
> Better to specify kmalloc's current alignment and why it fails to ensure
> (4 * sizeof(u64))-sized allocations wont cross page boundary.
> 

Is your comment referring to ARCH_KMALLOC_MINALIGN?  If so, I see
how that makes sense.  BUILD_BUG_ON (sizeof (*input) >
ARCH_KMALLOC_MINALIGN) would be a cleaner solution. 

Thanks,

Michael

> > + * required by the hypercall interface.
> > + */
> > +#define INPUTSIZE (4 * sizeof(u64))
> > +#define OUTPUTSIZE (2 * sizeof(u64))
> > +
> > +static void __hv_get_vpreg_128(u32 msr, struct hv_get_vp_register_output *res)
> > +{
> > +	union hv_hypercall_status		status;
> > +	struct hv_get_vp_register_input		*input;
> > +
> > +	BUILD_BUG_ON(sizeof(*input) > INPUTSIZE);
> > +
> > +	input = kzalloc(INPUTSIZE, GFP_ATOMIC);
> > +
> > +	input->partitionid = HV_PARTITION_ID_SELF;
> > +	input->vpindex = HV_VP_INDEX_SELF;
> > +	input->inputvtl = 0;
> > +	input->name0 = msr;
> > +	input->name1 = 0;
> > +
> > +
> > +	status.as_uint64 = hv_do_hypercall(
> > +		HVCALL_GET_VP_REGISTERS | HV_HYPERCALL_REP_COUNT_1,
> > +		input, res);
> > +
> > +	/*
> > +	 * Something is fundamentally broken in the hypervisor if
> > +	 * getting a VP register fails. There's really no way to
> > +	 * continue as a guest VM, so panic.
> > +	 */
> > +	BUG_ON(status.status != HV_STATUS_SUCCESS);
> > +
> > +	kfree(input);
> > +}