From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Katz Subject: Re: [PATCH] Paravirt framebuffer frontend kernel support [1/5] Date: Tue, 05 Sep 2006 12:11:53 -0400 Message-ID: <1157472713.7571.87.camel@aglarond.local> References: <1157227080.11059.38.camel@aglarond.local> <20060904090045.GA4812@cam.ac.uk> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20060904090045.GA4812@cam.ac.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Steven Smith Cc: aliguori , xen-devel , sos22@srcf.ucam.org, Markus Armbruster List-Id: xen-devel@lists.xenproject.org On Mon, 2006-09-04 at 10:00 +0100, Steven Smith wrote: > There are still a couple of things from the last round which are > unfixed. This is mostly my fault for going on holiday at an > inconvenient time; sorry about that. We won't begrudge you a holiday -- hope it was a good one :-) Leaving some of the stuff for Markus to reply to > The big problem with this is that you don't support shadow translate > mode guests, because the frontend exposes mfns directly to the > backend. The correct way of fixing this is to use grant tables, but, > as Anthony pointed out, that's quite hard due to the amount of memory > you need to grant access to. > > The easy way of doing this would be to just increase the size of the > grant tables. Alternatively, you could add support for granting > access to ranges of memory, but that sounds like a whole lot of work. > > You could also put in a quick hack of just translating the mfns in the > backend if the frontend is in shadow translate mode, but that's not > really very nice. I wouldn't complain if we go the route Keir suggested and have Xen do it ;-) But if not, I think that given the size of the memory in question + grant tables, the quick hack is probably going to be the "right" thing for this case. > > diff -r 5fa9b746d24f -r 510c6bceb87f linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c > > --- a/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c Sat Sep 02 12:11:54 2006 +0100 > > +++ b/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c Sat Sep 02 15:11:17 2006 -0400 > > @@ -1866,8 +1866,12 @@ void __init setup_arch(char **cmdline_p) > > #endif > > #endif > > } else { > > +#if defined(CONFIG_VT) && defined(CONFIG_DUMMY_CONSOLE) > > + conswitchp = &dummy_con; > > +#else > > extern int console_use_vt; > > console_use_vt = 0; > > +#endif > > } > > } > Not quite sure I understand what this is trying to achieve, or why it's > related to the PV framebuffer stuff. It makes it so that console switching is achieved by the dummy console and then when the fbcon driver is loaded, it takes over. [snip] > Perhaps XEN_KEYBOARD should depend on XEN_FRAMEBUFFER, given that it > doesn't work without it. Yeah, seems reasonable enough -- I've added to my tree. > > config XEN_SCRUB_PAGES > > bool "Scrub memory before freeing it to Xen" > > default y > > diff -r 5fa9b746d24f -r 510c6bceb87f linux-2.6-xen-sparse/drivers/xen/Makefile > > --- a/linux-2.6-xen-sparse/drivers/xen/Makefile Sat Sep 02 12:11:54 2006 +0100 > > +++ b/linux-2.6-xen-sparse/drivers/xen/Makefile Sat Sep 02 15:11:17 2006 -0400 > > @@ -15,3 +15,5 @@ obj-$(CONFIG_XEN_NETDEV_FRONTEND) += net > > obj-$(CONFIG_XEN_NETDEV_FRONTEND) += netfront/ > > obj-$(CONFIG_XEN_PCIDEV_BACKEND) += pciback/ > > obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += pcifront/ > > +obj-$(CONFIG_XEN_FRAMEBUFFER) += xenfb/ > > +obj-$(CONFIG_XEN_KEYBOARD) += xenkbd/ > > diff -r 5fa9b746d24f -r 510c6bceb87f linux-2.6-xen-sparse/mm/memory.c > > --- a/linux-2.6-xen-sparse/mm/memory.c Sat Sep 02 12:11:54 2006 +0100 > > +++ b/linux-2.6-xen-sparse/mm/memory.c Sat Sep 02 15:11:17 2006 -0400 > > @@ -881,6 +881,7 @@ unsigned long zap_page_range(struct vm_a > > tlb_finish_mmu(tlb, address, end); > > return end; > > } > > +EXPORT_SYMBOL(zap_page_range); > It's unfortunate that we have to modify an upstream file here, but I > can't really see any alternative. Well, if we don't allow the framebuffer to be modular then it doesn't have to be exported. But that also seems like the wrong answer :-) > > +#if 0 > > + /* if we're not set up to use graphics mode, then don't initialize */ > > + if (xenbus_scanf(XBT_NIL, "console", "use_graphics", "%d", &ret) < 0) > > + return -ENODEV; > > + if (ret == 0) > > + return -ENODEV; > > +#endif > That's interesting. What is this here for? It looks like it got accidentally #if 0'd while merging patches. Whoops. Basically, if the guest isn't set up to use a graphical console, then we shouldn't initialize the devices. Jeremy