[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/2] xenconsoled: use grant references instead of map_foreign_range



On Thu, 10 Jan 2013, Daniel De Graaf wrote:
> Grant references for the xenstore and xenconsole shared pages exist, but
> currently only xenstore uses these references.  Change the xenconsole
> daemon to prefer using the grant reference over map_foreign_range when
> mapping the shared console ring.
> 
> This allows xenconsoled to be run in a domain other than dom0 if set up
> correctly - for libxl, the xenstore path /tool/xenconsoled/domid
> specifies the domain containing xenconsoled.

Unfortunately at the moment xc_dom_gnttab_init doesn't work with an
autotranslated dom0, that is the only type of dom0 that you get on ARM.
As a consequence this patch would break xenconsoled on ARM. I expect the
same to happen with PVH as well.

I also have the same issue with xenstored and I was thinking about
writing a patch to make it gracefully fallback to xc_map_foreign_range
if xc_gnttab_map_grant_ref fails.

The reason why xc_dom_gnttab_init is broken is that it uses
xc_map_foreign_range to map the grant table page of the foreign domain,
that is not a regular gpfn page, therefore it fails.
Specifically on ARM it hits the case XENMAPSPACE_gmfn_foreign in
xenmem_add_to_physmap_one, then it fails the p2m lookup and returns
EINVAL.

Do you feel up for making xc_dom_gnttab_init work for an autotranslated
dom0?
Otherwise I think that for the moment is best to modify this patch to
gracefully fallback to xc_map_foreign_range. Then we need a couple of
other patches to do the same with xenstored and libxl.



> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  tools/console/daemon/io.c   | 48 
> ++++++++++++++++++++++++++++++++++++++-------
>  tools/console/daemon/io.h   |  1 -
>  tools/console/daemon/main.c |  2 --
>  3 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 48fe151..e1720b6 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -24,6 +24,7 @@
>  #include "io.h"
>  #include <xenstore.h>
>  #include <xen/io/console.h>
> +#include <xen/grant_table.h>
>  
>  #include <stdlib.h>
>  #include <errno.h>
> @@ -69,6 +70,7 @@ static int log_hv_fd = -1;
>  static evtchn_port_or_error_t log_hv_evtchn = -1;
>  static xc_interface *xch; /* why does xenconsoled have two xc handles ? */
>  static xc_evtchn *xce_handle = NULL;
> +static xc_gnttab *xcg_handle = NULL;
>  
>  struct buffer {
>       char *data;
> @@ -501,6 +503,18 @@ static int xs_gather(struct xs_handle *xs, const char 
> *dir, ...)
>       va_end(ap);
>       return ret;
>  }
> +
> +static void domain_unmap_interface(struct domain *dom)
> +{
> +     if (dom->interface == NULL)
> +             return;
> +     if (xcg_handle && dom->ring_ref == -1)
> +             xc_gnttab_munmap(xcg_handle, dom->interface, 1);
> +     else
> +             munmap(dom->interface, getpagesize());
> +     dom->interface = NULL;
> +     dom->ring_ref = -1;
> +}
>   
>  static int domain_create_ring(struct domain *dom)
>  {
> @@ -522,9 +536,19 @@ static int domain_create_ring(struct domain *dom)
>       }
>       free(type);
>  
> -     if (ring_ref != dom->ring_ref) {
> -             if (dom->interface != NULL)
> -                     munmap(dom->interface, getpagesize());
> +     /* If using ring_ref and it has changed, remap */
> +     if (ring_ref != dom->ring_ref && dom->ring_ref != -1)
> +             domain_unmap_interface(dom);
> +
> +     if (!dom->interface && xcg_handle) {
> +             /* Prefer using grant table */
> +             dom->interface = xc_gnttab_map_grant_ref(xcg_handle,
> +                     dom->domid, GNTTAB_RESERVED_CONSOLE,
> +                     PROT_READ|PROT_WRITE);
> +             dom->ring_ref = -1;
> +     }
> +     if (!dom->interface) {
> +             /* Fall back to xc_map_foreign_range */
>               dom->interface = xc_map_foreign_range(
>                       xc, dom->domid, getpagesize(),
>                       PROT_READ|PROT_WRITE,
> @@ -720,9 +744,7 @@ static void shutdown_domain(struct domain *d)
>  {
>       d->is_dead = true;
>       watch_domain(d, false);
> -     if (d->interface != NULL)
> -             munmap(d->interface, getpagesize());
> -     d->interface = NULL;
> +     domain_unmap_interface(d);
>       if (d->xce_handle != NULL)
>               xc_evtchn_close(d->xce_handle);
>       d->xce_handle = NULL;
> @@ -730,7 +752,7 @@ static void shutdown_domain(struct domain *d)
>  
>  static unsigned enum_pass = 0;
>  
> -void enum_domains(void)
> +static void enum_domains(void)
>  {
>       int domid = 1;
>       xc_dominfo_t dominfo;
> @@ -957,6 +979,14 @@ void handle_io(void)
>               }
>       }
>  
> +     xcg_handle = xc_gnttab_open(NULL, 0);
> +     if (xcg_handle == NULL) {
> +             dolog(LOG_DEBUG, "Failed to open xcg handle: %d (%s)",
> +                   errno, strerror(errno));
> +     }
> +
> +     enum_domains();
> +
>       for (;;) {
>               struct domain *d, *n;
>               int max_fd = -1;
> @@ -1097,6 +1127,10 @@ void handle_io(void)
>               xc_evtchn_close(xce_handle);
>               xce_handle = NULL;
>       }
> +     if (xcg_handle != NULL) {
> +             xc_gnttab_close(xcg_handle);
> +             xcg_handle = NULL;
> +     }
>       log_hv_evtchn = -1;
>  }
>  
> diff --git a/tools/console/daemon/io.h b/tools/console/daemon/io.h
> index 8fa04b6..f658bfc 100644
> --- a/tools/console/daemon/io.h
> +++ b/tools/console/daemon/io.h
> @@ -21,7 +21,6 @@
>  #ifndef CONSOLED_IO_H
>  #define CONSOLED_IO_H
>  
> -void enum_domains(void);
>  void handle_io(void);
>  
>  #endif
> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
> index 789baa6..92d2fc4 100644
> --- a/tools/console/daemon/main.c
> +++ b/tools/console/daemon/main.c
> @@ -161,8 +161,6 @@ int main(int argc, char **argv)
>       if (!xen_setup())
>               exit(1);
>  
> -     enum_domains();
> -
>       handle_io();
>  
>       closelog();
> -- 
> 1.7.11.7
> 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.