Hi John,
On 2/25/25 11:01, John Allen wrote:
> 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.
Yes. We need to verify it on pre-EPYC hardware. Please let us know how it
goes.
But, this series updates only the EPYC based CPU models. It should not be
a concern here. Right?
>
> 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
>>
>>
>
--
Thanks
Babu Moger
