Re: [Xen-devel] [RFC PATCH 03/10] Add cpuid_vmware_leaves

[Don Slutz <dslutz@xxxxxxxxxxx>]

On 12/13/13 08:38, Andrew Cooper wrote:
> On 13/12/2013 10:55, Jan Beulich wrote:
> > > > > On 12.12.13 at 23:27, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> > > On 12/12/2013 19:15, Don Slutz wrote:
> > > >   int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
> > > >                  uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
> > > >   {
> > > >       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 base = 0x40000000;
> > > >       uint32_t limit;
> > > >   
> > > > +    if ( is_viridian_domain(d) )
> > > > +        base += 0x100;
> > > > +    if ( is_vmware_domain(d) )
> > > > +        base += 0x100;
> > > > +
As a related question: Should I reply to each e-mail, or is the reply at the 
thread place (like here) ok?
> > > These bases need a far more scalable solution, especially as the result
> > > of each of these clauses can be changed at runtime with a cunning
> > > hvm_param_set hypercall.
I see.  Did not think about HVM_PARAM_VIRIDIAN changing and the effect it has 
on all the related code.  I just saw what was there for viridian and tried to 
add vmware support the same way.  I will look into a patch that will make the 
bases more scalable, not run time changeable.  I see that right now you can get 
the code to do strange (and maybe bad) things by changing HVM_PARAM_VIRIDIAN.  
So this patch (or patches) would make is_viridian_domain() an unchangable 
result.  Same for is_vmware_domain() when I add it.
> > > I think that both "is pretending to be HyperV" and "is pretending to be
> > > VMware" need to be domain creation flags which are strictly static for
> > > the lifetime of the domain.
I agree with this.
> > And it seems highly questionable to me whether having both at the
> > same time makes much sense.
> >
> > Plus the new function doesn't belong in xen/arch/x86/traps.c ...
I read this as "Change to some sort of data structure to get the answer instead of a 
function".  This is because I do not see how to return the correct data from a 
function if it is not in xen/arch/x86/traps.c (like cpuid_hypervisor_leaves is) or called 
from there, which I would see as more confusing.
> > Jan
> I would certainly agree on the sentiment of it not making much sense.
>
> However, as Xen needs viridian to run windows, I would be astounded if
> vmware VMs didn't have viridian as well, in which case it is probably
> quite likely that a vmware windows vm with vmware tools would expect to
> find viridian and vmware extensions.
I also find it strange, but could not see a reason not to try and support it.  
This could be related to VMware stating that using CPUID is not 100% the way to 
find out you are running on VMware.

   -Don Slutz
> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel