RE: [PATCH v8 1/6] arm64: hyperv: Add Hyper-V hypercall and register access utilities

[Michael Kelley]

From: Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx> Sent: Tuesday, March 2, 2021 4:35 PM
> 
> > +}
> > +EXPORT_SYMBOL_GPL(hv_do_fast_hypercall8);
> > +
> > +
> nit: Extra line, here and few other places

I'll try to clean these up.

> 
> > +u64 hv_get_vpreg(u32 msr)
> > +{
> > +	struct hv_get_vp_registers_input	*input;
> > +	struct hv_get_vp_registers_output	*output;
> > +	u64					result;
> It seems like the code is using both spaces and tabs to align variable names. Can
> we stick to one or the other, preferably spaces.

I think the general preference is to use tabs for alignment, not
spaces, so I'll try to clean these up in the next version.

> 
> > +
> > +	/*
> > +	 * Allocate a power of 2 size so alignment to that size is
> > +	 * guaranteed, since the hypercall input and output areas
> > +	 * must not cross a page boundary.
> > +	 */
> > +	input = kzalloc(roundup_pow_of_two(sizeof(input->header) +
> > +				sizeof(input->element[0])), GFP_ATOMIC);
> > +	output = kmalloc(roundup_pow_of_two(sizeof(*output)), GFP_ATOMIC);
> > +
> Check for null from these malloc routines? Here and in other places.

See my comment back to Boqun Feng on the same point.

> 
> > +	__hv_get_vpreg_128(msr, input, output);
> 
> + * Linux-specific definitions for managing interactions with Microsoft's
> + * Hyper-V hypervisor. The definitions in this file are specific to
> + * the ARM64 architecture.  See include/asm-generic/mshyperv.h for
> nit: Two space before 'See'. Here and in couple of other places.

The debate rages in the broader world of writing about using one
space vs. two spaces. :-)   I'm still old school on this point, and use two
spaces out of decades of habit, particularly with monospaced fonts,
though I'm probably not 100% consistent.

Michael