Re: [PATCH v7 05/10] arm64: hyperv: Add interrupt handlers for VMbus and stimer
From: Arnd Bergmann <arnd@arndb.de>
Date: 2020-08-26 07:15:01
Also in:
linux-arch, linux-arm-kernel, linux-efi, lkml
On Wed, Aug 26, 2020 at 12:04 AM Michael Kelley [off-list ref] wrote:
From: Arnd Bergmann <arnd@arndb.de> Sent: Monday, August 24, 2020 11:54 AM
quoted
I'm not sure what the correct solution should be, but what I'd try to do here is to move every function that just considers the platform rather than the architecture somewhere into drivers/hv where it can be linked into the same modules as the existing files when building for arm64, while trying to keep architecture specific code in the header file where it can be included from those modules.OK. The concept of separating platform from architecture makes sense to me. The original separation of the Hyper-V code into architecture independent portions and x86-specific portions could use some tweaking now that we're dealing with n=2 architectures. With that tweaking, I can reduce the amount of Hyper-V code under arch/x86 and under arch/arm64. On the flip side, the Hyper-V implementation on x86 and ARM64 has differences that are semi-related to the architecture. For example, on x86 Hyper-V uses synthetic MSRs for a lot of guest-hypervisor setup, while hypercalls are required on ARM64. So I'm assuming those differences will end up in code under arch/x86 and arch/arm64.
Yes, that absolutely makes sense.
Arguably, I could introduce a level of indirection (such as CONFIG_HYPERV_USE_MSRS vs. CONFIG_HYPERV_USE_HYPERCALLS) to distinguish the two behaviors. The selection would be tied to the architecture, and then code in drivers/hv can #ifdef the two cases. But I wonder if getting code out of arch/x86 and arch/arm64 is worth that additional messiness.
No, I think that would take it a little too far, and conflicts with the generic rule that code under drivers/* should be written to be portable even if can only run on a particular target platform.
Looking at the Xen code in drivers/xen, it looks like a lot of the Xen functionality is implemented in hypercalls that can be consistent across architectures, though I was a bit surprised to see a dozen or so instances of #ifdef CONFIG_X86. Xen also #ifdefs on PV vs. PVHVM, which may handle some architecture differences implicitly. But I'm assuming that doing #ifdef <architecture> in the Hyper-V code in order to reduce code under arch/x86 or arch/arm64 is not the right way to go.
In general that is true, adding a lot of #ifdefs makes code less readable and
harder to test. OTOH there are cases where a single #ifdef can be useful when
it avoids adding a larger amount of complexity elsewhere. Many subsystems
try to restrict the #ifdef checks to header files while keeping the
drivers/* code
free of them.
Arnd