Re: [PATCH v7 17/19] xen: xenstore changes to support PVH

On Thursday, December 19, 2013 1:54:54 pm Roger Pau Monne wrote:
> ---
>  sys/xen/xenstore/xenstore.c |   21 ++++++++++-----------
>  1 files changed, 10 insertions(+), 11 deletions(-)

This looks fine modulo possibly making it a xenpv child instead of a nexus 
child.

Note that even in the old XEN code in FreeBSD I would argue that attaching
directly to nexus is wrong and that even the old XEN code should create
some sort of top-level device below nexus0 that is XEN-specific for XEN-
specific drivers to attach to.  I've no idea what shape the old XEN-specific 
code is in or if it can be tested, but I'd really like PVH to do the right 
thing if at all possible.

> diff --git a/sys/xen/xenstore/xenstore.c b/sys/xen/xenstore/xenstore.c
> index bcf6357..2893c84 100644
> --- a/sys/xen/xenstore/xenstore.c
> +++ b/sys/xen/xenstore/xenstore.c
> _at__at_ -229,13 +229,11 _at__at_ struct xs_softc {
>  	 */
>  	struct sx xenwatch_mutex;
>  
> -#ifdef XENHVM
>  	/**
>  	 * The HVM guest pseudo-physical frame number.  This is Xen's mapping
>  	 * of the true machine frame number into our "physical address space".
>  	 */
>  	unsigned long gpfn;
> -#endif
>  
>  	/**
>  	 * The event channel for communicating with the
> _at__at_ -1141,13 +1139,15 _at__at_ xs_attach(device_t dev)
>  	/* Initialize the interface to xenstore. */
>  	struct proc *p;
>  
> -#ifdef XENHVM
> -	xs.evtchn = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN);
> -	xs.gpfn = hvm_get_parameter(HVM_PARAM_STORE_PFN);
> -	xen_store = pmap_mapdev(xs.gpfn * PAGE_SIZE, PAGE_SIZE);
> -#else
> -	xs.evtchn = xen_start_info->store_evtchn;
> -#endif
> +	if (xen_hvm_domain()) {
> +		xs.evtchn = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN);
> +		xs.gpfn = hvm_get_parameter(HVM_PARAM_STORE_PFN);
> +		xen_store = pmap_mapdev(xs.gpfn * PAGE_SIZE, PAGE_SIZE);
> +	} else if (xen_pv_domain()) {
> +		xs.evtchn = HYPERVISOR_start_info->store_evtchn;
> +	} else {
> +		panic("Unknown domain type, cannot initialize xenstore\n");
> +	}
>  
>  	TAILQ_INIT(&xs.reply_list);
>  	TAILQ_INIT(&xs.watch_events);
> _at__at_ -1256,9 +1256,8 _at__at_ static devclass_t xenstore_devclass;
>   
>  #ifdef XENHVM
>  DRIVER_MODULE(xenstore, xenpci, xenstore_driver, xenstore_devclass, 0, 0);
> -#else
> -DRIVER_MODULE(xenstore, nexus, xenstore_driver, xenstore_devclass, 0, 0);
>  #endif
> +DRIVER_MODULE(xenstore, nexus, xenstore_driver, xenstore_devclass, 0, 0);
>  
>  /*------------------------------- Sysctl Data 
--------------------------------*/
>  /* XXX Shouldn't the node be somewhere else? */
> -- 
> 1.7.7.5 (Apple Git-26)
> 
> 

-- 
John Baldwin