Re: [PATCH] Re: Re: [PATCH 2/2] PV framebuffer

[Markus Armbruster <armbru@redhat.com>]

Atsushi SAKAI <sakaia@jp.fujitsu.com> writes:

> Hi, Markus
>
>  This is a patch for work PV frame buffer on IA64.
> We also prepare the patch for FC6 and RHEL5B2 and its confirmed working 
> on IA64 and x86.
>
> In this patch code, 
> #ifdef __ia64__ exists, but it does not need.
> The purpose is avoid the unnecessary error handling for x86.

Do you mean your __ia64__ code works fine everywhere?

> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> Signed-off-by: Masami Watanabe <masami.watanabe@jp.fujitsu.com>
> Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>
>
>
>>From seeing your patches policy,
> We do memory address translation in Dom0 application side.

Looks like this is support for shadow translate mode guests.  My
current code has that, but I didn't catch all places.  Cool.

> Thanks
> Atsushi SAKAI
[...]
> diff -r 92fddc5af648 tools/xenfb/xenfb.c
> --- a/tools/xenfb/xenfb.c	Wed Nov 22 20:34:45 2006 +0900
> +++ b/tools/xenfb/xenfb.c	Wed Nov 22 20:36:35 2006 +0900
> @@ -344,6 +344,9 @@ bool xenfb_attach_dom(struct xenfb *xenf
>          int serrno;
>          int fbdev_evtchn, kbd_evtchn;
>  	unsigned long fbdev_mfn, kbd_mfn;
> +#ifdef __ia64__
> +	xen_pfn_t *page_array;
> +#endif
>  
>  	if (xenfb->domid != -1) {
>  		xenfb_detach_dom(xenfb);
> @@ -432,16 +435,24 @@ bool xenfb_attach_dom(struct xenfb *xenf
>  	if (xenfb->kbd_port == -1) 
>  		goto error;
>  
> +#ifdef __ia64__
>  	ret = xc_domain_translate_gpfn_list(xenfb->xc, domid, 1,
>  					    &fbdev_mfn, &fbdev_mfn);
>  	if (ret < 0 && errno != EINVAL)
>  		goto error;
> +#endif
>  	xenfb->fb_info = xc_map_foreign_range(xenfb->xc, domid, XC_PAGE_SIZE,
>  					      PROT_READ | PROT_WRITE,
>  					      fbdev_mfn);
>  	if (xenfb->fb_info == NULL)
>  		goto error;
>  
> +#ifdef __ia64__
> +	ret = xc_domain_translate_gpfn_list(xenfb->xc, domid, 1,
> +					    &kbd_mfn, &kbd_mfn);
> +	if (ret < 0 && errno != EINVAL)
> +		goto error;
> +#endif
>  	xenfb->kbd_info = xc_map_foreign_range(xenfb->xc, domid, XC_PAGE_SIZE,
>  					       PROT_READ | PROT_WRITE,
>  					       kbd_mfn);
> @@ -466,10 +477,36 @@ bool xenfb_attach_dom(struct xenfb *xenf
>  	 * happens, we happily continue here, and later crash on
>  	 * access.
>  	 */
> -	fbmfns = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ, xenfb->fb_info->pd, n_fbdirs);
> +	
> +#ifdef __ia64__
> +	page_array = (xen_pfn_t *)malloc(n_fbdirs * sizeof(xen_pfn_t));
> +	ret = xc_domain_translate_gpfn_list(xenfb->xc, domid,
> +		n_fbdirs, xenfb->fb_info->pd, page_array);
> +	if (ret == 0) {
> +		memcpy(xenfb->fb_info->pd, page_array,
> +			n_fbdirs * sizeof(xen_pfn_t));
> +	} else {
> +		if (ret < 0 && errno != EINVAL)
> +			goto error;
> +	}

Stupid question: why can't you translate in place here instead of
bouncing through page_array?

> +	free(page_array);
> +#endif
> +	fbmfns = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ | PROT_WRITE, xenfb->fb_info->pd, n_fbdirs);
>  	if (fbmfns == NULL)
>  		goto error;
>  
> +#ifdef __ia64__
> +	page_array = (xen_pfn_t *)malloc(n_fbmfns * sizeof(xen_pfn_t));
> +	ret = xc_domain_translate_gpfn_list(xenfb->xc, domid,
> +		n_fbmfns, fbmfns, page_array);
> +	if (ret == 0) {
> +		memcpy(fbmfns, page_array, n_fbmfns * sizeof(xen_pfn_t));
> +	} else {
> +		if (ret < 0 && errno != EINVAL)
> +			goto error;
> +	}
> +	free(page_array);
> +#endif
>  	xenfb->fb = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ | PROT_WRITE, fbmfns, n_fbmfns);
>  	if (xenfb->fb == NULL)
>  		goto error;