Re: [PATCH v1 2/2] x86/cpuid: Detect null segment behaviour on Zen2 CPUs

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/09/2021 16:17, Jan Beulich wrote:
> On 06.09.2021 14:00, Jane Malalane wrote:
>> --- a/xen/arch/x86/cpu/amd.c
>> +++ b/xen/arch/x86/cpu/amd.c
>> @@ -681,6 +681,19 @@ void amd_init_lfence(struct cpuinfo_x86 *c)
>>                        c->x86_capability);
>>  }
>>  
>> +void detect_zen2_null_seg_behaviour(void)
> This can in principle be marked __init.
>
>> +{
>> +    uint64_t base;
>> +
>> +    wrmsrl(MSR_FS_BASE, 1);
>> +    asm volatile ( "mov %0, %%fs" :: "rm" (0) );
> While I don't strictly mind the "m" part of the constraint to remain
> there (in the hope for compilers actually to support this), iirc it's
> not useful to have when the value is a constant: Last time I checked,
> the compiler would not instantiate an anonymous (stack) variable to
> fulfill this constraint (as can be seen when dropping the "r" part of
> the constraint).

This is "rm" because it is what we use elsewhere in Xen for selectors,
and because it is the correct constraints based on the legal instruction
encodings.

If you want to work around what you perceive to be bugs in compilers
then submit a independent change yourself.

>> @@ -731,6 +744,11 @@ static void init_amd(struct cpuinfo_x86 *c)
>>      else /* Implicily "== 0x10 || >= 0x12" by being 64bit. */
>>              amd_init_lfence(c);
>>  
>> +    /* Probe for NSCB on Zen2 CPUs when not virtualised */
>> +    if (!cpu_has_hypervisor && !cpu_has_nscb && c == &boot_cpu_data &&
>> +        c->x86 == 0x17 && c->x86_model >= 30 && c->x86_model <= 0x5f)
> DYM 0x30 here?

0x30, although it turns out that some of the mobile Zen2 CPUs exceed
0x60 in terms of model number.

As Zen3 changes the family number to 0x19, I'd just drop the upper bound.

>  Or 0x1e? In any event 0x5f should be accompanied by
> another hex constant. And it would also help if in the description
> you said where these bounds

>From talking to people at AMD.

>  as well as ...
>
>> --- a/xen/arch/x86/cpu/hygon.c
>> +++ b/xen/arch/x86/cpu/hygon.c
>> @@ -34,6 +34,11 @@ static void init_hygon(struct cpuinfo_x86 *c)
>>  
>>      amd_init_lfence(c);
>>  
>> +    /* Probe for NSCB on Zen2 CPUs when not virtualised */
>> +    if (!cpu_has_hypervisor && !cpu_has_nscb && c == &boot_cpu_data &&
>> +        c->x86 == 0x18 && c->x86_model >= 4)
> ... this one come from.

>From talking to people at Hygon.

~Andrew