Re: [PATCH 1/3] x86/hvm: Revert 80ecb40362365ba77e68fc609de8bd3b7208ae19

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 03/07/2014 05:46 AM, Jan Beulich wrote:
>>>> On 06.03.14 at 19:31, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
>> The Solaris bug that commit 80ecb40362365ba77e68fc609de8bd3b7208ae19
>> addressed
>> has been fixed and backported to earlier releases.
> I don't think this is sufficient justification for the revert: Suppose
> someone's still running an un-patched Solaris guest (for a cloud
> provider this may even be unknowingly) and then updates the
> hypervisor (i.e. by migrating the guest to an updated host). I
> can see why you want this reverted for the subsequent patches,
> but I'm afraid a different solution will need to be found (if nothing
> else, via explicit guest config option).


I did check with our Solaris folks and we (Oracle) don't recommend 
running versions of Solaris that are susceptible to this issue (not 
because of this bug specifically but mostly because those releases are 
too old and presumably there are other problems that needed to be 
addressed).

Having said that, we could provide a way to reduce the number of leaves 
by using the 'cpuid' option in xl.conf. In fact, supporting this from 
configuration file should be a useful feature anyway.


-boris


>
> Jan
>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>>   xen/arch/x86/traps.c |   11 ++---------
>>   1 files changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
>> index c462317..d8f83a0 100644
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -677,23 +677,16 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
>>       struct domain *d = current->domain;
>>       /* Optionally shift out of the way of Viridian architectural leaves. */
>>       uint32_t base = is_viridian_domain(d) ? 0x40000100 : 0x40000000;
>> -    uint32_t limit;
>>   
>>       idx -= base;
>>   
>> -    /*
>> -     * Some Solaris PV drivers fail if max > base + 2. Help them out by
>> -     * hiding the PVRDTSCP leaf if PVRDTSCP is disabled.
>> -     */
>> -    limit = (d->arch.tsc_mode < TSC_MODE_PVRDTSCP) ? 2 : 3;
>> -
>> -    if ( idx > limit )
>> +    if ( idx > 3 )
>>           return 0;
>>   
>>       switch ( idx )
>>       {
>>       case 0:
>> -        *eax = base + limit; /* Largest leaf */
>> +        *eax = base + 3; /* Largest leaf */
>>           *ebx = XEN_CPUID_SIGNATURE_EBX;
>>           *ecx = XEN_CPUID_SIGNATURE_ECX;
>>           *edx = XEN_CPUID_SIGNATURE_EDX;
>> -- 
>> 1.7.1
>
>