KVM: arm64: Force HCR_EL2.xMO to 1 at all times in VHE mode
authorMarc Zyngier <maz@kernel.org>
Tue, 29 Apr 2025 11:43:26 +0000 (12:43 +0100)
committerMarc Zyngier <maz@kernel.org>
Tue, 6 May 2025 08:41:32 +0000 (09:41 +0100)
We keep setting and clearing these bits depending on the role of
the host kernel, mimicking what we do for nVHE. But that's actually
pretty pointless, as we always want physical interrupts to make it
to the host, at EL2.

This has also two problems:

- it prevents IRQs from being taken when these bits are cleared
  if the implementation has chosen to implement these bits as
  masks when HCR_EL2.{TGE,xMO}=={0,0}

- it triggers a bad erratum on the AmpereOne HW, which catches
  fire on clearing these bits while an interrupt is being taken
  (AC03_CPU_36).

Let's kill these two birds with a single stone, and permanently
set the xMO bits when running VHE. This involves a bit of surgery
on code paths that rely on flipping these bits on and off for
other purposes.

Note that the earliest setting of hcr_el2 (in the init_hcr_el2
macro) is left untouched as is runs extremely early, with interrupts
disabled, and soon enough overwritten with the final value containing
the xMO bits.

Reported-by: D Scott Phillips <scott@os.amperecomputing.com>
Link: https://lore.kernel.org/r/20250429114326.3618875-1-maz@kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
arch/arm64/include/asm/kvm_arm.h
arch/arm64/kvm/hyp/vgic-v3-sr.c

index 974d72b5905b8616f6422f1d0cfc2c7e853c9fe8..bba4b0e930915410dcc82b68216f2ea067abc40e 100644 (file)
                         HCR_FMO | HCR_IMO | HCR_PTW | HCR_TID3 | HCR_TID1)
 #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA)
 #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
-#define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)
+#define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H | HCR_AMO | HCR_IMO | HCR_FMO)
 
 #define HCRX_HOST_FLAGS (HCRX_EL2_MSCEn | HCRX_EL2_TCR2En | HCRX_EL2_EnFPM)
 #define MPAMHCR_HOST_FLAGS     0
index ed363aa3027e59691d9b139ab104ce1712f443b7..50aa8dbcae75b059cfbef93ff19adc27e3f9277b 100644 (file)
@@ -429,23 +429,27 @@ u64 __vgic_v3_get_gic_config(void)
        /*
         * To check whether we have a MMIO-based (GICv2 compatible)
         * CPU interface, we need to disable the system register
-        * view. To do that safely, we have to prevent any interrupt
-        * from firing (which would be deadly).
+        * view.
         *
-        * Note that this only makes sense on VHE, as interrupts are
-        * already masked for nVHE as part of the exception entry to
-        * EL2.
-        */
-       if (has_vhe())
-               flags = local_daif_save();
-
-       /*
         * Table 11-2 "Permitted ICC_SRE_ELx.SRE settings" indicates
         * that to be able to set ICC_SRE_EL1.SRE to 0, all the
         * interrupt overrides must be set. You've got to love this.
+        *
+        * As we always run VHE with HCR_xMO set, no extra xMO
+        * manipulation is required in that case.
+        *
+        * To safely disable SRE, we have to prevent any interrupt
+        * from firing (which would be deadly). This only makes sense
+        * on VHE, as interrupts are already masked for nVHE as part
+        * of the exception entry to EL2.
         */
-       sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
-       isb();
+       if (has_vhe()) {
+               flags = local_daif_save();
+       } else {
+               sysreg_clear_set(hcr_el2, 0, HCR_AMO | HCR_FMO | HCR_IMO);
+               isb();
+       }
+
        write_gicreg(0, ICC_SRE_EL1);
        isb();
 
@@ -453,11 +457,13 @@ u64 __vgic_v3_get_gic_config(void)
 
        write_gicreg(sre, ICC_SRE_EL1);
        isb();
-       sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
-       isb();
 
-       if (has_vhe())
+       if (has_vhe()) {
                local_daif_restore(flags);
+       } else {
+               sysreg_clear_set(hcr_el2, HCR_AMO | HCR_FMO | HCR_IMO, 0);
+               isb();
+       }
 
        val  = (val & ICC_SRE_EL1_SRE) ? 0 : (1ULL << 63);
        val |= read_gicreg(ICH_VTR_EL2);