Re: [Xen-devel] [PATCH RFC v12 05/21] Introduce pv guest type and has_hvm_container macros

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.13 at 18:25, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> @@ -72,7 +72,7 @@ void *map_domain_page(unsigned long mfn)
>  #endif
>  
>      v = mapcache_current_vcpu();
> -    if ( !v || is_hvm_vcpu(v) )
> +    if ( !v || has_hvm_container_vcpu(v) )
>          return mfn_to_virt(mfn);
>  
>      dcache = &v->domain->arch.pv_domain.mapcache;
> @@ -177,7 +177,7 @@ void unmap_domain_page(const void *ptr)
>      ASSERT(va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END);
>  
>      v = mapcache_current_vcpu();
> -    ASSERT(v && !is_hvm_vcpu(v));
> +    ASSERT(v && is_pv_vcpu(v));

This is inconsistent with the above change to map_domain_page()
and the changes later in this file.

> @@ -1237,7 +1237,7 @@ void arch_get_info_guest(struct vcpu *v, 
> vcpu_guest_context_u c)
>      bool_t compat = is_pv_32on64_domain(v->domain);
>  #define c(fld) (!compat ? (c.nat->fld) : (c.cmp->fld))
>  
> -    if ( is_hvm_vcpu(v) )
> +    if ( has_hvm_container_vcpu(v) )
>          memset(c.nat, 0, sizeof(*c.nat));
>      memcpy(&c.nat->fpu_ctxt, v->arch.fpu_ctxt, sizeof(c.nat->fpu_ctxt));

I think best would be to drop the condition here altogether.

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -119,6 +119,7 @@ static void show_guest_stack(struct vcpu *v, struct 
> cpu_user_regs *regs)
>      unsigned long *stack, addr;
>      unsigned long mask = STACK_SIZE;
>  
> +    /* Avoid HVM as we don't know what the stack looks like */
>      if ( is_hvm_vcpu(v) )

So why do you not change this one to !is_pv_vcpu()? In particular
the do_page_walk() further down is inherently PV.

> @@ -732,8 +736,12 @@ void watchdog_domain_destroy(struct domain *d);
>  
>  #define VM_ASSIST(_d,_t) (test_bit((_t), &(_d)->vm_assist))
>  
> -#define is_hvm_domain(d) ((d)->is_hvm)
> +#define is_pv_domain(d) ((d)->guest_type == guest_type_pv)
> +#define is_pv_vcpu(v)   (is_pv_domain((v)->domain))
> +#define is_hvm_domain(d) ((d)->guest_type == guest_type_hvm)
>  #define is_hvm_vcpu(v)   (is_hvm_domain(v->domain))
> +#define has_hvm_container_domain(d) ((d)->guest_type != guest_type_pv)
> +#define has_hvm_container_vcpu(v)   (has_hvm_container_domain((v)->domain))

So in the end is_pv_...() = !has_hvm_container_...(), i.e. I
don't really see the point in having both.

Jan


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