From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiageng Yu Subject: Re: Linux Stubdom Problem Date: Fri, 29 Jul 2011 23:09:05 +0800 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0569962655==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Stefano Stabellini Cc: Ian Campbell , Anthony PERARD , "Xen-devel@lists.xensource.com" , Samuel Thibault List-Id: xen-devel@lists.xenproject.org --===============0569962655== Content-Type: multipart/alternative; boundary=bcaec53af11e946c0404a936a91d --bcaec53af11e946c0404a936a91d Content-Type: text/plain; charset=UTF-8 2011/7/29 Stefano Stabellini > On Fri, 29 Jul 2011, Jiageng Yu wrote: > > I have noticed the mistake. In fact, we shall stop the map_foreign_pages > of xen_console and xenfb devices in qemu. Because > > the front drivers in stubdom has already map the memories. This is my new > patch. But it is not stable, I am testing it. > > > > We use xc_handle to map foreign pages in xenfb and xen_console devices. > If qemu running on stubdom, the xc_handle is > > invalid. > > > > > > diff --git a/hw/xen_backend.c b/hw/xen_backend.c > > index cfb53c8..11c53fe 100644 > > --- a/hw/xen_backend.c > > +++ b/hw/xen_backend.c > > @@ -47,6 +47,7 @@ XenXC xen_xc = XC_HANDLER_INITIAL_VALUE; > > XenGnttab xen_xcg = XC_HANDLER_INITIAL_VALUE; > > struct xs_handle *xenstore = NULL; > > const char *xen_protocol; > > +XenXC xc_handle = XC_HANDLER_INITIAL_VALUE; > > > > /* private */ > > static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = > QTAILQ_HEAD_INITIALIZER(xendevs); > > @@ -655,6 +656,7 @@ static void xen_be_evtchn_event(void *opaque) > > > > int xen_be_init(void) > > { > > +#ifndef CONFIG_STUBDOM > > xenstore = xs_daemon_open(); > > if (!xenstore) { > > xen_be_printf(NULL, 0, "can't connect to xenstored\n"); > > @@ -665,10 +667,16 @@ int xen_be_init(void) > > goto err; > > } > > > > + if (xc_handle == XC_HANDLER_INITIAL_VALUE) { > > + goto err; > > + } > > +#endif > > + > > if (xen_xc == XC_HANDLER_INITIAL_VALUE) { > > /* Check if xen_init() have been called */ > > goto err; > > } > > + > > return 0; > > > > err: > > diff --git a/hw/xen_backend.h b/hw/xen_backend.h > > index 9d36df3..bc5a157 100644 > > --- a/hw/xen_backend.h > > +++ b/hw/xen_backend.h > > @@ -59,6 +59,9 @@ extern XenXC xen_xc; > > extern struct xs_handle *xenstore; > > extern const char *xen_protocol; > > > > +/* invalid in linux stubdom */ > > +extern XenXC xc_handle; > > + > > /* xenstore helper functions */ > > int xenstore_write_str(const char *base, const char *node, const char > *val); > > int xenstore_write_int(const char *base, const char *node, int ival); > > diff --git a/hw/xen_console.c b/hw/xen_console.c > > index c6c8163..66b6dd7 100644 > > --- a/hw/xen_console.c > > +++ b/hw/xen_console.c > > @@ -213,7 +213,7 @@ static int con_connect(struct XenDevice *xendev) > > if (xenstore_read_int(con->console, "limit", &limit) == 0) > > con->buffer.max_capacity = limit; > > > > - con->sring = xc_map_foreign_range(xen_xc, con->xendev.dom, > > + con->sring = xc_map_foreign_range(xc_handle, con->xendev.dom, > > XC_PAGE_SIZE, > > PROT_READ|PROT_WRITE, > > con->ring_ref); > > diff --git a/hw/xenfb.c b/hw/xenfb.c > > index 039076a..278fa60 100644 > > --- a/hw/xenfb.c > > +++ b/hw/xenfb.c > > @@ -104,7 +104,7 @@ static int common_bind(struct common *c) > > if (xenstore_read_fe_int(&c->xendev, "event-channel", > &c->xendev.remote_port) == -1) > > return -1; > > > > - c->page = xc_map_foreign_range(xen_xc, c->xendev.dom, > > + c->page = xc_map_foreign_range(xc_handle, c->xendev.dom, > > XC_PAGE_SIZE, > > PROT_READ | PROT_WRITE, mfn); > > if (c->page == NULL) > > @@ -482,14 +482,14 @@ static int xenfb_map_fb(struct XenFB *xenfb) > > fbmfns = qemu_mallocz(sizeof(unsigned long) * xenfb->fbpages); > > > > xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd); > > - map = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom, > > + map = xc_map_foreign_pages(xc_handle, xenfb->c.xendev.dom, > > PROT_READ, pgmfns, n_fbdirs); > > if (map == NULL) > > goto out; > > xenfb_copy_mfns(mode, xenfb->fbpages, fbmfns, map); > > munmap(map, n_fbdirs * XC_PAGE_SIZE); > > > > - xenfb->pixels = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom, > > + xenfb->pixels = xc_map_foreign_pages(xc_handle, xenfb->c.xendev.dom, > > PROT_READ | PROT_WRITE, fbmfns, xenfb->fbpages); > > if (xenfb->pixels == NULL) > > goto out; > > diff --git a/xen-all.c b/xen-all.c > > index b73fc43..04dfb51 100644 > > --- a/xen-all.c > > +++ b/xen-all.c > > @@ -527,12 +534,22 @@ int xen_init(void) > > return -1; > > } > > > > +#ifdef CONFIG_STUBDOM > > + return 0; > > +#endif > > + > > + xc_handle = xen_xc_interface_open(0, 0, 0); > > + if (xc_handle == XC_HANDLER_INITIAL_VALUE) { > > + xen_be_printf(NULL, 0, "can't open xen interface\n"); > > + return -1; > > + } > > + > > return 0; > > } > > > > > > I think that the backends shouldn't be initialized at all when running > in the stubdom, so I would do something like this instead: > > diff --git a/xen-all.c b/xen-all.c > index 167bed6..e3f630b 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -922,6 +922,7 @@ int xen_hvm_init(void) > cpu_register_phys_memory_client(&state->client); > state->log_for_dirtybit = NULL; > > +#ifndef CONFIG_STUBDOM > /* Initialize backend core & drivers */ > if (xen_be_init() != 0) { > fprintf(stderr, "%s: xen backend core setup failed\n", > __FUNCTION__); > @@ -930,7 +931,7 @@ int xen_hvm_init(void) > xen_be_register("console", &xen_console_ops); > xen_be_register("vkbd", &xen_kbdmouse_ops); > xen_be_register("qdisk", &xen_blkdev_ops); > - > +#endif > return 0; > } > Yes. this implementation of stubdom is more closer to the old qemu's implementation. Which branch this patch works on? The qemu-dm-v15 would not so lucky. --bcaec53af11e946c0404a936a91d Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

2011/7/29 Stefano Stabellini <stefano.st= abellini@eu.citrix.com>
On Fri, 29 Jul 2011, Jiageng Yu wrote: > I have noticed the mistake. In fact, we shall stop the map_foreign_pag= es of xen_console and xenfb devices in qemu. Because
> the front drivers in stubdom has already map the memories. This is my = new patch. But it is not stable, I am testing it.
> =C2=A0
> We use xc_handle to map foreign pages in xenfb and xen_console devices= . If qemu running on stubdom, the xc_handle is
> invalid.
> =C2=A0
> =C2=A0
> diff --git a/hw/xen_backend.c b/hw/xen_backend.c
> index cfb53c8..11c53fe 100644
> --- a/hw/xen_backend.c
> +++ b/hw/xen_backend.c
> @@ -47,6 +47,7 @@ XenXC xen_xc =3D XC_HANDLER_INITIAL_VALUE;
> =C2=A0XenGnttab xen_xcg =3D XC_HANDLER_INITIAL_VALUE;
> =C2=A0struct xs_handle *xenstore =3D NULL;
> =C2=A0const char *xen_protocol;
> +XenXC xc_handle =3D XC_HANDLER_INITIAL_VALUE;
> =C2=A0
> =C2=A0/* private */
> =C2=A0static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs =3D QTAILQ_= HEAD_INITIALIZER(xendevs);
> @@ -655,6 +656,7 @@ static void xen_be_evtchn_event(void *opaque)
> =C2=A0
> =C2=A0int xen_be_init(void)
> =C2=A0{
> +#ifndef CONFIG_STUBDOM
> =C2=A0=C2=A0=C2=A0=C2=A0 xenstore =3D xs_daemon_open();
> =C2=A0=C2=A0=C2=A0=C2=A0 if (!xenstore) {
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 xen_be_printf(NULL, 0= , "can't connect to xenstored\n");
> @@ -665,10 +667,16 @@ int xen_be_init(void)
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto err;
> =C2=A0=C2=A0=C2=A0=C2=A0 }
> =C2=A0
> +=C2=A0=C2=A0=C2=A0 if (xc_handle =3D=3D XC_HANDLER_INITIAL_VALUE) { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto err;
> +=C2=A0=C2=A0=C2=A0 }
> +#endif
> +
> =C2=A0=C2=A0=C2=A0=C2=A0 if (xen_xc =3D=3D XC_HANDLER_INITIAL_VALUE) {=
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Check if xen_init(= ) have been called */
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto err;
> =C2=A0=C2=A0=C2=A0=C2=A0 }
> +
> =C2=A0=C2=A0=C2=A0=C2=A0 return 0;
> =C2=A0
> =C2=A0err:
> diff --git a/hw/xen_backend.h b/hw/xen_backend.h
> index 9d36df3..bc5a157 100644
> --- a/hw/xen_backend.h
> +++ b/hw/xen_backend.h
> @@ -59,6 +59,9 @@ extern XenXC xen_xc;
> =C2=A0extern struct xs_handle *xenstore;
> =C2=A0extern const char *xen_protocol;
> =C2=A0
> +/* invalid in linux stubdom */
> +extern XenXC xc_handle;
> +
> =C2=A0/* xenstore helper functions */
> =C2=A0int xenstore_write_str(const char *base, const char *node, const= char *val);
> =C2=A0int xenstore_write_int(const char *base, const char *node, int i= val);
> diff --git a/hw/xen_console.c b/hw/xen_console.c
> index c6c8163..66b6dd7 100644
> --- a/hw/xen_console.c
> +++ b/hw/xen_console.c
> @@ -213,7 +213,7 @@ static int con_connect(struct XenDevice *xendev) > =C2=A0=C2=A0=C2=A0=C2=A0 if (xenstore_read_int(con->console, "= limit", &limit) =3D=3D 0)
> =C2=A0=C2=A0con->buffer.max_capacity =3D limit;
> =C2=A0
> -=C2=A0=C2=A0=C2=A0 con->sring =3D xc_map_foreign_range(xen_xc, con= ->xendev.dom,
> +=C2=A0=C2=A0=C2=A0con->sring =3D xc_map_foreign_range(xc_handle, c= on->xendev.dom,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 XC_PAGE_S= IZE,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PROT_READ= |PROT_WRITE,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 con->r= ing_ref);
> diff --git a/hw/xenfb.c b/hw/xenfb.c
> index 039076a..278fa60 100644
> --- a/hw/xenfb.c
> +++ b/hw/xenfb.c
> @@ -104,7 +104,7 @@ static int common_bind(struct common *c)
> =C2=A0=C2=A0=C2=A0=C2=A0 if (xenstore_read_fe_int(&c->xendev, &= quot;event-channel", &c->xendev.remote_port) =3D=3D -1)
> =C2=A0=C2=A0return -1;
> =C2=A0
> -=C2=A0=C2=A0=C2=A0 c->page =3D xc_map_foreign_range(xen_xc, c->= xendev.dom,
> +=C2=A0=C2=A0=C2=A0c->page =3D xc_map_foreign_range(xc_handle, c-&g= t;xendev.dom,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 XC_PAGE_SIZE,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PROT_READ | PROT_WRITE, mfn= );
> =C2=A0=C2=A0=C2=A0=C2=A0 if (c->page =3D=3D NULL)
> @@ -482,14 +482,14 @@ static int xenfb_map_fb(struct XenFB *xenfb)
> =C2=A0=C2=A0=C2=A0=C2=A0 fbmfns =3D qemu_mallocz(sizeof(unsigned long)= * xenfb->fbpages);
> =C2=A0
> =C2=A0=C2=A0=C2=A0=C2=A0 xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);<= br> > -=C2=A0=C2=A0=C2=A0 map =3D xc_map_foreign_pages(xen_xc, xenfb->c.x= endev.dom,
> +=C2=A0=C2=A0=C2=A0map =3D xc_map_foreign_pages(xc_handle, xenfb->c= .xendev.dom,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PROT_READ= , pgmfns, n_fbdirs);
> =C2=A0=C2=A0=C2=A0=C2=A0 if (map =3D=3D NULL)
> =C2=A0=C2=A0goto out;
> =C2=A0=C2=A0=C2=A0=C2=A0 xenfb_copy_mfns(mode, xenfb->fbpages, fbmf= ns, map);
> =C2=A0=C2=A0=C2=A0=C2=A0 munmap(map, n_fbdirs * XC_PAGE_SIZE);
> =C2=A0
> -=C2=A0=C2=A0=C2=A0 xenfb->pixels =3D xc_map_foreign_pages(xen_xc, = xenfb->c.xendev.dom,
> +=C2=A0=C2=A0=C2=A0xenfb->pixels =3D xc_map_foreign_pages(xc_handle= , xenfb->c.xendev.dom,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PROT_READ | PROT_WRITE, fbmfns, x= enfb->fbpages);
> =C2=A0=C2=A0=C2=A0=C2=A0 if (xenfb->pixels =3D=3D NULL)
> =C2=A0=C2=A0goto out;
> diff --git a/xen-all.c b/xen-all.c
> index b73fc43..04dfb51 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -527,12 +534,22 @@ int xen_init(void)
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return -1;
> =C2=A0=C2=A0=C2=A0=C2=A0 }
> =C2=A0
> +#ifdef CONFIG_STUBDOM
> +=C2=A0=C2=A0=C2=A0 return 0;
> +#endif
> +
> +=C2=A0=C2=A0=C2=A0 xc_handle =3D xen_xc_interface_open(0, 0, 0);
> +=C2=A0=C2=A0=C2=A0 if (xc_handle =3D=3D XC_HANDLER_INITIAL_VALUE) { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 xen_be_printf(NULL, 0, &qu= ot;can't open xen interface\n");
> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return -1;
> +=C2=A0=C2=A0=C2=A0 }
> +
> =C2=A0=C2=A0=C2=A0=C2=A0 return 0;
> =C2=A0}
>
>

I think that the backends shouldn't be initialized at all w= hen running
in the stubdom, so I would do something like this instead:

diff --git a/xen-all.c b/xen-all.c
index 167bed6..e3f630b 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -922,6 +922,7 @@ int xen_hvm_init(void)
=C2=A0 =C2=A0 cpu_register_phys_memory_client(&state->client);
=C2=A0 =C2=A0 state->log_for_dirtybit =3D NULL;

+#ifndef CONFIG_STUBDOM
=C2=A0 =C2=A0 /* Initialize backend core & drivers */
=C2=A0 =C2=A0 if (xen_be_init() !=3D 0) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 fprintf(stderr, "%s: xen backend core set= up failed\n", __FUNCTION__);
@@ -930,7 +931,7 @@ int xen_hvm_init(void)
=C2=A0 =C2=A0 xen_be_register("console", &xen_console_ops);<= br> =C2=A0 =C2=A0 xen_be_register("vkbd", &xen_kbdmouse_ops); =C2=A0 =C2=A0 xen_be_register("qdisk", &xen_blkdev_ops);
-
+#endif
=C2=A0 =C2=A0 return 0;
=C2=A0}
=C2=A0

=C2=A0
Yes. this implementation= of stubdom=C2=A0is more closer to the old qemu's implementation. Which= branch this patch works on? The qemu-dm-v15 would not so lucky.
--bcaec53af11e946c0404a936a91d-- --===============0569962655== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --===============0569962655==--