Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <de8dbc64-ae2d-aa9f-a973-171feb5874d6@redhat.com>
Date:   Fri, 22 Oct 2021 10:27:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.1.0
Subject: Re: [PATCH] KVM: x86: advertise absence of X86_BUG_NULL_SEG via CPUID
Content-Language: en-US
To:     Jim Mattson <jmattson@google.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        thomas.lendacky@amd.com, dgilbert@redhat.com
References: <20211021211958.1531754-1-pbonzini@redhat.com>
 <CALMp9eR3bt5P_+ZTJqcckL1pbZCyS6dCNK8o2OQR-atU_A_jtQ@mail.gmail.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <CALMp9eR3bt5P_+ZTJqcckL1pbZCyS6dCNK8o2OQR-atU_A_jtQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 21/10/21 23:36, Jim Mattson wrote:
> On Thu, Oct 21, 2021 at 2:20 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> Guests have X86_BUG_NULL_SEG if and only if the host have it.  Use
>> the info from static_cpu_has_bug to form the 0x80000021 CPUID leaf that
>> was defined for Zen3.  Userspace can then set the bit even on older
>> CPUs that do not have the bug, such as Zen2.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   arch/x86/kvm/cpuid.c | 17 ++++++++++++++++-
>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 2d70edb0f323..b51398e1727b 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -902,7 +902,13 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>>                  entry->edx = 0;
>>                  break;
>>          case 0x80000000:
>> -               entry->eax = min(entry->eax, 0x8000001f);
>> +               entry->eax = min(entry->eax, 0x80000021);
>> +               /*
>> +                * X86_BUG_NULL_SEG is not reported in CPUID on Zen2; in
>> +                * that case, provide the CPUID leaf ourselves.
>> +                */
> 
> I think this is backwards. !X86_BUG_NULL_SEG isn't reported in CPUID on Zen2.

Right I should use the name of the bit instead.

>> +               if (!static_cpu_has_bug(X86_BUG_NULL_SEG))
>> +                       entry->eax = max(entry->eax, 0x80000021);
>>                  break;
>>          case 0x80000001:
>>                  cpuid_entry_override(entry, CPUID_8000_0001_EDX);
>> @@ -973,6 +979,15 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>>                          entry->ebx &= ~GENMASK(11, 6);
>>                  }
>>                  break;
>> +       case 0x80000020:
>> +               entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
>> +               break;
>> +       case 0x80000021:
>> +               entry->ebx = entry->ecx = entry->edx = 0;
>> +               entry->eax &= BIT(6);
> 
> While we're here, shouldn't bit 0 (Processor ignores nested data
> breakpoints) and bit 2 (LFENCE is always dispatch serializing) also
> match the hardware?

Yes, that makes sense.  Just wanted to gauge whether anybody thought it 
a really bad idea.

Paolo

> 
>> +               if (!static_cpu_has_bug(X86_BUG_NULL_SEG))
>> +                       entry->eax |= BIT(6);
>> +               break;
>>          /*Add support for Centaur's CPUID instruction*/
>>          case 0xC0000000:
>>                  /*Just support up to 0xC0000004 now*/
>> --
>> 2.27.0
>>
>