Re: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions

Re: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions

[Boqun Feng]

Hi Michael,

On Thu, Oct 03, 2019 at 07:03:19PM +0000, Michael Kelley wrote:
> Add ARM64-specific code to make Hyper-V hypercalls and to
> access virtual processor synthetic registers via hypercalls.
> Hypercalls use a Hyper-V specific calling sequence with a non-zero
> immediate value per Section 2.9 of the SMC Calling Convention
> spec.
> 
> This code is architecture dependent and is mostly driven by
> architecture independent code in the VMbus driver and the
> Hyper-V timer clocksource driver.
> 
> This code is built only when CONFIG_HYPERV is enabled.
> 
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>  MAINTAINERS                 |   1 +
>  arch/arm64/Kbuild           |   1 +
>  arch/arm64/hyperv/Makefile  |   2 +
>  arch/arm64/hyperv/hv_hvc.S  |  44 +++++++++++++++
>  arch/arm64/hyperv/hv_init.c | 133 ++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 181 insertions(+)
>  create mode 100644 arch/arm64/hyperv/Makefile
>  create mode 100644 arch/arm64/hyperv/hv_hvc.S
>  create mode 100644 arch/arm64/hyperv/hv_init.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d464067..84f76f9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7566,6 +7566,7 @@ F:	arch/x86/kernel/cpu/mshyperv.c
>  F:	arch/x86/hyperv
>  F:	arch/arm64/include/asm/hyperv-tlfs.h
>  F:	arch/arm64/include/asm/mshyperv.h
> +F:	arch/arm64/hyperv
>  F:	drivers/clocksource/hyperv_timer.c
>  F:	drivers/hid/hid-hyperv.c
>  F:	drivers/hv/
> diff --git a/arch/arm64/Kbuild b/arch/arm64/Kbuild
> index d646582..2469421 100644
> --- a/arch/arm64/Kbuild
> +++ b/arch/arm64/Kbuild
> @@ -3,4 +3,5 @@ obj-y			+= kernel/ mm/
>  obj-$(CONFIG_NET)	+= net/
>  obj-$(CONFIG_KVM)	+= kvm/
>  obj-$(CONFIG_XEN)	+= xen/
> +obj-$(CONFIG_HYPERV)	+= hyperv/

I did a kernel built with CONFIG_HYPERV=m today, and found out this line
should be (similar to x86):

	+obj-$(subst m,y,$(CONFIG_HYPERV))      += hyperv/

, otherwise, when CONFIG_HYPERV=m, files in arch/arm64/hyperv/ will be
compiled as obj-m, and symbols defined in those files cannot be
used by kernel builtin, e.g. hyperv_timer (since CONFIG_HYPERV_TIMER=y
in this case).

A compile/link error I hit today is:

| /home/boqun/linux-arm64/drivers/clocksource/hyperv_timer.c:98: undefined reference to `hv_set_vpreg'
| aarch64-linux-gnu-ld: /home/boqun/linux-arm64/drivers/clocksource/hyperv_timer.c:98: undefined reference to `hv_set_vpreg'

[...]

Besides, another problem I hit when compiled with CONFIG_HYPERV=m is:

| ERROR: "screen_info" [drivers/hv/hv_vmbus.ko] undefined!

, which can be fixed by the following change.

Regards,
Boqun

---------------->8
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index d0cf596db82c..8ff557ae5cc6 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c

@@ -55,6 +55,7 @@ static __init pteval_t create_mapping_protection(efi_memory_desc_t *md)

 /* we will fill this structure from the stub, so don't put it in .bss */
 struct screen_info screen_info __section(.data);
+EXPORT_SYMBOL(screen_info);

 int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
 {

Re: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions

[Marc Zyngier]

On 2019-10-03 20:12, Michael Kelley wrote:
> Add ARM64-specific code to make Hyper-V hypercalls and to
> access virtual processor synthetic registers via hypercalls.
> Hypercalls use a Hyper-V specific calling sequence with a non-zero
> immediate value per Section 2.9 of the SMC Calling Convention
> spec.

I find this "following the spec by actively sidestepping it" counter
productive. You (or rather the Hyper-V people) are reinventing the
wheel (of the slightly square variety) instead of using the standard
that the whole of the ARM ecosystem seems happy to take advantage
of.

I wonder what is the rational for this. If something doesn't quite
work for Hyper-V, I think we'd all like to know.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions

[Marc Zyngier]

On 2019-11-06 19:08, Michael Kelley wrote:
> From: Marc Zyngier <maz@kernel.org>  Sent: Wednesday, November 6,
> 2019 2:20 AM
>>
>> On 2019-10-03 20:12, Michael Kelley wrote:
>> > Add ARM64-specific code to make Hyper-V hypercalls and to
>> > access virtual processor synthetic registers via hypercalls.
>> > Hypercalls use a Hyper-V specific calling sequence with a non-zero
>> > immediate value per Section 2.9 of the SMC Calling Convention
>> > spec.
>>
>> I find this "following the spec by actively sidestepping it" counter
>> productive. You (or rather the Hyper-V people) are reinventing the
>> wheel (of the slightly square variety) instead of using the standard
>> that the whole of the ARM ecosystem seems happy to take advantage
>> of.
>>
>> I wonder what is the rational for this. If something doesn't quite
>> work for Hyper-V, I think we'd all like to know.
>>
>
> I'll go another round internally with the Hyper-V people on this
> topic and impress upon them the desire of the Linux community to
> have Hyper-V adopt the true spirit of the spec.  But I know they are
> fairly set in their approach at this point, regardless of the 
> technical
> merits or lack thereof.  Hyper-V is shipping and in use as a 
> commercial
> product on ARM64 hardware, which makes it harder to change.  I
> hope we can find a way to avoid a complete impasse ....

Hyper-V shipping with their own calling convention is fine by me. Linux
having to implement multiple calling conventions because the Hyper-V
folks refuse (for undisclosed reason) to adopt the standard isn't fine 
at
all.

HV can perfectly retain its interface for Windows or other things, but
please *at least* implement the standard interface on which all 
existing
operating systems rely.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v5 3/8] arm64: hyperv: Add memory alloc/free functions for Hyper-V size pages

[Marc Zyngier]

On 2019-10-03 20:12, Michael Kelley wrote:
> Add ARM64-specific code to allocate memory with HV_HYP_PAGE_SIZE
> size and alignment. These are for use when pages need to be shared
> with Hyper-V. Separate functions are needed as the page size used
> by Hyper-V may not be the same as the guest page size.  Free
> operations are rarely done, so no attempt is made to combine
> freed pages into larger chunks.
>
> This code is built only when CONFIG_HYPERV is enabled.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>  arch/arm64/hyperv/hv_init.c    | 68
> ++++++++++++++++++++++++++++++++++++++++++
>  include/asm-generic/mshyperv.h |  5 ++++
>  2 files changed, 73 insertions(+)
>
> diff --git a/arch/arm64/hyperv/hv_init.c 
> b/arch/arm64/hyperv/hv_init.c
> index 6808bc8..9c294f6 100644
> --- a/arch/arm64/hyperv/hv_init.c
> +++ b/arch/arm64/hyperv/hv_init.c
> @@ -15,10 +15,78 @@
>  #include <linux/export.h>
>  #include <linux/mm.h>
>  #include <linux/hyperv.h>
> +#include <linux/spinlock.h>
> +#include <linux/list.h>
> +#include <linux/string.h>
>  #include <asm-generic/bug.h>
>  #include <asm/hyperv-tlfs.h>
>  #include <asm/mshyperv.h>
>
> +
> +/*
> + * Functions for allocating and freeing memory with size and
> + * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> + * the guest page size may not be the same as the Hyper-V page
> + * size. And while kalloc() could allocate the memory, it does not
> + * guarantee the required alignment. So a separate small memory
> + * allocator is needed.  The free function is rarely used, so it
> + * does not try to combine freed pages into larger chunks.

Is this still needed now that kmalloc has alignment guarantees
(see 59bb47985c1d)?

         M.
-- 
Jazz is not dead. It just smells funny...