WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: Linux Stubdom Problem

To: Jiageng Yu <yujiageng734@xxxxxxxxx>
Subject: [Xen-devel] Re: Linux Stubdom Problem
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Mon, 22 Aug 2011 20:36:23 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxx>, "Xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Mon, 22 Aug 2011 12:30:05 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <CAJ0pt16CLqt+bHUwvO_V=c39CqN4Y4BCgbcSW26wik4hFTHq_g@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <CAJ0pt14RBmT+bCGhU6szMW4Aje-mBQ-WVR8Vb7wOLefgauatbA@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1107271431070.12963@kaball-desktop> <CAJ0pt15-X7psh5Fzxzo0=8BR9G-hdVjdPqQO7CYLDCgNx9zNZg@xxxxxxxxxxxxxx> <CAJ0pt175GrngJTxnvx0GRf1RvmXe_JAMxBch+SprKOArNx42ng@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1107281928020.12963@kaball-desktop> <CAJ0pt146Te2hCFpxCdwU518HpwRuGHiUj7sSkVOU75YLpoXSBw@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1107291601070.12963@kaball-desktop> <CAJ0pt15t_7==0rX9qEHyDMBRWUgfpMyLCmP0yBJsZg410FoutA@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1107291617130.12963@kaball-desktop> <CAJ0pt15MVvw5_rJ_xFxR6aQpRYtTWFmR0OHJ54wuXSE+7hNkGQ@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1107291628000.12963@kaball-desktop> <CAJ0pt155eqcyUwd-m--m3n+t-hLYixe+xA8J5MWgEWj58+o_Gw@xxxxxxxxxxxxxx> <CAJ0pt14OXshWn5NQ3JSfqRNdHkWMyhKWC0OwMixu9Xmk0b1oqw@xxxxxxxxxxxxxx> <alpine.DEB.2.00.1108190027450.12963@kaball-desktop> <CAJ0pt16CLqt+bHUwvO_V=c39CqN4Y4BCgbcSW26wik4hFTHq_g@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
On Mon, 22 Aug 2011, Jiageng Yu wrote:
> Hi Stefano,
> 
>      I am trying to fix the vram mapping issue. That is my new patch.
> It is still needed to debug. Please check it for me and make sure I am
> running on the right way.
> 
>     I define a new enmu type Stubdom_Vga_State which is used to notify
> xen_remap_bucket whether to map the vram memory. In
> fbdev_pv_display_prepare function, I map the xen_fbfront memory to
> qemu (fb_mem) and directly incoke ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH,
> &ioctlx) to map the vram of hvm guest.
> 

The current implementation of IOCTL_PRIVCMD_MMAPBATCH is only capable of
mapping foreign mfns into the address space of the caller, while we need
to remap a set of pages already mapped in the address space of the
caller to some gmfns of a foreign domain. In other words we need the
same functionality but in the other direction.

First of all we need an hypercall to remap a given mfn to a guest gmfn
and currently there are no hypercalls that do that, so we need to add a
new one or extend an existing hypercall.
I suggest extending xc_domain_add_to_physmap with a new XENMAPSPACE,
called XENMAPSPACE_mfn that takes an mfn and maps it into a particular
gmfn.


>From the Xen point of view we need to add a new XENMAPSPACE_mfn case to
xen/arch/x86/mm.c:arch_memory_op, the basic implementation should be
something like the following (uncompiled, untested):

diff -r a79c1d5b946e xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c Fri Aug 19 10:00:25 2011 +0100
+++ b/xen/arch/x86/mm.c Mon Aug 22 17:51:41 2011 +0000
@@ -4618,6 +4618,16 @@ long arch_memory_op(int op, XEN_GUEST_HA
             page = mfn_to_page(mfn);
             break;
         }
+        case XENMAPSPACE_mfn:
+        {
+            if ( !IS_PRIV_FOR(current->domain, d) )
+                return -EINVAL;
+            mfn = xatp.idx;
+            page = mfn_to_page(mfn);
+            break;
+        }
         default:
             break;
         }
@@ -4648,10 +4658,12 @@ long arch_memory_op(int op, XEN_GUEST_HA
         }
 
         /* Unmap from old location, if any. */
-        gpfn = get_gpfn_from_mfn(mfn);
-        ASSERT( gpfn != SHARED_M2P_ENTRY );
-        if ( gpfn != INVALID_M2P_ENTRY )
-            guest_physmap_remove_page(d, gpfn, mfn, 0);
+        if ( xatp.space != XENMAPSPACE_mfn && d != current->domain ) {
+            gpfn = get_gpfn_from_mfn(mfn);
+            ASSERT( gpfn != SHARED_M2P_ENTRY );
+            if ( gpfn != INVALID_M2P_ENTRY )
+                guest_physmap_remove_page(d, gpfn, mfn, 0);
+        }
 
         /* Map at new location. */
         rc = guest_physmap_add_page(d, xatp.gpfn, mfn, 0);


Unfortunately qemu doesn't know how to find the mfns corresponding to
the mmap'ed framebuffer and I would rather avoid writing a pagetable
walker in qemu.
Thus we need to use the linux kernel to do the virtual address to mfn
translation and issue the hypercall.
We can add a new privcmd ioctl IOCTL_PRIVCMD_FOREIGN_MAP: qemu calls this
ioctl with the mmap'ed framebuffer address, the size of the framebuffer
and the destination gmfns.
The implementation of the ioctl in the kernel would:

- find the list of mfns corresponding to the arguments, using
arbitrary_virt_to_machine;

- for each mfn, call the hypercall we extended with the appropriate
arguments, it would end up being
HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);

- there would be no "if (!xen_initial_domain())" check, because this
ioctl can be called by stubdoms.


So the call trace would be:

qemu: ioctl(fd, IOCTL_PRIVCMD_FOREIGN_MAP, &ioctlx);

|
v

linux: HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);

|
v

xen: guest_physmap_add_page


Has anybody any better ideas?


> diff --git a/fbdev.c b/fbdev.c
> index a601b48..f7ff682 100644
> --- a/fbdev.c
> +++ b/fbdev.c
> @@ -30,6 +30,12 @@
>  #include "sysemu.h"
>  #include "pflib.h"
> 
> +#include "hw/xen_backend.h"
> +#include <xen/hvm/params.h>
> +#include <sys/ioctl.h>
> +#include "xenctrlosdep.h"
> +#include <xen/privcmd.h>
> +
>  /* -------------------------------------------------------------------- */
> 
>  /* file handles */
> @@ -541,29 +547,23 @@ static void fbdev_cleanup(void)
>      }
>  }
> 
> -static int fbdev_init(const char *device)
> +static int fbdev_init(int prot, unsigned long size)
>  {
>      struct vt_stat vts;
>         unsigned long page_mask;
>      char ttyname[32];
> 
>      /* open framebuffer */
> -    if (device == NULL) {
> -       device = getenv("FRAMEBUFFER");
> -    }
> -    if (device == NULL) {
> -       device = "/dev/fb0";
> -    }
> -    fb = open(device, O_RDWR);
> +    fb = open("/dev/fb0", O_RDWR);
>      if (fb == -1) {
> -       fprintf(stderr, "open %s: %s\n", device, strerror(errno));
> +        fprintf(stderr, "open /dev/fb0: %s\n", strerror(errno));
>          return -1;
>      }
> 
>      /* open virtual console */
>      tty = 0;
>      if (ioctl(tty, VT_GETSTATE, &vts) < 0) {
>              fprintf(stderr, "Not started from virtual terminal, trying to
> open one.\n");
> 
>          snprintf(ttyname, sizeof(ttyname), "/dev/tty0");
>          tty = open(ttyname, O_RDWR);
> @@ -632,14 +632,14 @@ static int fbdev_init(const char *device)
>         goto err;
>      }
> 
>      page_mask = getpagesize()-1;
>      fb_mem_offset = (unsigned long)(fb_fix.smem_start) & page_mask;
> -    fb_mem = mmap(NULL,fb_fix.smem_len+fb_mem_offset,
> -                 PROT_READ|PROT_WRITE,MAP_SHARED,fb,0);
> +    fb_mem = mmap(NULL, size << XC_PAGE_SHIFT, prot, MAP_SHARED, fb, 0);
>      if (fb_mem == MAP_FAILED) {
>         perror("mmap");
>         goto err;
>      }
> +
>      /* move viewport to upper left corner */
>      if (fb_var.xoffset != 0 || fb_var.yoffset != 0) {
>         fb_var.xoffset = 0;
> @@ -930,3 +928,71 @@ void fbdev_display_uninit(void)
>      qemu_remove_exit_notifier(&exit_notifier);
>      uninit_mouse();
>  }

I would rather avoid modifing fbdev_init and add a new function to
xen-all.c that independently mmaps the xen_fbfront buffer with the right
size and maps it into the guest.


> +int fbdev_pv_display_prepare(unsigned long domid, int prot, const
> unsigned long *arr,
> +                                                               int *err, 
> unsigned int num)
> +{
> +       xen_pfn_t *pfn;
> +       privcmd_mmapbatch_t ioctlx;
> +       int fd,i,rc;
> +
> +    if (fbdev_init(prot,num) != 0) {
> +        exit(1);
> +    }
> +
> +       pfn = malloc(num * sizeof(*pfn));
> +       if(!pfn){
> +               errno = -ENOMEM;
> +               rc = -1;
> +               return rc;
> +       }
> +       memcpy(pfn, arr, num*sizeof(*arr));
> +
> +       fd = open("/proc/xen/privcmd", O_RDWR);
> +       if(fd == -1){
> +               fprintf(stderr,"Could not obtain handle on privcmd device\n");
> +               exit(1);
> +       }
> +
> +       ioctlx.num = num;
> +       ioctlx.dom = domid;
> +       ioctlx.addr = (unsigned long)fb_mem;
> +       ioctlx.arr = pfn;
> +
> +       rc = ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx);
> +
> +       for(i=0; i<num; i++)
> +       {
> +               switch(pfn[i] ^ arr[i])
> +               {
> +                       case 0:
> +                               err[i] = rc != -ENOENT ? rc:0;
> +                               continue;
> +                       default:
> +                               err[i] = -EINVAL;
> +                               continue;
> +               }
> +               break;
> +       }
> +
> +       free(pfn);
> +
> +       if (rc == -ENOENT && i == num)
> +               rc=0;
> +       else if(rc)
> +       {
> +               errno = -rc;
> +               rc = -1;
> +       }
> +
> +       if(rc < 0)
> +       {
> +               fprintf(stderr,"privcmd ioctl failed\n");
> +               munmap(fb_mem, num << XC_PAGE_SHIFT);
> +               return NULL;
> +       }
> +
> +       close(fd);
> +
> +       return fb_mem;
> +}

We shouldn't use IOCTL_PRIVCMD_MMAPBATCH, we need a new ioctl, see above.


> diff --git a/hw/vga.c b/hw/vga.c
> index 0f54734..de7dd85 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -28,6 +28,7 @@
>  #include "vga_int.h"
>  #include "pixel_ops.h"
>  #include "qemu-timer.h"
> +#include "xen_backend.h"
> 
>  //#define DEBUG_VGA
>  //#define DEBUG_VGA_MEM
> @@ -2237,7 +2238,12 @@ void vga_common_init(VGACommonState *s, int 
> vga_ram_size)
>      s->is_vbe_vmstate = 0;
>  #endif
>      s->vram_offset = qemu_ram_alloc(NULL, "vga.vram", vga_ram_size);
> +#ifdef CONFIG_STUBDOM
> +       stubdom_vga_state = VGA_INIT_READY;
> +#endif
>      s->vram_ptr = qemu_get_ram_ptr(s->vram_offset);
>      s->vram_size = vga_ram_size;
>      s->get_bpp = vga_get_bpp;
>      s->get_offsets = vga_get_offsets;
> diff --git a/hw/xen_backend.c b/hw/xen_backend.c
> index c506dfe..f4ecce4 100644
> --- a/hw/xen_backend.c
> +++ b/hw/xen_backend.c
> @@ -48,6 +48,10 @@ XenGnttab xen_xcg = XC_HANDLER_INITIAL_VALUE;
>  struct xs_handle *xenstore = NULL;
>  const char *xen_protocol;
> 
> +#ifdef CONFIG_STUBDOM
> +enum Stubdom_Vga_State stubdom_vga_state=0;
> +#endif
> +
>  /* private */
>  static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs =
> QTAILQ_HEAD_INITIALIZER(xendevs);
>  static int debug = 0;
> diff --git a/hw/xen_backend.h b/hw/xen_backend.h
> index 3305630..36167d1 100644
> --- a/hw/xen_backend.h
> +++ b/hw/xen_backend.h
> @@ -60,6 +60,16 @@ extern XenXC xen_xc;
>  extern struct xs_handle *xenstore;
>  extern const char *xen_protocol;
> 
> +#ifdef CONFIG_STUBDOM
> +/* linux stubdom vga initialization*/
> +enum Stubdom_Vga_State{
> +       VGA_INIT_WAIT = 0,
> +       VGA_INIT_READY,
> +       VGA_INIT_COMPLETE
> +};
> +extern enum Stubdom_Vga_State stubdom_vga_state;
> +#endif
> +
>  /* 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/xen-mapcache.c b/xen-mapcache.c
> index 007136a..e615f98 100644
> --- a/xen-mapcache.c
> +++ b/xen-mapcache.c
> @@ -20,6 +20,7 @@
>  #include "xen-mapcache.h"
>  #include "trace.h"
> 
> +#include "hw/xen.h"
> 
>  //#define MAPCACHE_DEBUG
> 
> @@ -144,8 +145,19 @@ static void xen_remap_bucket(MapCacheEntry *entry,
>          pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT)) + i;
>      }
> 
> +#ifdef CONFIG_STUBDOM
> +       if(stubdom_vga_state == VGA_INIT_READY){
> +               fprintf(stderr,"xen_remap_bucket: start linux stubdom vga\n");
> +               vaddr_base = fbdev_pv_display_prepare(xen_domid, 
> PROT_READ|PROT_WRITE,
> +                                                                             
>   pfns, err, nb_pfn);
> +               stubdom_vga_state = VGA_INIT_COMPLETE;
> +       }else
> +       vaddr_base = xc_map_foreign_bulk(xen_xc, xen_domid, 
> PROT_READ|PROT_WRITE,
> +                                       pfns, err, nb_pfn);
> +#else
>      vaddr_base = xc_map_foreign_bulk(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
>                                       pfns, err, nb_pfn);
> +#endif
>      if (vaddr_base == NULL) {
>          perror("xc_map_foreign_bulk");
>          exit(-1);
> 
> 

I don't like the stubdom_vga_init approach: in general it is a good idea
to avoid state machines unless necessary.
I would implement a new function called xen_vga_ram_map in xen-all.c
that mmaps the xen_fbfront buffer and uses the new ioctl to
map the buffer into the guest.
xen_vga_ram_map should be called instead of xen_ram_alloc by
qemu_ram_alloc_from_ptr if name == "vga.vram".


Another suggestion: before starting to write any new lines of code, I
would make sure that mmaping the /dev/fb device actually works correctly.
For debugging purposes you can try to modify fbdev_init and write
something to the framebuffer right after the mmap, to see if the new
pattern is displayed correctly on the screen.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel