Re: [Xen-devel] [PATCH 06/17] [V3]PVH xen: Introduce PVH guest type

[Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>]

On Mon, 15 Apr 2013 16:12:50 +0100
"Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> >>> On 13.04.13 at 03:02, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> >>> wrote:
> > @@ -158,7 +158,7 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t *buf,
> > int len, struct domain *dp, 
> >          pagecnt = min_t(long, PAGE_SIZE - (addr & ~PAGE_MASK),
> > len); 
> > -        mfn = (dp->is_hvm
> > +        mfn = (is_hvm_domain(dp)
> >                 ? dbg_hvm_va2mfn(addr, dp, toaddr, &gfn)
> >                 : dbg_pv_va2mfn(addr, dp, pgd3));
> 
> Doesn't this rather need to be !is_pv_domain()?
> 
> > @@ -256,10 +256,9 @@ struct arch_domain
> >  
> >      struct list_head pdev_list;
> >  
> > -    union {
> > -        struct pv_domain pv_domain;
> > -        struct hvm_domain hvm_domain;
> > -    };
> > +    /* PVH : pvh uses fields from both pv and hvm, hence not a
> > union */
> > +    struct pv_domain pv_domain;
> > +    struct hvm_domain hvm_domain;
> 
> This isn't nice, as it needlessly grows struct domain (limited to a
> page in size). And I think I said before that I'm of the opinion that
> you ought to pull out the shared fields, but leave the private ones
> in place. Of course, unless that's _almost all_ of them...
> 
> Jan
> 

pv_domain is very small, but should be easy to go back to union. Just
need to pull the e820* related fields to PVH struct. 

thanks,
Mukesh

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