linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot
@ 2025-03-15  0:19 Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence Roman Kisel
                   ` (10 more replies)
  0 siblings, 11 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

This patch set allows 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.

The OpenHCL paravisor https://github.com/microsoft/openvmm/tree/main/openhcl
can serve as a practical application of these patches on ARM64.

For validation, I built kernels for the {x86_64, ARM64} x {VTL0, VTL2} set with
a small initrd embedded into the kernel and booted VMs managed by Hyper-V and
OpenVMM off of that.

Starting from V5, the patch series includes a non-functional change to KVM on
arm64 which I tested as well.

[V6]
    - Use more intuitive Kconfig update.
    - Remove ifdef for getting IRQ number
    ** Thank you, Arnd! **

    - Simplify code for finding the parent IRQ domain.
    ** Thank you, Bjorn! **

    - Remove a superfluous check.
    ** Thank you, Dan! **

    - Make the commit title and descrtiption legible.
    - Don't set additionalProperties to true.
    ** Thank you, Krzysztof! **

    - Fix spelling in the commit title and description.
    - Trade-offs for options in Kconfig.
    - Export the new symbol as hyperv-pci can be built as a module.
    ** Thank you, Michael! **

    - Simplify code for getting IRQ number.
    ** Thank you, Rob! **

    - Add comment to clarify when running in VTL mode is reported.
    ** Thank you, Wei! **

[V5]
  https://lore.kernel.org/linux-hyperv/20250307220304.247725-1-romank@linux.microsoft.com/
    - Provide and use a common SMCCC-based infra for the arm64 hypervisor guests
      to detect hypervisor presence.
    ** Thank you, Arnd! **

    - Fix line wraps to follow the rest of the code.
    - Open-code getting IRQ domain parent in the ACPI case to make the code
      better.
    ** Thank you, Bjorn! **

    - Test the binding with the latest dtschema.
    - Clean up the commit title and description.
    - Use proper defines for known constants.
    ** Thank you, Krzysztof! **

    - Extend comment on why ACPI v6 is checked for.
    - Reorder patches to make sure that even with partial series application
      the compilation succeeds.
    - Report VTL the kernel runs in.
    - Use "X86_64" in Kconfig rather than "X86".
    - Extract a non-functional change for hv_get_vmbus_root_device() into
      a separate patch.
    ** Thank you, Michael! **

[V4]
    https://lore.kernel.org/linux-hyperv/20250212014321.1108840-1-romank@linux.microsoft.com/
    - Fixed wording to match acronyms defined in the "Terms and Abbreviations"
      section of the SMCCC specification throughout the patch series.
      **Thank you, Michael!**

    - Replaced the hypervisor ID containing ASCII with an UUID as
      required by the specification.
      **Thank you, Michael!**

    - Added an explicit check for `SMCCC_RET_NOT_SUPPORTED` when discovering the
      hypervisor presence to make the backward compatibility obvious.
      **Thank you, Saurabh!**

    - Split the fix for `get_vtl(void)` out to make it easier to backport.
    - Refactored the configuration options as requested to eliminate the risk
      of building non-functional kernels with randomly selected options.
      **Thank you, Michael!**

    - Refactored the changes not to introduce an additional file with
      a one-line function.
      **Thank you, Wei!**

    - Fixed change description for the VMBus DeviceTree changes, used
      `scripts/get_maintainers.pl` on the latest kernel to get the up-to-date list
      of maintainers as requested.
      **Thank you, Krzysztof!**

    - Removed the added (paranoidal+superfluous) checks for DMA coherence in the
      VMBus driver and instead relied on the DMA and the OF subsystem code.
      **Thank you, Arnd, Krzysztof, Michael!**

    - Used another set of APIs for discovering the hardware interrupt number
      in the VMBus driver to be able to build the driver as a module.
      **Thank you, Michael, Saurabh!**

    - Renamed the newly introduced `get_vmbus_root_device(void)` function to
      `hv_get_vmbus_root_device(void)` as requested.
      **Thank you, Wei!**

    - Applied the suggested small-scale refactoring to simplify changes to the Hyper-V
      PCI driver. Taking the offered liberty of doing the large scale refactoring
      in another patch series.
      **Thank you, Michael!**

    - Added a fix for the issue discovered internally where the CPU would not
      get the interrupt from a PCI device attached to VTL2 as the shared peripheral
      interrupt number (SPI) was not offset by 32 (the first valid SPI number).
      **Thank you, Brian!**

[V3]
    https://lore.kernel.org/lkml/20240726225910.1912537-1-romank@linux.microsoft.com/
    - Employed the SMCCC function recently implemented in the Microsoft Hyper-V
      hypervisor to detect running on Hyper-V/arm64. No dependence on ACPI/DT is
      needed anymore although the source code still falls back to ACPI as the new
      hypervisor might be available only in the Windows Insiders channel just
      yet.
    - As a part of the above, refactored detecting the hypervisor via ACPI FADT.
    - There was a suggestion to explore whether it is feasible or not to express
      that ACPI must be absent for the VTL mode and present for the regular guests
      in the Hyper-V Kconfig file.
      My current conclusion is that this will require refactoring in many places.
      That becomes especially convoluted on x86_64 due to the MSI and APIC
      dependencies. I'd ask to let us tackle that in another patch series (or chalk
      up to nice-have's rather than fires to put out) to separate concerns and
      decrease chances of breakage.
    - While refactoring `get_vtl(void)` and the related code, fixed the hypercall
      output address not to overlap with the input as the Hyper-V TLFS mandates:
      "The input and output parameter lists cannot overlap or cross page boundaries."
      See https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
      for more.
      Some might argue that should've been a topic for a separate patch series;
      I'd counter that the change is well-contained (one line), has no dependencies,
      and makes the code legal.
    - Made the VTL boot code (c)leaner as was suggested.
    - Set DMA cache coherency for the VMBus.
    - Updated DT bindings in the VMBus documentation (separated out into a new patch).
    - Fixed `vmbus_set_irq` to use the API that works both for the ACPI and OF.
    - Reworked setting up the vPCI MSI IRQ domain in the non-ACPI case. The logic
      looks a bit fiddly/ad-hoc as I couldn't find the API that would fit the bill.
      Added comments to explain myself.

[V2]
    https://lore.kernel.org/all/20240514224508.212318-1-romank@linux.microsoft.com/
    - Decreased number of #ifdef's
    - Updated the wording in the commit messages to adhere to the guidlines
    - Sending to the correct set of maintainers and mail lists

[V1]
    https://lore.kernel.org/all/20240510160602.1311352-1-romank@linux.microsoft.com/

Roman Kisel (11):
  arm64: kvm, smccc: Introduce and use API for detecting hypervisor
    presence
  arm64: hyperv: Use SMCCC to detect hypervisor presence
  Drivers: hv: Enable VTL mode for arm64
  Drivers: hv: Provide arch-neutral implementation of get_vtl()
  arm64: hyperv: Initialize the Virtual Trust Level field
  arm64, x86: hyperv: Report the VTL the system boots in
  dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence
    properties
  Drivers: hv: vmbus: Get the IRQ number from DeviceTree
  Drivers: hv: vmbus: Introduce hv_get_vmbus_root_device()
  ACPI: irq: Introduce  acpi_get_gsi_dispatcher()
  PCI: hv: Get vPCI MSI IRQ domain from DeviceTree

 .../bindings/bus/microsoft,vmbus.yaml         | 17 ++++-
 arch/arm64/hyperv/mshyperv.c                  | 46 ++++++++++--
 arch/arm64/kvm/hypercalls.c                   |  5 +-
 arch/x86/hyperv/hv_init.c                     | 34 ---------
 arch/x86/hyperv/hv_vtl.c                      |  7 +-
 drivers/acpi/irq.c                            | 15 +++-
 drivers/firmware/smccc/kvm_guest.c            | 10 +--
 drivers/firmware/smccc/smccc.c                | 19 +++++
 drivers/hv/Kconfig                            |  6 +-
 drivers/hv/hv_common.c                        | 31 ++++++++
 drivers/hv/vmbus_drv.c                        | 53 ++++++++++++--
 drivers/pci/controller/pci-hyperv.c           | 73 +++++++++++++++++--
 include/asm-generic/mshyperv.h                |  6 ++
 include/hyperv/hvgdk_mini.h                   |  2 +-
 include/linux/acpi.h                          |  5 +-
 include/linux/arm-smccc.h                     | 55 +++++++++++++-
 include/linux/hyperv.h                        |  2 +
 17 files changed, 307 insertions(+), 79 deletions(-)


base-commit: d9608f6dc3e3229a40e1db1dab573d6882f38c2a
-- 
2.43.0



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:27   ` Roman Kisel
  2025-03-17 11:29   ` Mark Rutland
  2025-03-15  0:19 ` [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect " Roman Kisel
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

The KVM/arm64 uses SMCCC to detect hypervisor presence. That code is
private, and it follows the SMCCC specification. Other existing and
emerging hypervisor guest implementations can and should use that
standard approach as well.

Factor out a common infrastructure that the guests can use, update KVM
to employ the new API. The central notion of the SMCCC method is the
UUID of the hypervisor, and the API follows that.

No functional changes. Validated with a KVM/arm64 guest.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm64/kvm/hypercalls.c        |  5 +--
 drivers/firmware/smccc/kvm_guest.c | 10 ++----
 drivers/firmware/smccc/smccc.c     | 19 +++++++++++
 include/linux/arm-smccc.h          | 55 +++++++++++++++++++++++++++---
 4 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 27ce4cb44904..92b9bc1ea8e8 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -353,10 +353,7 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
 			val[0] = gpa;
 		break;
 	case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
-		val[0] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0;
-		val[1] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1;
-		val[2] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2;
-		val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
+		UUID_TO_SMCCC_RES(ARM_SMCCC_VENDOR_HYP_UID_KVM, val);
 		break;
 	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
 		val[0] = smccc_feat->vendor_hyp_bmap;
diff --git a/drivers/firmware/smccc/kvm_guest.c b/drivers/firmware/smccc/kvm_guest.c
index f3319be20b36..b5084b309ea0 100644
--- a/drivers/firmware/smccc/kvm_guest.c
+++ b/drivers/firmware/smccc/kvm_guest.c
@@ -14,17 +14,11 @@ static DECLARE_BITMAP(__kvm_arm_hyp_services, ARM_SMCCC_KVM_NUM_FUNCS) __ro_afte
 
 void __init kvm_init_hyp_services(void)
 {
+	uuid_t kvm_uuid = ARM_SMCCC_VENDOR_HYP_UID_KVM;
 	struct arm_smccc_res res;
 	u32 val[4];
 
-	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
-		return;
-
-	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
-	if (res.a0 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0 ||
-	    res.a1 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1 ||
-	    res.a2 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2 ||
-	    res.a3 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3)
+	if (!arm_smccc_hyp_present(&kvm_uuid))
 		return;
 
 	memset(&res, 0, sizeof(res));
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index a74600d9f2d7..7399f27d58e5 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -67,6 +67,25 @@ s32 arm_smccc_get_soc_id_revision(void)
 }
 EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
 
+bool arm_smccc_hyp_present(const uuid_t *hyp_uuid)
+{
+	struct arm_smccc_res res = {};
+
+	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
+		return false;
+	arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
+	if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
+		return false;
+
+	return ({
+		const uuid_t uuid = SMCCC_RES_TO_UUID(res.a0, res.a1, res.a2, res.a3);
+		const bool present = uuid_equal(&uuid, hyp_uuid);
+
+		present;
+	});
+}
+EXPORT_SYMBOL_GPL(arm_smccc_hyp_present);
+
 static int __init smccc_devices_init(void)
 {
 	struct platform_device *pdev;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 67f6fdf2e7cd..726f18221f1c 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -7,6 +7,11 @@
 
 #include <linux/args.h>
 #include <linux/init.h>
+
+#ifndef __ASSEMBLER__
+#include <linux/uuid.h>
+#endif
+
 #include <uapi/linux/const.h>
 
 /*
@@ -107,10 +112,10 @@
 			   ARM_SMCCC_FUNC_QUERY_CALL_UID)
 
 /* KVM UID value: 28b46fb6-2ec5-11e9-a9ca-4b564d003a74 */
-#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0	0xb66fb428U
-#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1	0xe911c52eU
-#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2	0x564bcaa9U
-#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3	0x743a004dU
+#define ARM_SMCCC_VENDOR_HYP_UID_KVM UUID_INIT(\
+	0xb66fb428, 0xc52e, 0xe911, \
+	0xa9, 0xca, 0x4b, 0x56, \
+	0x4d, 0x00, 0x3a, 0x74)
 
 /* KVM "vendor specific" services */
 #define ARM_SMCCC_KVM_FUNC_FEATURES		0
@@ -333,6 +338,48 @@ s32 arm_smccc_get_soc_id_version(void);
  */
 s32 arm_smccc_get_soc_id_revision(void);
 
+#ifndef __ASSEMBLER__
+
+/**
+ * arm_smccc_hyp_present(const uuid_t *hyp_uuid)
+ *
+ * Returns `true` if the hypervisor advertises its presence via SMCCC.
+ *
+ * When the function returns `false`, the caller shall not assume that
+ * there is no hypervisor running. Instead, the caller must fall back to
+ * other approaches if any are available.
+ */
+bool arm_smccc_hyp_present(const uuid_t *hyp_uuid);
+
+#define SMCCC_RES_TO_UUID(r0, r1, r2, r3) \
+	UUID_INIT( \
+		cpu_to_le32(lower_32_bits(r0)), \
+		cpu_to_le32(lower_32_bits(r1)) & 0xffff, \
+		cpu_to_le32(lower_32_bits(r1)) >> 16, \
+		cpu_to_le32(lower_32_bits(r2)) & 0xff, \
+		(cpu_to_le32(lower_32_bits(r2)) >> 8) & 0xff, \
+		(cpu_to_le32(lower_32_bits(r2)) >> 16) & 0xff, \
+		(cpu_to_le32(lower_32_bits(r2)) >> 24) & 0xff, \
+		cpu_to_le32(lower_32_bits(r3)) & 0xff, \
+		(cpu_to_le32(lower_32_bits(r3)) >> 8) & 0xff, \
+		(cpu_to_le32(lower_32_bits(r3)) >> 16) & 0xff, \
+		(cpu_to_le32(lower_32_bits(r3)) >> 24) & 0xff \
+	)
+
+#define UUID_TO_SMCCC_RES(uuid_init, regs) do { \
+		const uuid_t uuid = uuid_init; \
+		(regs)[0] = le32_to_cpu((u32)uuid.b[0] | (uuid.b[1] << 8) | \
+						((uuid.b[2]) << 16) | ((uuid.b[3]) << 24)); \
+		(regs)[1] = le32_to_cpu((u32)uuid.b[4] | (uuid.b[5] << 8) | \
+						((uuid.b[6]) << 16) | ((uuid.b[7]) << 24)); \
+		(regs)[2] = le32_to_cpu((u32)uuid.b[8] | (uuid.b[9] << 8) | \
+						((uuid.b[10]) << 16) | ((uuid.b[11]) << 24)); \
+		(regs)[3] = le32_to_cpu((u32)uuid.b[12] | (uuid.b[13] << 8) | \
+						((uuid.b[14]) << 16) | ((uuid.b[15]) << 24)); \
+	} while (0)
+
+#endif /* !__ASSEMBLER__ */
+
 /**
  * struct arm_smccc_res - Result from SMC/HVC call
  * @a0-a3 result values from registers 0 to 3
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect hypervisor presence
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-17 11:37   ` Mark Rutland
  2025-03-15  0:19 ` [PATCH hyperv-next v6 03/11] Drivers: hv: Enable VTL mode for arm64 Roman Kisel
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

The arm64 Hyper-V startup path relies on ACPI to detect
running under a Hyper-V compatible hypervisor. That
doesn't work on non-ACPI systems.

Hoist the ACPI detection logic into a separate function. Then
use the vendor-specific hypervisor service call (implemented
recently in Hyper-V) via SMCCC in the non-ACPI case.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 arch/arm64/hyperv/mshyperv.c | 43 +++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index 2265ea5ce5ad..c5b03d3af7c5 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -27,6 +27,41 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
 	return 0;
 }
 
+static bool __init hyperv_detect_via_acpi(void)
+{
+	if (acpi_disabled)
+		return false;
+#if IS_ENABLED(CONFIG_ACPI)
+	/*
+	 * Hypervisor ID is only available in ACPI v6+, and the
+	 * structure layout was extended in v6 to accommodate that
+	 * new field.
+	 *
+	 * At the very minimum, this check makes sure not to read
+	 * past the FADT structure.
+	 *
+	 * It is also needed to catch running in some unknown
+	 * non-Hyper-V environment that has ACPI 5.x or less.
+	 * In such a case, it can't be Hyper-V.
+	 */
+	if (acpi_gbl_FADT.header.revision < 6)
+		return false;
+	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
+#else
+	return false;
+#endif
+}
+
+static bool __init hyperv_detect_via_smccc(void)
+{
+	uuid_t hyperv_uuid = UUID_INIT(
+		0x4d32ba58, 0x4764, 0xcd24,
+		0x75, 0x6c, 0xef, 0x8e,
+		0x24, 0x70, 0x59, 0x16);
+
+	return arm_smccc_hyp_present(&hyperv_uuid);
+}
+
 static int __init hyperv_init(void)
 {
 	struct hv_get_vp_registers_output	result;
@@ -35,13 +70,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_via_acpi() && !hyperv_detect_via_smccc())
 		return 0;
 
 	/* Setup the guest ID */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 03/11] Drivers: hv: Enable VTL mode for arm64
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect " Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 04/11] Drivers: hv: Provide arch-neutral implementation of get_vtl() Roman Kisel
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

Kconfig dependencies for arm64 guests on Hyper-V require that be
ACPI enabled, and limit VTL mode to x86/x64. To enable VTL mode
on arm64 as well, update the dependencies. Since VTL mode requires
DeviceTree instead of ACPI, don’t require arm64 guests on Hyper-V
to have ACPI unconditionally.

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 794e88e9dc6b..11b07a6d594a 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.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 04/11] Drivers: hv: Provide arch-neutral implementation of get_vtl()
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (2 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 03/11] Drivers: hv: Enable VTL mode for arm64 Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 05/11] arm64: hyperv: Initialize the Virtual Trust Level field Roman Kisel
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

To run in the VTL mode, Hyper-V drivers have to know what
VTL the system boots in, and the arm64/hyperv code does not
have the means to compute that.

Refactor the code to hoist the function that detects VTL,
make it arch-neutral to be able to employ it to get the VTL
on arm64.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
---
 arch/x86/hyperv/hv_init.c      | 34 ----------------------------------
 drivers/hv/hv_common.c         | 31 +++++++++++++++++++++++++++++++
 include/asm-generic/mshyperv.h |  6 ++++++
 include/hyperv/hvgdk_mini.h    |  2 +-
 4 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index ddeb40930bc8..3b569291dfed 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -390,40 +390,6 @@ static void __init hv_stimer_setup_percpu_clockev(void)
 		old_setup_percpu_clockev();
 }
 
-#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_input_get_vp_registers *input;
-	struct hv_output_get_vp_registers *output;
-	unsigned long flags;
-	u64 ret;
-
-	local_irq_save(flags);
-	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
-	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
-
-	memset(input, 0, struct_size(input, names, 1));
-	input->partition_id = HV_PARTITION_ID_SELF;
-	input->vp_index = HV_VP_INDEX_SELF;
-	input->input_vtl.as_uint8 = 0;
-	input->names[0] = HV_REGISTER_VSM_VP_STATUS;
-
-	ret = hv_do_hypercall(control, input, output);
-	if (hv_result_success(ret)) {
-		ret = output->values[0].reg8 & 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/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 9804adb4cc56..dce4b34e293b 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -304,6 +304,37 @@ void __init hv_get_partition_id(void)
 		pr_err("Hyper-V: failed to get partition ID: %#x\n",
 		       hv_result(status));
 }
+#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_input_get_vp_registers *input;
+	struct hv_output_get_vp_registers *output;
+	unsigned long flags;
+	u64 ret;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+	memset(input, 0, struct_size(input, names, 1));
+	input->partition_id = HV_PARTITION_ID_SELF;
+	input->vp_index = HV_VP_INDEX_SELF;
+	input->input_vtl.as_uint8 = 0;
+	input->names[0] = HV_REGISTER_VSM_VP_STATUS;
+
+	ret = hv_do_hypercall(control, input, output);
+	if (hv_result_success(ret)) {
+		ret = output->values[0].reg8 & HV_VTL_MASK;
+	} else {
+		pr_err("Failed to get VTL(error: %lld) exiting...\n", ret);
+		BUG();
+	}
+
+	local_irq_restore(flags);
+	return ret;
+}
+#endif
 
 int __init hv_common_init(void)
 {
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index b13b0cda4ac8..c5150a2b12d6 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -348,4 +348,10 @@ static inline int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u3
 }
 #endif /* CONFIG_MSHV_ROOT */
 
+#if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
+u8 __init get_vtl(void);
+#else
+static inline u8 get_vtl(void) { return 0; }
+#endif
+
 #endif
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 58895883f636..98aa64e8844c 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -1202,7 +1202,7 @@ struct hv_send_ipi {	 /* HV_INPUT_SEND_SYNTHETIC_CLUSTER_IPI */
 	u64 cpu_mask;
 } __packed;
 
-#define	HV_X64_VTL_MASK			GENMASK(3, 0)
+#define	HV_VTL_MASK			GENMASK(3, 0)
 
 /* Hyper-V memory host visibility */
 enum hv_mem_host_visibility {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 05/11] arm64: hyperv: Initialize the Virtual Trust Level field
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (3 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 04/11] Drivers: hv: Provide arch-neutral implementation of get_vtl() Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 06/11] arm64, x86: hyperv: Report the VTL the system boots in Roman Kisel
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

Various parts of the hyperv code need to know what VTL
the kernel runs at, most notably VMBus needs that to
establish communication with the host.

Initialize the Virtual Trust Level field to enable
booting in the Virtual Trust Level.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 arch/arm64/hyperv/mshyperv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index c5b03d3af7c5..f251a08ada5b 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -109,6 +109,7 @@ static int __init hyperv_init(void)
 
 	if (ms_hyperv.priv_high & HV_ACCESS_PARTITION_ID)
 		hv_get_partition_id();
+	ms_hyperv.vtl = get_vtl();
 
 	ms_hyperv_late_init();
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 06/11] arm64, x86: hyperv: Report the VTL the system boots in
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (4 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 05/11] arm64: hyperv: Initialize the Virtual Trust Level field Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties Roman Kisel
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

The hyperv guest code might run in various Virtual Trust Levels.

Report the level when the kernel boots in the non-default (0)
one.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 arch/arm64/hyperv/mshyperv.c | 2 ++
 arch/x86/hyperv/hv_vtl.c     | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index f251a08ada5b..ecbd2f5c955e 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -110,6 +110,8 @@ static int __init hyperv_init(void)
 	if (ms_hyperv.priv_high & HV_ACCESS_PARTITION_ID)
 		hv_get_partition_id();
 	ms_hyperv.vtl = get_vtl();
+	if (ms_hyperv.vtl > 0) /* non default VTL */
+		pr_info("Linux runs in Hyper-V Virtual Trust Level %d\n", ms_hyperv.vtl);
 
 	ms_hyperv_late_init();
 
diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 582fe820e29c..038c896fdd60 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -55,7 +55,12 @@ static void  __noreturn hv_vtl_restart(char __maybe_unused *cmd)
 
 void __init hv_vtl_init_platform(void)
 {
-	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
+	/*
+	 * This function is a no-op if the VTL mode is not enabled.
+	 * If it is, this function runs if and only the kernel boots in
+	 * VTL2 which the x86 hv initialization path makes sure of.
+	 */
+	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.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (5 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 06/11] arm64, x86: hyperv: Report the VTL the system boots in Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-16 17:36   ` Krzysztof Kozlowski
  2025-03-15  0:19 ` [PATCH hyperv-next v6 08/11] Drivers: hv: vmbus: Get the IRQ number from DeviceTree Roman Kisel
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

To boot in the VTL mode, VMBus on arm64 needs interrupt description
which the binding documentation lacks. The transactions on the bus are
DMA coherent which is not mentioned as well.

Add the interrupt property and the DMA coherence property to the VMBus
binding. Update the example to match that. Fix typos.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 .../bindings/bus/microsoft,vmbus.yaml           | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
index a8d40c766dcd..ca288ea54b34 100644
--- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
+++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
@@ -10,8 +10,8 @@ maintainers:
   - Saurabh Sengar <ssengar@linux.microsoft.com>
 
 description:
-  VMBus is a software bus that implement the protocols for communication
-  between the root or host OS and guest OSs (virtual machines).
+  VMBus is a software bus that implements the protocols for communication
+  between the root or host OS and guest OS'es (virtual machines).
 
 properties:
   compatible:
@@ -25,9 +25,17 @@ properties:
   '#size-cells':
     const: 1
 
+  dma-coherent: true
+
+  interrupts:
+    maxItems: 1
+    description: |
+      This interrupt is used to report a message from the host.
+
 required:
   - compatible
   - ranges
+  - interrupts
   - '#address-cells'
   - '#size-cells'
 
@@ -35,6 +43,8 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
     soc {
         #address-cells = <2>;
         #size-cells = <1>;
@@ -49,6 +59,9 @@ examples:
                 #address-cells = <2>;
                 #size-cells = <1>;
                 ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>;
+                dma-coherent;
+                interrupt-parent = <&gic>;
+                interrupts = <GIC_PPI 2 IRQ_TYPE_EDGE_RISING>;
             };
         };
     };
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 08/11] Drivers: hv: vmbus: Get the IRQ number from DeviceTree
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (6 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 09/11] Drivers: hv: vmbus: Introduce hv_get_vmbus_root_device() Roman Kisel
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

The VMBus driver uses ACPI for interrupt assignment on
arm64 hence it won't function in the VTL mode where only
DeviceTree can be used.

Update the VMBus driver to discover interrupt configuration
from DT.

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

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 22afebfc28ff..e8f2c3e92d1f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2345,6 +2345,31 @@ static int vmbus_acpi_add(struct platform_device *pdev)
 }
 #endif
 
+static int vmbus_set_irq(struct platform_device *pdev)
+{
+	struct irq_data *data;
+	int irq;
+	irq_hw_number_t hwirq;
+
+	irq = platform_get_irq(pdev, 0);
+	/* platform_get_irq() may not return 0. */
+	if (irq < 0)
+		return irq;
+
+	data = irq_get_irq_data(irq);
+	if (!data) {
+		pr_err("No interrupt data for VMBus virq %d\n", irq);
+		return -ENODEV;
+	}
+	hwirq = irqd_to_hwirq(data);
+
+	vmbus_irq = irq;
+	vmbus_interrupt = 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;
@@ -2359,6 +2384,11 @@ static int vmbus_device_add(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (!__is_defined(HYPERVISOR_CALLBACK_VECTOR))
+		ret = vmbus_set_irq(pdev);
+	if (ret)
+		return ret;
+
 	for_each_of_range(&parser, &range) {
 		struct resource *res;
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 09/11] Drivers: hv: vmbus: Introduce hv_get_vmbus_root_device()
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (7 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 08/11] Drivers: hv: vmbus: Get the IRQ number from DeviceTree Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher() Roman Kisel
  2025-03-15  0:19 ` [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree Roman Kisel
  10 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

The ARM64 PCI code for hyperv needs to know the VMBus root
device, and it is private.

Provide a function that returns it. Rename it from "hv_dev"
as "hv_dev" as a symbol is very overloaded. No functional
changes.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/hv/vmbus_drv.c | 23 +++++++++++++++--------
 include/linux/hyperv.h |  2 ++
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e8f2c3e92d1f..df18b4070b01 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -45,7 +45,8 @@ struct vmbus_dynid {
 	struct hv_vmbus_device_id id;
 };
 
-static struct device  *hv_dev;
+/* VMBus Root Device */
+static struct device  *vmbus_root_device;
 
 static int hyperv_cpuhp_online;
 
@@ -80,9 +81,15 @@ static struct resource *fb_mmio;
 static struct resource *hyperv_mmio;
 static DEFINE_MUTEX(hyperv_mmio_lock);
 
+struct device *hv_get_vmbus_root_device(void)
+{
+	return vmbus_root_device;
+}
+EXPORT_SYMBOL_GPL(hv_get_vmbus_root_device);
+
 static int vmbus_exists(void)
 {
-	if (hv_dev == NULL)
+	if (vmbus_root_device == NULL)
 		return -ENODEV;
 
 	return 0;
@@ -861,7 +868,7 @@ static int vmbus_dma_configure(struct device *child_device)
 	 * On x86/x64 coherence is assumed and these calls have no effect.
 	 */
 	hv_setup_dma_ops(child_device,
-		device_get_dma_attr(hv_dev) == DEV_DMA_COHERENT);
+		device_get_dma_attr(vmbus_root_device) == DEV_DMA_COHERENT);
 	return 0;
 }
 
@@ -1930,7 +1937,7 @@ int vmbus_device_register(struct hv_device *child_device_obj)
 		     &child_device_obj->channel->offermsg.offer.if_instance);
 
 	child_device_obj->device.bus = &hv_bus;
-	child_device_obj->device.parent = hv_dev;
+	child_device_obj->device.parent = vmbus_root_device;
 	child_device_obj->device.release = vmbus_device_release;
 
 	child_device_obj->device.dma_parms = &child_device_obj->dma_parms;
@@ -2292,7 +2299,7 @@ static int vmbus_acpi_add(struct platform_device *pdev)
 	struct acpi_device *ancestor;
 	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 
-	hv_dev = &device->dev;
+	vmbus_root_device = &device->dev;
 
 	/*
 	 * Older versions of Hyper-V for ARM64 fail to include the _CCA
@@ -2378,7 +2385,7 @@ static int vmbus_device_add(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
-	hv_dev = &pdev->dev;
+	vmbus_root_device = &pdev->dev;
 
 	ret = of_range_parser_init(&parser, np);
 	if (ret)
@@ -2696,7 +2703,7 @@ static int __init hv_acpi_init(void)
 	if (ret)
 		return ret;
 
-	if (!hv_dev) {
+	if (!vmbus_root_device) {
 		ret = -ENODEV;
 		goto cleanup;
 	}
@@ -2727,7 +2734,7 @@ static int __init hv_acpi_init(void)
 
 cleanup:
 	platform_driver_unregister(&vmbus_platform_driver);
-	hv_dev = NULL;
+	vmbus_root_device = NULL;
 	return ret;
 }
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 675959fb97ba..1f310fbbc4f9 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1277,6 +1277,8 @@ static inline void *hv_get_drvdata(struct hv_device *dev)
 	return dev_get_drvdata(&dev->device);
 }
 
+struct device *hv_get_vmbus_root_device(void);
+
 struct hv_ring_buffer_debug_info {
 	u32 current_interrupt_mask;
 	u32 current_read_index;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce  acpi_get_gsi_dispatcher()
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (8 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 09/11] Drivers: hv: vmbus: Introduce hv_get_vmbus_root_device() Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-26 14:55   ` Rafael J. Wysocki
  2025-03-15  0:19 ` [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree Roman Kisel
  10 siblings, 1 reply; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

Using acpi_irq_create_hierarchy() in the cases where the code
also handles OF leads to code duplication as the ACPI subsystem
doesn't provide means to compute the IRQ domain parent whereas
the OF does.

Introduce acpi_get_gsi_dispatcher() so that the drivers relying
on both ACPI and OF may use irq_domain_create_hierarchy() in the
common code paths.

No functional changes.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/acpi/irq.c   | 15 +++++++++++++--
 include/linux/acpi.h |  5 ++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index 1687483ff319..8eb09e45e5c5 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -12,7 +12,7 @@
 
 enum acpi_irq_model_id acpi_irq_model;
 
-static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
+static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
 static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
 
 /**
@@ -307,12 +307,23 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
  *	for a given GSI
  */
 void __init acpi_set_irq_model(enum acpi_irq_model_id model,
-			       struct fwnode_handle *(*fn)(u32))
+	acpi_gsi_domain_disp_fn fn)
 {
 	acpi_irq_model = model;
 	acpi_get_gsi_domain_id = fn;
 }
 
+/**
+ * acpi_get_gsi_dispatcher - Returns dispatcher function that
+ *                           computes the domain fwnode for a
+ *                           given GSI.
+ */
+acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
+{
+	return acpi_get_gsi_domain_id;
+}
+EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
+
 /**
  * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
  * callback to fallback to arch specified implementation.
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4e495b29c640..abc51288e867 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
 
+typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
+
 void acpi_set_irq_model(enum acpi_irq_model_id model,
-			struct fwnode_handle *(*)(u32));
+	acpi_gsi_domain_disp_fn fn);
+acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
 void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
 
 struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
  2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
                   ` (9 preceding siblings ...)
  2025-03-15  0:19 ` [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher() Roman Kisel
@ 2025-03-15  0:19 ` Roman Kisel
  2025-03-15 18:49   ` Bjorn Helgaas
  2025-03-26 14:56   ` Rafael J. Wysocki
  10 siblings, 2 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:19 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
arm64. It won't be able to do that in the VTL mode where only DeviceTree
can be used.

Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
case, too.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 6084b38bdda1..cbff19e8a07c 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -50,6 +50,7 @@
 #include <linux/irqdomain.h>
 #include <linux/acpi.h>
 #include <linux/sizes.h>
+#include <linux/of_irq.h>
 #include <asm/mshyperv.h>
 
 /*
@@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
 	int ret;
 
 	fwspec.fwnode = domain->parent->fwnode;
-	fwspec.param_count = 2;
-	fwspec.param[0] = hwirq;
-	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
+	if (is_of_node(fwspec.fwnode)) {
+		/* SPI lines for OF translations start at offset 32 */
+		fwspec.param_count = 3;
+		fwspec.param[0] = 0;
+		fwspec.param[1] = hwirq - 32;
+		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+	} else {
+		fwspec.param_count = 2;
+		fwspec.param[0] = hwirq;
+		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
+	}
 
 	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
 	if (ret)
@@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
 	.activate = hv_pci_vec_irq_domain_activate,
 };
 
+#ifdef CONFIG_OF
+
+static struct irq_domain *hv_pci_of_irq_domain_parent(void)
+{
+	struct device_node *parent;
+	struct irq_domain *domain;
+
+	parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
+	if (!parent)
+		return NULL;
+	domain = irq_find_host(parent);
+	of_node_put(parent);
+
+	return domain;
+}
+
+#endif
+
+#ifdef CONFIG_ACPI
+
+static struct irq_domain *hv_pci_acpi_irq_domain_parent(void)
+{
+	struct irq_domain *domain;
+	acpi_gsi_domain_disp_fn gsi_domain_disp_fn;
+
+	if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
+		return NULL;
+	gsi_domain_disp_fn = acpi_get_gsi_dispatcher();
+	if (!gsi_domain_disp_fn)
+		return NULL;
+	return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
+				     DOMAIN_BUS_ANY);
+}
+
+#endif
+
 static int hv_pci_irqchip_init(void)
 {
 	static struct hv_pci_chip_data *chip_data;
 	struct fwnode_handle *fn = NULL;
+	struct irq_domain *irq_domain_parent = NULL;
 	int ret = -ENOMEM;
 
 	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
@@ -907,9 +953,24 @@ 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);
+#ifdef CONFIG_ACPI
+	if (!acpi_disabled)
+		irq_domain_parent = hv_pci_acpi_irq_domain_parent();
+#endif
+#if defined(CONFIG_OF)
+	if (!irq_domain_parent)
+		irq_domain_parent = hv_pci_of_irq_domain_parent();
+#endif
+	if (!irq_domain_parent) {
+		WARN_ONCE(1, "Invalid firmware configuration for VMBus interrupts\n");
+		ret = -EINVAL;
+		goto free_chip;
+	}
+
+	hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
+		irq_domain_parent, 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");
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence
  2025-03-15  0:19 ` [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence Roman Kisel
@ 2025-03-15  0:27   ` Roman Kisel
  2025-03-15 13:12     ` Arnd Bergmann
  2025-03-17 11:29   ` Mark Rutland
  1 sibling, 1 reply; 29+ messages in thread
From: Roman Kisel @ 2025-03-15  0:27 UTC (permalink / raw)
  To: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut



On 3/14/2025 5:19 PM, Roman Kisel wrote:
> The KVM/arm64 uses SMCCC to detect hypervisor presence. That code is
> private, and it follows the SMCCC specification. Other existing and
> emerging hypervisor guest implementations can and should use that
> standard approach as well.
> 
> Factor out a common infrastructure that the guests can use, update KVM
> to employ the new API. The central notion of the SMCCC method is the
> UUID of the hypervisor, and the API follows that.
> 
> No functional changes. Validated with a KVM/arm64 guest.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

While the change is Acked, here is the caveat maybe.

This patch produces warnings wtih sparse and CHECK_ENDING.
That said, the kernel build produces a whole lot more other warnings
from building with sparse by itself and/or with CHECK_ENDING.

I am not sure how to proceed with that, thinking I should
not add warnings yet at the same time there are many others.
Not certain if folks take these signals as fyi or blockers.

Decided to send V6 to trim the V5 discussion threads by implementing
suggestions and fixes.

> ---
>   arch/arm64/kvm/hypercalls.c        |  5 +--
>   drivers/firmware/smccc/kvm_guest.c | 10 ++----
>   drivers/firmware/smccc/smccc.c     | 19 +++++++++++
>   include/linux/arm-smccc.h          | 55 +++++++++++++++++++++++++++---
>   4 files changed, 73 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 27ce4cb44904..92b9bc1ea8e8 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -353,10 +353,7 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
>   			val[0] = gpa;
>   		break;
>   	case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> -		val[0] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0;
> -		val[1] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1;
> -		val[2] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2;
> -		val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
> +		UUID_TO_SMCCC_RES(ARM_SMCCC_VENDOR_HYP_UID_KVM, val);
>   		break;
>   	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
>   		val[0] = smccc_feat->vendor_hyp_bmap;
> diff --git a/drivers/firmware/smccc/kvm_guest.c b/drivers/firmware/smccc/kvm_guest.c
> index f3319be20b36..b5084b309ea0 100644
> --- a/drivers/firmware/smccc/kvm_guest.c
> +++ b/drivers/firmware/smccc/kvm_guest.c
> @@ -14,17 +14,11 @@ static DECLARE_BITMAP(__kvm_arm_hyp_services, ARM_SMCCC_KVM_NUM_FUNCS) __ro_afte
>   
>   void __init kvm_init_hyp_services(void)
>   {
> +	uuid_t kvm_uuid = ARM_SMCCC_VENDOR_HYP_UID_KVM;
>   	struct arm_smccc_res res;
>   	u32 val[4];
>   
> -	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
> -		return;
> -
> -	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> -	if (res.a0 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0 ||
> -	    res.a1 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1 ||
> -	    res.a2 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2 ||
> -	    res.a3 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3)
> +	if (!arm_smccc_hyp_present(&kvm_uuid))
>   		return;
>   
>   	memset(&res, 0, sizeof(res));
> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index a74600d9f2d7..7399f27d58e5 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
> @@ -67,6 +67,25 @@ s32 arm_smccc_get_soc_id_revision(void)
>   }
>   EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
>   
> +bool arm_smccc_hyp_present(const uuid_t *hyp_uuid)
> +{
> +	struct arm_smccc_res res = {};
> +
> +	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
> +		return false;
> +	arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> +	if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> +		return false;
> +
> +	return ({
> +		const uuid_t uuid = SMCCC_RES_TO_UUID(res.a0, res.a1, res.a2, res.a3);
> +		const bool present = uuid_equal(&uuid, hyp_uuid);
> +
> +		present;
> +	});
> +}
> +EXPORT_SYMBOL_GPL(arm_smccc_hyp_present);
> +
>   static int __init smccc_devices_init(void)
>   {
>   	struct platform_device *pdev;
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 67f6fdf2e7cd..726f18221f1c 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -7,6 +7,11 @@
>   
>   #include <linux/args.h>
>   #include <linux/init.h>
> +
> +#ifndef __ASSEMBLER__
> +#include <linux/uuid.h>
> +#endif
> +
>   #include <uapi/linux/const.h>
>   
>   /*
> @@ -107,10 +112,10 @@
>   			   ARM_SMCCC_FUNC_QUERY_CALL_UID)
>   
>   /* KVM UID value: 28b46fb6-2ec5-11e9-a9ca-4b564d003a74 */
> -#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0	0xb66fb428U
> -#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1	0xe911c52eU
> -#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2	0x564bcaa9U
> -#define ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3	0x743a004dU
> +#define ARM_SMCCC_VENDOR_HYP_UID_KVM UUID_INIT(\
> +	0xb66fb428, 0xc52e, 0xe911, \
> +	0xa9, 0xca, 0x4b, 0x56, \
> +	0x4d, 0x00, 0x3a, 0x74)
>   
>   /* KVM "vendor specific" services */
>   #define ARM_SMCCC_KVM_FUNC_FEATURES		0
> @@ -333,6 +338,48 @@ s32 arm_smccc_get_soc_id_version(void);
>    */
>   s32 arm_smccc_get_soc_id_revision(void);
>   
> +#ifndef __ASSEMBLER__
> +
> +/**
> + * arm_smccc_hyp_present(const uuid_t *hyp_uuid)
> + *
> + * Returns `true` if the hypervisor advertises its presence via SMCCC.
> + *
> + * When the function returns `false`, the caller shall not assume that
> + * there is no hypervisor running. Instead, the caller must fall back to
> + * other approaches if any are available.
> + */
> +bool arm_smccc_hyp_present(const uuid_t *hyp_uuid);
> +
> +#define SMCCC_RES_TO_UUID(r0, r1, r2, r3) \
> +	UUID_INIT( \
> +		cpu_to_le32(lower_32_bits(r0)), \
> +		cpu_to_le32(lower_32_bits(r1)) & 0xffff, \
> +		cpu_to_le32(lower_32_bits(r1)) >> 16, \
> +		cpu_to_le32(lower_32_bits(r2)) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r2)) >> 8) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r2)) >> 16) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r2)) >> 24) & 0xff, \
> +		cpu_to_le32(lower_32_bits(r3)) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r3)) >> 8) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r3)) >> 16) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r3)) >> 24) & 0xff \
> +	)
> +
> +#define UUID_TO_SMCCC_RES(uuid_init, regs) do { \
> +		const uuid_t uuid = uuid_init; \
> +		(regs)[0] = le32_to_cpu((u32)uuid.b[0] | (uuid.b[1] << 8) | \
> +						((uuid.b[2]) << 16) | ((uuid.b[3]) << 24)); \
> +		(regs)[1] = le32_to_cpu((u32)uuid.b[4] | (uuid.b[5] << 8) | \
> +						((uuid.b[6]) << 16) | ((uuid.b[7]) << 24)); \
> +		(regs)[2] = le32_to_cpu((u32)uuid.b[8] | (uuid.b[9] << 8) | \
> +						((uuid.b[10]) << 16) | ((uuid.b[11]) << 24)); \
> +		(regs)[3] = le32_to_cpu((u32)uuid.b[12] | (uuid.b[13] << 8) | \
> +						((uuid.b[14]) << 16) | ((uuid.b[15]) << 24)); \
> +	} while (0)
> +
> +#endif /* !__ASSEMBLER__ */
> +
>   /**
>    * struct arm_smccc_res - Result from SMC/HVC call
>    * @a0-a3 result values from registers 0 to 3

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence
  2025-03-15  0:27   ` Roman Kisel
@ 2025-03-15 13:12     ` Arnd Bergmann
  2025-03-17 17:28       ` Roman Kisel
  0 siblings, 1 reply; 29+ messages in thread
From: Arnd Bergmann @ 2025-03-15 13:12 UTC (permalink / raw)
  To: Roman Kisel, bhelgaas, Borislav Petkov, Catalin Marinas,
	Conor Dooley, Dan Carpenter, Dave Hansen, Dexuan Cui,
	Haiyang Zhang, H. Peter Anvin, Joey Gouly, krzk+dt,
	Krzysztof Wilczyński, K. Y. Srinivasan, Len Brown,
	Lorenzo Pieralisi, Manivannan Sadhasivam, Mark Rutland,
	Marc Zyngier, Ingo Molnar, Oliver Upton, Rafael J . Wysocki,
	Rob Herring, ssengar, Sudeep Holla, Suzuki K Poulose,
	Thomas Gleixner, Wei Liu, Will Deacon, Zenghui Yu, devicetree,
	kvmarm, linux-acpi, Linux-Arch, linux-arm-kernel, linux-hyperv,
	linux-kernel, linux-pci, x86
  Cc: apais, benhill, bperkins, sunilmut

On Sat, Mar 15, 2025, at 01:27, Roman Kisel wrote:
> On 3/14/2025 5:19 PM, Roman Kisel wrote:
>
> While the change is Acked, here is the caveat maybe.
>
> This patch produces warnings wtih sparse and CHECK_ENDING.
> That said, the kernel build produces a whole lot more other warnings
> from building with sparse by itself and/or with CHECK_ENDING.
>
> I am not sure how to proceed with that, thinking I should
> not add warnings yet at the same time there are many others.
> Not certain if folks take these signals as fyi or blockers.

It would be best not to add warnigns. There is slow progress on
fixing all the endianess warnings and I hope this can eventually
complete, so even if there are many existing ones please try to
keep new code clean.

     Arnd


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
  2025-03-15  0:19 ` [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree Roman Kisel
@ 2025-03-15 18:49   ` Bjorn Helgaas
  2025-03-17 17:07     ` Roman Kisel
  2025-03-26 14:56   ` Rafael J. Wysocki
  1 sibling, 1 reply; 29+ messages in thread
From: Bjorn Helgaas @ 2025-03-15 18:49 UTC (permalink / raw)
  To: Roman Kisel
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut

On Fri, Mar 14, 2025 at 05:19:31PM -0700, Roman Kisel wrote:
> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
> arm64. It won't be able to do that in the VTL mode where only DeviceTree
> can be used.
> 
> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
> case, too.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Looks good to me; trivial whitespace comment below.

> ---
>  drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
>  1 file changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 6084b38bdda1..cbff19e8a07c 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -50,6 +50,7 @@
>  #include <linux/irqdomain.h>
>  #include <linux/acpi.h>
>  #include <linux/sizes.h>
> +#include <linux/of_irq.h>
>  #include <asm/mshyperv.h>
>  
>  /*
> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
>  	int ret;
>  
>  	fwspec.fwnode = domain->parent->fwnode;
> -	fwspec.param_count = 2;
> -	fwspec.param[0] = hwirq;
> -	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +	if (is_of_node(fwspec.fwnode)) {
> +		/* SPI lines for OF translations start at offset 32 */
> +		fwspec.param_count = 3;
> +		fwspec.param[0] = 0;
> +		fwspec.param[1] = hwirq - 32;
> +		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +	} else {
> +		fwspec.param_count = 2;
> +		fwspec.param[0] = hwirq;
> +		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +	}
>  
>  	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>  	if (ret)
> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
>  	.activate = hv_pci_vec_irq_domain_activate,
>  };
>  
> +#ifdef CONFIG_OF
> +
> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
> +{
> +	struct device_node *parent;
> +	struct irq_domain *domain;
> +
> +	parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
> +	if (!parent)
> +		return NULL;
> +	domain = irq_find_host(parent);
> +	of_node_put(parent);
> +
> +	return domain;
> +}
> +
> +#endif
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct irq_domain *hv_pci_acpi_irq_domain_parent(void)
> +{
> +	struct irq_domain *domain;
> +	acpi_gsi_domain_disp_fn gsi_domain_disp_fn;
> +
> +	if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
> +		return NULL;
> +	gsi_domain_disp_fn = acpi_get_gsi_dispatcher();
> +	if (!gsi_domain_disp_fn)
> +		return NULL;
> +	return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
> +				     DOMAIN_BUS_ANY);
> +}
> +
> +#endif
> +
>  static int hv_pci_irqchip_init(void)
>  {
>  	static struct hv_pci_chip_data *chip_data;
>  	struct fwnode_handle *fn = NULL;
> +	struct irq_domain *irq_domain_parent = NULL;
>  	int ret = -ENOMEM;
>  
>  	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> @@ -907,9 +953,24 @@ 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);
> +#ifdef CONFIG_ACPI
> +	if (!acpi_disabled)
> +		irq_domain_parent = hv_pci_acpi_irq_domain_parent();
> +#endif
> +#if defined(CONFIG_OF)
> +	if (!irq_domain_parent)
> +		irq_domain_parent = hv_pci_of_irq_domain_parent();
> +#endif
> +	if (!irq_domain_parent) {
> +		WARN_ONCE(1, "Invalid firmware configuration for VMBus interrupts\n");
> +		ret = -EINVAL;
> +		goto free_chip;
> +	}
> +
> +	hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
> +		irq_domain_parent, 0, HV_PCI_MSI_SPI_NR,
> +		fn, &hv_pci_domain_ops,
> +		chip_data);

This is a different style of indenting the parameters than other
similar cases in this file, which line up parameters on subsequent
lines under the open parenthesis.

>  	if (!hv_msi_gic_irq_domain) {
>  		pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ domain\n");
> -- 
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties
  2025-03-15  0:19 ` [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties Roman Kisel
@ 2025-03-16 17:36   ` Krzysztof Kozlowski
  2025-03-17 17:07     ` Roman Kisel
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2025-03-16 17:36 UTC (permalink / raw)
  To: Roman Kisel
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut

On Fri, Mar 14, 2025 at 05:19:27PM -0700, Roman Kisel wrote:
>  
> +  dma-coherent: true
> +
> +  interrupts:

> +    maxItems: 1
> +    description: |
> +      This interrupt is used to report a message from the host.

These could be just two lines:

items:
  - description: Interrupt used to report a message from the host.

(and note that just like we do not use "This" in commit msg, there is
really no benefit of using it in hardware descruption for simple
statements)

Regardless:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence
  2025-03-15  0:19 ` [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence Roman Kisel
  2025-03-15  0:27   ` Roman Kisel
@ 2025-03-17 11:29   ` Mark Rutland
  2025-03-17 17:11     ` Roman Kisel
  1 sibling, 1 reply; 29+ messages in thread
From: Mark Rutland @ 2025-03-17 11:29 UTC (permalink / raw)
  To: Roman Kisel
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, maz, mingo, oliver.upton,
	rafael, robh, ssengar, sudeep.holla, suzuki.poulose, tglx,
	wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut

On Fri, Mar 14, 2025 at 05:19:21PM -0700, Roman Kisel wrote:
> The KVM/arm64 uses SMCCC to detect hypervisor presence. That code is
> private, and it follows the SMCCC specification. Other existing and
> emerging hypervisor guest implementations can and should use that
> standard approach as well.
> 
> Factor out a common infrastructure that the guests can use, update KVM
> to employ the new API. The central notion of the SMCCC method is the
> UUID of the hypervisor, and the API follows that.
> 
> No functional changes. Validated with a KVM/arm64 guest.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm64/kvm/hypercalls.c        |  5 +--
>  drivers/firmware/smccc/kvm_guest.c | 10 ++----
>  drivers/firmware/smccc/smccc.c     | 19 +++++++++++
>  include/linux/arm-smccc.h          | 55 +++++++++++++++++++++++++++---
>  4 files changed, 73 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 27ce4cb44904..92b9bc1ea8e8 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -353,10 +353,7 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
>  			val[0] = gpa;
>  		break;
>  	case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> -		val[0] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0;
> -		val[1] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1;
> -		val[2] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2;
> -		val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
> +		UUID_TO_SMCCC_RES(ARM_SMCCC_VENDOR_HYP_UID_KVM, val);
>  		break;
>  	case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
>  		val[0] = smccc_feat->vendor_hyp_bmap;
> diff --git a/drivers/firmware/smccc/kvm_guest.c b/drivers/firmware/smccc/kvm_guest.c
> index f3319be20b36..b5084b309ea0 100644
> --- a/drivers/firmware/smccc/kvm_guest.c
> +++ b/drivers/firmware/smccc/kvm_guest.c
> @@ -14,17 +14,11 @@ static DECLARE_BITMAP(__kvm_arm_hyp_services, ARM_SMCCC_KVM_NUM_FUNCS) __ro_afte
>  
>  void __init kvm_init_hyp_services(void)
>  {
> +	uuid_t kvm_uuid = ARM_SMCCC_VENDOR_HYP_UID_KVM;
>  	struct arm_smccc_res res;
>  	u32 val[4];
>  
> -	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
> -		return;
> -
> -	arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> -	if (res.a0 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_0 ||
> -	    res.a1 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_1 ||
> -	    res.a2 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_2 ||
> -	    res.a3 != ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3)
> +	if (!arm_smccc_hyp_present(&kvm_uuid))
>  		return;
>  
>  	memset(&res, 0, sizeof(res));
> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index a74600d9f2d7..7399f27d58e5 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
> @@ -67,6 +67,25 @@ s32 arm_smccc_get_soc_id_revision(void)
>  }
>  EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
>  
> +bool arm_smccc_hyp_present(const uuid_t *hyp_uuid)
> +{
> +	struct arm_smccc_res res = {};
> +
> +	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
> +		return false;
> +	arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> +	if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> +		return false;
> +
> +	return ({
> +		const uuid_t uuid = SMCCC_RES_TO_UUID(res.a0, res.a1, res.a2, res.a3);
> +		const bool present = uuid_equal(&uuid, hyp_uuid);
> +
> +		present;
> +	});
> +}

This use of a statement expression is bizarre, and the function would be
clearer without it, e.g.

| bool arm_smccc_hyp_present(const uuid_t *hyp_uuid)
| {
| 	struct arm_smccc_res res = {};
| 	uuid_t uuid;
| 
| 	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
| 		return false;
| 
| 	arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
| 	if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
| 		return false;
| 	
| 	uuid_t = SMCCC_RES_TO_UUID(res.a0, res.a1, res.a2, res.a3);
| 	return uuid_equal(&uuid, hyp_uuid);
| }

As noted below, I'd prefer if this were renamed to something like
arm_smccc_hypervisor_has_uuid(), to more clearly indicate what is being
checked.

[...]

> +/**
> + * arm_smccc_hyp_present(const uuid_t *hyp_uuid)
> + *
> + * Returns `true` if the hypervisor advertises its presence via SMCCC.
> + *
> + * When the function returns `false`, the caller shall not assume that
> + * there is no hypervisor running. Instead, the caller must fall back to
> + * other approaches if any are available.
> + */
> +bool arm_smccc_hyp_present(const uuid_t *hyp_uuid);

I'd prefer if this were:

| /*
|  * Returns whether a specific hypervisor UUID is advertised for the
|  * Vendor Specific Hypervisor Service range.
|  */
| bool arm_smccc_hypervisor_has_uuid(const uuid_t *uuid);

> +#define SMCCC_RES_TO_UUID(r0, r1, r2, r3) \
> +	UUID_INIT( \
> +		cpu_to_le32(lower_32_bits(r0)), \
> +		cpu_to_le32(lower_32_bits(r1)) & 0xffff, \
> +		cpu_to_le32(lower_32_bits(r1)) >> 16, \
> +		cpu_to_le32(lower_32_bits(r2)) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r2)) >> 8) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r2)) >> 16) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r2)) >> 24) & 0xff, \
> +		cpu_to_le32(lower_32_bits(r3)) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r3)) >> 8) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r3)) >> 16) & 0xff, \
> +		(cpu_to_le32(lower_32_bits(r3)) >> 24) & 0xff \
> +	)

I think this'd be clearer if we did something similar to what we did for
the SMCCC SOC_ID name:

  https://lore.kernel.org/linux-arm-kernel/20250219005932.3466-1-paul@os.amperecomputing.com/

... and pack/unpack the bytes explicitly, e.g.

| static inline uuid smccc_res_to_uuid(u32 r0, u32, r1, u32 r2, u32 r3)
| {
| 	uuid_t uuid = {
| 		.b = {
| 			[0]  = (r0 >> 0)  & 0xff,
| 			[1]  = (r0 >> 8)  & 0xff,
| 			[2]  = (r0 >> 16) & 0xff,
| 			[3]  = (r0 >> 24) & 0xff,
| 
| 			[4]  = (r1 >> 0)  & 0xff,
| 			[5]  = (r1 >> 8)  & 0xff,
| 			[6]  = (r1 >> 16) & 0xff,
| 			[7]  = (r1 >> 24) & 0xff,
| 
| 			[8]  = (r2 >> 0)  & 0xff,
| 			[9]  = (r2 >> 8)  & 0xff,
| 			[10] = (r2 >> 16) & 0xff,
| 			[11] = (r2 >> 24) & 0xff,
| 
| 			[12] = (r3 >> 0)  & 0xff,
| 			[13] = (r3 >> 8)  & 0xff,
| 			[14] = (r3 >> 16) & 0xff,
| 			[15] = (r3 >> 24) & 0xff,
| 		},
| 	};
| 
| 	return uuid;
| }

... which is a bit more verbose, but clearly aligns with what the SMCCC
spec says w.r.t. packing/unpacking, and should avoid warnings about
endianness conversions.

> +
> +#define UUID_TO_SMCCC_RES(uuid_init, regs) do { \
> +		const uuid_t uuid = uuid_init; \
> +		(regs)[0] = le32_to_cpu((u32)uuid.b[0] | (uuid.b[1] << 8) | \
> +						((uuid.b[2]) << 16) | ((uuid.b[3]) << 24)); \
> +		(regs)[1] = le32_to_cpu((u32)uuid.b[4] | (uuid.b[5] << 8) | \
> +						((uuid.b[6]) << 16) | ((uuid.b[7]) << 24)); \
> +		(regs)[2] = le32_to_cpu((u32)uuid.b[8] | (uuid.b[9] << 8) | \
> +						((uuid.b[10]) << 16) | ((uuid.b[11]) << 24)); \
> +		(regs)[3] = le32_to_cpu((u32)uuid.b[12] | (uuid.b[13] << 8) | \
> +						((uuid.b[14]) << 16) | ((uuid.b[15]) << 24)); \
> +	} while (0)
> +
> +#endif /* !__ASSEMBLER__ */

IMO it'd be clearer to initialise a uuid_t beforehand, and then allow
the helper to unpack the bytes, e.g.

static inline u32 smccc_uuid_to_reg(const uuid_t uuid, int reg)
{
	u32 val = 0;

	val |= (u32)(uuid.b[4 * reg + 0] << 0)
	val |= (u32)(uuid.b[4 * reg + 1] << 8)
	val |= (u32)(uuid.b[4 * reg + 2] << 16)
	val |= (u32)(uuid.b[4 * reg + 3] << 24)

	return val:
}

#define UUID_TO_SMCCC_RES(uuid, regs)		\
do {						\
	(regs)[0] = smccc_uuid_to_reg(uuid, 0); \
	(regs)[1] = smccc_uuid_to_reg(uuid, 1); \
	(regs)[2] = smccc_uuid_to_reg(uuid, 2); \
	(regs)[3] = smccc_uuid_to_reg(uuid, 3); \
} while (0)

... though arguably at that point you can get rid of the
UUID_TO_SMCCC_RES() macro and just expand that directly at the callsite.

Mark.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect hypervisor presence
  2025-03-15  0:19 ` [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect " Roman Kisel
@ 2025-03-17 11:37   ` Mark Rutland
  2025-03-17 17:12     ` Roman Kisel
  2025-03-19 22:11     ` Michael Kelley
  0 siblings, 2 replies; 29+ messages in thread
From: Mark Rutland @ 2025-03-17 11:37 UTC (permalink / raw)
  To: Roman Kisel
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, maz, mingo, oliver.upton,
	rafael, robh, ssengar, sudeep.holla, suzuki.poulose, tglx,
	wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut

On Fri, Mar 14, 2025 at 05:19:22PM -0700, Roman Kisel wrote:
> The arm64 Hyper-V startup path relies on ACPI to detect
> running under a Hyper-V compatible hypervisor. That
> doesn't work on non-ACPI systems.
> 
> Hoist the ACPI detection logic into a separate function. Then
> use the vendor-specific hypervisor service call (implemented
> recently in Hyper-V) via SMCCC in the non-ACPI case.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> ---
>  arch/arm64/hyperv/mshyperv.c | 43 +++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index 2265ea5ce5ad..c5b03d3af7c5 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -27,6 +27,41 @@ int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
>  	return 0;
>  }
>  
> +static bool __init hyperv_detect_via_acpi(void)
> +{
> +	if (acpi_disabled)
> +		return false;
> +#if IS_ENABLED(CONFIG_ACPI)
> +	/*
> +	 * Hypervisor ID is only available in ACPI v6+, and the
> +	 * structure layout was extended in v6 to accommodate that
> +	 * new field.
> +	 *
> +	 * At the very minimum, this check makes sure not to read
> +	 * past the FADT structure.
> +	 *
> +	 * It is also needed to catch running in some unknown
> +	 * non-Hyper-V environment that has ACPI 5.x or less.
> +	 * In such a case, it can't be Hyper-V.
> +	 */
> +	if (acpi_gbl_FADT.header.revision < 6)
> +		return false;
> +	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
> +#else
> +	return false;
> +#endif
> +}
> +

The 'acpi_disabled' variable doesn't exist for !CONFIG_ACPI, so its use
prior to the ifdeffery looks misplaced.

Usual codestyle is to avoid ifdeffery if possible, using IS_ENABLED().
Otherwise, use a stub, e.g.

| #ifdef CONFIG_ACPI
| static bool __init hyperv_detect_via_acpi(void)
| {
| 	if (acpi_disabled)
| 		return false;
| 	
| 	if (acpi_gbl_FADT.header.revision < 6)
| 		return false;
| 	
| 	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
| }
| #else
| static inline bool hyperv_detect_via_acpi(void) { return false; }
| #endif

Mark.

> +static bool __init hyperv_detect_via_smccc(void)
> +{
> +	uuid_t hyperv_uuid = UUID_INIT(
> +		0x4d32ba58, 0x4764, 0xcd24,
> +		0x75, 0x6c, 0xef, 0x8e,
> +		0x24, 0x70, 0x59, 0x16);
> +
> +	return arm_smccc_hyp_present(&hyperv_uuid);
> +}
> +
>  static int __init hyperv_init(void)
>  {
>  	struct hv_get_vp_registers_output	result;
> @@ -35,13 +70,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_via_acpi() && !hyperv_detect_via_smccc())
>  		return 0;
>  
>  	/* Setup the guest ID */
> -- 
> 2.43.0
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
  2025-03-15 18:49   ` Bjorn Helgaas
@ 2025-03-17 17:07     ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-17 17:07 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut



On 3/15/2025 11:49 AM, Bjorn Helgaas wrote:
> On Fri, Mar 14, 2025 at 05:19:31PM -0700, Roman Kisel wrote:
>> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
>> arm64. It won't be able to do that in the VTL mode where only DeviceTree
>> can be used.
>>
>> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
>> case, too.
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Looks good to me; trivial whitespace comment below.
> 

Thanks a bunch!! I'll fix that the whitespace (and remove the unused
variable the robot noticed).

>> ---
>>   drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
>>   1 file changed, 67 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
>> index 6084b38bdda1..cbff19e8a07c 100644
>> --- a/drivers/pci/controller/pci-hyperv.c
>> +++ b/drivers/pci/controller/pci-hyperv.c
>> @@ -50,6 +50,7 @@
>>   #include <linux/irqdomain.h>
>>   #include <linux/acpi.h>
>>   #include <linux/sizes.h>
>> +#include <linux/of_irq.h>
>>   #include <asm/mshyperv.h>
>>   
>>   /*
>> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
>>   	int ret;
>>   
>>   	fwspec.fwnode = domain->parent->fwnode;
>> -	fwspec.param_count = 2;
>> -	fwspec.param[0] = hwirq;
>> -	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
>> +	if (is_of_node(fwspec.fwnode)) {
>> +		/* SPI lines for OF translations start at offset 32 */
>> +		fwspec.param_count = 3;
>> +		fwspec.param[0] = 0;
>> +		fwspec.param[1] = hwirq - 32;
>> +		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
>> +	} else {
>> +		fwspec.param_count = 2;
>> +		fwspec.param[0] = hwirq;
>> +		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
>> +	}
>>   
>>   	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>>   	if (ret)
>> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
>>   	.activate = hv_pci_vec_irq_domain_activate,
>>   };
>>   
>> +#ifdef CONFIG_OF
>> +
>> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
>> +{
>> +	struct device_node *parent;
>> +	struct irq_domain *domain;
>> +
>> +	parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
>> +	if (!parent)
>> +		return NULL;
>> +	domain = irq_find_host(parent);
>> +	of_node_put(parent);
>> +
>> +	return domain;
>> +}
>> +
>> +#endif
>> +
>> +#ifdef CONFIG_ACPI
>> +
>> +static struct irq_domain *hv_pci_acpi_irq_domain_parent(void)
>> +{
>> +	struct irq_domain *domain;
>> +	acpi_gsi_domain_disp_fn gsi_domain_disp_fn;
>> +
>> +	if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
>> +		return NULL;
>> +	gsi_domain_disp_fn = acpi_get_gsi_dispatcher();
>> +	if (!gsi_domain_disp_fn)
>> +		return NULL;
>> +	return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
>> +				     DOMAIN_BUS_ANY);
>> +}
>> +
>> +#endif
>> +
>>   static int hv_pci_irqchip_init(void)
>>   {
>>   	static struct hv_pci_chip_data *chip_data;
>>   	struct fwnode_handle *fn = NULL;
>> +	struct irq_domain *irq_domain_parent = NULL;
>>   	int ret = -ENOMEM;
>>   
>>   	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
>> @@ -907,9 +953,24 @@ 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);
>> +#ifdef CONFIG_ACPI
>> +	if (!acpi_disabled)
>> +		irq_domain_parent = hv_pci_acpi_irq_domain_parent();
>> +#endif
>> +#if defined(CONFIG_OF)
>> +	if (!irq_domain_parent)
>> +		irq_domain_parent = hv_pci_of_irq_domain_parent();
>> +#endif
>> +	if (!irq_domain_parent) {
>> +		WARN_ONCE(1, "Invalid firmware configuration for VMBus interrupts\n");
>> +		ret = -EINVAL;
>> +		goto free_chip;
>> +	}
>> +
>> +	hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
>> +		irq_domain_parent, 0, HV_PCI_MSI_SPI_NR,
>> +		fn, &hv_pci_domain_ops,
>> +		chip_data);
> 
> This is a different style of indenting the parameters than other
> similar cases in this file, which line up parameters on subsequent
> lines under the open parenthesis.
> 
>>   	if (!hv_msi_gic_irq_domain) {
>>   		pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ domain\n");
>> -- 
>> 2.43.0
>>

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties
  2025-03-16 17:36   ` Krzysztof Kozlowski
@ 2025-03-17 17:07     ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-17 17:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut



On 3/16/2025 10:36 AM, Krzysztof Kozlowski wrote:
> On Fri, Mar 14, 2025 at 05:19:27PM -0700, Roman Kisel wrote:
>>   
>> +  dma-coherent: true
>> +
>> +  interrupts:
> 
>> +    maxItems: 1
>> +    description: |
>> +      This interrupt is used to report a message from the host.
> 
> These could be just two lines:
> 
> items:
>    - description: Interrupt used to report a message from the host.
> 
> (and note that just like we do not use "This" in commit msg, there is
> really no benefit of using it in hardware descruption for simple
> statements)
> 

Appreciate your help very much!! Will fix the formatting.

> Regardless:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
> 

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence
  2025-03-17 11:29   ` Mark Rutland
@ 2025-03-17 17:11     ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-17 17:11 UTC (permalink / raw)
  To: Mark Rutland
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, maz, mingo, oliver.upton,
	rafael, robh, ssengar, sudeep.holla, suzuki.poulose, tglx,
	wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut



On 3/17/2025 4:29 AM, Mark Rutland wrote:
> On Fri, Mar 14, 2025 at 05:19:21PM -0700, Roman Kisel wrote:

[...]

>> +}
> 
> This use of a statement expression is bizarre, and the function would be
> clearer without it, e.g.

I'll change that to what you're suggesting, thanks for your help!

> 
> | bool arm_smccc_hyp_present(const uuid_t *hyp_uuid)
> | {
> | 	struct arm_smccc_res res = {};
> | 	uuid_t uuid;
> |
> | 	if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
> | 		return false;
> |
> | 	arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> | 	if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> | 		return false;
> | 	
> | 	uuid_t = SMCCC_RES_TO_UUID(res.a0, res.a1, res.a2, res.a3);
> | 	return uuid_equal(&uuid, hyp_uuid);
> | }
> 
> As noted below, I'd prefer if this were renamed to something like
> arm_smccc_hypervisor_has_uuid(), to more clearly indicate what is being
> checked.
> 
> [...]

Will update, thanks for your help!

> 
>> +/**
>> + * arm_smccc_hyp_present(const uuid_t *hyp_uuid)
>> + *
>> + * Returns `true` if the hypervisor advertises its presence via SMCCC.
>> + *
>> + * When the function returns `false`, the caller shall not assume that
>> + * there is no hypervisor running. Instead, the caller must fall back to
>> + * other approaches if any are available.
>> + */
>> +bool arm_smccc_hyp_present(const uuid_t *hyp_uuid);
> 
> I'd prefer if this were:
> 
> | /*
> |  * Returns whether a specific hypervisor UUID is advertised for the
> |  * Vendor Specific Hypervisor Service range.
> |  */
> | bool arm_smccc_hypervisor_has_uuid(const uuid_t *uuid);
> 
[...]



> 
> I think this'd be clearer if we did something similar to what we did for
> the SMCCC SOC_ID name:
> 
>    https://lore.kernel.org/linux-arm-kernel/20250219005932.3466-1-paul@os.amperecomputing.com/
> 
> ... and pack/unpack the bytes explicitly, e.g.

That looks great, thanks for the suggestion!

> 
> | static inline uuid smccc_res_to_uuid(u32 r0, u32, r1, u32 r2, u32 r3)
> | {
> | 	uuid_t uuid = {
> | 		.b = {
> | 			[0]  = (r0 >> 0)  & 0xff,
> | 			[1]  = (r0 >> 8)  & 0xff,
> | 			[2]  = (r0 >> 16) & 0xff,
> | 			[3]  = (r0 >> 24) & 0xff,
> |
> | 			[4]  = (r1 >> 0)  & 0xff,
> | 			[5]  = (r1 >> 8)  & 0xff,
> | 			[6]  = (r1 >> 16) & 0xff,
> | 			[7]  = (r1 >> 24) & 0xff,
> |
> | 			[8]  = (r2 >> 0)  & 0xff,
> | 			[9]  = (r2 >> 8)  & 0xff,
> | 			[10] = (r2 >> 16) & 0xff,
> | 			[11] = (r2 >> 24) & 0xff,
> |
> | 			[12] = (r3 >> 0)  & 0xff,
> | 			[13] = (r3 >> 8)  & 0xff,
> | 			[14] = (r3 >> 16) & 0xff,
> | 			[15] = (r3 >> 24) & 0xff,
> | 		},
> | 	};
> |
> | 	return uuid;
> | }
> 
> ... which is a bit more verbose, but clearly aligns with what the SMCCC
> spec says w.r.t. packing/unpacking, and should avoid warnings about
> endianness conversions.
> 

I believe what you're proposing hits a better trade-off, thanks again!

>> +
>> +#define UUID_TO_SMCCC_RES(uuid_init, regs) do { \
>> +		const uuid_t uuid = uuid_init; \
>> +		(regs)[0] = le32_to_cpu((u32)uuid.b[0] | (uuid.b[1] << 8) | \
>> +						((uuid.b[2]) << 16) | ((uuid.b[3]) << 24)); \
>> +		(regs)[1] = le32_to_cpu((u32)uuid.b[4] | (uuid.b[5] << 8) | \
>> +						((uuid.b[6]) << 16) | ((uuid.b[7]) << 24)); \
>> +		(regs)[2] = le32_to_cpu((u32)uuid.b[8] | (uuid.b[9] << 8) | \
>> +						((uuid.b[10]) << 16) | ((uuid.b[11]) << 24)); \
>> +		(regs)[3] = le32_to_cpu((u32)uuid.b[12] | (uuid.b[13] << 8) | \
>> +						((uuid.b[14]) << 16) | ((uuid.b[15]) << 24)); \
>> +	} while (0)
>> +
>> +#endif /* !__ASSEMBLER__ */
> 
> IMO it'd be clearer to initialise a uuid_t beforehand, and then allow
> the helper to unpack the bytes, e.g.
> 
> static inline u32 smccc_uuid_to_reg(const uuid_t uuid, int reg)
> {
> 	u32 val = 0;
> 
> 	val |= (u32)(uuid.b[4 * reg + 0] << 0)
> 	val |= (u32)(uuid.b[4 * reg + 1] << 8)
> 	val |= (u32)(uuid.b[4 * reg + 2] << 16)
> 	val |= (u32)(uuid.b[4 * reg + 3] << 24)
> 
> 	return val:
> }
> 
> #define UUID_TO_SMCCC_RES(uuid, regs)		\
> do {						\
> 	(regs)[0] = smccc_uuid_to_reg(uuid, 0); \
> 	(regs)[1] = smccc_uuid_to_reg(uuid, 1); \
> 	(regs)[2] = smccc_uuid_to_reg(uuid, 2); \
> 	(regs)[3] = smccc_uuid_to_reg(uuid, 3); \
> } while (0)
> 
> ... though arguably at that point you can get rid of the
> UUID_TO_SMCCC_RES() macro and just expand that directly at the callsite.
> 

I'll work on that, thanks!!

> Mark.

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect hypervisor presence
  2025-03-17 11:37   ` Mark Rutland
@ 2025-03-17 17:12     ` Roman Kisel
  2025-03-19 22:11     ` Michael Kelley
  1 sibling, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-17 17:12 UTC (permalink / raw)
  To: Mark Rutland
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, maz, mingo, oliver.upton,
	rafael, robh, ssengar, sudeep.holla, suzuki.poulose, tglx,
	wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut



On 3/17/2025 4:37 AM, Mark Rutland wrote:
> On Fri, Mar 14, 2025 at 05:19:22PM -0700, Roman Kisel wrote:

[...]

> The 'acpi_disabled' variable doesn't exist for !CONFIG_ACPI, so its use
> prior to the ifdeffery looks misplaced.
> 
> Usual codestyle is to avoid ifdeffery if possible, using IS_ENABLED().
> Otherwise, use a stub, e.g.
> 

That looks much better, thanks for the review! Will implement in
the next version.

> | #ifdef CONFIG_ACPI
> | static bool __init hyperv_detect_via_acpi(void)
> | {
> | 	if (acpi_disabled)
> | 		return false;
> | 	
> | 	if (acpi_gbl_FADT.header.revision < 6)
> | 		return false;
> | 	
> | 	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
> | }
> | #else
> | static inline bool hyperv_detect_via_acpi(void) { return false; }
> | #endif
> 
> Mark.
> 
>> +static bool __init hyperv_detect_via_smccc(void)
>> +{
>> +	uuid_t hyperv_uuid = UUID_INIT(
>> +		0x4d32ba58, 0x4764, 0xcd24,
>> +		0x75, 0x6c, 0xef, 0x8e,
>> +		0x24, 0x70, 0x59, 0x16);
>> +
>> +	return arm_smccc_hyp_present(&hyperv_uuid);
>> +}
>> +
>>   static int __init hyperv_init(void)
>>   {
>>   	struct hv_get_vp_registers_output	result;
>> @@ -35,13 +70,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_via_acpi() && !hyperv_detect_via_smccc())
>>   		return 0;
>>   
>>   	/* Setup the guest ID */
>> -- 
>> 2.43.0
>>

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence
  2025-03-15 13:12     ` Arnd Bergmann
@ 2025-03-17 17:28       ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-17 17:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: apais, benhill, bperkins, sunilmut, bhelgaas, Borislav Petkov,
	Catalin Marinas, Conor Dooley, Dan Carpenter, Dave Hansen,
	Dexuan Cui, Haiyang Zhang, H. Peter Anvin, Joey Gouly, krzk+dt,
	Krzysztof Wilczyński, K. Y. Srinivasan, Len Brown,
	Lorenzo Pieralisi, Manivannan Sadhasivam, Mark Rutland,
	Marc Zyngier, Ingo Molnar, Oliver Upton, Rafael J . Wysocki,
	Rob Herring, ssengar, Sudeep Holla, Suzuki K Poulose,
	Thomas Gleixner, Wei Liu, Will Deacon, Zenghui Yu, devicetree,
	kvmarm, linux-acpi, Linux-Arch, linux-arm-kernel, linux-hyperv,
	linux-kernel, linux-pci, x86



On 3/15/2025 6:12 AM, Arnd Bergmann wrote:
> On Sat, Mar 15, 2025, at 01:27, Roman Kisel wrote:
>> On 3/14/2025 5:19 PM, Roman Kisel wrote:
>>
>> While the change is Acked, here is the caveat maybe.
>>
>> This patch produces warnings wtih sparse and CHECK_ENDING.
>> That said, the kernel build produces a whole lot more other warnings
>> from building with sparse by itself and/or with CHECK_ENDING.
>>
>> I am not sure how to proceed with that, thinking I should
>> not add warnings yet at the same time there are many others.
>> Not certain if folks take these signals as fyi or blockers.
> 
> It would be best not to add warnigns. There is slow progress on
> fixing all the endianess warnings and I hope this can eventually
> complete, so even if there are many existing ones please try to
> keep new code clean.
> 

Will fix the warnings, appreciate providing the guidance!

>       Arnd

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* RE: [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect hypervisor presence
  2025-03-17 11:37   ` Mark Rutland
  2025-03-17 17:12     ` Roman Kisel
@ 2025-03-19 22:11     ` Michael Kelley
  2025-03-20 18:47       ` Roman Kisel
  1 sibling, 1 reply; 29+ messages in thread
From: Michael Kelley @ 2025-03-19 22:11 UTC (permalink / raw)
  To: Mark Rutland, Roman Kisel
  Cc: arnd@arndb.de, bhelgaas@google.com, bp@alien8.de,
	catalin.marinas@arm.com, conor+dt@kernel.org,
	dan.carpenter@linaro.org, dave.hansen@linux.intel.com,
	decui@microsoft.com, haiyangz@microsoft.com, hpa@zytor.com,
	joey.gouly@arm.com, krzk+dt@kernel.org, kw@linux.com,
	kys@microsoft.com, lenb@kernel.org, lpieralisi@kernel.org,
	manivannan.sadhasivam@linaro.org, maz@kernel.org,
	mingo@redhat.com, oliver.upton@linux.dev, rafael@kernel.org,
	robh@kernel.org, ssengar@linux.microsoft.com,
	sudeep.holla@arm.com, suzuki.poulose@arm.com, tglx@linutronix.de,
	wei.liu@kernel.org, will@kernel.org, yuzenghui@huawei.com,
	devicetree@vger.kernel.org, kvmarm@lists.linux.dev,
	linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, x86@kernel.org, apais@microsoft.com,
	benhill@microsoft.com, bperkins@microsoft.com,
	sunilmut@microsoft.com

From: Mark Rutland <mark.rutland@arm.com> Sent: Monday, March 17, 2025 4:38 AM
> 
> On Fri, Mar 14, 2025 at 05:19:22PM -0700, Roman Kisel wrote:
> > The arm64 Hyper-V startup path relies on ACPI to detect
> > running under a Hyper-V compatible hypervisor. That
> > doesn't work on non-ACPI systems.
> >
> > Hoist the ACPI detection logic into a separate function. Then
> > use the vendor-specific hypervisor service call (implemented
> > recently in Hyper-V) via SMCCC in the non-ACPI case.
> >
> > Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> > ---
> >  arch/arm64/hyperv/mshyperv.c | 43 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 38 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> > index 2265ea5ce5ad..c5b03d3af7c5 100644
> > --- a/arch/arm64/hyperv/mshyperv.c
> > +++ b/arch/arm64/hyperv/mshyperv.c
> > @@ -27,6 +27,41 @@ int hv_get_hypervisor_version(union
> hv_hypervisor_version_info *info)
> >  	return 0;
> >  }
> >
> > +static bool __init hyperv_detect_via_acpi(void)
> > +{
> > +	if (acpi_disabled)
> > +		return false;
> > +#if IS_ENABLED(CONFIG_ACPI)
> > +	/*
> > +	 * Hypervisor ID is only available in ACPI v6+, and the
> > +	 * structure layout was extended in v6 to accommodate that
> > +	 * new field.
> > +	 *
> > +	 * At the very minimum, this check makes sure not to read
> > +	 * past the FADT structure.
> > +	 *
> > +	 * It is also needed to catch running in some unknown
> > +	 * non-Hyper-V environment that has ACPI 5.x or less.
> > +	 * In such a case, it can't be Hyper-V.
> > +	 */
> > +	if (acpi_gbl_FADT.header.revision < 6)
> > +		return false;
> > +	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
> > +#else
> > +	return false;
> > +#endif
> > +}
> > +
> 
> The 'acpi_disabled' variable doesn't exist for !CONFIG_ACPI, so its use
> prior to the ifdeffery looks misplaced.

FWIW, include/linux/acpi.h has 

#define acpi_disabled 1

when !CONFIG_ACPI.  But I agree that using a stub is better.

Michael

> 
> Usual codestyle is to avoid ifdeffery if possible, using IS_ENABLED().
> Otherwise, use a stub, e.g.
> 
> | #ifdef CONFIG_ACPI
> | static bool __init hyperv_detect_via_acpi(void)
> | {
> | 	if (acpi_disabled)
> | 		return false;
> |
> | 	if (acpi_gbl_FADT.header.revision < 6)
> | 		return false;
> |
> | 	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
> | }
> | #else
> | static inline bool hyperv_detect_via_acpi(void) { return false; }
> | #endif
> 
> Mark.
> 
> > +static bool __init hyperv_detect_via_smccc(void)
> > +{
> > +	uuid_t hyperv_uuid = UUID_INIT(
> > +		0x4d32ba58, 0x4764, 0xcd24,
> > +		0x75, 0x6c, 0xef, 0x8e,
> > +		0x24, 0x70, 0x59, 0x16);
> > +
> > +	return arm_smccc_hyp_present(&hyperv_uuid);
> > +}
> > +
> >  static int __init hyperv_init(void)
> >  {
> >  	struct hv_get_vp_registers_output	result;
> > @@ -35,13 +70,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_via_acpi() && !hyperv_detect_via_smccc())
> >  		return 0;
> >
> >  	/* Setup the guest ID */
> > --
> > 2.43.0
> >



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect hypervisor presence
  2025-03-19 22:11     ` Michael Kelley
@ 2025-03-20 18:47       ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-20 18:47 UTC (permalink / raw)
  To: Michael Kelley, Mark Rutland
  Cc: arnd@arndb.de, bhelgaas@google.com, bp@alien8.de,
	catalin.marinas@arm.com, conor+dt@kernel.org,
	dan.carpenter@linaro.org, dave.hansen@linux.intel.com,
	decui@microsoft.com, haiyangz@microsoft.com, hpa@zytor.com,
	joey.gouly@arm.com, krzk+dt@kernel.org, kw@linux.com,
	kys@microsoft.com, lenb@kernel.org, lpieralisi@kernel.org,
	manivannan.sadhasivam@linaro.org, maz@kernel.org,
	mingo@redhat.com, oliver.upton@linux.dev, rafael@kernel.org,
	robh@kernel.org, ssengar@linux.microsoft.com,
	sudeep.holla@arm.com, suzuki.poulose@arm.com, tglx@linutronix.de,
	wei.liu@kernel.org, will@kernel.org, yuzenghui@huawei.com,
	devicetree@vger.kernel.org, kvmarm@lists.linux.dev,
	linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, x86@kernel.org, apais@microsoft.com,
	benhill@microsoft.com, bperkins@microsoft.com,
	sunilmut@microsoft.com



On 3/19/2025 3:11 PM, Michael Kelley wrote:
> From: Mark Rutland <mark.rutland@arm.com> Sent: Monday, March 17, 2025 4:38 AM

>>
>> The 'acpi_disabled' variable doesn't exist for !CONFIG_ACPI, so its use
>> prior to the ifdeffery looks misplaced.
> 
> FWIW, include/linux/acpi.h has
> 
> #define acpi_disabled 1
> 
> when !CONFIG_ACPI.  But I agree that using a stub is better.

Thanks, Michael! I didn't "fact-checked" what was suggested indeed.
Agreed that the stub makes the code look better.

> 
> Michael
> 
>>
>> Usual codestyle is to avoid ifdeffery if possible, using IS_ENABLED().
>> Otherwise, use a stub, e.g.
>>
>> | #ifdef CONFIG_ACPI
>> | static bool __init hyperv_detect_via_acpi(void)
>> | {
>> | 	if (acpi_disabled)
>> | 		return false;
>> |
>> | 	if (acpi_gbl_FADT.header.revision < 6)
>> | 		return false;
>> |
>> | 	return strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8) == 0;
>> | }
>> | #else
>> | static inline bool hyperv_detect_via_acpi(void) { return false; }
>> | #endif
>>
>> Mark.
>>
>>> +static bool __init hyperv_detect_via_smccc(void)
>>> +{
>>> +	uuid_t hyperv_uuid = UUID_INIT(
>>> +		0x4d32ba58, 0x4764, 0xcd24,
>>> +		0x75, 0x6c, 0xef, 0x8e,
>>> +		0x24, 0x70, 0x59, 0x16);
>>> +
>>> +	return arm_smccc_hyp_present(&hyperv_uuid);
>>> +}
>>> +
>>>   static int __init hyperv_init(void)
>>>   {
>>>   	struct hv_get_vp_registers_output	result;
>>> @@ -35,13 +70,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_via_acpi() && !hyperv_detect_via_smccc())
>>>   		return 0;
>>>
>>>   	/* Setup the guest ID */
>>> --
>>> 2.43.0
>>>
> 

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher()
  2025-03-15  0:19 ` [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher() Roman Kisel
@ 2025-03-26 14:55   ` Rafael J. Wysocki
  2025-03-26 15:32     ` Roman Kisel
  0 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2025-03-26 14:55 UTC (permalink / raw)
  To: Roman Kisel
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut

On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> Using acpi_irq_create_hierarchy() in the cases where the code
> also handles OF leads to code duplication as the ACPI subsystem
> doesn't provide means to compute the IRQ domain parent whereas
> the OF does.
>
> Introduce acpi_get_gsi_dispatcher() so that the drivers relying
> on both ACPI and OF may use irq_domain_create_hierarchy() in the
> common code paths.
>
> No functional changes.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>

This basically looks OK to me except for a couple of coding style
related nits below.

> ---
>  drivers/acpi/irq.c   | 15 +++++++++++++--
>  include/linux/acpi.h |  5 ++++-
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index 1687483ff319..8eb09e45e5c5 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -12,7 +12,7 @@
>
>  enum acpi_irq_model_id acpi_irq_model;
>
> -static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
> +static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
>  static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
>
>  /**
> @@ -307,12 +307,23 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
>   *     for a given GSI
>   */
>  void __init acpi_set_irq_model(enum acpi_irq_model_id model,
> -                              struct fwnode_handle *(*fn)(u32))

Please retain the indentation here and analogously below.

> +       acpi_gsi_domain_disp_fn fn)
>  {
>         acpi_irq_model = model;
>         acpi_get_gsi_domain_id = fn;
>  }
>
> +/**
> + * acpi_get_gsi_dispatcher - Returns dispatcher function that
> + *                           computes the domain fwnode for a
> + *                           given GSI.
> + */

I would format this kerneldoc comment a bit differently:

/*
 * acpi_get_gsi_dispatcher() - Get the GSI dispatcher function
 *
 * Return the dispatcher function that computes the domain fwnode for
a given GSI.
 */

> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
> +{
> +       return acpi_get_gsi_domain_id;
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
> +
>  /**
>   * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
>   * callback to fallback to arch specified implementation.
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4e495b29c640..abc51288e867 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
>  int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
>  int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>
> +typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
> +
>  void acpi_set_irq_model(enum acpi_irq_model_id model,
> -                       struct fwnode_handle *(*)(u32));
> +       acpi_gsi_domain_disp_fn fn);
> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
>  void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
>
>  struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> --

With the above addressed, please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

to the patch and route it along with the rest of the series.

Thanks!


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
  2025-03-15  0:19 ` [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree Roman Kisel
  2025-03-15 18:49   ` Bjorn Helgaas
@ 2025-03-26 14:56   ` Rafael J. Wysocki
  2025-03-26 15:37     ` Roman Kisel
  1 sibling, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2025-03-26 14:56 UTC (permalink / raw)
  To: Roman Kisel
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, rafael, robh, ssengar, sudeep.holla, suzuki.poulose,
	tglx, wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut

On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
> arm64. It won't be able to do that in the VTL mode where only DeviceTree
> can be used.
>
> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
> case, too.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
>  1 file changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 6084b38bdda1..cbff19e8a07c 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -50,6 +50,7 @@
>  #include <linux/irqdomain.h>
>  #include <linux/acpi.h>
>  #include <linux/sizes.h>
> +#include <linux/of_irq.h>
>  #include <asm/mshyperv.h>
>
>  /*
> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
>         int ret;
>
>         fwspec.fwnode = domain->parent->fwnode;
> -       fwspec.param_count = 2;
> -       fwspec.param[0] = hwirq;
> -       fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +       if (is_of_node(fwspec.fwnode)) {
> +               /* SPI lines for OF translations start at offset 32 */
> +               fwspec.param_count = 3;
> +               fwspec.param[0] = 0;
> +               fwspec.param[1] = hwirq - 32;
> +               fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +       } else {
> +               fwspec.param_count = 2;
> +               fwspec.param[0] = hwirq;
> +               fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +       }
>
>         ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>         if (ret)
> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
>         .activate = hv_pci_vec_irq_domain_activate,
>  };
>
> +#ifdef CONFIG_OF
> +
> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
> +{
> +       struct device_node *parent;
> +       struct irq_domain *domain;
> +
> +       parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
> +       if (!parent)
> +               return NULL;
> +       domain = irq_find_host(parent);
> +       of_node_put(parent);
> +
> +       return domain;
> +}
> +
> +#endif
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct irq_domain *hv_pci_acpi_irq_domain_parent(void)
> +{
> +       struct irq_domain *domain;
> +       acpi_gsi_domain_disp_fn gsi_domain_disp_fn;
> +
> +       if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
> +               return NULL;
> +       gsi_domain_disp_fn = acpi_get_gsi_dispatcher();
> +       if (!gsi_domain_disp_fn)
> +               return NULL;
> +       return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
> +                                    DOMAIN_BUS_ANY);
> +}
> +
> +#endif
> +
>  static int hv_pci_irqchip_init(void)
>  {
>         static struct hv_pci_chip_data *chip_data;
>         struct fwnode_handle *fn = NULL;
> +       struct irq_domain *irq_domain_parent = NULL;
>         int ret = -ENOMEM;
>
>         chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> @@ -907,9 +953,24 @@ 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);
> +#ifdef CONFIG_ACPI
> +       if (!acpi_disabled)
> +               irq_domain_parent = hv_pci_acpi_irq_domain_parent();
> +#endif
> +#if defined(CONFIG_OF)

Why don't you do

#ifdef CONFIG_OF

here for consistency?

> +       if (!irq_domain_parent)
> +               irq_domain_parent = hv_pci_of_irq_domain_parent();
> +#endif
> +       if (!irq_domain_parent) {
> +               WARN_ONCE(1, "Invalid firmware configuration for VMBus interrupts\n");
> +               ret = -EINVAL;
> +               goto free_chip;
> +       }
> +
> +       hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
> +               irq_domain_parent, 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");
> --


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher()
  2025-03-26 14:55   ` Rafael J. Wysocki
@ 2025-03-26 15:32     ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-26 15:32 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, robh, ssengar, sudeep.holla, suzuki.poulose, tglx,
	wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut



On 3/26/2025 7:55 AM, Rafael J. Wysocki wrote:
> On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:
[...]
> 
> This basically looks OK to me except for a couple of coding style
> related nits below.
> 

Appreciate taking time to review this very much! Will squash the nits in
the next version.

>> ---
>>   drivers/acpi/irq.c   | 15 +++++++++++++--
>>   include/linux/acpi.h |  5 ++++-
>>   2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>> index 1687483ff319..8eb09e45e5c5 100644
>> --- a/drivers/acpi/irq.c
>> +++ b/drivers/acpi/irq.c
>> @@ -12,7 +12,7 @@
>>
>>   enum acpi_irq_model_id acpi_irq_model;
>>
>> -static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
>> +static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
>>   static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
>>
>>   /**
>> @@ -307,12 +307,23 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
>>    *     for a given GSI
>>    */
>>   void __init acpi_set_irq_model(enum acpi_irq_model_id model,
>> -                              struct fwnode_handle *(*fn)(u32))
> 
> Please retain the indentation here and analogously below.
> 
>> +       acpi_gsi_domain_disp_fn fn)
>>   {
>>          acpi_irq_model = model;
>>          acpi_get_gsi_domain_id = fn;
>>   }
>>
>> +/**
>> + * acpi_get_gsi_dispatcher - Returns dispatcher function that
>> + *                           computes the domain fwnode for a
>> + *                           given GSI.
>> + */
> 
> I would format this kerneldoc comment a bit differently:
> 
> /*
>   * acpi_get_gsi_dispatcher() - Get the GSI dispatcher function
>   *
>   * Return the dispatcher function that computes the domain fwnode for
> a given GSI.
>   */
> 
>> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
>> +{
>> +       return acpi_get_gsi_domain_id;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
>> +
>>   /**
>>    * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
>>    * callback to fallback to arch specified implementation.
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 4e495b29c640..abc51288e867 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
>>   int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
>>   int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>>
>> +typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
>> +
>>   void acpi_set_irq_model(enum acpi_irq_model_id model,
>> -                       struct fwnode_handle *(*)(u32));
>> +       acpi_gsi_domain_disp_fn fn);
>> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
>>   void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
>>
>>   struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
>> --
> 
> With the above addressed, please feel free to add
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> to the patch and route it along with the rest of the series.
> 

Will do, thanks!

> Thanks!

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
  2025-03-26 14:56   ` Rafael J. Wysocki
@ 2025-03-26 15:37     ` Roman Kisel
  0 siblings, 0 replies; 29+ messages in thread
From: Roman Kisel @ 2025-03-26 15:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: arnd, bhelgaas, bp, catalin.marinas, conor+dt, dan.carpenter,
	dave.hansen, decui, haiyangz, hpa, joey.gouly, krzk+dt, kw, kys,
	lenb, lpieralisi, manivannan.sadhasivam, mark.rutland, maz, mingo,
	oliver.upton, robh, ssengar, sudeep.holla, suzuki.poulose, tglx,
	wei.liu, will, yuzenghui, devicetree, kvmarm, linux-acpi,
	linux-arch, linux-arm-kernel, linux-hyperv, linux-kernel,
	linux-pci, x86, apais, benhill, bperkins, sunilmut



On 3/26/2025 7:56 AM, Rafael J. Wysocki wrote:
> On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:

[...]

>> -                                                         chip_data);
>> +#ifdef CONFIG_ACPI
>> +       if (!acpi_disabled)
>> +               irq_domain_parent = hv_pci_acpi_irq_domain_parent();
>> +#endif
>> +#if defined(CONFIG_OF)
> 
> Why don't you do
> 
> #ifdef CONFIG_OF
> 
> here for consistency?
> 

Agree, that'd be easier on the eyes :) Will fix in the next version,
thanks for the suggestion!

>> +       if (!irq_domain_parent)
>> +               irq_domain_parent = hv_pci_of_irq_domain_parent();
>> +#endif
>> +       if (!irq_domain_parent) {
>> +               WARN_ONCE(1, "Invalid firmware configuration for VMBus interrupts\n");
>> +               ret = -EINVAL;
>> +               goto free_chip;
>> +       }
>> +
>> +       hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
>> +               irq_domain_parent, 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");
>> --

-- 
Thank you,
Roman



^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2025-03-26 16:02 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-15  0:19 [PATCH hyperv-next v6 00/11] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 01/11] arm64: kvm, smccc: Introduce and use API for detecting hypervisor presence Roman Kisel
2025-03-15  0:27   ` Roman Kisel
2025-03-15 13:12     ` Arnd Bergmann
2025-03-17 17:28       ` Roman Kisel
2025-03-17 11:29   ` Mark Rutland
2025-03-17 17:11     ` Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 02/11] arm64: hyperv: Use SMCCC to detect " Roman Kisel
2025-03-17 11:37   ` Mark Rutland
2025-03-17 17:12     ` Roman Kisel
2025-03-19 22:11     ` Michael Kelley
2025-03-20 18:47       ` Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 03/11] Drivers: hv: Enable VTL mode for arm64 Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 04/11] Drivers: hv: Provide arch-neutral implementation of get_vtl() Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 05/11] arm64: hyperv: Initialize the Virtual Trust Level field Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 06/11] arm64, x86: hyperv: Report the VTL the system boots in Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 07/11] dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties Roman Kisel
2025-03-16 17:36   ` Krzysztof Kozlowski
2025-03-17 17:07     ` Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 08/11] Drivers: hv: vmbus: Get the IRQ number from DeviceTree Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 09/11] Drivers: hv: vmbus: Introduce hv_get_vmbus_root_device() Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher() Roman Kisel
2025-03-26 14:55   ` Rafael J. Wysocki
2025-03-26 15:32     ` Roman Kisel
2025-03-15  0:19 ` [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree Roman Kisel
2025-03-15 18:49   ` Bjorn Helgaas
2025-03-17 17:07     ` Roman Kisel
2025-03-26 14:56   ` Rafael J. Wysocki
2025-03-26 15:37     ` Roman Kisel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).