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: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: Linux Stubdom Problem
From: Jiageng Yu <yujiageng734@xxxxxxxxx>
Date: Tue, 23 Aug 2011 17:39:44 +0800
Cc: "Xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, tim@xxxxxxx, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxx>, Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
Delivery-date: Tue, 23 Aug 2011 02:40:49 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=JmDCONaW/nGkcHXMx+rrQ8FlQoINwqx+vnvbRbkrSdE=; b=eL5aBEuR6utr7EPbUrwNhFQ8DOIsTJ4ORNHLN/gZlVdBoraXpoeEznKPnjLrhwVBSC TvHY88xAn36q6LA2sGnmEX4MXePcAU8/UIPqf0iy88UC8P2q/LQrgbDoB6Bqtgz0xF+m A85q4T6CqO1ylyfT4M8J4GVDWCdtjtKHT47JI=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1108221834340.12963@kaball-desktop>
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> <alpine.DEB.2.00.1108221834340.12963@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
2011/8/23 Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>:
> 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


Yes. I am already working on it.

>
>
> 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".
>

I have question here. I think xen_vga_ram_map() is used to instead of
xc_map_foreign_bulk() which maps hvm guest's vram.

vga_common_init
    -->qemu_get_ram_ptr
             -->xen_map_cache
                     -->xen_remap_bucket
                               -->xc_map_foreign_bulk

The xen_ram_alloc() calls xc_domain_populate_physmap_exact() to
increase the physical memory of hvm guest. And the increased physical
memory becomes hvm guest's vram.

For the xen_remap_bucket(), there is no "vga.vram" input parameter. So
we need the notification mechanism to invoke xen_vga_ram_map().


>
> 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.
>

I have tested the xen_fbfront driver in linux stubdom. As shown in
attached file, I could print a small colored region.

Attachment: Screenshot-QEMU (fedora14-dm)-4.png
Description: PNG image

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