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

[Jeremy Fitzhardinge <jeremy@xxxxxxxx>]

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.

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

    J

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