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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> --- 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.

> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1928,7 +1928,7 @@ void tsc_set_info(struct domain *d,
>                    uint32_t tsc_mode, uint64_t elapsed_nsec,
>                    uint32_t gtsc_khz, uint32_t incarnation)
>  {
> -    if ( is_idle_domain(d) || (d->domain_id == 0) )
> +    if ( is_idle_domain(d) || is_hardware_domain(d) )

Just like with the CPUID faulting, it is unclear (without a proper
model described end established first) which domains we really
want to special case here. Hence, rather than papering over the
issue, I'd prefer keeping the explicit domid check in place until
such a model can get established.

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -737,7 +737,7 @@ static void pv_cpuid(struct cpu_user_regs *regs)
>      c = regs->ecx;
>      d = regs->edx;
>  
> -    if ( current->domain->domain_id != 0 )
> +    if ( !is_hardware_domain(current->domain) )

Again, if anything this is more likely is_control_domain() or the or
of both.

Jan


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