From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Smith Subject: Re: [PATCH] Paravirt framebuffer frontend kernel support [1/5] Date: Wed, 6 Sep 2006 10:11:41 +0100 Message-ID: <20060906091140.GA3257@cam.ac.uk> References: <1157227080.11059.38.camel@aglarond.local> <20060904090045.GA4812@cam.ac.uk> <1157472713.7571.87.camel@aglarond.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0680402731==" Return-path: In-Reply-To: <1157472713.7571.87.camel@aglarond.local> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Mime-version: 1.0 Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jeremy Katz Cc: aliguori , xen-devel , sos22@srcf.ucam.org, Markus Armbruster List-Id: xen-devel@lists.xenproject.org --===============0680402731== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0OAP2g/MAC+5xKAE" Content-Disposition: inline --0OAP2g/MAC+5xKAE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable > > 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 :-) Yes, thank you. > > 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. > >=20 > > 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. > >=20 > > 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. Okay. Adding translations to the hypervisor isn't going to happen until after 3.0.3, so we should probably just leave this for a little while. > > > diff -r 5fa9b746d24f -r 510c6bceb87f linux-2.6-xen-sparse/arch/i386/k= ernel/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 =3D &dummy_con; > > > +#else > > > extern int console_use_vt; > > > console_use_vt =3D 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. =20 Okay. >=20 > [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. Thanks. > > > 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 2= 006 +0100 > > > +++ b/linux-2.6-xen-sparse/drivers/xen/Makefile Sat Sep 02 15:11:17 2= 006 -0400 > > > @@ -15,3 +15,5 @@ obj-$(CONFIG_XEN_NETDEV_FRONTEND) +=3D net > > > obj-$(CONFIG_XEN_NETDEV_FRONTEND) +=3D netfront/ > > > obj-$(CONFIG_XEN_PCIDEV_BACKEND) +=3D pciback/ > > > obj-$(CONFIG_XEN_PCIDEV_FRONTEND) +=3D pcifront/ > > > +obj-$(CONFIG_XEN_FRAMEBUFFER) +=3D xenfb/ > > > +obj-$(CONFIG_XEN_KEYBOARD) +=3D 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 :-) Yeah, adding the EXPORT_SYMBOL is the right thing to do here, it's just kind of annoying. Oh well. >=20 > > > +#if 0 > > > + /* if we're not set up to use graphics mode, then don't init= ialize */ > > > + if (xenbus_scanf(XBT_NIL, "console", "use_graphics", "%d", &ret) < = 0) > > > + return -ENODEV; > > > + if (ret =3D=3D 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. That seems like a good idea. Please put this back in. Steven. --0OAP2g/MAC+5xKAE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (GNU/Linux) iD8DBQFE/pDMO4S8/gLNrjcRAo6hAKDPNaBwOfdKFTZSs5sMhwmWLWN72ACfTE0U dfI6Z6bYQwPNXPQL+Sj67UY= =Vtte -----END PGP SIGNATURE----- --0OAP2g/MAC+5xKAE-- --===============0680402731== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --===============0680402731==--