RE: [PATCH 0/2] hyperv: Move some features to common code

[Michael Kelley]

From: Nuno Das Neves <nunodasneves@xxxxxxxxxxxxxxxxxxx> Sent: Friday, December 6, 2024 2:22 PM
> 
> There are several bits of Hyper-V-related code that today live in
> arch/x86 but are not really specific to x86_64 and will work on arm64
> too.
> 
> Some of these will be needed in the upcoming mshv driver code (for
> Linux as root partition on Hyper-V).

Previously, Linux as the root partition on Hyper-V was x86 only, which is
why the code is currently under arch/x86. So evidently the mshv driver
is being expanded to support both x86 and arm64, correct? Assuming
that's the case, I have some thoughts about how the source code should
be organized and built. It's probably best to get this right to start with so
it doesn't need to be changed again.

* Patch 2 of this series moves hv_call_deposit_pages() and
   hv_call_create_vp() to common code, but does not move
   hv_call_add_logical_proc(). All three are used together, so
   I'm wondering why hv_call_add_logical_proc() isn't moved.

* These three functions were originally put in a separate source
   code file because of being specific to running in the root partition,
   and not needed for generic Linux guest support. I think there's
   value in keeping them in a separate file, rather than merging them
   into hv_common.c. Maybe just move the entire hv_proc.c file?
   And then later, perhaps move the entire irqdomain.c file as well?
   There's also an interesting question of whether to move them into
   drivers/hv, or create a new directory virt/hyperv. Hyper-V support
   started out 15 years ago structured as a driver, hence "drivers/hv".
   But over the time, the support has become significantly more than
   just a driver, so "virt/hyperv" might be a better location for
   non-driver code that had previously been under arch/x86 but is
   now common to all architectures.

* Today, the code for running in the root partition is built along
   with the rest of the Hyper-V support, and so is present in kernels
   built for normal Linux guests on Hyper-V. I haven't thought about
   all the implications, but perhaps there's value in having a CONFIG
   option to build for the root partition, so that code can be dropped
   from normal kernels. There's a significant amount of new code still
   to come for mshv that could be excluded from normal guests in this
   way. Also, the tests of the hv_root_partition variable could be
   changed to a function the compiler detects is always "false" in a
   kernel built without the CONFIG option, in which case it can drop
   the code for where hv_root_partition is "true".

* The code currently in hv_proc.c is built for x86 only, and validly
   assumes the page size is 4K. But when the code moves to be
   common across architectures, that assumption is no longer
   valid in the general case. Perhaps the intent is that kernels for
   the root partition should always be built with page size 4K on
   arm64, but nothing enforces that intent. Personally, I think the code
   should be made to work with page sizes other than 4K so as to not
   leave technical debt. But I realize you may have other priorities. If
   there were a CONFIG option for building for the root partition,
   that option could be setup to enforce the 4K page size on arm64.

Anyway, thinking through these decisions up front could avoid
the need for additional moves later on.

Michael

> So this is a good time to move
> them to hv_common.c.
> 
> Signed-off-by: Nuno Das Neves <nudasnev@xxxxxxxxxxxxx>
> 
> Nuno Das Neves (2):
>   hyperv: Move hv_current_partition_id to arch-generic code
>   hyperv: Move create_vp and deposit_pages hvcalls to hv_common.c
> 
>  arch/arm64/hyperv/mshyperv.c    |   3 +
>  arch/x86/hyperv/hv_init.c       |  25 +----
>  arch/x86/hyperv/hv_proc.c       | 144 ---------------------------
>  arch/x86/include/asm/mshyperv.h |   4 -
>  drivers/hv/hv_common.c          | 168 ++++++++++++++++++++++++++++++++
>  include/asm-generic/mshyperv.h  |   4 +
>  6 files changed, 176 insertions(+), 172 deletions(-)
> 
> --
> 2.34.1