[PATCH v4 0/3] KVM: arm64: Errata management for VM Live migration

[PATCH v4 0/3] KVM: arm64: Errata management for VM Live migration

[Shameer Kolothum]

Hi,

v3 --> v4(Minor updates)
https://lore.kernel.org/kvmarm/20241209115311.40496-1-shameerali.kolothum.thodi@huawei.com/

 -Changed MIDR/REVIDR to 64 bits based on feedback from Connie
  and Marc(Patch #3).
 -Added R-by tags from Sebastian (Thanks!).

Thanks,
Shameer

RFC v2 --> v3
https://lore.kernel.org/kvmarm/20241024094012.29452-1-shameerali.kolothum.thodi@huawei.com/

 -Addressed comments from Oliver(Thanks!).
 -Using implementation CPUs MIDR/REVIDR when it is set for
  _midr_range() functions(Patch 1 & 3)
 -New hypercall for retrieving implementation CPUs(Patch #2).
 -Dropped RFC.

RFC v1 --> RFCv2:
https://lore.kernel.org/kvmarm/20241011075053.80540-1-shameerali.kolothum.thodi@huawei.com/
 -Introduced hypercalls to retrieve target CPUs info from user space VMM.
  see patch #1 for details.
 -Patch #2 uses the hypercall to retrieve the target CPU info if any.
 -Use the target CPUs MIDR/REVIDR in errata enablement. See patch #3.

Background from v1:

On ARM64 platforms most of the errata workarounds are based on CPU
MIDR/REVIDR values and a number of these workarounds need to be
implemented by the Guest kernel as well. This creates a problem when
Guest needs to be migrated to a platform that differs in these
MIDR/REVIDR values even if the VMM can come up with a common minimum
feature list for the Guest using the recently introduced "Writable
ID registers" support.

(This is roughly based on a discussion I had with Marc and Oliver
at KVM forum. Marc outlined his idea for a solution and this is an
attempt to implement it. Thanks to both and I take all the blame
if this is nowhere near what is intended/required)

Shameer Kolothum (3):
  arm64: Modify _midr_range() functions to read MIDR/REVIDR internally
  KVM: arm64: Introduce hypercall support for retrieving target
    implementations
  arm64: paravirt: Enable errata based on implementation CPUs

 Documentation/virt/kvm/arm/hypercalls.rst | 30 +++++++++++++
 arch/arm64/include/asm/cputype.h          | 51 ++++++++++++++++-------
 arch/arm64/include/asm/mmu.h              |  3 +-
 arch/arm64/include/asm/paravirt.h         |  3 ++
 arch/arm64/kernel/cpu_errata.c            | 37 +++++++++++-----
 arch/arm64/kernel/cpufeature.c            |  8 ++--
 arch/arm64/kernel/image-vars.h            |  2 +
 arch/arm64/kernel/paravirt.c              | 31 ++++++++++++++
 arch/arm64/kernel/proton-pack.c           | 17 ++++----
 arch/arm64/kvm/vgic/vgic-v3.c             |  2 +-
 drivers/clocksource/arm_arch_timer.c      |  2 +-
 include/linux/arm-smccc.h                 |  7 ++++
 12 files changed, 152 insertions(+), 41 deletions(-)

-- 
2.47.0

[PATCH v4 1/3] arm64: Modify _midr_range() functions to read MIDR/REVIDR internally

[Shameer Kolothum]

These changes lay the groundwork for adding support for guest kernels,
allowing them to leverage target CPU implementations provided by the
VMM.

No functional changes intended.

Suggested-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 arch/arm64/include/asm/cputype.h     | 28 ++++++++++++++--------------
 arch/arm64/include/asm/mmu.h         |  3 +--
 arch/arm64/kernel/cpu_errata.c       | 23 +++++++++++++----------
 arch/arm64/kernel/cpufeature.c       |  6 +++---
 arch/arm64/kernel/proton-pack.c      | 17 ++++++++---------
 arch/arm64/kvm/vgic/vgic-v3.c        |  2 +-
 drivers/clocksource/arm_arch_timer.c |  2 +-
 7 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 488f8e751349..dcf0e1ce892d 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -229,6 +229,16 @@
 
 #define read_cpuid(reg)			read_sysreg_s(SYS_ ## reg)
 
+/*
+ * The CPU ID never changes at run time, so we might as well tell the
+ * compiler that it's constant.  Use this function to read the CPU ID
+ * rather than directly reading processor_id or read_cpuid() directly.
+ */
+static inline u32 __attribute_const__ read_cpuid_id(void)
+{
+	return read_cpuid(MIDR_EL1);
+}
+
 /*
  * Represent a range of MIDR values for a given CPU model and a
  * range of variant/revision values.
@@ -264,31 +274,21 @@ static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
 	return _model == model && rv >= rv_min && rv <= rv_max;
 }
 
-static inline bool is_midr_in_range(u32 midr, struct midr_range const *range)
+static inline bool is_midr_in_range(struct midr_range const *range)
 {
-	return midr_is_cpu_model_range(midr, range->model,
+	return midr_is_cpu_model_range(read_cpuid_id(), range->model,
 				       range->rv_min, range->rv_max);
 }
 
 static inline bool
-is_midr_in_range_list(u32 midr, struct midr_range const *ranges)
+is_midr_in_range_list(struct midr_range const *ranges)
 {
 	while (ranges->model)
-		if (is_midr_in_range(midr, ranges++))
+		if (is_midr_in_range(ranges++))
 			return true;
 	return false;
 }
 
-/*
- * The CPU ID never changes at run time, so we might as well tell the
- * compiler that it's constant.  Use this function to read the CPU ID
- * rather than directly reading processor_id or read_cpuid() directly.
- */
-static inline u32 __attribute_const__ read_cpuid_id(void)
-{
-	return read_cpuid(MIDR_EL1);
-}
-
 static inline u64 __attribute_const__ read_cpuid_mpidr(void)
 {
 	return read_cpuid(MPIDR_EL1);
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 2ec96d91acc6..bfd303e074be 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -101,8 +101,7 @@ static inline bool kaslr_requires_kpti(void)
 	if (IS_ENABLED(CONFIG_CAVIUM_ERRATUM_27456)) {
 		extern const struct midr_range cavium_erratum_27456_cpus[];
 
-		if (is_midr_in_range_list(read_cpuid_id(),
-					  cavium_erratum_27456_cpus))
+		if (is_midr_in_range_list(cavium_erratum_27456_cpus))
 			return false;
 	}
 
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index a78f247029ae..929685c00263 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -15,30 +15,34 @@
 #include <asm/smp_plat.h>
 
 static bool __maybe_unused
-is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
+__is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
+			 u32 midr, u32 revidr)
 {
 	const struct arm64_midr_revidr *fix;
-	u32 midr = read_cpuid_id(), revidr;
-
-	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	if (!is_midr_in_range(midr, &entry->midr_range))
+	if (!is_midr_in_range(&entry->midr_range))
 		return false;
 
 	midr &= MIDR_REVISION_MASK | MIDR_VARIANT_MASK;
-	revidr = read_cpuid(REVIDR_EL1);
 	for (fix = entry->fixed_revs; fix && fix->revidr_mask; fix++)
 		if (midr == fix->midr_rv && (revidr & fix->revidr_mask))
 			return false;
-
 	return true;
 }
 
+static bool __maybe_unused
+is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
+	return __is_affected_midr_range(entry, read_cpuid_id(),
+					read_cpuid(REVIDR_EL1));
+}
+
 static bool __maybe_unused
 is_affected_midr_range_list(const struct arm64_cpu_capabilities *entry,
 			    int scope)
 {
 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return is_midr_in_range_list(read_cpuid_id(), entry->midr_range_list);
+	return is_midr_in_range_list(entry->midr_range_list);
 }
 
 static bool __maybe_unused
@@ -186,12 +190,11 @@ static bool __maybe_unused
 has_neoverse_n1_erratum_1542419(const struct arm64_cpu_capabilities *entry,
 				int scope)
 {
-	u32 midr = read_cpuid_id();
 	bool has_dic = read_cpuid_cachetype() & BIT(CTR_EL0_DIC_SHIFT);
 	const struct midr_range range = MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1);
 
 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return is_midr_in_range(midr, &range) && has_dic;
+	return is_midr_in_range(&range) && has_dic;
 }
 
 #ifdef CONFIG_ARM64_WORKAROUND_REPEAT_TLBI
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6ce71f444ed8..4cc4ae16b28d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1783,7 +1783,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 	char const *str = "kpti command line option";
 	bool meltdown_safe;
 
-	meltdown_safe = is_midr_in_range_list(read_cpuid_id(), kpti_safe_list);
+	meltdown_safe = is_midr_in_range_list(kpti_safe_list);
 
 	/* Defer to CPU feature registers */
 	if (has_cpuid_feature(entry, scope))
@@ -1853,7 +1853,7 @@ static bool has_nv1(const struct arm64_cpu_capabilities *entry, int scope)
 
 	return (__system_matches_cap(ARM64_HAS_NESTED_VIRT) &&
 		!(has_cpuid_feature(entry, scope) ||
-		  is_midr_in_range_list(read_cpuid_id(), nv1_ni_list)));
+		  is_midr_in_range_list(nv1_ni_list)));
 }
 
 #if defined(ID_AA64MMFR0_EL1_TGRAN_LPA2) && defined(ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2)
@@ -2036,7 +2036,7 @@ static bool cpu_has_broken_dbm(void)
 		{},
 	};
 
-	return is_midr_in_range_list(read_cpuid_id(), cpus);
+	return is_midr_in_range_list(cpus);
 }
 
 static bool cpu_can_use_dbm(const struct arm64_cpu_capabilities *cap)
diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
index da53722f95d4..a573fa40d4b6 100644
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -172,7 +172,7 @@ static enum mitigation_state spectre_v2_get_cpu_hw_mitigation_state(void)
 		return SPECTRE_UNAFFECTED;
 
 	/* Alternatively, we have a list of unaffected CPUs */
-	if (is_midr_in_range_list(read_cpuid_id(), spectre_v2_safe_list))
+	if (is_midr_in_range_list(spectre_v2_safe_list))
 		return SPECTRE_UNAFFECTED;
 
 	return SPECTRE_VULNERABLE;
@@ -331,7 +331,7 @@ bool has_spectre_v3a(const struct arm64_cpu_capabilities *entry, int scope)
 	};
 
 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return is_midr_in_range_list(read_cpuid_id(), spectre_v3a_unsafe_list);
+	return is_midr_in_range_list(spectre_v3a_unsafe_list);
 }
 
 void spectre_v3a_enable_mitigation(const struct arm64_cpu_capabilities *__unused)
@@ -475,7 +475,7 @@ static enum mitigation_state spectre_v4_get_cpu_hw_mitigation_state(void)
 		{ /* sentinel */ },
 	};
 
-	if (is_midr_in_range_list(read_cpuid_id(), spectre_v4_safe_list))
+	if (is_midr_in_range_list(spectre_v4_safe_list))
 		return SPECTRE_UNAFFECTED;
 
 	/* CPU features are detected first */
@@ -878,13 +878,13 @@ u8 spectre_bhb_loop_affected(int scope)
 			{},
 		};
 
-		if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
+		if (is_midr_in_range_list(spectre_bhb_k32_list))
 			k = 32;
-		else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k24_list))
+		else if (is_midr_in_range_list(spectre_bhb_k24_list))
 			k = 24;
-		else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k11_list))
+		else if (is_midr_in_range_list(spectre_bhb_k11_list))
 			k = 11;
-		else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k8_list))
+		else if (is_midr_in_range_list(spectre_bhb_k8_list))
 			k =  8;
 
 		max_bhb_k = max(max_bhb_k, k);
@@ -926,8 +926,7 @@ static bool is_spectre_bhb_fw_affected(int scope)
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
 		{},
 	};
-	bool cpu_in_list = is_midr_in_range_list(read_cpuid_id(),
-					 spectre_bhb_firmware_mitigated_list);
+	bool cpu_in_list = is_midr_in_range_list(spectre_bhb_firmware_mitigated_list);
 
 	if (scope != SCOPE_LOCAL_CPU)
 		return system_affected;
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index f267bc2486a1..b815d910c7e4 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -632,7 +632,7 @@ static const struct midr_range broken_seis[] = {
 static bool vgic_v3_broken_seis(void)
 {
 	return ((kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) &&
-		is_midr_in_range_list(read_cpuid_id(), broken_seis));
+		is_midr_in_range_list(broken_seis));
 }
 
 /**
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 808f259781fd..981a578043a5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -842,7 +842,7 @@ static u64 __arch_timer_check_delta(void)
 		{},
 	};
 
-	if (is_midr_in_range_list(read_cpuid_id(), broken_cval_midrs)) {
+	if (is_midr_in_range_list(broken_cval_midrs)) {
 		pr_warn_once("Broken CNTx_CVAL_EL1, using 31 bit TVAL instead.\n");
 		return CLOCKSOURCE_MASK(31);
 	}
-- 
2.47.0

Re: [PATCH v4 1/3] arm64: Modify _midr_range() functions to read MIDR/REVIDR internally

[Cornelia Huck]

On Wed, Dec 18 2024, Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:

> These changes lay the groundwork for adding support for guest kernels,
> allowing them to leverage target CPU implementations provided by the
> VMM.
>
> No functional changes intended.
>
> Suggested-by: Oliver Upton <oliver.upton@linux.dev>
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/include/asm/cputype.h     | 28 ++++++++++++++--------------
>  arch/arm64/include/asm/mmu.h         |  3 +--
>  arch/arm64/kernel/cpu_errata.c       | 23 +++++++++++++----------
>  arch/arm64/kernel/cpufeature.c       |  6 +++---
>  arch/arm64/kernel/proton-pack.c      | 17 ++++++++---------
>  arch/arm64/kvm/vgic/vgic-v3.c        |  2 +-
>  drivers/clocksource/arm_arch_timer.c |  2 +-
>  7 files changed, 41 insertions(+), 40 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[PATCH v4 2/3] KVM: arm64: Introduce hypercall support for retrieving target implementations

[Shameer Kolothum]

If the Guest requires migration to multiple targets, this hypercall
will provide a way to retrieve the target CPU implementations from
the user space VMM.

Subsequent patch will use this to enable the associated errata.

Suggested-by: Oliver Upton <oliver.upton@linux.dev>
Reviewed-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 Documentation/virt/kvm/arm/hypercalls.rst | 30 +++++++++++++++++++++++
 include/linux/arm-smccc.h                 |  7 ++++++
 2 files changed, 37 insertions(+)

diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst
index af7bc2c2e0cb..16b4c02cf9d9 100644
--- a/Documentation/virt/kvm/arm/hypercalls.rst
+++ b/Documentation/virt/kvm/arm/hypercalls.rst
@@ -142,3 +142,33 @@ region is equal to the memory protection granule advertised by
 |                     |          |    +---------------------------------------------+
 |                     |          |    | ``INVALID_PARAMETER (-3)``                  |
 +---------------------+----------+----+---------------------------------------------+
+
+``ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID``
+-------------------------------------------------------
+
+Request the target CPU implementation information for the Guest VM. This hypercall
+must be handled by the userspace VMM. The Guest kernel will use this information to
+enable the associated errata. A maximum of 64 implementations is supported.
+
++---------------------+-------------------------------------------------------------+
+| Presence:           | Optional;  KVM/ARM64 Guests only                            |
++---------------------+-------------------------------------------------------------+
+| Calling convention: | HVC64                                                       |
++---------------------+----------+--------------------------------------------------+
+| Function ID:        | (uint32) | 0xC600007E                                       |
++---------------------+----------+----+---------------------------------------------+
+| Arguments:          | (uint64) | R1 | selected implementation index               |
+|                     +----------+----+---------------------------------------------+
+|                     | (uint64) | R2 | Reserved / Must be zero                     |
+|                     +----------+----+---------------------------------------------+
+|                     | (uint64) | R3 | Reserved / Must be zero                     |
++---------------------+----------+----+---------------------------------------------+
+| Return Values:      | (int64)  | R0 | -1 on error else the maximum required CPU   |
+|                     |          |    | implementation index.                       |
+|                     +----------+----+---------------------------------------------+
+|                     | (uint64) | R1 | MIDR_EL1 of the selected implementation     |
+|                     +----------+----+---------------------------------------------+
+|                     | (uint64) | R2 | REVIDR_EL1 of the selected implementation   |
+|                     +----------+----+---------------------------------------------+
+|                     | (uint64) | R3 | Reserved / Must be zero                     |
++---------------------+----------+----+---------------------------------------------+
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 67f6fdf2e7cd..6c080fa9d345 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -179,6 +179,7 @@
 #define ARM_SMCCC_KVM_FUNC_PKVM_RESV_62		62
 #define ARM_SMCCC_KVM_FUNC_PKVM_RESV_63		63
 /* End of pKVM hypercall range */
+#define ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS	126
 #define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
 #define ARM_SMCCC_KVM_NUM_FUNCS			128
 
@@ -225,6 +226,12 @@
 			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
 			   ARM_SMCCC_KVM_FUNC_MMIO_GUARD)
 
+#define ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID		\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,				\
+			   ARM_SMCCC_SMC_64,				\
+			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
+			   ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS)
+
 /* ptp_kvm counter type ID */
 #define KVM_PTP_VIRT_COUNTER			0
 #define KVM_PTP_PHYS_COUNTER			1
-- 
2.47.0

Re: [PATCH v4 2/3] KVM: arm64: Introduce hypercall support for retrieving target implementations

[Cornelia Huck]

On Wed, Dec 18 2024, Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:

> If the Guest requires migration to multiple targets, this hypercall
> will provide a way to retrieve the target CPU implementations from
> the user space VMM.
>
> Subsequent patch will use this to enable the associated errata.
>
> Suggested-by: Oliver Upton <oliver.upton@linux.dev>
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  Documentation/virt/kvm/arm/hypercalls.rst | 30 +++++++++++++++++++++++
>  include/linux/arm-smccc.h                 |  7 ++++++
>  2 files changed, 37 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v4 2/3] KVM: arm64: Introduce hypercall support for retrieving target implementations

[Marc Zyngier]

Hi Shameer,

On Wed, 18 Dec 2024 10:53:44 +0000,
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> If the Guest requires migration to multiple targets, this hypercall
> will provide a way to retrieve the target CPU implementations from
> the user space VMM.
> 
> Subsequent patch will use this to enable the associated errata.
> 
> Suggested-by: Oliver Upton <oliver.upton@linux.dev>
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  Documentation/virt/kvm/arm/hypercalls.rst | 30 +++++++++++++++++++++++
>  include/linux/arm-smccc.h                 |  7 ++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst
> index af7bc2c2e0cb..16b4c02cf9d9 100644
> --- a/Documentation/virt/kvm/arm/hypercalls.rst
> +++ b/Documentation/virt/kvm/arm/hypercalls.rst
> @@ -142,3 +142,33 @@ region is equal to the memory protection granule advertised by
>  |                     |          |    +---------------------------------------------+
>  |                     |          |    | ``INVALID_PARAMETER (-3)``                  |
>  +---------------------+----------+----+---------------------------------------------+
> +
> +``ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID``
> +-------------------------------------------------------
> +
> +Request the target CPU implementation information for the Guest VM. This hypercall
> +must be handled by the userspace VMM. The Guest kernel will use this information to
> +enable the associated errata. A maximum of 64 implementations is supported.

In the interest of being able to support this long term, I really
think there should be a separate function returning:

- the version of the extension, modelled after the PSCI versioning

- how many different implementations the extension supports

This would avoid hardcoding this 'maximum of 64 implementations' in
the spec, and allow us to add new ID registers if they are added in
the future.

> +
> ++---------------------+-------------------------------------------------------------+
> +| Presence:           | Optional;  KVM/ARM64 Guests only                            |
> ++---------------------+-------------------------------------------------------------+
> +| Calling convention: | HVC64                                                       |
> ++---------------------+----------+--------------------------------------------------+
> +| Function ID:        | (uint32) | 0xC600007E                                       |
> ++---------------------+----------+----+---------------------------------------------+
> +| Arguments:          | (uint64) | R1 | selected implementation index               |
> +|                     +----------+----+---------------------------------------------+
> +|                     | (uint64) | R2 | Reserved / Must be zero                     |
> +|                     +----------+----+---------------------------------------------+
> +|                     | (uint64) | R3 | Reserved / Must be zero                     |
> ++---------------------+----------+----+---------------------------------------------+
> +| Return Values:      | (int64)  | R0 | -1 on error else the maximum required CPU   |
> +|                     |          |    | implementation index.                       |

With the above introduced, this can be simplified to always return
"SUCCESS (0)" or "INVALID_PARAMETERS (-3)". Please use the SMCCC names
to describe standard return values (see section 7.1 in the spec).

> +|                     +----------+----+---------------------------------------------+
> +|                     | (uint64) | R1 | MIDR_EL1 of the selected implementation     |
> +|                     +----------+----+---------------------------------------------+
> +|                     | (uint64) | R2 | REVIDR_EL1 of the selected implementation   |
> +|                     +----------+----+---------------------------------------------+
> +|                     | (uint64) | R3 | Reserved / Must be zero                     |
> ++---------------------+----------+----+---------------------------------------------+

This is missing AIDR_EL1. Although Linux never had to use it, its
purpose is to describe additional implementation details that we may
find relevant in the future.

> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 67f6fdf2e7cd..6c080fa9d345 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -179,6 +179,7 @@
>  #define ARM_SMCCC_KVM_FUNC_PKVM_RESV_62		62
>  #define ARM_SMCCC_KVM_FUNC_PKVM_RESV_63		63
>  /* End of pKVM hypercall range */
> +#define ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS	126

No need to stay at the back of the room, you can happily take slot 64!

>  #define ARM_SMCCC_KVM_FUNC_FEATURES_2		127
>  #define ARM_SMCCC_KVM_NUM_FUNCS			128
>  
> @@ -225,6 +226,12 @@
>  			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
>  			   ARM_SMCCC_KVM_FUNC_MMIO_GUARD)
>  
> +#define ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID		\
> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,				\
> +			   ARM_SMCCC_SMC_64,				\
> +			   ARM_SMCCC_OWNER_VENDOR_HYP,			\
> +			   ARM_SMCCC_KVM_FUNC_DISCOVER_IMPL_CPUS)
> +
>  /* ptp_kvm counter type ID */
>  #define KVM_PTP_VIRT_COUNTER			0
>  #define KVM_PTP_PHYS_COUNTER			1

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[PATCH v4 3/3] arm64: paravirt: Enable errata based on implementation CPUs

[Shameer Kolothum]

Retrieve any migration target implementation CPUs using the hypercall
and enable associated errata.

Reviewed-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 arch/arm64/include/asm/cputype.h  | 25 +++++++++++++++++++++++--
 arch/arm64/include/asm/paravirt.h |  3 +++
 arch/arm64/kernel/cpu_errata.c    | 20 +++++++++++++++++---
 arch/arm64/kernel/cpufeature.c    |  2 ++
 arch/arm64/kernel/image-vars.h    |  2 ++
 arch/arm64/kernel/paravirt.c      | 31 +++++++++++++++++++++++++++++++
 6 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index dcf0e1ce892d..019d1b7dae80 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -265,6 +265,16 @@ struct midr_range {
 #define MIDR_REV(m, v, r) MIDR_RANGE(m, v, r, v, r)
 #define MIDR_ALL_VERSIONS(m) MIDR_RANGE(m, 0, 0, 0xf, 0xf)
 
+#define MAX_TARGET_IMPL_CPUS 64
+
+struct target_impl_cpu {
+	u64 midr;
+	u64 revidr;
+};
+
+extern u32 target_impl_cpu_num;
+extern struct target_impl_cpu target_impl_cpus[];
+
 static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
 					   u32 rv_max)
 {
@@ -276,8 +286,19 @@ static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
 
 static inline bool is_midr_in_range(struct midr_range const *range)
 {
-	return midr_is_cpu_model_range(read_cpuid_id(), range->model,
-				       range->rv_min, range->rv_max);
+	int i;
+
+	if (!target_impl_cpu_num)
+		return midr_is_cpu_model_range(read_cpuid_id(), range->model,
+					       range->rv_min, range->rv_max);
+
+	for (i = 0; i < target_impl_cpu_num; i++) {
+		if (midr_is_cpu_model_range(target_impl_cpus[i].midr,
+					    range->model,
+					    range->rv_min, range->rv_max))
+			return true;
+	}
+	return false;
 }
 
 static inline bool
diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h
index 9aa193e0e8f2..95f1c15bbb7d 100644
--- a/arch/arm64/include/asm/paravirt.h
+++ b/arch/arm64/include/asm/paravirt.h
@@ -19,11 +19,14 @@ static inline u64 paravirt_steal_clock(int cpu)
 }
 
 int __init pv_time_init(void);
+void __init pv_target_impl_cpu_init(void);
 
 #else
 
 #define pv_time_init() do {} while (0)
 
+#define pv_target_impl_cpu_init() do {} while (0)
+
 #endif // CONFIG_PARAVIRT
 
 #endif
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 929685c00263..4055082ce69b 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -14,6 +14,9 @@
 #include <asm/kvm_asm.h>
 #include <asm/smp_plat.h>
 
+u32 target_impl_cpu_num;
+struct target_impl_cpu target_impl_cpus[MAX_TARGET_IMPL_CPUS];
+
 static bool __maybe_unused
 __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
 			 u32 midr, u32 revidr)
@@ -32,9 +35,20 @@ __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
 static bool __maybe_unused
 is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
 {
-	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
-	return __is_affected_midr_range(entry, read_cpuid_id(),
-					read_cpuid(REVIDR_EL1));
+	int i;
+
+	if (!target_impl_cpu_num) {
+		WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
+		return __is_affected_midr_range(entry, read_cpuid_id(),
+						read_cpuid(REVIDR_EL1));
+	}
+
+	for (i = 0; i < target_impl_cpu_num; i++) {
+		if (__is_affected_midr_range(entry, target_impl_cpus[i].midr,
+					     target_impl_cpus[i].midr))
+			return true;
+	}
+	return false;
 }
 
 static bool __maybe_unused
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4cc4ae16b28d..d32c767bf189 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -85,6 +85,7 @@
 #include <asm/kvm_host.h>
 #include <asm/mmu_context.h>
 #include <asm/mte.h>
+#include <asm/paravirt.h>
 #include <asm/processor.h>
 #include <asm/smp.h>
 #include <asm/sysreg.h>
@@ -3642,6 +3643,7 @@ unsigned long cpu_get_elf_hwcap3(void)
 
 static void __init setup_boot_cpu_capabilities(void)
 {
+	pv_target_impl_cpu_init();
 	/*
 	 * The boot CPU's feature register values have been recorded. Detect
 	 * boot cpucaps and local cpucaps for the boot CPU, then enable and
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 8f5422ed1b75..694e19709c46 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -49,6 +49,8 @@ PROVIDE(__pi_arm64_sw_feature_override	= arm64_sw_feature_override);
 PROVIDE(__pi_arm64_use_ng_mappings	= arm64_use_ng_mappings);
 #ifdef CONFIG_CAVIUM_ERRATUM_27456
 PROVIDE(__pi_cavium_erratum_27456_cpus	= cavium_erratum_27456_cpus);
+PROVIDE(__pi_target_impl_cpu_num	= target_impl_cpu_num);
+PROVIDE(__pi_target_impl_cpus		= target_impl_cpus);
 #endif
 PROVIDE(__pi__ctype			= _ctype);
 PROVIDE(__pi_memstart_offset_seed	= memstart_offset_seed);
diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
index aa718d6a9274..95fc3aae4a27 100644
--- a/arch/arm64/kernel/paravirt.c
+++ b/arch/arm64/kernel/paravirt.c
@@ -153,6 +153,37 @@ static bool __init has_pv_steal_clock(void)
 	return (res.a0 == SMCCC_RET_SUCCESS);
 }
 
+void  __init pv_target_impl_cpu_init(void)
+{
+	struct arm_smccc_res res;
+	int index = 0, max_idx = -1;
+
+	/* Check we have already set targets */
+	if (target_impl_cpu_num)
+		return;
+
+	do {
+		arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID,
+				     index, &res);
+		if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
+			return;
+
+		if (max_idx < 0) {
+			/* res.a0 should have a valid maximum CPU implementation index */
+			if (res.a0 >= MAX_TARGET_IMPL_CPUS)
+				return;
+			max_idx = res.a0;
+		}
+
+		target_impl_cpus[index].midr = res.a1;
+		target_impl_cpus[index].revidr = res.a2;
+		index++;
+	} while (index <= max_idx);
+
+	target_impl_cpu_num = index;
+	pr_info("Number of target implementation CPUs is %d\n", target_impl_cpu_num);
+}
+
 int __init pv_time_init(void)
 {
 	int ret;
-- 
2.47.0

Re: [PATCH v4 3/3] arm64: paravirt: Enable errata based on implementation CPUs

[Cornelia Huck]

On Wed, Dec 18 2024, Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:

> Retrieve any migration target implementation CPUs using the hypercall
> and enable associated errata.
>
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/include/asm/cputype.h  | 25 +++++++++++++++++++++++--
>  arch/arm64/include/asm/paravirt.h |  3 +++
>  arch/arm64/kernel/cpu_errata.c    | 20 +++++++++++++++++---
>  arch/arm64/kernel/cpufeature.c    |  2 ++
>  arch/arm64/kernel/image-vars.h    |  2 ++
>  arch/arm64/kernel/paravirt.c      | 31 +++++++++++++++++++++++++++++++
>  6 files changed, 78 insertions(+), 5 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v4 3/3] arm64: paravirt: Enable errata based on implementation CPUs

[Marc Zyngier]

On Wed, 18 Dec 2024 10:53:45 +0000,
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> Retrieve any migration target implementation CPUs using the hypercall
> and enable associated errata.
> 
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/include/asm/cputype.h  | 25 +++++++++++++++++++++++--
>  arch/arm64/include/asm/paravirt.h |  3 +++
>  arch/arm64/kernel/cpu_errata.c    | 20 +++++++++++++++++---
>  arch/arm64/kernel/cpufeature.c    |  2 ++
>  arch/arm64/kernel/image-vars.h    |  2 ++
>  arch/arm64/kernel/paravirt.c      | 31 +++++++++++++++++++++++++++++++
>  6 files changed, 78 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index dcf0e1ce892d..019d1b7dae80 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -265,6 +265,16 @@ struct midr_range {
>  #define MIDR_REV(m, v, r) MIDR_RANGE(m, v, r, v, r)
>  #define MIDR_ALL_VERSIONS(m) MIDR_RANGE(m, 0, 0, 0xf, 0xf)
>  
> +#define MAX_TARGET_IMPL_CPUS 64
> +
> +struct target_impl_cpu {
> +	u64 midr;
> +	u64 revidr;
> +};
> +
> +extern u32 target_impl_cpu_num;
> +extern struct target_impl_cpu target_impl_cpus[];
> +
>  static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
>  					   u32 rv_max)
>  {
> @@ -276,8 +286,19 @@ static inline bool midr_is_cpu_model_range(u32 midr, u32 model, u32 rv_min,
>  
>  static inline bool is_midr_in_range(struct midr_range const *range)
>  {
> -	return midr_is_cpu_model_range(read_cpuid_id(), range->model,
> -				       range->rv_min, range->rv_max);
> +	int i;
> +
> +	if (!target_impl_cpu_num)
> +		return midr_is_cpu_model_range(read_cpuid_id(), range->model,
> +					       range->rv_min, range->rv_max);
> +
> +	for (i = 0; i < target_impl_cpu_num; i++) {
> +		if (midr_is_cpu_model_range(target_impl_cpus[i].midr,
> +					    range->model,
> +					    range->rv_min, range->rv_max))
> +			return true;
> +	}
> +	return false;
>  }
>  
>  static inline bool
> diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h
> index 9aa193e0e8f2..95f1c15bbb7d 100644
> --- a/arch/arm64/include/asm/paravirt.h
> +++ b/arch/arm64/include/asm/paravirt.h
> @@ -19,11 +19,14 @@ static inline u64 paravirt_steal_clock(int cpu)
>  }
>  
>  int __init pv_time_init(void);
> +void __init pv_target_impl_cpu_init(void);
>  
>  #else
>  
>  #define pv_time_init() do {} while (0)
>  
> +#define pv_target_impl_cpu_init() do {} while (0)
> +
>  #endif // CONFIG_PARAVIRT
>  
>  #endif
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 929685c00263..4055082ce69b 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -14,6 +14,9 @@
>  #include <asm/kvm_asm.h>
>  #include <asm/smp_plat.h>
>  
> +u32 target_impl_cpu_num;
> +struct target_impl_cpu target_impl_cpus[MAX_TARGET_IMPL_CPUS];
> +
>  static bool __maybe_unused
>  __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
>  			 u32 midr, u32 revidr)
> @@ -32,9 +35,20 @@ __is_affected_midr_range(const struct arm64_cpu_capabilities *entry,
>  static bool __maybe_unused
>  is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
>  {
> -	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
> -	return __is_affected_midr_range(entry, read_cpuid_id(),
> -					read_cpuid(REVIDR_EL1));
> +	int i;
> +
> +	if (!target_impl_cpu_num) {
> +		WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
> +		return __is_affected_midr_range(entry, read_cpuid_id(),
> +						read_cpuid(REVIDR_EL1));
> +	}
> +
> +	for (i = 0; i < target_impl_cpu_num; i++) {
> +		if (__is_affected_midr_range(entry, target_impl_cpus[i].midr,
> +					     target_impl_cpus[i].midr))
> +			return true;
> +	}
> +	return false;
>  }
>  
>  static bool __maybe_unused
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 4cc4ae16b28d..d32c767bf189 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -85,6 +85,7 @@
>  #include <asm/kvm_host.h>
>  #include <asm/mmu_context.h>
>  #include <asm/mte.h>
> +#include <asm/paravirt.h>
>  #include <asm/processor.h>
>  #include <asm/smp.h>
>  #include <asm/sysreg.h>
> @@ -3642,6 +3643,7 @@ unsigned long cpu_get_elf_hwcap3(void)
>  
>  static void __init setup_boot_cpu_capabilities(void)
>  {
> +	pv_target_impl_cpu_init();
>  	/*
>  	 * The boot CPU's feature register values have been recorded. Detect
>  	 * boot cpucaps and local cpucaps for the boot CPU, then enable and
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 8f5422ed1b75..694e19709c46 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -49,6 +49,8 @@ PROVIDE(__pi_arm64_sw_feature_override	= arm64_sw_feature_override);
>  PROVIDE(__pi_arm64_use_ng_mappings	= arm64_use_ng_mappings);
>  #ifdef CONFIG_CAVIUM_ERRATUM_27456
>  PROVIDE(__pi_cavium_erratum_27456_cpus	= cavium_erratum_27456_cpus);
> +PROVIDE(__pi_target_impl_cpu_num	= target_impl_cpu_num);
> +PROVIDE(__pi_target_impl_cpus		= target_impl_cpus);
>  #endif
>  PROVIDE(__pi__ctype			= _ctype);
>  PROVIDE(__pi_memstart_offset_seed	= memstart_offset_seed);
> diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
> index aa718d6a9274..95fc3aae4a27 100644
> --- a/arch/arm64/kernel/paravirt.c
> +++ b/arch/arm64/kernel/paravirt.c
> @@ -153,6 +153,37 @@ static bool __init has_pv_steal_clock(void)
>  	return (res.a0 == SMCCC_RET_SUCCESS);
>  }
>  
> +void  __init pv_target_impl_cpu_init(void)
> +{
> +	struct arm_smccc_res res;
> +	int index = 0, max_idx = -1;
> +
> +	/* Check we have already set targets */
> +	if (target_impl_cpu_num)
> +		return;
> +
> +	do {
> +		arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID,
> +				     index, &res);
> +		if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> +			return;

Can't you probe for this as part of the KVM guest services?

> +
> +		if (max_idx < 0) {
> +			/* res.a0 should have a valid maximum CPU implementation index */
> +			if (res.a0 >= MAX_TARGET_IMPL_CPUS)
> +				return;
> +			max_idx = res.a0;
> +		}
> +
> +		target_impl_cpus[index].midr = res.a1;
> +		target_impl_cpus[index].revidr = res.a2;
> +		index++;
> +	} while (index <= max_idx);
> +
> +	target_impl_cpu_num = index;
> +	pr_info("Number of target implementation CPUs is %d\n", target_impl_cpu_num);
> +}
> +
>  int __init pv_time_init(void)
>  {
>  	int ret;

Independent of this, I wonder what we should output in sysfs
(/sys/devices/system/cpu/cpu*/regs/identification/*).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v4 3/3] arm64: paravirt: Enable errata based on implementation CPUs

[Oliver Upton]

On Thu, Dec 19, 2024 at 10:04:22AM +0000, Marc Zyngier wrote:
> > +void  __init pv_target_impl_cpu_init(void)
> > +{
> > +	struct arm_smccc_res res;
> > +	int index = 0, max_idx = -1;
> > +
> > +	/* Check we have already set targets */
> > +	if (target_impl_cpu_num)
> > +		return;
> > +
> > +	do {
> > +		arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_DISCOVER_IMPL_CPUS_FUNC_ID,
> > +				     index, &res);
> > +		if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> > +			return;
> 
> Can't you probe for this as part of the KVM guest services?

+1, this needs to be predicated on actually detecting the hypervisor as
KVM.

> > +
> > +		if (max_idx < 0) {
> > +			/* res.a0 should have a valid maximum CPU implementation index */
> > +			if (res.a0 >= MAX_TARGET_IMPL_CPUS)
> > +				return;
> > +			max_idx = res.a0;
> > +		}
> > +
> > +		target_impl_cpus[index].midr = res.a1;
> > +		target_impl_cpus[index].revidr = res.a2;
> > +		index++;
> > +	} while (index <= max_idx);
> > +
> > +	target_impl_cpu_num = index;
> > +	pr_info("Number of target implementation CPUs is %d\n", target_impl_cpu_num);
> > +}
> > +
> >  int __init pv_time_init(void)
> >  {
> >  	int ret;
> 
> Independent of this, I wonder what we should output in sysfs
> (/sys/devices/system/cpu/cpu*/regs/identification/*).

It's a bit crap, but maybe implementation index 0 gets reported through
the 'main' midr/revidr files, otherwise have a directory per
implementation index of midr/revidr.

-- 
Thanks,
Oliver

Re: [PATCH v4 3/3] arm64: paravirt: Enable errata based on implementation CPUs

[Marc Zyngier]

On Thu, 19 Dec 2024 17:40:55 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> > Independent of this, I wonder what we should output in sysfs
> > (/sys/devices/system/cpu/cpu*/regs/identification/*).
> 
> It's a bit crap, but maybe implementation index 0 gets reported through
> the 'main' midr/revidr files, otherwise have a directory per
> implementation index of midr/revidr.

Having slept on that one, I'm starting to think that we should keep
the status-quo of reporting what the kernel snapshot at boot time.
There is no good reason to force the VMM to report the potential
implementations in any specific order.

The "alternative-implementations" is interesting, but we don't keep it
per-CPU, so I don't  think it fits the current scheme. But maybe
something in /sys/devices/system/cpu, outside of the cpu* hierarchy?

Either way, this isn't something we should worry too much right now.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v4 3/3] arm64: paravirt: Enable errata based on implementation CPUs

[Cornelia Huck]

On Fri, Dec 20 2024, Marc Zyngier <maz@kernel.org> wrote:

> On Thu, 19 Dec 2024 17:40:55 +0000,
> Oliver Upton <oliver.upton@linux.dev> wrote:
>> 
>> > Independent of this, I wonder what we should output in sysfs
>> > (/sys/devices/system/cpu/cpu*/regs/identification/*).
>> 
>> It's a bit crap, but maybe implementation index 0 gets reported through
>> the 'main' midr/revidr files, otherwise have a directory per
>> implementation index of midr/revidr.
>
> Having slept on that one, I'm starting to think that we should keep
> the status-quo of reporting what the kernel snapshot at boot time.
> There is no good reason to force the VMM to report the potential
> implementations in any specific order.
>
> The "alternative-implementations" is interesting, but we don't keep it
> per-CPU, so I don't  think it fits the current scheme. But maybe
> something in /sys/devices/system/cpu, outside of the cpu* hierarchy?

I agree that this should be reported outside of the cpu* hierarchy,
maybe smth like

/sys/devices/system/cpu/alternatives/
   identification/
     regs0/
       aidr_el1
       midr_el1
       revidr_el1
     regs1/
       aidr_el1
       midr_el1
       revidr_el1

(with no guarantee as to _which_ triplet will show up under regs<n>)

Also depends on the intended consumers, I guess.

>
> Either way, this isn't something we should worry too much right now.

I agree as well.

Re: [PATCH v4 0/3] KVM: arm64: Errata management for VM Live migration

[Marc Zyngier]

Hi Shameer,

On Wed, 18 Dec 2024 10:53:42 +0000,
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> Hi,
> 
> v3 --> v4(Minor updates)
> https://lore.kernel.org/kvmarm/20241209115311.40496-1-shameerali.kolothum.thodi@huawei.com/
> 
>  -Changed MIDR/REVIDR to 64 bits based on feedback from Connie
>   and Marc(Patch #3).
>  -Added R-by tags from Sebastian (Thanks!).

Thanks again for putting this together.

I think it would be really good to have a sample userspace
implementation of this extension so that we can play with it for real
before fully committing to it.

I am also wondering is we should make it mandatory that a guest is
presented with an MIDR_EL1.Implementer value set to 0, which denotes
SW use, and would make it plain to the guest (and crucially, guest
userspace) that we are going to play tricks.

Thoughts?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v4 0/3] KVM: arm64: Errata management for VM Live migration

[Oliver Upton]

On Thu, Dec 19, 2024 at 10:07:39AM +0000, Marc Zyngier wrote:
> Hi Shameer,
> 
> On Wed, 18 Dec 2024 10:53:42 +0000,
> Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:
> > 
> > Hi,
> > 
> > v3 --> v4(Minor updates)
> > https://lore.kernel.org/kvmarm/20241209115311.40496-1-shameerali.kolothum.thodi@huawei.com/
> > 
> >  -Changed MIDR/REVIDR to 64 bits based on feedback from Connie
> >   and Marc(Patch #3).
> >  -Added R-by tags from Sebastian (Thanks!).
> 
> Thanks again for putting this together.
> 
> I think it would be really good to have a sample userspace
> implementation of this extension so that we can play with it for real
> before fully committing to it.
> 
> I am also wondering is we should make it mandatory that a guest is
> presented with an MIDR_EL1.Implementer value set to 0, which denotes
> SW use, and would make it plain to the guest (and crucially, guest
> userspace) that we are going to play tricks.
> 
> Thoughts?

I see no issues with giving userspace the option of doing this, but I'd
rather not enforce this to use the PV interface.

At least in a cloud setting it seems highly likely that you'd be running
a mix of guests on the same VM definition, some aware of the PV
interface and others not. If we use a software MIDR, all old VMs will
lose errata mitigations and whatever else userspace might key off of MIDR.

Userspace can make the call whether or not a VM can be migrated onto a
different implementation based on whether or not the guest used the
hypercall interface. And maybe just terminate the VM if it didn't.

-- 
Thanks,
Oliver