[PATCH 0/6] arm64/hyperv: Support Virtual Trust Level boot

[PATCH 0/6] arm64/hyperv: Support Virtual Trust Level boot

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

This set of patches enables the Hyper-V code to boot on ARM64
inside a Virtual Trust Level. These levels are a part of the
Virtual Secure Mode documented in the Top-Level Functional
Specification available at
https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/vsm

Roman Kisel (6):
  arm64/hyperv: Support DeviceTree
  drivers/hv: Enable VTL mode for arm64
  arm64/hyperv: Boot in a Virtual Trust Level
  drivers/hv: arch-neutral implementation of get_vtl()
  drivers/hv/vmbus: Get the irq number from DeviceTree
  drivers/pci/hyperv/arm64: vPCI MSI IRQ domain from DT

 arch/arm64/hyperv/Makefile          |  1 +
 arch/arm64/hyperv/hv_vtl.c          | 19 +++++++++++++
 arch/arm64/hyperv/mshyperv.c        | 40 +++++++++++++++++++++++----
 arch/arm64/include/asm/mshyperv.h   |  8 ++++++
 arch/x86/hyperv/hv_init.c           | 34 -----------------------
 arch/x86/hyperv/hv_vtl.c            |  2 +-
 arch/x86/include/asm/hyperv-tlfs.h  |  7 -----
 drivers/hv/Kconfig                  |  6 ++--
 drivers/hv/hv_common.c              | 43 +++++++++++++++++++++++++++++
 drivers/hv/vmbus_drv.c              | 37 +++++++++++++++++++++++++
 drivers/pci/controller/pci-hyperv.c | 13 +++++++--
 include/asm-generic/hyperv-tlfs.h   |  7 +++++
 include/asm-generic/mshyperv.h      |  6 ++++
 include/linux/acpi.h                | 10 +++++++
 14 files changed, 180 insertions(+), 53 deletions(-)
 create mode 100644 arch/arm64/hyperv/hv_vtl.c

-- 
2.45.0

[PATCH 1/6] arm64/hyperv: Support DeviceTree

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

Update the driver to support DeviceTree boot as well along with ACPI.
This enables the Virtual Trust Level platforms boot up on ARM64.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index b1a4de4eee29..208a3bcb9686 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -15,6 +15,9 @@
 #include <linux/errno.h>
 #include <linux/version.h>
 #include <linux/cpuhotplug.h>
+#include <linux/libfdt.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
 #include <asm/mshyperv.h>
 
 static bool hyperv_initialized;
@@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
 	return 0;
 }
 
+static bool hyperv_detect_fdt(void)
+{
+#ifdef CONFIG_OF
+	const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
+			of_get_flat_dt_root(), "hypervisor");
+
+	return (hyp_node != -FDT_ERR_NOTFOUND) &&
+			of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
+#else
+	return false;
+#endif
+}
+
+static bool hyperv_detect_acpi(void)
+{
+#ifdef CONFIG_ACPI
+	return !acpi_disabled &&
+			!strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
+#else
+	return false;
+#endif
+}
+
 static int __init hyperv_init(void)
 {
 	struct hv_get_vp_registers_output	result;
@@ -35,13 +61,11 @@ static int __init hyperv_init(void)
 
 	/*
 	 * Allow for a kernel built with CONFIG_HYPERV to be running in
-	 * a non-Hyper-V environment, including on DT instead of ACPI.
+	 * a non-Hyper-V environment.
+	 *
 	 * In such cases, do nothing and return success.
 	 */
-	if (acpi_disabled)
-		return 0;
-
-	if (strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8))
+	if (!hyperv_detect_fdt() && !hyperv_detect_acpi())
 		return 0;
 
 	/* Setup the guest ID */
-- 
2.45.0

[PATCH 2/6] drivers/hv: Enable VTL mode for arm64

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

This change removes dependency on ACPI when buidling the hv drivers to
allow Virtual Trust Level boot with DeviceTree.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 drivers/hv/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 862c47b191af..a5cd1365e248 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -5,7 +5,7 @@ menu "Microsoft Hyper-V guest support"
 config HYPERV
 	tristate "Microsoft Hyper-V client drivers"
 	depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
-		|| (ACPI && ARM64 && !CPU_BIG_ENDIAN)
+		|| (ARM64 && !CPU_BIG_ENDIAN)
 	select PARAVIRT
 	select X86_HV_CALLBACK_VECTOR if X86
 	select OF_EARLY_FLATTREE if OF
@@ -15,7 +15,7 @@ config HYPERV
 
 config HYPERV_VTL_MODE
 	bool "Enable Linux to boot in VTL context"
-	depends on X86_64 && HYPERV
+	depends on HYPERV
 	depends on SMP
 	default n
 	help
@@ -31,7 +31,7 @@ config HYPERV_VTL_MODE
 
 	  Select this option to build a Linux kernel to run at a VTL other than
 	  the normal VTL0, which currently is only VTL2.  This option
-	  initializes the x86 platform for VTL2, and adds the ability to boot
+	  initializes the kernel to run in VTL2, and adds the ability to boot
 	  secondary CPUs directly into 64-bit context as required for VTLs other
 	  than 0.  A kernel built with this option must run at VTL2, and will
 	  not run as a normal guest.
-- 
2.45.0

[PATCH 3/6] arm64/hyperv: Boot in a Virtual Trust Level

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

This change builds upon the previous ones to boot
in a Virtual Trust Level and provide configuration
for the drivers.

Also print the VTL the code runs in the new and the
existing code.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 arch/arm64/hyperv/Makefile        |  1 +
 arch/arm64/hyperv/hv_vtl.c        | 19 +++++++++++++++++++
 arch/arm64/hyperv/mshyperv.c      |  6 ++++++
 arch/arm64/include/asm/mshyperv.h |  8 ++++++++
 arch/x86/hyperv/hv_vtl.c          |  2 +-
 5 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/hyperv/hv_vtl.c

diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile
index 87c31c001da9..9701a837a6e1 100644
--- a/arch/arm64/hyperv/Makefile
+++ b/arch/arm64/hyperv/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-y		:= hv_core.o mshyperv.o
+obj-$(CONFIG_HYPERV_VTL_MODE)	+= hv_vtl.o
diff --git a/arch/arm64/hyperv/hv_vtl.c b/arch/arm64/hyperv/hv_vtl.c
new file mode 100644
index 000000000000..9b44cc49594c
--- /dev/null
+++ b/arch/arm64/hyperv/hv_vtl.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023, Microsoft, Inc.
+ *
+ * Author : Roman Kisel <romank@linux.microsoft.com>
+ */
+
+#include <asm/mshyperv.h>
+
+void __init hv_vtl_init_platform(void)
+{
+	pr_info("Linux runs in Hyper-V Virtual Trust Level %d\n", ms_hyperv.vtl);
+}
+
+int __init hv_vtl_early_init(void)
+{
+	return 0;
+}
+early_initcall(hv_vtl_early_init);
diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index 208a3bcb9686..cbde483b167a 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -96,6 +96,12 @@ static int __init hyperv_init(void)
 		return ret;
 	}
 
+	/* Find the VTL */
+	ms_hyperv.vtl = get_vtl();
+	if (ms_hyperv.vtl > 0) /* non default VTL */
+		hv_vtl_early_init();
+
+	hv_vtl_init_platform();
 	ms_hyperv_late_init();
 
 	hyperv_initialized = true;
diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
index a975e1a689dd..4a8ff6be389c 100644
--- a/arch/arm64/include/asm/mshyperv.h
+++ b/arch/arm64/include/asm/mshyperv.h
@@ -49,6 +49,14 @@ static inline u64 hv_get_msr(unsigned int reg)
 				ARM_SMCCC_OWNER_VENDOR_HYP,	\
 				HV_SMCCC_FUNC_NUMBER)
 
+#ifdef CONFIG_HYPERV_VTL_MODE
+void __init hv_vtl_init_platform(void);
+int __init hv_vtl_early_init(void);
+#else
+static inline void __init hv_vtl_init_platform(void) {}
+static inline int __init hv_vtl_early_init(void) { return 0; }
+#endif
+
 #include <asm-generic/mshyperv.h>
 
 #endif
diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 92bd5a55f093..ae3105375a12 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -19,7 +19,7 @@ static struct real_mode_header hv_vtl_real_mode_header;
 
 void __init hv_vtl_init_platform(void)
 {
-	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
+	pr_info("Linux runs in Hyper-V Virtual Trust Level %d\n", ms_hyperv.vtl);
 
 	x86_platform.realmode_reserve = x86_init_noop;
 	x86_platform.realmode_init = x86_init_noop;
-- 
2.45.0

[PATCH 4/6] drivers/hv: arch-neutral implementation of get_vtl()

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

This change generalizes the x86-specific implementation
of get_vtl() so that it can be used on arm64.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 arch/x86/hyperv/hv_init.c          | 34 -----------------------
 arch/x86/include/asm/hyperv-tlfs.h |  7 -----
 drivers/hv/hv_common.c             | 43 ++++++++++++++++++++++++++++++
 include/asm-generic/hyperv-tlfs.h  |  7 +++++
 include/asm-generic/mshyperv.h     |  6 +++++
 5 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 17a71e92a343..c350fa05ee59 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -413,40 +413,6 @@ static void __init hv_get_partition_id(void)
 	local_irq_restore(flags);
 }
 
-#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
-static u8 __init get_vtl(void)
-{
-	u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS;
-	struct hv_get_vp_registers_input *input;
-	struct hv_get_vp_registers_output *output;
-	unsigned long flags;
-	u64 ret;
-
-	local_irq_save(flags);
-	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
-	output = (struct hv_get_vp_registers_output *)input;
-
-	memset(input, 0, struct_size(input, element, 1));
-	input->header.partitionid = HV_PARTITION_ID_SELF;
-	input->header.vpindex = HV_VP_INDEX_SELF;
-	input->header.inputvtl = 0;
-	input->element[0].name0 = HV_X64_REGISTER_VSM_VP_STATUS;
-
-	ret = hv_do_hypercall(control, input, output);
-	if (hv_result_success(ret)) {
-		ret = output->as64.low & HV_X64_VTL_MASK;
-	} else {
-		pr_err("Failed to get VTL(error: %lld) exiting...\n", ret);
-		BUG();
-	}
-
-	local_irq_restore(flags);
-	return ret;
-}
-#else
-static inline u8 get_vtl(void) { return 0; }
-#endif
-
 /*
  * This function is to be invoked early in the boot sequence after the
  * hypervisor has been detected.
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 3787d26810c1..9ee68eb8e6ff 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -309,13 +309,6 @@ enum hv_isolation_type {
 #define HV_MSR_STIMER0_CONFIG	(HV_X64_MSR_STIMER0_CONFIG)
 #define HV_MSR_STIMER0_COUNT	(HV_X64_MSR_STIMER0_COUNT)
 
-/*
- * Registers are only accessible via HVCALL_GET_VP_REGISTERS hvcall and
- * there is not associated MSR address.
- */
-#define	HV_X64_REGISTER_VSM_VP_STATUS	0x000D0003
-#define	HV_X64_VTL_MASK			GENMASK(3, 0)
-
 /* Hyper-V memory host visibility */
 enum hv_mem_host_visibility {
 	VMBUS_PAGE_NOT_VISIBLE		= 0,
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index dde3f9b6871a..d4cf477a4d0c 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -660,3 +660,46 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
 	return HV_STATUS_INVALID_PARAMETER;
 }
 EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
+
+#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
+u8 __init get_vtl(void)
+{
+	u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS;
+	struct hv_get_vp_registers_input *input;
+	struct hv_get_vp_registers_output *output;
+	unsigned long flags;
+	u64 ret;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	output = (struct hv_get_vp_registers_output *)input;
+
+	memset(input, 0, struct_size(input, element, 1));
+	input->header.partitionid = HV_PARTITION_ID_SELF;
+	input->header.vpindex = HV_VP_INDEX_SELF;
+	input->header.inputvtl = 0;
+	input->element[0].name0 = HV_REGISTER_VSM_VP_STATUS;
+
+	ret = hv_do_hypercall(control, input, output);
+	if (hv_result_success(ret)) {
+		ret = output->as64.low & HV_VTL_MASK;
+	} else {
+		pr_err("Failed to get VTL(error: %lld) exiting...\n", ret);
+
+		/*
+		 * This is a dead end, something fundamental is broken.
+		 *
+		 * There is no sensible way of continuing as the Hyper-V drivers
+		 * transitively depend via the vmbus driver on knowing which VTL
+		 * they run in to establish communication with the host. The kernel
+		 * is going to be worse off if continued booting than a panicked one,
+		 * just hung and stuck, producing second-order failures, with neither
+		 * a way to recover nor to provide expected services.
+		 */
+		BUG();
+	}
+
+	local_irq_restore(flags);
+	return ret;
+}
+#endif
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index 87e3d49a4e29..682bcda3124f 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -75,6 +75,13 @@
 /* AccessTscInvariantControls privilege */
 #define HV_ACCESS_TSC_INVARIANT			BIT(15)
 
+/*
+ * This synthetic register is only accessible via the HVCALL_GET_VP_REGISTERS
+ * hvcall, and there is no an associated MSR on x86.
+ */
+#define	HV_REGISTER_VSM_VP_STATUS	0x000D0003
+#define	HV_VTL_MASK			GENMASK(3, 0)
+
 /*
  * Group B features.
  */
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 99935779682d..ea434186d765 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -301,4 +301,10 @@ static inline enum hv_isolation_type hv_get_isolation_type(void)
 }
 #endif /* CONFIG_HYPERV */
 
+#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
+u8 __init get_vtl(void);
+#else
+static inline u8 get_vtl(void) { return 0; }
+#endif
+
 #endif
-- 
2.45.0

[PATCH 5/6] drivers/hv/vmbus: Get the irq number from DeviceTree

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

When booting on arm64, one has to configure the irq
using the data from the system configuration. There
has already been support for ACPI, support DeviceTree
as booting in a Virtual Trust Level relies on DT.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 drivers/hv/vmbus_drv.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e25223cee3ab..52f01bd1c947 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -36,6 +36,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/dma-map-ops.h>
 #include <linux/pci.h>
+#include <linux/of_irq.h>
 #include <clocksource/hyperv_timer.h>
 #include <asm/mshyperv.h>
 #include "hyperv_vmbus.h"
@@ -2316,6 +2317,34 @@ static int vmbus_acpi_add(struct platform_device *pdev)
 }
 #endif
 
+static int __maybe_unused vmbus_of_set_irq(struct device_node *np)
+{
+	struct irq_desc *desc;
+	int irq;
+
+	irq = of_irq_get(np, 0);
+	if (irq == 0) {
+		pr_err("VMBus interrupt mapping failure\n");
+		return -EINVAL;
+	}
+	if (irq < 0) {
+		pr_err("VMBus interrupt data can't be read from DeviceTree, error %d\n", irq);
+		return irq;
+	}
+
+	desc = irq_to_desc(irq);
+	if (!desc) {
+		pr_err("VMBus interrupt description can't be found for virq %d\n", irq);
+		return -ENODEV;
+	}
+
+	vmbus_irq = irq;
+	vmbus_interrupt = desc->irq_data.hwirq;
+	pr_debug("VMBus virq %d, hwirq %d\n", vmbus_irq, vmbus_interrupt);
+
+	return 0;
+}
+
 static int vmbus_device_add(struct platform_device *pdev)
 {
 	struct resource **cur_res = &hyperv_mmio;
@@ -2324,12 +2353,20 @@ static int vmbus_device_add(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
+	pr_debug("VMBus is present in DeviceTree\n");
+
 	hv_dev = &pdev->dev;
 
 	ret = of_range_parser_init(&parser, np);
 	if (ret)
 		return ret;
 
+#ifndef HYPERVISOR_CALLBACK_VECTOR
+	ret = vmbus_of_set_irq(np);
+	if (ret)
+		return ret;
+#endif
+
 	for_each_of_range(&parser, &range) {
 		struct resource *res;
 
-- 
2.45.0

[PATCH 6/6] drivers/pci/hyperv/arm64: vPCI MSI IRQ domain from DT

[romank]

From: Roman Kisel <romank@linux.microsoft.com>

This change allows Hyper-V PCI to be enabled on arm64
via DT when booting in a Virtual Trust Level.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 13 ++++++++++---
 include/linux/acpi.h                | 10 ++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 1eaffff40b8d..ccc2b54206f4 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -906,9 +906,16 @@ static int hv_pci_irqchip_init(void)
 	 * way to ensure that all the corresponding devices are also gone and
 	 * no interrupts will be generated.
 	 */
-	hv_msi_gic_irq_domain = acpi_irq_create_hierarchy(0, HV_PCI_MSI_SPI_NR,
-							  fn, &hv_pci_domain_ops,
-							  chip_data);
+	if (acpi_disabled)
+		hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
+			irq_find_matching_fwnode(fn, DOMAIN_BUS_ANY),
+			0, HV_PCI_MSI_SPI_NR,
+			fn, &hv_pci_domain_ops,
+			chip_data);
+	else
+		hv_msi_gic_irq_domain = acpi_irq_create_hierarchy(0, HV_PCI_MSI_SPI_NR,
+			fn, &hv_pci_domain_ops,
+			chip_data);
 
 	if (!hv_msi_gic_irq_domain) {
 		pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ domain\n");
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index b7165e52b3c6..eb93d355bb6d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1077,6 +1077,16 @@ static inline bool acpi_sleep_state_supported(u8 sleep_state)
 	return false;
 }
 
+
+static inline struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
+					     unsigned int size,
+					     struct fwnode_handle *fwnode,
+					     const struct irq_domain_ops *ops,
+					     void *host_data)
+{
+	return NULL;
+}
+
 #endif	/* !CONFIG_ACPI */
 
 extern void arch_post_acpi_subsys_init(void);
-- 
2.45.0

Re: [PATCH 1/6] arm64/hyperv: Support DeviceTree

[Easwar Hariharan]

On 5/10/2024 9:05 AM, romank@linux.microsoft.com wrote:
> From: Roman Kisel <romank@linux.microsoft.com>
> 
> Update the driver to support DeviceTree boot as well along with ACPI.
> This enables the Virtual Trust Level platforms boot up on ARM64.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index b1a4de4eee29..208a3bcb9686 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -15,6 +15,9 @@
>  #include <linux/errno.h>
>  #include <linux/version.h>
>  #include <linux/cpuhotplug.h>
> +#include <linux/libfdt.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
>  #include <asm/mshyperv.h>
>  
>  static bool hyperv_initialized;
> @@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>  	return 0;
>  }
>  
> +static bool hyperv_detect_fdt(void)
> +{
> +#ifdef CONFIG_OF
> +	const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
> +			of_get_flat_dt_root(), "hypervisor");
> +
> +	return (hyp_node != -FDT_ERR_NOTFOUND) &&
> +			of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
> +#else
> +	return false;
> +#endif
> +}
> +
> +static bool hyperv_detect_acpi(void)
> +{
> +#ifdef CONFIG_ACPI
> +	return !acpi_disabled &&
> +			!strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
> +#else
> +	return false;
> +#endif
> +}
> +

Could using IS_ENABLED() allow collapsing these two functions into one hyperv_detect_fw()?
I am wondering if #ifdef was explicitly chosen to allow for the code to be compiled in if CONFIG* is defined
v/s IS_ENABLED() only being true if the CONFIG value is true.


>  static int __init hyperv_init(void)
>  {
>  	struct hv_get_vp_registers_output	result;
> @@ -35,13 +61,11 @@ static int __init hyperv_init(void)
>  
>  	/*
>  	 * Allow for a kernel built with CONFIG_HYPERV to be running in
> -	 * a non-Hyper-V environment, including on DT instead of ACPI.
> +	 * a non-Hyper-V environment.
> +	 *
>  	 * In such cases, do nothing and return success.
>  	 */
> -	if (acpi_disabled)
> -		return 0;
> -
> -	if (strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8))
> +	if (!hyperv_detect_fdt() && !hyperv_detect_acpi())
>  		return 0;
>  
>  	/* Setup the guest ID */

Re: [PATCH 1/6] arm64/hyperv: Support DeviceTree

[Roman Kisel]

On 5/10/2024 10:04 AM, Easwar Hariharan wrote:
> On 5/10/2024 9:05 AM, romank@linux.microsoft.com wrote:
>> From: Roman Kisel <romank@linux.microsoft.com>
>>
>> Update the driver to support DeviceTree boot as well along with ACPI.
>> This enables the Virtual Trust Level platforms boot up on ARM64.
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> ---
>>   arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
>>   1 file changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
>> index b1a4de4eee29..208a3bcb9686 100644
>> --- a/arch/arm64/hyperv/mshyperv.c
>> +++ b/arch/arm64/hyperv/mshyperv.c
>> @@ -15,6 +15,9 @@
>>   #include <linux/errno.h>
>>   #include <linux/version.h>
>>   #include <linux/cpuhotplug.h>
>> +#include <linux/libfdt.h>
>> +#include <linux/of.h>
>> +#include <linux/of_fdt.h>
>>   #include <asm/mshyperv.h>
>>   
>>   static bool hyperv_initialized;
>> @@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>>   	return 0;
>>   }
>>   
>> +static bool hyperv_detect_fdt(void)
>> +{
>> +#ifdef CONFIG_OF
>> +	const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>> +			of_get_flat_dt_root(), "hypervisor");
>> +
>> +	return (hyp_node != -FDT_ERR_NOTFOUND) &&
>> +			of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
>> +#else
>> +	return false;
>> +#endif
>> +}
>> +
>> +static bool hyperv_detect_acpi(void)
>> +{
>> +#ifdef CONFIG_ACPI
>> +	return !acpi_disabled &&
>> +			!strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
>> +#else
>> +	return false;
>> +#endif
>> +}
>> +
> 
> Could using IS_ENABLED() allow collapsing these two functions into one hyperv_detect_fw()?
> I am wondering if #ifdef was explicitly chosen to allow for the code to be compiled in if CONFIG* is defined
> v/s IS_ENABLED() only being true if the CONFIG value is true.
> 
In the hyperv_detect_fdt function, the #ifdef has been chosen due to the 
functions being declared only when the macro is defined, hence I could 
not rely on `if IS_ENABLED()` and the run-time detection. For the 
compile-time option, `#if IS_ENABLED()` would convey the intent better, 
could update with that.

In the hyperv_detect_acpi function, using of the #ifdef appears to be 
not needed, will remove that in the next version of the patch set. 
Appreciate your help!

As for combining the functions into one, the result looks less readable 
due to more if statements and is mixing the concerns to an extent. That 
said, unless you feel strongly about having just one function here, 
perhaps could keep the both functions.

> 
>>   static int __init hyperv_init(void)
>>   {
>>   	struct hv_get_vp_registers_output	result;
>> @@ -35,13 +61,11 @@ static int __init hyperv_init(void)
>>   
>>   	/*
>>   	 * Allow for a kernel built with CONFIG_HYPERV to be running in
>> -	 * a non-Hyper-V environment, including on DT instead of ACPI.
>> +	 * a non-Hyper-V environment.
>> +	 *
>>   	 * In such cases, do nothing and return success.
>>   	 */
>> -	if (acpi_disabled)
>> -		return 0;
>> -
>> -	if (strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8))
>> +	if (!hyperv_detect_fdt() && !hyperv_detect_acpi())
>>   		return 0;
>>   
>>   	/* Setup the guest ID */
> 

-- 
Thank you,
Roman

Re: [PATCH 2/6] drivers/hv: Enable VTL mode for arm64

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on arnd-asm-generic/master linus/master v6.9-rc7 next-20240510]
[cannot apply to tip/x86/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/romank-linux-microsoft-com/arm64-hyperv-Support-DeviceTree/20240511-000917
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20240510160602.1311352-3-romank%40linux.microsoft.com
patch subject: [PATCH 2/6] drivers/hv: Enable VTL mode for arm64
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240512/202405121034.fVYQIs8h-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240512/202405121034.fVYQIs8h-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405121034.fVYQIs8h-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   arch/x86/hyperv/hv_vtl.c: In function 'hv_vtl_system_desc_base':
>> arch/x86/hyperv/hv_vtl.c:54:28: error: 'struct ldttss_desc' has no member named 'base3'; did you mean 'base0'?
      54 |         return ((u64)desc->base3 << 32) | ((u64)desc->base2 << 24) |
         |                            ^~~~~
         |                            base0
   arch/x86/hyperv/hv_vtl.c: In function 'hv_vtl_ap_entry':
>> arch/x86/hyperv/hv_vtl.c:66:35: error: 'secondary_startup_64' undeclared (first use in this function); did you mean 'secondary_startup_64_fn'?
      66 |         ((secondary_startup_64_fn)secondary_startup_64)(&boot_params, &boot_params);
         |                                   ^~~~~~~~~~~~~~~~~~~~
         |                                   secondary_startup_64_fn
   arch/x86/hyperv/hv_vtl.c:66:35: note: each undeclared identifier is reported only once for each function it appears in
   arch/x86/hyperv/hv_vtl.c: In function 'hv_vtl_bringup_vcpu':
>> arch/x86/hyperv/hv_vtl.c:86:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      86 |         u64 rip = (u64)&hv_vtl_ap_entry;
         |                   ^
   arch/x86/hyperv/hv_vtl.c: In function 'hv_vtl_system_desc_base':
>> arch/x86/hyperv/hv_vtl.c:56:1: warning: control reaches end of non-void function [-Wreturn-type]
      56 | }
         | ^


vim +54 arch/x86/hyperv/hv_vtl.c

3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   51  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   52  static inline u64 hv_vtl_system_desc_base(struct ldttss_desc *desc)
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   53  {
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  @54  	return ((u64)desc->base3 << 32) | ((u64)desc->base2 << 24) |
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   55  		(desc->base1 << 16) | desc->base0;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  @56  }
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   57  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   58  static inline u32 hv_vtl_system_desc_limit(struct ldttss_desc *desc)
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   59  {
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   60  	return ((u32)desc->limit1 << 16) | (u32)desc->limit0;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   61  }
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   62  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   63  typedef void (*secondary_startup_64_fn)(void*, void*);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   64  static void hv_vtl_ap_entry(void)
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   65  {
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  @66  	((secondary_startup_64_fn)secondary_startup_64)(&boot_params, &boot_params);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   67  }
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   68  
2b4b90e053a290 Saurabh Sengar 2024-03-03   69  static int hv_vtl_bringup_vcpu(u32 target_vp_index, int cpu, u64 eip_ignored)
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   70  {
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   71  	u64 status;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   72  	int ret = 0;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   73  	struct hv_enable_vp_vtl *input;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   74  	unsigned long irq_flags;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   75  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   76  	struct desc_ptr gdt_ptr;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   77  	struct desc_ptr idt_ptr;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   78  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   79  	struct ldttss_desc *tss;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   80  	struct ldttss_desc *ldt;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   81  	struct desc_struct *gdt;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   82  
2b4b90e053a290 Saurabh Sengar 2024-03-03   83  	struct task_struct *idle = idle_thread_get(cpu);
2b4b90e053a290 Saurabh Sengar 2024-03-03   84  	u64 rsp = (unsigned long)idle->thread.sp;
2b4b90e053a290 Saurabh Sengar 2024-03-03   85  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  @86  	u64 rip = (u64)&hv_vtl_ap_entry;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   87  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   88  	native_store_gdt(&gdt_ptr);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   89  	store_idt(&idt_ptr);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   90  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   91  	gdt = (struct desc_struct *)((void *)(gdt_ptr.address));
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   92  	tss = (struct ldttss_desc *)(gdt + GDT_ENTRY_TSS);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   93  	ldt = (struct ldttss_desc *)(gdt + GDT_ENTRY_LDT);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   94  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   95  	local_irq_save(irq_flags);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   96  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   97  	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   98  	memset(input, 0, sizeof(*input));
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10   99  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  100  	input->partition_id = HV_PARTITION_ID_SELF;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  101  	input->vp_index = target_vp_index;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  102  	input->target_vtl.target_vtl = HV_VTL_MGMT;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  103  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  104  	/*
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  105  	 * The x86_64 Linux kernel follows the 16-bit -> 32-bit -> 64-bit
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  106  	 * mode transition sequence after waking up an AP with SIPI whose
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  107  	 * vector points to the 16-bit AP startup trampoline code. Here in
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  108  	 * VTL2, we can't perform that sequence as the AP has to start in
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  109  	 * the 64-bit mode.
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  110  	 *
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  111  	 * To make this happen, we tell the hypervisor to load a valid 64-bit
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  112  	 * context (most of which is just magic numbers from the CPU manual)
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  113  	 * so that AP jumps right to the 64-bit entry of the kernel, and the
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  114  	 * control registers are loaded with values that let the AP fetch the
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  115  	 * code and data and carry on with work it gets assigned.
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  116  	 */
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  117  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  118  	input->vp_context.rip = rip;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  119  	input->vp_context.rsp = rsp;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  120  	input->vp_context.rflags = 0x0000000000000002;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  121  	input->vp_context.efer = __rdmsr(MSR_EFER);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  122  	input->vp_context.cr0 = native_read_cr0();
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  123  	input->vp_context.cr3 = __native_read_cr3();
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  124  	input->vp_context.cr4 = native_read_cr4();
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  125  	input->vp_context.msr_cr_pat = __rdmsr(MSR_IA32_CR_PAT);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  126  	input->vp_context.idtr.limit = idt_ptr.size;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  127  	input->vp_context.idtr.base = idt_ptr.address;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  128  	input->vp_context.gdtr.limit = gdt_ptr.size;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  129  	input->vp_context.gdtr.base = gdt_ptr.address;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  130  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  131  	/* Non-system desc (64bit), long, code, present */
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  132  	input->vp_context.cs.selector = __KERNEL_CS;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  133  	input->vp_context.cs.base = 0;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  134  	input->vp_context.cs.limit = 0xffffffff;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  135  	input->vp_context.cs.attributes = 0xa09b;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  136  	/* Non-system desc (64bit), data, present, granularity, default */
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  137  	input->vp_context.ss.selector = __KERNEL_DS;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  138  	input->vp_context.ss.base = 0;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  139  	input->vp_context.ss.limit = 0xffffffff;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  140  	input->vp_context.ss.attributes = 0xc093;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  141  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  142  	/* System desc (128bit), present, LDT */
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  143  	input->vp_context.ldtr.selector = GDT_ENTRY_LDT * 8;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  144  	input->vp_context.ldtr.base = hv_vtl_system_desc_base(ldt);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  145  	input->vp_context.ldtr.limit = hv_vtl_system_desc_limit(ldt);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  146  	input->vp_context.ldtr.attributes = 0x82;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  147  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  148  	/* System desc (128bit), present, TSS, 0x8b - busy, 0x89 -- default */
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  149  	input->vp_context.tr.selector = GDT_ENTRY_TSS * 8;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  150  	input->vp_context.tr.base = hv_vtl_system_desc_base(tss);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  151  	input->vp_context.tr.limit = hv_vtl_system_desc_limit(tss);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  152  	input->vp_context.tr.attributes = 0x8b;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  153  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  154  	status = hv_do_hypercall(HVCALL_ENABLE_VP_VTL, input, NULL);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  155  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  156  	if (!hv_result_success(status) &&
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  157  	    hv_result(status) != HV_STATUS_VTL_ALREADY_ENABLED) {
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  158  		pr_err("HVCALL_ENABLE_VP_VTL failed for VP : %d ! [Err: %#llx\n]",
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  159  		       target_vp_index, status);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  160  		ret = -EINVAL;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  161  		goto free_lock;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  162  	}
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  163  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  164  	status = hv_do_hypercall(HVCALL_START_VP, input, NULL);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  165  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  166  	if (!hv_result_success(status)) {
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  167  		pr_err("HVCALL_START_VP failed for VP : %d ! [Err: %#llx]\n",
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  168  		       target_vp_index, status);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  169  		ret = -EINVAL;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  170  	}
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  171  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  172  free_lock:
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  173  	local_irq_restore(irq_flags);
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  174  
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  175  	return ret;
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  176  }
3be1bc2fe9d2e4 Saurabh Sengar 2023-04-10  177  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/6] arm64/hyperv: Support DeviceTree

[Easwar Hariharan]

On 5/10/2024 10:42 AM, Roman Kisel wrote:
> 
> 
> On 5/10/2024 10:04 AM, Easwar Hariharan wrote:
>> On 5/10/2024 9:05 AM, romank@linux.microsoft.com wrote:
>>> From: Roman Kisel <romank@linux.microsoft.com>
>>>
>>> Update the driver to support DeviceTree boot as well along with ACPI.
>>> This enables the Virtual Trust Level platforms boot up on ARM64.
>>>
>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>> ---
>>>   arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
>>>   1 file changed, 29 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
>>> index b1a4de4eee29..208a3bcb9686 100644
>>> --- a/arch/arm64/hyperv/mshyperv.c
>>> +++ b/arch/arm64/hyperv/mshyperv.c
>>> @@ -15,6 +15,9 @@
>>>   #include <linux/errno.h>
>>>   #include <linux/version.h>
>>>   #include <linux/cpuhotplug.h>
>>> +#include <linux/libfdt.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_fdt.h>
>>>   #include <asm/mshyperv.h>
>>>     static bool hyperv_initialized;
>>> @@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>>>       return 0;
>>>   }
>>>   +static bool hyperv_detect_fdt(void)
>>> +{
>>> +#ifdef CONFIG_OF
>>> +    const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>>> +            of_get_flat_dt_root(), "hypervisor");
>>> +
>>> +    return (hyp_node != -FDT_ERR_NOTFOUND) &&
>>> +            of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
>>> +#else
>>> +    return false;
>>> +#endif
>>> +}
>>> +
>>> +static bool hyperv_detect_acpi(void)
>>> +{
>>> +#ifdef CONFIG_ACPI
>>> +    return !acpi_disabled &&
>>> +            !strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
>>> +#else
>>> +    return false;
>>> +#endif
>>> +}
>>> +
>>
>> Could using IS_ENABLED() allow collapsing these two functions into one hyperv_detect_fw()?
>> I am wondering if #ifdef was explicitly chosen to allow for the code to be compiled in if CONFIG* is defined
>> v/s IS_ENABLED() only being true if the CONFIG value is true.
>>
> In the hyperv_detect_fdt function, the #ifdef has been chosen due to the functions being declared only when the macro is defined, hence I could not rely on `if IS_ENABLED()` and the run-time detection. For the compile-time option, `#if IS_ENABLED()` would convey the intent better, could update with that.

In patch 2/6, just under the diff you have, we already `select OF_EARLY_FLATTREE if OF`, so the declarations (and definitions)
of the functions being present is already handled, AFAIK. Are we thinking there may be some weird config where neither OF nor
ACPI is selected? If so, we should set a `depends on ACPI || OF` for config HYPERV to prevent that.

I don't know if I'm missing something obvious here, so please correct me if I'm wrong.

> 
> In the hyperv_detect_acpi function, using of the #ifdef appears to be not needed, will remove that in the next version of the patch set. Appreciate your help!
> 
> As for combining the functions into one, the result looks less readable due to more if statements and is mixing the concerns to an extent. That said, unless you feel strongly about having just one function here, perhaps could keep the both functions.

Agreed on the readability, let's keep both functions.

Thanks,
Easwar

Re: [PATCH 1/6] arm64/hyperv: Support DeviceTree

[Roman Kisel]

On 5/14/2024 3:46 PM, Easwar Hariharan wrote:
> On 5/10/2024 10:42 AM, Roman Kisel wrote:
>>
>>
>> On 5/10/2024 10:04 AM, Easwar Hariharan wrote:
>>> On 5/10/2024 9:05 AM, romank@linux.microsoft.com wrote:
>>>> From: Roman Kisel <romank@linux.microsoft.com>
>>>>
>>>> Update the driver to support DeviceTree boot as well along with ACPI.
>>>> This enables the Virtual Trust Level platforms boot up on ARM64.
>>>>
>>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>>> ---
>>>>    arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
>>>>    1 file changed, 29 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
>>>> index b1a4de4eee29..208a3bcb9686 100644
>>>> --- a/arch/arm64/hyperv/mshyperv.c
>>>> +++ b/arch/arm64/hyperv/mshyperv.c
>>>> @@ -15,6 +15,9 @@
>>>>    #include <linux/errno.h>
>>>>    #include <linux/version.h>
>>>>    #include <linux/cpuhotplug.h>
>>>> +#include <linux/libfdt.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_fdt.h>
>>>>    #include <asm/mshyperv.h>
>>>>      static bool hyperv_initialized;
>>>> @@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>>>>        return 0;
>>>>    }
>>>>    +static bool hyperv_detect_fdt(void)
>>>> +{
>>>> +#ifdef CONFIG_OF
>>>> +    const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>>>> +            of_get_flat_dt_root(), "hypervisor");
>>>> +
>>>> +    return (hyp_node != -FDT_ERR_NOTFOUND) &&
>>>> +            of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
>>>> +#else
>>>> +    return false;
>>>> +#endif
>>>> +}
>>>> +
>>>> +static bool hyperv_detect_acpi(void)
>>>> +{
>>>> +#ifdef CONFIG_ACPI
>>>> +    return !acpi_disabled &&
>>>> +            !strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
>>>> +#else
>>>> +    return false;
>>>> +#endif
>>>> +}
>>>> +
>>>
>>> Could using IS_ENABLED() allow collapsing these two functions into one hyperv_detect_fw()?
>>> I am wondering if #ifdef was explicitly chosen to allow for the code to be compiled in if CONFIG* is defined
>>> v/s IS_ENABLED() only being true if the CONFIG value is true.
>>>
>> In the hyperv_detect_fdt function, the #ifdef has been chosen due to the functions being declared only when the macro is defined, hence I could not rely on `if IS_ENABLED()` and the run-time detection. For the compile-time option, `#if IS_ENABLED()` would convey the intent better, could update with that.
> 
> In patch 2/6, just under the diff you have, we already `select OF_EARLY_FLATTREE if OF`, so the declarations (and definitions)
> of the functions being present is already handled, AFAIK. Are we thinking there may be some weird config where neither OF nor
> ACPI is selected? If so, we should set a `depends on ACPI || OF` for config HYPERV to prevent that.
> 
> I don't know if I'm missing something obvious here, so please correct me if I'm wrong.
> 
I just sent out the v2 of the patch set, and (un?)fortunately missed the 
change I had for the #ifdef's in this chunk (to use #if IS_ENABLED() and 
remove pre-processor directives from the ACPI-related function).

I am making the point that the change you are suggesting (as I 
understand) is this conditional statement

if IS_ENABLED(CONFIG_OF) {
     const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
				of_get_flat_dt_root(), "hypervisor");

     return (hyp_node != -FDT_ERR_NOTFOUND) &&
				of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
}

and for it to link successfully, one needs 
of_get_flat_dt_subnode_by_name defined. From the source code, that needs 
CONFIG_OF_EARLY_FLATTREE as there is no stub available when 
CONFIG_OF_EARLY_FLATTREE is not defined. I'll check on successful 
linking with the config without CONFIG_OF and the above snippet. Do feel 
free to provide the code you have in mind.

>>
>> In the hyperv_detect_acpi function, using of the #ifdef appears to be not needed, will remove that in the next version of the patch set. Appreciate your help!
>>
>> As for combining the functions into one, the result looks less readable due to more if statements and is mixing the concerns to an extent. That said, unless you feel strongly about having just one function here, perhaps could keep the both functions.
> 
> Agreed on the readability, let's keep both functions.
> 
> Thanks,
> Easwar

-- 
Thank you,
Roman

Re: [PATCH 1/6] arm64/hyperv: Support DeviceTree

[Easwar Hariharan]

On 5/14/2024 4:17 PM, Roman Kisel wrote:
> 
> 
> On 5/14/2024 3:46 PM, Easwar Hariharan wrote:
>> On 5/10/2024 10:42 AM, Roman Kisel wrote:
>>>
>>>
>>> On 5/10/2024 10:04 AM, Easwar Hariharan wrote:
>>>> On 5/10/2024 9:05 AM, romank@linux.microsoft.com wrote:
>>>>> From: Roman Kisel <romank@linux.microsoft.com>
>>>>>
>>>>> Update the driver to support DeviceTree boot as well along with ACPI.
>>>>> This enables the Virtual Trust Level platforms boot up on ARM64.
>>>>>
>>>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>>>> ---
>>>>>    arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
>>>>>    1 file changed, 29 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
>>>>> index b1a4de4eee29..208a3bcb9686 100644
>>>>> --- a/arch/arm64/hyperv/mshyperv.c
>>>>> +++ b/arch/arm64/hyperv/mshyperv.c
>>>>> @@ -15,6 +15,9 @@
>>>>>    #include <linux/errno.h>
>>>>>    #include <linux/version.h>
>>>>>    #include <linux/cpuhotplug.h>
>>>>> +#include <linux/libfdt.h>
>>>>> +#include <linux/of.h>
>>>>> +#include <linux/of_fdt.h>
>>>>>    #include <asm/mshyperv.h>
>>>>>      static bool hyperv_initialized;
>>>>> @@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>>>>>        return 0;
>>>>>    }
>>>>>    +static bool hyperv_detect_fdt(void)
>>>>> +{
>>>>> +#ifdef CONFIG_OF
>>>>> +    const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>>>>> +            of_get_flat_dt_root(), "hypervisor");
>>>>> +
>>>>> +    return (hyp_node != -FDT_ERR_NOTFOUND) &&
>>>>> +            of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
>>>>> +#else
>>>>> +    return false;
>>>>> +#endif
>>>>> +}
>>>>> +
>>>>> +static bool hyperv_detect_acpi(void)
>>>>> +{
>>>>> +#ifdef CONFIG_ACPI
>>>>> +    return !acpi_disabled &&
>>>>> +            !strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
>>>>> +#else
>>>>> +    return false;
>>>>> +#endif
>>>>> +}
>>>>> +
>>>>
>>>> Could using IS_ENABLED() allow collapsing these two functions into one hyperv_detect_fw()?
>>>> I am wondering if #ifdef was explicitly chosen to allow for the code to be compiled in if CONFIG* is defined
>>>> v/s IS_ENABLED() only being true if the CONFIG value is true.
>>>>
>>> In the hyperv_detect_fdt function, the #ifdef has been chosen due to the functions being declared only when the macro is defined, hence I could not rely on `if IS_ENABLED()` and the run-time detection. For the compile-time option, `#if IS_ENABLED()` would convey the intent better, could update with that.
>>
>> In patch 2/6, just under the diff you have, we already `select OF_EARLY_FLATTREE if OF`, so the declarations (and definitions)
>> of the functions being present is already handled, AFAIK. Are we thinking there may be some weird config where neither OF nor
>> ACPI is selected? If so, we should set a `depends on ACPI || OF` for config HYPERV to prevent that.
>>
>> I don't know if I'm missing something obvious here, so please correct me if I'm wrong.
>>
> I just sent out the v2 of the patch set, and (un?)fortunately missed the change I had for the #ifdef's in this chunk (to use #if IS_ENABLED() and remove pre-processor directives from the ACPI-related function).
> 
> I am making the point that the change you are suggesting (as I understand) is this conditional statement
> 
> if IS_ENABLED(CONFIG_OF) {
>     const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>                 of_get_flat_dt_root(), "hypervisor");
> 
>     return (hyp_node != -FDT_ERR_NOTFOUND) &&
>                 of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
> }
> 

That's right, that's the suggestion I'm making.

> and for it to link successfully, one needs of_get_flat_dt_subnode_by_name defined. From the source code, that needs CONFIG_OF_EARLY_FLATTREE as there is no stub available when CONFIG_OF_EARLY_FLATTREE is not defined.

We agree that you need of_get_flat_dt_subnode_by_name declared and defined, and it's not available, stub or otherwise, if CONFIG_OF_EARLY_FLATTREE is not defined.

In my mind, I believed that either ACPI or OF had to be selected for some sort of firmware handoff to occur, but I did some experimentation and ended up with an
x86_64 kernel config that has neither enabled, but nonetheless has CONFIG_HYPERV enabled. The kernel builds with such a config, whether it's useful for anything
is another question. ARM64 requires CONFIG_OF so that side of the equation is clear.

That's where my question above came in: Are we thinking there may be some weird config where neither OF nor ACPI is selected? I thought that was not a reasonable
config, thus my prescription to set `depends on ACPI || OF` for config HYPERV. If there is a use case for an x86_64 kernel that has neither ACPI nor OpenFirmware/FDT,
but nevertheless sets CONFIG_HYPERV, feel free to ignore this comment thread entirely.


Thanks,
Easwar

Re: [PATCH 2/6] drivers/hv: Enable VTL mode for arm64

[Wei Liu]

On Fri, May 10, 2024 at 09:05:01AM -0700, romank@linux.microsoft.com wrote:
> From: Roman Kisel <romank@linux.microsoft.com>
> 
> This change removes dependency on ACPI when buidling the hv drivers to
> allow Virtual Trust Level boot with DeviceTree.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  drivers/hv/Kconfig | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 862c47b191af..a5cd1365e248 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -5,7 +5,7 @@ menu "Microsoft Hyper-V guest support"
>  config HYPERV
>  	tristate "Microsoft Hyper-V client drivers"
>  	depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
> -		|| (ACPI && ARM64 && !CPU_BIG_ENDIAN)
> +		|| (ARM64 && !CPU_BIG_ENDIAN)
>  	select PARAVIRT
>  	select X86_HV_CALLBACK_VECTOR if X86
>  	select OF_EARLY_FLATTREE if OF
> @@ -15,7 +15,7 @@ config HYPERV
>  
>  config HYPERV_VTL_MODE
>  	bool "Enable Linux to boot in VTL context"
> -	depends on X86_64 && HYPERV
> +	depends on HYPERV

As Ktest bot pointed out, this change broke the build.

Thanks,
Wei.

>  	depends on SMP
>  	default n
>  	help
> @@ -31,7 +31,7 @@ config HYPERV_VTL_MODE
>  
>  	  Select this option to build a Linux kernel to run at a VTL other than
>  	  the normal VTL0, which currently is only VTL2.  This option
> -	  initializes the x86 platform for VTL2, and adds the ability to boot
> +	  initializes the kernel to run in VTL2, and adds the ability to boot
>  	  secondary CPUs directly into 64-bit context as required for VTLs other
>  	  than 0.  A kernel built with this option must run at VTL2, and will
>  	  not run as a normal guest.
> -- 
> 2.45.0
> 

Re: [PATCH 2/6] drivers/hv: Enable VTL mode for arm64

[Roman Kisel]

On 5/15/2024 12:43 AM, Wei Liu wrote:
> On Fri, May 10, 2024 at 09:05:01AM -0700, romank@linux.microsoft.com wrote:
>> From: Roman Kisel <romank@linux.microsoft.com>
>>
>> This change removes dependency on ACPI when buidling the hv drivers to
>> allow Virtual Trust Level boot with DeviceTree.
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> ---
>>   drivers/hv/Kconfig | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>> index 862c47b191af..a5cd1365e248 100644
>> --- a/drivers/hv/Kconfig
>> +++ b/drivers/hv/Kconfig
>> @@ -5,7 +5,7 @@ menu "Microsoft Hyper-V guest support"
>>   config HYPERV
>>   	tristate "Microsoft Hyper-V client drivers"
>>   	depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
>> -		|| (ACPI && ARM64 && !CPU_BIG_ENDIAN)
>> +		|| (ARM64 && !CPU_BIG_ENDIAN)
>>   	select PARAVIRT
>>   	select X86_HV_CALLBACK_VECTOR if X86
>>   	select OF_EARLY_FLATTREE if OF
>> @@ -15,7 +15,7 @@ config HYPERV
>>   
>>   config HYPERV_VTL_MODE
>>   	bool "Enable Linux to boot in VTL context"
>> -	depends on X86_64 && HYPERV
>> +	depends on HYPERV
> 
> As Ktest bot pointed out, this change broke the build.
> 
Appreciate your help! I've sent out v2 with the base commit set, and so 
far, robots have been fine with it :) I haven't noticed an alert of a 
broken build due to this change.

> Thanks,
> Wei.
> 
>>   	depends on SMP
>>   	default n
>>   	help
>> @@ -31,7 +31,7 @@ config HYPERV_VTL_MODE
>>   
>>   	  Select this option to build a Linux kernel to run at a VTL other than
>>   	  the normal VTL0, which currently is only VTL2.  This option
>> -	  initializes the x86 platform for VTL2, and adds the ability to boot
>> +	  initializes the kernel to run in VTL2, and adds the ability to boot
>>   	  secondary CPUs directly into 64-bit context as required for VTLs other
>>   	  than 0.  A kernel built with this option must run at VTL2, and will
>>   	  not run as a normal guest.
>> -- 
>> 2.45.0
>>

-- 
Thank you,
Roman

Re: [PATCH 1/6] arm64/hyperv: Support DeviceTree

[Roman Kisel]

On 5/14/2024 5:00 PM, Easwar Hariharan wrote:
> On 5/14/2024 4:17 PM, Roman Kisel wrote:
>>
>>
>> On 5/14/2024 3:46 PM, Easwar Hariharan wrote:
>>> On 5/10/2024 10:42 AM, Roman Kisel wrote:
>>>>
>>>>
>>>> On 5/10/2024 10:04 AM, Easwar Hariharan wrote:
>>>>> On 5/10/2024 9:05 AM, romank@linux.microsoft.com wrote:
>>>>>> From: Roman Kisel <romank@linux.microsoft.com>
>>>>>>
>>>>>> Update the driver to support DeviceTree boot as well along with ACPI.
>>>>>> This enables the Virtual Trust Level platforms boot up on ARM64.
>>>>>>
>>>>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>>>>> ---
>>>>>>     arch/arm64/hyperv/mshyperv.c | 34 +++++++++++++++++++++++++++++-----
>>>>>>     1 file changed, 29 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
>>>>>> index b1a4de4eee29..208a3bcb9686 100644
>>>>>> --- a/arch/arm64/hyperv/mshyperv.c
>>>>>> +++ b/arch/arm64/hyperv/mshyperv.c
>>>>>> @@ -15,6 +15,9 @@
>>>>>>     #include <linux/errno.h>
>>>>>>     #include <linux/version.h>
>>>>>>     #include <linux/cpuhotplug.h>
>>>>>> +#include <linux/libfdt.h>
>>>>>> +#include <linux/of.h>
>>>>>> +#include <linux/of_fdt.h>
>>>>>>     #include <asm/mshyperv.h>
>>>>>>       static bool hyperv_initialized;
>>>>>> @@ -27,6 +30,29 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>>>>>>         return 0;
>>>>>>     }
>>>>>>     +static bool hyperv_detect_fdt(void)
>>>>>> +{
>>>>>> +#ifdef CONFIG_OF
>>>>>> +    const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>>>>>> +            of_get_flat_dt_root(), "hypervisor");
>>>>>> +
>>>>>> +    return (hyp_node != -FDT_ERR_NOTFOUND) &&
>>>>>> +            of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
>>>>>> +#else
>>>>>> +    return false;
>>>>>> +#endif
>>>>>> +}
>>>>>> +
>>>>>> +static bool hyperv_detect_acpi(void)
>>>>>> +{
>>>>>> +#ifdef CONFIG_ACPI
>>>>>> +    return !acpi_disabled &&
>>>>>> +            !strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8);
>>>>>> +#else
>>>>>> +    return false;
>>>>>> +#endif
>>>>>> +}
>>>>>> +
>>>>>
>>>>> Could using IS_ENABLED() allow collapsing these two functions into one hyperv_detect_fw()?
>>>>> I am wondering if #ifdef was explicitly chosen to allow for the code to be compiled in if CONFIG* is defined
>>>>> v/s IS_ENABLED() only being true if the CONFIG value is true.
>>>>>
>>>> In the hyperv_detect_fdt function, the #ifdef has been chosen due to the functions being declared only when the macro is defined, hence I could not rely on `if IS_ENABLED()` and the run-time detection. For the compile-time option, `#if IS_ENABLED()` would convey the intent better, could update with that.
>>>
>>> In patch 2/6, just under the diff you have, we already `select OF_EARLY_FLATTREE if OF`, so the declarations (and definitions)
>>> of the functions being present is already handled, AFAIK. Are we thinking there may be some weird config where neither OF nor
>>> ACPI is selected? If so, we should set a `depends on ACPI || OF` for config HYPERV to prevent that.
>>>
>>> I don't know if I'm missing something obvious here, so please correct me if I'm wrong.
>>>
>> I just sent out the v2 of the patch set, and (un?)fortunately missed the change I had for the #ifdef's in this chunk (to use #if IS_ENABLED() and remove pre-processor directives from the ACPI-related function).
>>
>> I am making the point that the change you are suggesting (as I understand) is this conditional statement
>>
>> if IS_ENABLED(CONFIG_OF) {
>>      const unsigned long hyp_node = of_get_flat_dt_subnode_by_name(
>>                  of_get_flat_dt_root(), "hypervisor");
>>
>>      return (hyp_node != -FDT_ERR_NOTFOUND) &&
>>                  of_flat_dt_is_compatible(hyp_node, "microsoft,hyperv");
>> }
>>
> 
> That's right, that's the suggestion I'm making.
> 
>> and for it to link successfully, one needs of_get_flat_dt_subnode_by_name defined. From the source code, that needs CONFIG_OF_EARLY_FLATTREE as there is no stub available when CONFIG_OF_EARLY_FLATTREE is not defined.
> 
> We agree that you need of_get_flat_dt_subnode_by_name declared and defined, and it's not available, stub or otherwise, if CONFIG_OF_EARLY_FLATTREE is not defined.
> 
> In my mind, I believed that either ACPI or OF had to be selected for some sort of firmware handoff to occur, but I did some experimentation and ended up with an
> x86_64 kernel config that has neither enabled, but nonetheless has CONFIG_HYPERV enabled. The kernel builds with such a config, whether it's useful for anything
> is another question. ARM64 requires CONFIG_OF so that side of the equation is clear.
> 
> That's where my question above came in: Are we thinking there may be some weird config where neither OF nor ACPI is selected? I thought that was not a reasonable
> config, thus my prescription to set `depends on ACPI || OF` for config HYPERV. If there is a use case for an x86_64 kernel that has neither ACPI nor OpenFirmware/FDT,
> but nevertheless sets CONFIG_HYPERV, feel free to ignore this comment thread entirely.
> 
Thanks for laying out the details so patiently for me! I believe I 
understand your concerns better now. In the V2 of the patch series, 
Michael Kelly suggested rethinking how the Kconfig change is carried 
out. Appears natural to try out what your suggesting alongside with the 
Michael's suggestions.

As for your other point, I wouldn't think, too, the system could boot or 
be useful without some coordinates of the environment passed via ACPI || DT.

> 
> Thanks,
> Easwar

-- 
Thank you,
Roman