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

4 messages, 2 authors, 2021-03-05 · open the first message on its own page

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

From: Sunil Muthuswamy <hidden>
Date: 2021-03-03 11:03:40

+}
+EXPORT_SYMBOL_GPL(hv_do_fast_hypercall8);
+
+
nit: Extra line, here and few other places
+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.
+
+	/*
+	 * 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.
+	__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.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

From: Sunil Muthuswamy <hidden>
Date: 2021-03-03 20:58:19

quoted
+
+	/*
+	 * 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.
Between, is there a plan to setup a percpu input/output page for hypercall
input/output on ARM (like we do for x64)? I didn't check the specific size requirement
for this particular call, but, that generally will remove the need for these
allocations.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

From: Michael Kelley <hidden>
Date: 2021-03-05 20:59:19

From: Sunil Muthuswamy <redacted> Sent: Wednesday, March 3, 2021 10:28 AM
quoted
quoted
+
+	/*
+	 * 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.
Between, is there a plan to setup a percpu input/output page for hypercall
input/output on ARM (like we do for x64)? I didn't check the specific size requirement
for this particular call, but, that generally will remove the need for these
allocations.
I'm not planning to add percpu input/output pages in this patch set.  As noted
in another reply, we can eliminate these memory allocations entirely by using
"fast" hypercalls.  At that point, there would be no usage of percpu input/output
pages in this patch set.  If the need arises in the context of future work, we can
add them at that time.

Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

From: Michael Kelley <hidden>
Date: 2021-03-05 20:57:10

From: Sunil Muthuswamy <redacted> Sent: Tuesday, March 2, 2021 4:35 PM
quoted
+}
+EXPORT_SYMBOL_GPL(hv_do_fast_hypercall8);
+
+
nit: Extra line, here and few other places
I'll try to clean these up.
quoted
+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.
quoted
+
+	/*
+	 * 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.
quoted
+	__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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help