[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Re: Linux Stubdom Problem
On 22/08/2011 20:36, "Stefano Stabellini" <stefano.stabellini@xxxxxxxxxxxxx> wrote: > 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. I suggest XENMAPSPACE_caller_gmfn. I'm slightly worried about the reference counting implications of mapping these foreign pages into an HVM guest. It might need checking with Tim Deegan. > >> 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); Also need something like get_page(page, current->domain). > + 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 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |