| Message ID | 1269543582-2242-1-git-send-email-chase.douglas@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Andy Whitcroft |
| Headers | show |
Version 2 spits out the following at boot: [ 19.806985] vga16fb: initializing [ 19.806989] vga16fb: mapped to 0xffff8800000a0000 [ 19.806992] vga16fb: not registering due to another framebuffer present -- Chase On Thu, Mar 25, 2010 at 2:59 PM, Chase Douglas <chase.douglas@canonical.com> wrote: > Using the vga16fb framebuffer is not safe when other framebuffers are > present. This fixes the case where the vga16fb module is loaded after > a better framebuffer is loaded (since vga16fb is by definition the > worst-case framebuffer). > > There does not appear to be any locking around the num_registered_fb, so > this is a hack at best. However, in Lucid we build vga16fb as a module. > Modules are loaded serially, so this should be ok for Lucid. In M, we > will transition to efifb and drop vga16fb, so this is a one time hack. > > This prevents sudo lshw from corrupting /dev/fb0 by writing to vga16fb > through /dev/fb1. > > BugLink: http://bugs.launchpad.net/bugs/527369 > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > --- > drivers/video/vga16fb.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c > index efde41d..d8f8313 100644 > --- a/drivers/video/vga16fb.c > +++ b/drivers/video/vga16fb.c > @@ -1355,6 +1355,16 @@ static int __init vga16fb_probe(struct platform_device *dev) > > vga16fb_update_fix(info); > > + /* > + * Ubuntu: Don't register vga16fb if another fb exists. Bad interactions > + * can occur. > + */ > + if (num_registered_fb > 0) { > + printk(KERN_NOTICE "vga16fb: not registering due to another " > + "framebuffer present\n"); > + goto err_check_var; > + } > + > if (register_framebuffer(info) < 0) { > printk(KERN_ERR "vga16fb: unable to register framebuffer\n"); > ret = -EINVAL; > -- > 1.7.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team >
I should learn to read my email in the reverse time-stamp order. I'd save on many replies :) On 10 Mar 25, Chase Douglas wrote: > Using the vga16fb framebuffer is not safe when other framebuffers are > present. This fixes the case where the vga16fb module is loaded after > a better framebuffer is loaded (since vga16fb is by definition the > worst-case framebuffer). So what happens in the case where vga16fb is the first one loaded and some other (better) fb is loaded later? > There does not appear to be any locking around the num_registered_fb, so > this is a hack at best. However, in Lucid we build vga16fb as a module. > Modules are loaded serially, so this should be ok for Lucid. In M, we > will transition to efifb and drop vga16fb, so this is a one time hack. > > This prevents sudo lshw from corrupting /dev/fb0 by writing to vga16fb > through /dev/fb1. > > BugLink: http://bugs.launchpad.net/bugs/527369 > > Signed-off-by: Chase Douglas <chase.douglas@canonical.com> > --- > drivers/video/vga16fb.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c > index efde41d..d8f8313 100644 > --- a/drivers/video/vga16fb.c > +++ b/drivers/video/vga16fb.c > @@ -1355,6 +1355,16 @@ static int __init vga16fb_probe(struct platform_device *dev) > > vga16fb_update_fix(info); > > + /* > + * Ubuntu: Don't register vga16fb if another fb exists. Bad interactions > + * can occur. > + */ > + if (num_registered_fb > 0) { > + printk(KERN_NOTICE "vga16fb: not registering due to another " > + "framebuffer present\n"); > + goto err_check_var; > + } > + > if (register_framebuffer(info) < 0) { > printk(KERN_ERR "vga16fb: unable to register framebuffer\n"); > ret = -EINVAL; > -- > 1.7.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Fri, Mar 26, 2010 at 4:12 AM, Amit Kucheria <amit.kucheria@canonical.com> wrote: > On 10 Mar 25, Chase Douglas wrote: >> Using the vga16fb framebuffer is not safe when other framebuffers are >> present. This fixes the case where the vga16fb module is loaded after >> a better framebuffer is loaded (since vga16fb is by definition the >> worst-case framebuffer). > > So what happens in the case where vga16fb is the first one loaded and some > other (better) fb is loaded later? There's two answers: 1. This patch is in vga16fb itself, so it only prevents itself from registering as a framebuffer. If vga16fb gets /dev/fb0, any other framebuffer can still register further framebuffers. 2. That doesn't really matter cause we're screwed if vga16fb is first when we try to use KMS. KMS drivers have their own fb, but only /dev/fb0 is ever used, so you get bad interactions in that case. Thankfully, no one seems to be having this issue anymore because the KMS drivers are loaded before vga16fb and module loading is serialized by the kernel. If we do find it happens, we may have to tell people to blacklist vga16fb manually. All this is secondary to the issue fixed by this patch, however. -- Chase
Chase Douglas wrote: > On Fri, Mar 26, 2010 at 4:12 AM, Amit Kucheria > <amit.kucheria@canonical.com> wrote: >> On 10 Mar 25, Chase Douglas wrote: >>> Using the vga16fb framebuffer is not safe when other framebuffers are >>> present. This fixes the case where the vga16fb module is loaded after >>> a better framebuffer is loaded (since vga16fb is by definition the >>> worst-case framebuffer). >> So what happens in the case where vga16fb is the first one loaded and some >> other (better) fb is loaded later? > > There's two answers: > > 1. This patch is in vga16fb itself, so it only prevents itself from > registering as a framebuffer. If vga16fb gets /dev/fb0, any other > framebuffer can still register further framebuffers. > > 2. That doesn't really matter cause we're screwed if vga16fb is first > when we try to use KMS. KMS drivers have their own fb, but only > /dev/fb0 is ever used, so you get bad interactions in that case. > Thankfully, no one seems to be having this issue anymore because the > KMS drivers are loaded before vga16fb and module loading is serialized > by the kernel. If we do find it happens, we may have to tell people to > blacklist vga16fb manually. All this is secondary to the issue fixed > by this patch, however. > > -- Chase > I think I will proceed with this by a) currently test building with it applied b) test that build on a few local systems c) if it seems to work include it in this weeks upload but we will need to pay close attention on it -Stefan
I tested the patch on 4 systems, 2 radeonfb, 1 nouveau and one i915. All behaved in the same (more or less broken) way as before. Three would have loaded vga16fb while they already had another fb running. As I see no regression here, I went ahead, acked and applied it. Lets have a keen eye on gfx bugs next week. Stefan Applied to Lucid master
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c index efde41d..d8f8313 100644 --- a/drivers/video/vga16fb.c +++ b/drivers/video/vga16fb.c @@ -1355,6 +1355,16 @@ static int __init vga16fb_probe(struct platform_device *dev) vga16fb_update_fix(info); + /* + * Ubuntu: Don't register vga16fb if another fb exists. Bad interactions + * can occur. + */ + if (num_registered_fb > 0) { + printk(KERN_NOTICE "vga16fb: not registering due to another " + "framebuffer present\n"); + goto err_check_var; + } + if (register_framebuffer(info) < 0) { printk(KERN_ERR "vga16fb: unable to register framebuffer\n"); ret = -EINVAL;
Using the vga16fb framebuffer is not safe when other framebuffers are present. This fixes the case where the vga16fb module is loaded after a better framebuffer is loaded (since vga16fb is by definition the worst-case framebuffer). There does not appear to be any locking around the num_registered_fb, so this is a hack at best. However, in Lucid we build vga16fb as a module. Modules are loaded serially, so this should be ok for Lucid. In M, we will transition to efifb and drop vga16fb, so this is a one time hack. This prevents sudo lshw from corrupting /dev/fb0 by writing to vga16fb through /dev/fb1. BugLink: http://bugs.launchpad.net/bugs/527369 Signed-off-by: Chase Douglas <chase.douglas@canonical.com> --- drivers/video/vga16fb.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)