Re: [Xen-devel] [PATCH 8/9] xen/arch/x86: use is_hardware_domain instead of domid == 0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.04.13 at 16:24, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> On 04/12/2013 04:07 AM, Jan Beulich wrote:
>>>>> On 11.04.13 at 22:13, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1571,7 +1571,7 @@ void context_switch(struct vcpu *prev, struct vcpu 
> *next)
>>>           }
>>>
>>>           set_cpuid_faulting(!is_hvm_vcpu(next) &&
>>> -                           (next->domain->domain_id != 0));
>>> +                           !is_hardware_domain(next->domain));
>>
>> This one is highly questionable. Following the comments on the
>> previous patch, I'd think is_control_domain() here is more
>> appropriate, but maybe it would really need to become the or
>> of both? The question really is which domains we specifically
>> don't want CPUID faulting for.
> 
> My impression was that this would be the hardware domain since it is
> the one that would be parsing ACPI tables and similar things that need
> CPUID to work. The control domain would just be involved in creating
> and managing domains, which doesn't need anything out of CPUID that a
> normal PV domain would need.

Parsing ACPI tables is unrelated to CPUID, as is most other
hardware management. The only reason I could see to override
this would be to have some domains know the true capabilities of
the processor. And that would be the control domain(s), in order
to be able to sensibly set up CPUID emulation for the guests.

>>> --- a/xen/arch/x86/hvm/i8254.c
>>> +++ b/xen/arch/x86/hvm/i8254.c
>>> @@ -541,7 +541,7 @@ int pv_pit_handler(int port, int data, int write)
>>>           .data = data
>>>       };
>>>
>>> -    if ( (current->domain->domain_id == 0) && dom0_pit_access(&ioreq) )
>>> +    if ( is_hardware_domain(current->domain) && dom0_pit_access(&ioreq) )
>>
>> And this one is now becoming inconsistent in itself: You remove the
>> explicit 0 on the left side, but leave the "dom0" in place on the right
>> side. If we drop the association with domid == 0 being the special
>> one, then all uses of "dom0" would logically also need to go away.
> 
> I didn't want to take on the mass function rename that this would imply. I
> had considered naming the function "is_dom0_domain" but this seemed less
> clear given that I was trying to isolate hardware accesses.

I understood this would have been the main reason, but then we
can as well leave the domid == 0 check in place...

Jan


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