Re: [PATCH v2 2/2][4.15] x86/AMD: expose HWCR.TscFreqSel to guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.03.2021 13:29, Roger Pau Monné wrote:
> On Mon, Mar 08, 2021 at 12:47:44PM +0100, Jan Beulich wrote:
>> On 08.03.2021 12:37, Roger Pau Monné wrote:
>>> On Fri, Mar 05, 2021 at 10:50:54AM +0100, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/msr.c
>>>> +++ b/xen/arch/x86/msr.c
>>>> @@ -315,6 +315,12 @@ int guest_rdmsr(struct vcpu *v, uint32_t
>>>>          *val = msrs->tsc_aux;
>>>>          break;
>>>>  
>>>> +    case MSR_K8_HWCR:
>>>> +        if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
>>>> +            goto gp_fault;
>>>> +        *val = K8_HWCR_TSC_FREQ_SEL;
>>>
>>> I've been only able to find information about this MSR up to family
>>> 10h, but I think in theory Xen might also run on family 0Fh, do you
>>> know if the MSR is present there, and the bit has the same meaning?
>>
>> From its name (and its K7 alternative name) it's clear the register
>> had been there at that point. And indeed the bit has a different
>> meaning there (its the bottom bit of a 6-bit START_FID field if the
>> BKDG I'm looking at can be trusted.
> 
> OK, I cannot seem to find the BKDG for family 0Fh. The oldest BKDG I
> can find is for Family 10h [0].
> 
>> Since I don't think it matters
>> much whether we expose a value of 0x00 or a value of 0x01 there,
>> and since we likely don't want to make #GP raising dependent upon
>> family when we don't _really_ need to, I would want to propose that
>> the value used is good enough uniformly.
> 
> I would be fine with setting it to 0 if Fam < 10h if you think that's
> acceptable. I think the chances of someone running Xen >= 4.15 on such
> old hardware are quite dim.

Would you mind explaining how returning 0 in this case would be
better? No hard-coded value will ever be guaranteed to reflect the
truth. See my reply to Andrew - if anything we'd need to let the
hardware field shine through, and in _that_ case I of course I
agree that we then should treat Fam0F specially.

I will admit though that as per the BKDG I'm looking at only even
values are defined for the field. Reporting 1 here therefore may
do good (keep OSes from trying to use any of this P-state stuff)
or bad (confuse OSes).

Jan