On Thu, Feb 20, 2025 at 06:59:34PM +0800, Zhao Liu wrote:
> And one more thing :-) ...
>
> > static const CPUCaches epyc_rome_cache_info = {
> > .l1d_cache = &(CPUCacheInfo) {
> > .type = DATA_CACHE,
> > @@ -5207,6 +5261,25 @@ static const X86CPUDefinition builtin_x86_defs[] = {
> > },
> > .cache_info = &epyc_v4_cache_info
> > },
> > + {
> > + .version = 5,
> > + .props = (PropValue[]) {
> > + { "overflow-recov", "on" },
> > + { "succor", "on" },
>
> When I checks the "overflow-recov" and "succor" enabling, I find these 2
> bits are set unconditionally.
>
> I'm not sure if all AMD platforms support both bits, do you think it's
> necessary to check the host support?
Hi Zhao,
IIRC, we intentionally set these unconditionally since there is no
specific support needed from the host side for guests to use these bits
to handle MCEs. See the original discussion and rationale in this
thread:
https://lore.kernel.org/all/20230706194022.2485195-2-john.allen@xxxxxxx/
However, this discussion only applied to the SUCCOR feature and not the
OVERFLOW_RECOV feature and now that you bring it up, I'm second guessing
whether we can apply the same thinking to OVERFLOW_RECOV. I think we may
want to keep setting the SUCCOR bit unconditionally, but we may want to
handle OVERFLOW_RECOV normally. I'll have to track down some old
hardware to see how this behaves when the hardware doesn't support it.
Thanks,
John
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 6c749d4ee812..03e463076632 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -555,7 +555,10 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
> cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
> ret |= cpuid_1_edx & CPUID_EXT2_AMD_ALIASES;
> } else if (function == 0x80000007 && reg == R_EBX) {
> - ret |= CPUID_8000_0007_EBX_OVERFLOW_RECOV | CPUID_8000_0007_EBX_SUCCOR;
> + uint32_t ebx;
> + host_cpuid(0x80000007, 0, &unused, &ebx, &unused, &unused);
> +
> + ret |= ebx & (CPUID_8000_0007_EBX_OVERFLOW_RECOV | CPUID_8000_0007_EBX_SUCCOR);
> } else if (function == KVM_CPUID_FEATURES && reg == R_EAX) {
> /* kvm_pv_unhalt is reported by GET_SUPPORTED_CPUID, but it can't
> * be enabled without the in-kernel irqchip
>
> Thanks,
> Zhao
>
>
