Re: [Xen-devel] [PATCH 2 of 6] early PV on HVM

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 23 Apr 2010, Jeremy Fitzhardinge wrote:
> On 04/22/2010 08:15 AM, Stefano Stabellini wrote:
> > From: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
> >
> > this patch:
> >
> > - adds a xen_guest_init hook in setup.c right after kvm_guest_init;
> >
> > - initializes basic pv on hvm features in xen_guest_init;
> >   
> 
> Looks mostly OK, except:
> 
> > diff --git a/drivers/xen/xenbus/xenbus_probe.c 
> > b/drivers/xen/xenbus/xenbus_probe.c
> > index ab04a1b..7d11957 100644
> > --- a/drivers/xen/xenbus/xenbus_probe.c
> > +++ b/drivers/xen/xenbus/xenbus_probe.c
> > @@ -55,6 +55,8 @@
> >  #include <xen/events.h>
> >  #include <xen/page.h>
> >  
> > +#include <xen/hvm.h>
> > +
> >  #include "xenbus_comms.h"
> >  #include "xenbus_probe.h"
> >  
> > @@ -708,10 +710,19 @@ static int __init xenbus_probe_init(void)
> >             xen_store_interface = mfn_to_virt(xen_store_mfn);
> >     } else {
> >             xenstored_ready = 1;
> > -           xen_store_evtchn = xen_start_info->store_evtchn;
> > -           xen_store_mfn = xen_start_info->store_mfn;
> > +           if (xen_hvm_domain()) {
> > +                   xen_store_evtchn =
> > +                           hvm_get_parameter(HVM_PARAM_STORE_EVTCHN);
> > +                   xen_store_mfn =
> > +                           hvm_get_parameter(HVM_PARAM_STORE_PFN);
> > +                   xen_store_interface =
> > +                           ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
> >   
> 
> I think I'd prefer to see these wrap 80 cols than be line-broken like this.

Ok

> 
> > +           } else {
> > +                   xen_store_evtchn = xen_start_info->store_evtchn;
> > +                   xen_store_mfn = xen_start_info->store_mfn;
> > +                   xen_store_interface = mfn_to_virt(xen_store_mfn);
> > +           }
> >     }
> > -   xen_store_interface = mfn_to_virt(xen_store_mfn);
> >  
> >     /* Initialize the interface to xenstore. */
> >     err = xs_init();
> > diff --git a/include/xen/xen.h b/include/xen/xen.h
> > index a164024..cb8c48b 100644
> > --- a/include/xen/xen.h
> > +++ b/include/xen/xen.h
> > @@ -9,8 +9,10 @@ enum xen_domain_type {
> >  
> >  #ifdef CONFIG_XEN
> >  extern enum xen_domain_type xen_domain_type;
> > +extern void xen_guest_init(void);
> >  #else
> >  #define xen_domain_type            XEN_NATIVE
> > +#define xen_guest_init() do { } while (0)
> >  #endif
> >   
> 
> This isn't an appropriate header for this.  Something under
> arch/x86/include/asm would be better.
> 
 

I moved it to arch/x86/include/asm/xen/xen.h, is that OK?


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel