|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] 32-on-64: pvfb issue
Gerd Hoffmann <kraxel@xxxxxxx> writes:
> Gerd Hoffmann wrote:
>
>> I'll go code up both front and back bits for block and pvfb to see how
>> it works out in practice, I think I'll have patches later today or
>> tomorrow ...
>
> Here we go. Compile-tested on 32bit, more tests coming, full rebuild
> still in progress ...
>
> Attached are:
>
> protocol-bimodal.diff
> short header file with the protocol names.
>
> {blk,fb}front-bimodal.diff
> small frontend driver patches, just add the protocol name to xenstore.
>
> {blk,fb}back-bimodal.diff
> backend patches (for unstable only).
>
> cheers,
> Gerd
>
> --
> Gerd Hoffmann <kraxel@xxxxxxx>
> ---
> xen/include/public/io/protocols.h | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> Index: build-32-unstable-13495/xen/include/public/io/protocols.h
> ===================================================================
> --- /dev/null
> +++ build-32-unstable-13495/xen/include/public/io/protocols.h
> @@ -0,0 +1,21 @@
> +#ifndef __XEN_PROTOCOLS_H__
> +#define __XEN_PROTOCOLS_H__
> +
> +#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi"
> +#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi"
> +#define XEN_IO_PROTO_ABI_IA64 "ia64-abi"
> +#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi"
> +
> +#if defined(__i386__)
> +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
> +#elif defined(__x86_64__)
> +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64
> +#elif defined(__ia64__)
> +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
> +#elif defined(__powerpc64__)
> +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
> +#else
> +# error arch fixup needed here
> +#endif
> +
> +#endif
> ---
> linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> Index:
> build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c
> ===================================================================
> ---
> build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c
> +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c
> @@ -27,6 +27,7 @@
> #include <asm/hypervisor.h>
> #include <xen/evtchn.h>
> #include <xen/interface/io/fbif.h>
> +#include <xen/interface/io/protocols.h>
> #include <xen/xenbus.h>
> #include <linux/kthread.h>
>
> @@ -479,7 +480,7 @@ static int __devinit xenfb_probe(struct
> goto error_nomem;
>
> /* set up shared page */
> - info->page = (void *)__get_free_page(GFP_KERNEL);
> + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> if (!info->page)
> goto error_nomem;
>
> @@ -640,6 +641,10 @@ static int xenfb_connect_backend(struct
> irq_to_evtchn_port(info->irq));
> if (ret)
> goto error_xenbus;
> + ret = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
> + XEN_IO_PROTO_ABI_NATIVE);
> + if (ret)
> + goto error_xenbus;
This identifies the ABI, but how does it provide versioning?
> ret = xenbus_printf(xbt, dev->nodename, "feature-update", "1");
> if (ret)
> goto error_xenbus;
> ---
> linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> Index:
> build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
> ===================================================================
> ---
> build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
> +++
> build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
> @@ -44,6 +44,7 @@
> #include <xen/evtchn.h>
> #include <xen/xenbus.h>
> #include <xen/interface/grant_table.h>
> +#include <xen/interface/io/protocols.h>
> #include <xen/gnttab.h>
> #include <asm/hypervisor.h>
> #include <asm/maddr.h>
> @@ -180,6 +181,12 @@ again:
> message = "writing event-channel";
> goto abort_transaction;
> }
> + err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
> + XEN_IO_PROTO_ABI_NATIVE);
> + if (err) {
> + message = "writing protocol";
> + goto abort_transaction;
> + }
>
> err = xenbus_transaction_end(xbt, 0);
> if (err) {
> ---
> tools/xenfb/xenfb.c | 103
> ++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 92 insertions(+), 11 deletions(-)
>
> Index: build-32-unstable-13495/tools/xenfb/xenfb.c
> ===================================================================
> --- build-32-unstable-13495.orig/tools/xenfb/xenfb.c
> +++ build-32-unstable-13495/tools/xenfb/xenfb.c
> @@ -7,6 +7,7 @@
> #include <xen/io/xenbus.h>
> #include <xen/io/fbif.h>
> #include <xen/io/kbdif.h>
> +#include <xen/io/protocols.h>
> #include <sys/select.h>
> #include <stdbool.h>
> #include <xen/linux/evtchn.h>
> @@ -40,6 +41,7 @@ struct xenfb_private {
> struct xs_handle *xsh; /* xs daemon handle */
> struct xenfb_device fb, kbd;
> size_t fb_len; /* size of framebuffer */
> + char protocol[64]; /* frontend protocol */
> };
>
> static void xenfb_detach_dom(struct xenfb_private *);
> @@ -324,36 +326,112 @@ static int xenfb_wait_for_frontend_initi
> return 0;
> }
>
> +static void xenfb_copy_mfns(int mode, int count, unsigned long *dst, void
> *src)
Nitpick: the argument order dst, src, count got burned into my
synapses; other orders tend to confuse me.
> +{
> + uint32_t *src32 = src;
> + uint64_t *src64 = src;
> + int i;
> +
> + if (32 == mode) {
> + for (i = 0; i < count; i++)
> + dst[i] = src32[i];
> + } else {
> + for (i = 0; i < count; i++)
> + dst[i] = src64[i];
> + }
> +}
> +
> static int xenfb_map_fb(struct xenfb_private *xenfb, int domid)
> {
> struct xenfb_page *page = xenfb->fb.page;
> int n_fbmfns;
> int n_fbdirs;
> - unsigned long *fbmfns;
> + unsigned long *pgmfns = NULL;
> + unsigned long *fbmfns = NULL;
> + void *map, *pd;
> + int mode, ret = -1;
> +
> + /* default to native */
> + pd = page->pd;
> + mode = sizeof(unsigned long) * 8;
Nitick: encoding modes as 4, 8 and so forth rather than 32, 64, ...
would be slightly simpler.
> +
> + if (0 == strlen(xenfb->protocol)) {
> + /*
> + * Undefined protocol, some guesswork needed.
Guesswork is only required if we want to support old domU with
different width than dom0. Do we?
> + *
> + * Old frontends which don't set the protocol use
> + * one page directory only, thus pd[1] must be zero.
> + * pd[1] of the 32bit struct layout and the lower
> + * 32 bits of pd[0] of the 64bit struct layout have
> + * the same location, so we can check that ...
> + */
> + uint32_t *ptr32 = NULL;
> + uint32_t *ptr64 = NULL;
> +#if defined(__i386_)
> + ptr32 = page->pd;
> + ptr64 = ((void*)page->pd) + 4;
> +#elif defined(__x86_64__)
> + ptr32 = ((void*)page->pd) - 4;
> + ptr64 = page->pd;
> +#endif
> + if (ptr32) {
> + if (0 == ptr32[1]) {
> + mode = 32;
> + pd = ptr32;
> + } else {
> + mode = 64;
> + pd = ptr64;
> + }
This clever hack tests those 32 pg bits that are guaranteed to be
initialized for both modes. In other words, it uses all the
information there is.
In 32-bit mode, ptr32[1] is pd[1], and that is certainly zero for old
frontends.
In 64-bit mode, ptr32[1] is pd[0] & 0xffffffff. Non-zero unless we
get very unlucky with the address of mfns. Why can't that happen?
> + }
> +#if defined(__x86_64__)
> + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_32)) {
> + /* 64bit dom0, 32bit domU */
> + mode = 32;
> + pd = ((void*)page->pd) - 4;
> +#elif defined(__i386_)
> + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_64)) {
> + /* 32bit dom0, 64bit domU */
> + mode = 64;
> + pd = ((void*)page->pd) + 4;
> +#endif
Hackery so that we can keep the old page layout. Could perhaps be
done in a cleaner way, but I don't care.
> + }
>
> n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
> - n_fbdirs = n_fbmfns * sizeof(unsigned long);
> + n_fbdirs = n_fbmfns * mode / 8;
> n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
>
> + pgmfns = malloc(sizeof(unsigned long) * n_fbdirs);
> + fbmfns = malloc(sizeof(unsigned long) * n_fbmfns);
> + if (!pgmfns || !fbmfns)
> + goto out;
> +
> /*
> * Bug alert: xc_map_foreign_batch() can fail partly and
> * return a non-null value. This is a design flaw. When it
> * happens, we happily continue here, and later crash on
> * access.
> */
> - fbmfns = xc_map_foreign_batch(xenfb->xc, domid,
> - PROT_READ, page->pd, n_fbdirs);
> - if (fbmfns == NULL)
> - return -1;
> + xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
> + map = xc_map_foreign_batch(xenfb->xc, domid,
> + PROT_READ, pgmfns, n_fbdirs);
> + if (map == NULL)
> + goto out;
> + xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map);
> + munmap(map, n_fbdirs * XC_PAGE_SIZE);
>
> xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid,
> PROT_READ | PROT_WRITE, fbmfns, n_fbmfns);
> - if (xenfb->pub.pixels == NULL) {
> - munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE);
> - return -1;
> - }
> + if (xenfb->pub.pixels == NULL)
> + goto out;
>
> - return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE);
> + ret = 0; /* all is fine */
> +
> + out:
> + if (pgmfns)
> + free(pgmfns);
> + if (fbmfns)
> + free(fbmfns);
> + return ret;
> }
>
> static int xenfb_bind(struct xenfb_device *dev)
> @@ -368,6 +446,9 @@ static int xenfb_bind(struct xenfb_devic
> if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "event-channel", "%u",
> &evtchn) < 0)
> return -1;
> + if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "protocol", "%63s",
> + xenfb->protocol) < 0)
> + xenfb->protocol[0] = '\0';
>
> dev->port = xc_evtchn_bind_interdomain(xenfb->evt_xch,
> dev->otherend_id, evtchn);
[...]
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |