Re: [Xen-devel] [PATCH v2] xen: use domid check in is_hardware_domain

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.07.13 at 12:57, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 09/07/13 21:28, Daniel De Graaf wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -1459,7 +1459,7 @@ void context_switch(struct vcpu *prev, struct vcpu 
>> *next)
>>           }
>>   
>>           set_cpuid_faulting(!is_hvm_vcpu(next) &&
>> -                           (next->domain->domain_id != 0));
>> +                           !is_control_domain(next->domain));
> 
> Won't this mean that in your separate hardware/control domain model that 
> the hardware domain *will* fault on cpuid? Is this what we want?

I think this is correct, as it's the control domain that ought to be
able to set up CPUID via the tool stack for all other domains.

The implication is that it's the first booted domain. If that's not
generally the case, we'd need another qualification here. And
perhaps that qualification would in the end be domid == 0...

>> @@ -1962,7 +1962,7 @@ static void dump_softtsc(unsigned char key)
>>                  "warp=%lu (count=%lu)\n", tsc_max_warp, tsc_check_count);
>>       for_each_domain ( d )
>>       {
>> -        if ( d->domain_id == 0 && d->arch.tsc_mode == TSC_MODE_DEFAULT )
>> +        if ( is_hardware_domain(d) && d->arch.tsc_mode == TSC_MODE_DEFAULT )
>>               continue;
>>           printk("dom%u%s: mode=%d",d->domain_id,
>>                   is_hvm_domain(d) ? "(hvm)" : "", d->arch.tsc_mode);
> 
> Why have direct access to the tsc for the hardware domain and not the 
> control domain?

Because, as its name says, it has direct access to the hardware
(including the TSC)?

>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -745,7 +745,7 @@ static void pv_cpuid(struct cpu_user_regs *regs)
>>       c = regs->ecx;
>>       d = regs->edx;
>>   
>> -    if ( current->domain->domain_id != 0 )
>> +    if ( !is_control_domain(current->domain) )
>>       {
>>           unsigned int cpuid_leaf = a, sub_leaf = c;
>>   
> 
> Same as above re cpuid.

Ditto.

>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -238,7 +238,7 @@ struct domain *domain_create(
>>       if ( domcr_flags & DOMCRF_hvm )
>>           d->is_hvm = 1;
>>   
>> -    if ( domid == 0 )
>> +    if ( is_hardware_domain(d) )
>>       {
>>           d->is_pinned = opt_dom0_vcpus_pin;
>>           d->disable_migrate = 1;
> 
> At some point this will have to be thought about a bit more -- which of 
> the disaggregated domains do we actually want pinned here?  But I think 
> this is fine for now.

The pinning really is mainly for the Dom0-controlled CPU frequency
scaling to work (and hence validly qualified by is_hardware_domain()).
Any other uses, no matter how frequently they are seen, are abusing
dom0_vcpus_pin afaict.

Jan


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