Re: [Qemu-devel] Massive read only kvm guests when backing file was missing
"Michael S. Tsirkin" writes: > On Wed, Mar 26, 2014 at 11:08:03PM -0300, Alejandro Comisario wrote: >> Hi List! >> Hope some one can help me, we had a big issue in our cloud the other >> day, a couple of our openstack regions ( +2000 kvm guests with qcow2 ) >> went read only filesystem from the guest side because the backing >> files directory (the openstack _base directory) was compromised and >> the data was lost, when we realized the data was lost, it took us 5 >> mins to restore the backup of the backing files, but by that time all >> the kvm guests received some kind of IO error from the hypervisor >> layer, and went read only on root filesystem. >> >> My question would be, is there a way to hold the IO operations against >> the backing files ( i thought that would be 99% READ operations ) for >> a little longer ( im asking this because i dont quite understand what >> is the process and when it raises the error ) in a case the backing >> files are missing (no IO possible) but is recoverable within minutes ? >> >> Any tip on how to achieve this if possible, or information about how >> backing files works on kvm, will be amazing. >> Waiting for feedback! >> >> kindest regards. >> Alejandro Comisario > > > I'm guessing this is what happened: guests timed out meanwhile. > You can increase the timeout within the guest: > echo 600 > /sys/block/sda/device/timeout > to timeout after 10 minutes. > > If you have installed qemu guest agent on your system, you can do this > from the host. Unfortunately by default it's memory can be pushed out to swap > and then on disk error access there might will fail :( > Maybe we should consider mlock on all its memory at least as an option. > > You could pause your guests, restart them after the issue is resolved, > and we could I guess add functionality to pause VM on disk errors > automatically. > Stefan? Would -drive rerror=stop do? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Qemu-devel] Massive read only kvm guests when backing file was missing
Michael S. Tsirkin m...@redhat.com writes: On Wed, Mar 26, 2014 at 11:08:03PM -0300, Alejandro Comisario wrote: Hi List! Hope some one can help me, we had a big issue in our cloud the other day, a couple of our openstack regions ( +2000 kvm guests with qcow2 ) went read only filesystem from the guest side because the backing files directory (the openstack _base directory) was compromised and the data was lost, when we realized the data was lost, it took us 5 mins to restore the backup of the backing files, but by that time all the kvm guests received some kind of IO error from the hypervisor layer, and went read only on root filesystem. My question would be, is there a way to hold the IO operations against the backing files ( i thought that would be 99% READ operations ) for a little longer ( im asking this because i dont quite understand what is the process and when it raises the error ) in a case the backing files are missing (no IO possible) but is recoverable within minutes ? Any tip on how to achieve this if possible, or information about how backing files works on kvm, will be amazing. Waiting for feedback! kindest regards. Alejandro Comisario I'm guessing this is what happened: guests timed out meanwhile. You can increase the timeout within the guest: echo 600 /sys/block/sda/device/timeout to timeout after 10 minutes. If you have installed qemu guest agent on your system, you can do this from the host. Unfortunately by default it's memory can be pushed out to swap and then on disk error access there might will fail :( Maybe we should consider mlock on all its memory at least as an option. You could pause your guests, restart them after the issue is resolved, and we could I guess add functionality to pause VM on disk errors automatically. Stefan? Would -drive rerror=stop do? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] fbdev: Make deferred I/O work as advertized
"Jaya Kumar" <[EMAIL PROTECTED]> writes: > On Mon, Feb 25, 2008 at 8:03 AM, Markus Armbruster <[EMAIL PROTECTED]> wrote: >> >> Subject: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb >> From: Jaya Kumar <[EMAIL PROTECTED]> >> Date: 2008-02-18 13:41:26 > > Hi Markus, > > Andrew pointed out that there may be race conditions associated with > this patch. [ http://marc.info/?l=linux-fbdev-devel=120376473020396=2 > ] So I would not encourage anyone to merge it. I'll try to figure > things out this weekend. > > Thanks, > jaya Thanks for the timely info. I'm not in an undue hurry to get this merged. As long as we're moving forward, I'm happy. What about pushing the fb_defio fixes independently of any new fb_defio users? If fb_defio was worth merging into Linus's tree, it should be worth fixing there, whether new users are in shape already or not. If we must have a new user, well, I could easily whip up something like FB_VIRTUAL on top of fb_defio. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] fbdev: Make deferred I/O work as advertized
Jaya Kumar [EMAIL PROTECTED] writes: On Mon, Feb 25, 2008 at 8:03 AM, Markus Armbruster [EMAIL PROTECTED] wrote: Subject: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb From: Jaya Kumar [EMAIL PROTECTED] Date: 2008-02-18 13:41:26 Hi Markus, Andrew pointed out that there may be race conditions associated with this patch. [ http://marc.info/?l=linux-fbdev-develm=120376473020396w=2 ] So I would not encourage anyone to merge it. I'll try to figure things out this weekend. Thanks, jaya Thanks for the timely info. I'm not in an undue hurry to get this merged. As long as we're moving forward, I'm happy. What about pushing the fb_defio fixes independently of any new fb_defio users? If fb_defio was worth merging into Linus's tree, it should be worth fixing there, whether new users are in shape already or not. If we must have a new user, well, I could easily whip up something like FB_VIRTUAL on top of fb_defio. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] xen pvfb: Para-virtual framebuffer, keyboard and pointer driver
This is a pair of Xen para-virtual frontend device drivers:
drivers/video/xen-fbfront.c provides a framebuffer, and
drivers/input/xen-kbdfront provides keyboard and mouse.
The backends run in dom0 user space.
The two drivers are not in two separate patches, because the
intermediate step (one driver, not the other) is somewhat problematic:
the backend in dom0 needs both drivers, and will refuse to complete
device initialization unless they're both present.
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
drivers/input/Kconfig|9
drivers/input/Makefile |2
drivers/input/xen-kbdfront.c | 340
drivers/video/Kconfig| 14
drivers/video/Makefile |1
drivers/video/xen-fbfront.c | 550 +++
include/xen/interface/io/fbif.h | 124
include/xen/interface/io/kbdif.h | 114
8 files changed, 1154 insertions(+)
diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 9dea14d..5f9d860 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -149,6 +149,15 @@ config INPUT_APMPOWER
To compile this driver as a module, choose M here: the
module will be called apm-power.
+config XEN_KBDDEV_FRONTEND
+ tristate "Xen virtual keyboard and mouse support"
+ depends on XEN_FBDEV_FRONTEND
+ default y
+ help
+ This driver implements the front-end of the Xen virtual
+ keyboard and mouse device driver. It communicates with a back-end
+ in another domain.
+
comment "Input Device Drivers"
source "drivers/input/keyboard/Kconfig"
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 2ae87b1..98c4f9a 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -23,3 +23,5 @@ obj-$(CONFIG_INPUT_TOUCHSCREEN) += touchscreen/
obj-$(CONFIG_INPUT_MISC) += misc/
obj-$(CONFIG_INPUT_APMPOWER) += apm-power.o
+
+obj-$(CONFIG_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
new file mode 100644
index 000..0f47f46
--- /dev/null
+++ b/drivers/input/xen-kbdfront.c
@@ -0,0 +1,340 @@
+/*
+ * Xen para-virtual input device
+ *
+ * Copyright (C) 2005 Anthony Liguori <[EMAIL PROTECTED]>
+ * Copyright (C) 2006-2008 Red Hat, Inc., Markus Armbruster <[EMAIL PROTECTED]>
+ *
+ * Based on linux/drivers/input/mouse/sermouse.c
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive for
+ * more details.
+ */
+
+/*
+ * TODO:
+ *
+ * Switch to grant tables together with xen-fbfront.c.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct xenkbd_info {
+ struct input_dev *kbd;
+ struct input_dev *ptr;
+ struct xenkbd_page *page;
+ int irq;
+ struct xenbus_device *xbdev;
+ char phys[32];
+};
+
+static int xenkbd_remove(struct xenbus_device *);
+static int xenkbd_connect_backend(struct xenbus_device *, struct xenkbd_info
*);
+static void xenkbd_disconnect_backend(struct xenkbd_info *);
+
+/*
+ * Note: if you need to send out events, see xenfb_do_update() for how
+ * to do that.
+ */
+
+static irqreturn_t input_handler(int rq, void *dev_id)
+{
+ struct xenkbd_info *info = dev_id;
+ struct xenkbd_page *page = info->page;
+ __u32 cons, prod;
+
+ prod = page->in_prod;
+ if (prod == page->in_cons)
+ return IRQ_HANDLED;
+ rmb(); /* ensure we see ring contents up to prod */
+ for (cons = page->in_cons; cons != prod; cons++) {
+ union xenkbd_in_event *event;
+ struct input_dev *dev;
+ event = _IN_RING_REF(page, cons);
+
+ dev = info->ptr;
+ switch (event->type) {
+ case XENKBD_TYPE_MOTION:
+ input_report_rel(dev, REL_X, event->motion.rel_x);
+ input_report_rel(dev, REL_Y, event->motion.rel_y);
+ break;
+ case XENKBD_TYPE_KEY:
+ dev = NULL;
+ if (test_bit(event->key.keycode, info->kbd->keybit))
+ dev = info->kbd;
+ if (test_bit(event->key.keycode, info->ptr->keybit))
+ dev = info->ptr;
+ if (dev)
+ input_report_key(dev, event->key.keycode,
+event->key.pressed);
+ else
+ printk(KERN_WARNING
+ "xenkbd: unhandled keycode 0x%x\n",
+
[PATCH 2/3] fbdev: Make deferred I/O work as advertized
Deferred I/O was utterly broken. Reading the mmap()ed framebuffer
worked, but writing it made the VM endlessly invoke
vm_ops.page_mkwrite(). That happened because we failed to set
page->mapping and page->index.
The fix is to set them, and clean up properly before the framebuffer
gets released.
Fix extracted from this linux-fbdev-devel message:
Subject: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb
From: Jaya Kumar <[EMAIL PROTECTED]>
Date: 2008-02-18 13:41:26
Signed-off-by: Jaya Kumar <[EMAIL PROTECTED]>
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
drivers/video/fb_defio.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fb_defio.c b/drivers/video/fb_defio.c
index 0f8cfb9..24843fd 100644
--- a/drivers/video/fb_defio.c
+++ b/drivers/video/fb_defio.c
@@ -4,7 +4,7 @@
* Copyright (C) 2006 Jaya Kumar
*
* This file is subject to the terms and conditions of the GNU General Public
- * License. See the file COPYING in the main directory of this archive
+ * License. See the file COPYING in the main directory of this archive
* for more details.
*/
@@ -31,7 +31,7 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
unsigned long offset;
struct page *page;
struct fb_info *info = vma->vm_private_data;
- /* info->screen_base is in System RAM */
+ /* info->screen_base is virtual memory */
void *screen_base = (void __force *) info->screen_base;
offset = vmf->pgoff << PAGE_SHIFT;
@@ -43,6 +43,15 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
return VM_FAULT_SIGBUS;
get_page(page);
+
+ if (vma->vm_file)
+ page->mapping = vma->vm_file->f_mapping;
+ else
+ printk(KERN_ERR "no mapping available\n");
+
+ BUG_ON(!page->mapping);
+ page->index = vmf->pgoff;
+
vmf->page = page;
return 0;
}
@@ -138,11 +147,20 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_init);
void fb_deferred_io_cleanup(struct fb_info *info)
{
+ void *screen_base = (void __force *) info->screen_base;
struct fb_deferred_io *fbdefio = info->fbdefio;
+ struct page *page;
+ int i;
BUG_ON(!fbdefio);
cancel_delayed_work(>deferred_work);
flush_scheduled_work();
+
+ /* clear out the mapping that we setup */
+ for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
+ page = vmalloc_to_page(screen_base + i);
+ page->mapping = NULL;
+ }
}
EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] xen: Make xen-blkfront write its protocol ABI to xenstore
Frontends are expected to write their protocol ABI to xenstore. Since
the protocol ABI defaults to the backend's native ABI, things work
fine without that as long as the frontend's native ABI is identical to
the backend's native ABI. This is not the case for xen-blkfront
running 32-on-64, because its ABI differs between 32 and 64 bit, and
thus needs this fix.
Based on http://xenbits.xensource.com/xen-unstable.hg?rev/c545932a18f3
and http://xenbits.xensource.com/xen-unstable.hg?rev/ffe52263b430 by
Gerd Hoffmann <[EMAIL PROTECTED]>
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
drivers/block/xen-blkfront.c |7 +++
include/xen/interface/io/protocols.h | 21 +
2 files changed, 28 insertions(+), 0 deletions(-)
create mode 100644 include/xen/interface/io/protocols.h
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8afce67..e69164a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -46,6 +46,7 @@
#include
#include
+#include
#include
@@ -597,6 +598,12 @@ again:
message = "writing event-channel";
goto abort_transaction;
}
+ err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
+ XEN_IO_PROTO_ABI_NATIVE);
+ if (err) {
+ message = "writing protocol";
+ goto abort_transaction;
+ }
err = xenbus_transaction_end(xbt, 0);
if (err) {
diff --git a/include/xen/interface/io/protocols.h
b/include/xen/interface/io/protocols.h
new file mode 100644
index 000..01fc8ae
--- /dev/null
+++ b/include/xen/interface/io/protocols.h
@@ -0,0 +1,21 @@
+#ifndef __XEN_PROTOCOLS_H__
+#define __XEN_PROTOCOLS_H__
+
+#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi"
+#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi"
+#define XEN_IO_PROTO_ABI_IA64 "ia64-abi"
+#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi"
+
+#if defined(__i386__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
+#elif defined(__x86_64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64
+#elif defined(__ia64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
+#elif defined(__powerpc64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
+#else
+# error arch fixup needed here
+#endif
+
+#endif
--
1.5.3.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] xen pvfb: Para-virtual framebuffer, keyboard and pointer
This is a pair of Xen para-virtual frontend device drivers: drivers/video/xen-fbfront.c provides a framebuffer, and drivers/input/xen-kbdfront provides keyboard and mouse. The backends run in dom0 user space. Differences since last post: * Required patch fixing 32-on-64 xen-blkfront included. * Cleanup when xenkbd_probe() fails fixed. * Don't store event channel in device info. I started with the Xen version at http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/ca05cf1a9bdc Differences to that Xen version, for those who care: * Rewritten on top of fb deferred I/O * IRQ handler names visible in /proc and /sys match the driver names. * Use framebuffer helper functions appropriate for framebuffer in system RAM. * write() refreshes the framebuffer properly. * off-by-one height of some screen refreshs fixed. * Crash when register_framebuffer() fails fixed. * Test for empty ring in input_handler() fixed. * Deadlock in xen-kbdfront resume fixed. * Cleanup when xenkbd_probe() fails fixed. * General clean up. I have a step-by-step patch series from that Xen version to my version, if anybody is interested. Might be useful for reviewers familiar with the Xen version. The patch consists of three parts: 1. xen: Make xen-blkfront write its protocol ABI to xenstore Could do without, but then I'd have to put the same bug in xen-fbfront and xen-kbdfront. 2. fbdev: Make deferred I/O work as advertized I need fb deferred I/O, but is utterly broken. A fix has been floating around on linux-fbdev-devel as part of a larger patch, which as far as I know has not been merged anywhere, yet. This is just the fix. 2. xen pvfb: Para-virtual framebuffer, keyboard and pointer driver The actual drivers. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] xen pvfb: Para-virtual framebuffer, keyboard and pointer
This is a pair of Xen para-virtual frontend device drivers: drivers/video/xen-fbfront.c provides a framebuffer, and drivers/input/xen-kbdfront provides keyboard and mouse. The backends run in dom0 user space. Differences since last post: * Required patch fixing 32-on-64 xen-blkfront included. * Cleanup when xenkbd_probe() fails fixed. * Don't store event channel in device info. I started with the Xen version at http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/ca05cf1a9bdc Differences to that Xen version, for those who care: * Rewritten on top of fb deferred I/O * IRQ handler names visible in /proc and /sys match the driver names. * Use framebuffer helper functions appropriate for framebuffer in system RAM. * write() refreshes the framebuffer properly. * off-by-one height of some screen refreshs fixed. * Crash when register_framebuffer() fails fixed. * Test for empty ring in input_handler() fixed. * Deadlock in xen-kbdfront resume fixed. * Cleanup when xenkbd_probe() fails fixed. * General clean up. I have a step-by-step patch series from that Xen version to my version, if anybody is interested. Might be useful for reviewers familiar with the Xen version. The patch consists of three parts: 1. xen: Make xen-blkfront write its protocol ABI to xenstore Could do without, but then I'd have to put the same bug in xen-fbfront and xen-kbdfront. 2. fbdev: Make deferred I/O work as advertized I need fb deferred I/O, but is utterly broken. A fix has been floating around on linux-fbdev-devel as part of a larger patch, which as far as I know has not been merged anywhere, yet. This is just the fix. 2. xen pvfb: Para-virtual framebuffer, keyboard and pointer driver The actual drivers. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] xen: Make xen-blkfront write its protocol ABI to xenstore
Frontends are expected to write their protocol ABI to xenstore. Since
the protocol ABI defaults to the backend's native ABI, things work
fine without that as long as the frontend's native ABI is identical to
the backend's native ABI. This is not the case for xen-blkfront
running 32-on-64, because its ABI differs between 32 and 64 bit, and
thus needs this fix.
Based on http://xenbits.xensource.com/xen-unstable.hg?rev/c545932a18f3
and http://xenbits.xensource.com/xen-unstable.hg?rev/ffe52263b430 by
Gerd Hoffmann [EMAIL PROTECTED]
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
drivers/block/xen-blkfront.c |7 +++
include/xen/interface/io/protocols.h | 21 +
2 files changed, 28 insertions(+), 0 deletions(-)
create mode 100644 include/xen/interface/io/protocols.h
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8afce67..e69164a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -46,6 +46,7 @@
#include xen/interface/grant_table.h
#include xen/interface/io/blkif.h
+#include xen/interface/io/protocols.h
#include asm/xen/hypervisor.h
@@ -597,6 +598,12 @@ again:
message = writing event-channel;
goto abort_transaction;
}
+ err = xenbus_printf(xbt, dev-nodename, protocol, %s,
+ XEN_IO_PROTO_ABI_NATIVE);
+ if (err) {
+ message = writing protocol;
+ goto abort_transaction;
+ }
err = xenbus_transaction_end(xbt, 0);
if (err) {
diff --git a/include/xen/interface/io/protocols.h
b/include/xen/interface/io/protocols.h
new file mode 100644
index 000..01fc8ae
--- /dev/null
+++ b/include/xen/interface/io/protocols.h
@@ -0,0 +1,21 @@
+#ifndef __XEN_PROTOCOLS_H__
+#define __XEN_PROTOCOLS_H__
+
+#define XEN_IO_PROTO_ABI_X86_32 x86_32-abi
+#define XEN_IO_PROTO_ABI_X86_64 x86_64-abi
+#define XEN_IO_PROTO_ABI_IA64 ia64-abi
+#define XEN_IO_PROTO_ABI_POWERPC64 powerpc64-abi
+
+#if defined(__i386__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
+#elif defined(__x86_64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64
+#elif defined(__ia64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
+#elif defined(__powerpc64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
+#else
+# error arch fixup needed here
+#endif
+
+#endif
--
1.5.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] fbdev: Make deferred I/O work as advertized
Deferred I/O was utterly broken. Reading the mmap()ed framebuffer
worked, but writing it made the VM endlessly invoke
vm_ops.page_mkwrite(). That happened because we failed to set
page-mapping and page-index.
The fix is to set them, and clean up properly before the framebuffer
gets released.
Fix extracted from this linux-fbdev-devel message:
Subject: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb
From: Jaya Kumar [EMAIL PROTECTED]
Date: 2008-02-18 13:41:26
Signed-off-by: Jaya Kumar [EMAIL PROTECTED]
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
drivers/video/fb_defio.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fb_defio.c b/drivers/video/fb_defio.c
index 0f8cfb9..24843fd 100644
--- a/drivers/video/fb_defio.c
+++ b/drivers/video/fb_defio.c
@@ -4,7 +4,7 @@
* Copyright (C) 2006 Jaya Kumar
*
* This file is subject to the terms and conditions of the GNU General Public
- * License. See the file COPYING in the main directory of this archive
+ * License. See the file COPYING in the main directory of this archive
* for more details.
*/
@@ -31,7 +31,7 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
unsigned long offset;
struct page *page;
struct fb_info *info = vma-vm_private_data;
- /* info-screen_base is in System RAM */
+ /* info-screen_base is virtual memory */
void *screen_base = (void __force *) info-screen_base;
offset = vmf-pgoff PAGE_SHIFT;
@@ -43,6 +43,15 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
return VM_FAULT_SIGBUS;
get_page(page);
+
+ if (vma-vm_file)
+ page-mapping = vma-vm_file-f_mapping;
+ else
+ printk(KERN_ERR no mapping available\n);
+
+ BUG_ON(!page-mapping);
+ page-index = vmf-pgoff;
+
vmf-page = page;
return 0;
}
@@ -138,11 +147,20 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_init);
void fb_deferred_io_cleanup(struct fb_info *info)
{
+ void *screen_base = (void __force *) info-screen_base;
struct fb_deferred_io *fbdefio = info-fbdefio;
+ struct page *page;
+ int i;
BUG_ON(!fbdefio);
cancel_delayed_work(info-deferred_work);
flush_scheduled_work();
+
+ /* clear out the mapping that we setup */
+ for (i = 0 ; i info-fix.smem_len; i += PAGE_SIZE) {
+ page = vmalloc_to_page(screen_base + i);
+ page-mapping = NULL;
+ }
}
EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] xen pvfb: Para-virtual framebuffer, keyboard and pointer driver
This is a pair of Xen para-virtual frontend device drivers:
drivers/video/xen-fbfront.c provides a framebuffer, and
drivers/input/xen-kbdfront provides keyboard and mouse.
The backends run in dom0 user space.
The two drivers are not in two separate patches, because the
intermediate step (one driver, not the other) is somewhat problematic:
the backend in dom0 needs both drivers, and will refuse to complete
device initialization unless they're both present.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
drivers/input/Kconfig|9
drivers/input/Makefile |2
drivers/input/xen-kbdfront.c | 340
drivers/video/Kconfig| 14
drivers/video/Makefile |1
drivers/video/xen-fbfront.c | 550 +++
include/xen/interface/io/fbif.h | 124
include/xen/interface/io/kbdif.h | 114
8 files changed, 1154 insertions(+)
diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 9dea14d..5f9d860 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -149,6 +149,15 @@ config INPUT_APMPOWER
To compile this driver as a module, choose M here: the
module will be called apm-power.
+config XEN_KBDDEV_FRONTEND
+ tristate Xen virtual keyboard and mouse support
+ depends on XEN_FBDEV_FRONTEND
+ default y
+ help
+ This driver implements the front-end of the Xen virtual
+ keyboard and mouse device driver. It communicates with a back-end
+ in another domain.
+
comment Input Device Drivers
source drivers/input/keyboard/Kconfig
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 2ae87b1..98c4f9a 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -23,3 +23,5 @@ obj-$(CONFIG_INPUT_TOUCHSCREEN) += touchscreen/
obj-$(CONFIG_INPUT_MISC) += misc/
obj-$(CONFIG_INPUT_APMPOWER) += apm-power.o
+
+obj-$(CONFIG_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
new file mode 100644
index 000..0f47f46
--- /dev/null
+++ b/drivers/input/xen-kbdfront.c
@@ -0,0 +1,340 @@
+/*
+ * Xen para-virtual input device
+ *
+ * Copyright (C) 2005 Anthony Liguori [EMAIL PROTECTED]
+ * Copyright (C) 2006-2008 Red Hat, Inc., Markus Armbruster [EMAIL PROTECTED]
+ *
+ * Based on linux/drivers/input/mouse/sermouse.c
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive for
+ * more details.
+ */
+
+/*
+ * TODO:
+ *
+ * Switch to grant tables together with xen-fbfront.c.
+ */
+
+#include linux/kernel.h
+#include linux/errno.h
+#include linux/module.h
+#include linux/input.h
+#include asm/xen/hypervisor.h
+#include xen/events.h
+#include xen/page.h
+#include xen/interface/io/fbif.h
+#include xen/interface/io/kbdif.h
+#include xen/xenbus.h
+
+struct xenkbd_info {
+ struct input_dev *kbd;
+ struct input_dev *ptr;
+ struct xenkbd_page *page;
+ int irq;
+ struct xenbus_device *xbdev;
+ char phys[32];
+};
+
+static int xenkbd_remove(struct xenbus_device *);
+static int xenkbd_connect_backend(struct xenbus_device *, struct xenkbd_info
*);
+static void xenkbd_disconnect_backend(struct xenkbd_info *);
+
+/*
+ * Note: if you need to send out events, see xenfb_do_update() for how
+ * to do that.
+ */
+
+static irqreturn_t input_handler(int rq, void *dev_id)
+{
+ struct xenkbd_info *info = dev_id;
+ struct xenkbd_page *page = info-page;
+ __u32 cons, prod;
+
+ prod = page-in_prod;
+ if (prod == page-in_cons)
+ return IRQ_HANDLED;
+ rmb(); /* ensure we see ring contents up to prod */
+ for (cons = page-in_cons; cons != prod; cons++) {
+ union xenkbd_in_event *event;
+ struct input_dev *dev;
+ event = XENKBD_IN_RING_REF(page, cons);
+
+ dev = info-ptr;
+ switch (event-type) {
+ case XENKBD_TYPE_MOTION:
+ input_report_rel(dev, REL_X, event-motion.rel_x);
+ input_report_rel(dev, REL_Y, event-motion.rel_y);
+ break;
+ case XENKBD_TYPE_KEY:
+ dev = NULL;
+ if (test_bit(event-key.keycode, info-kbd-keybit))
+ dev = info-kbd;
+ if (test_bit(event-key.keycode, info-ptr-keybit))
+ dev = info-ptr;
+ if (dev)
+ input_report_key(dev, event-key.keycode,
+event-key.pressed);
+ else
+ printk(KERN_WARNING
+ xenkbd: unhandled keycode 0x%x\n
[PATCH] 9p: Make uname and remotename parsing more robust
match_strcpy() is a somewhat creepy function: the caller needs to make
sure that the destination buffer is big enough, and when he screws up
or forgets, match_strcpy() happily overruns the buffer.
There's exactly one customer: v9fs_parse_options(). I believe it
currently can't overflow its buffer, but that's not exactly obvious.
The source string is a substring of the mount options. The kernel
silently truncates those to PAGE_SIZE bytes, including the terminating
zero. See compat_sys_mount() and do_mount().
The destination buffer is obtained from __getname(), which allocates
from name_cachep, which is initialized by vfs_caches_init() for size
PATH_MAX.
We're safe as long as PATH_MAX <= PAGE_SIZE. PATH_MAX is 4096. As
far as I know, the smallest PAGE_SIZE is also 4096.
Here's a patch that makes the code a bit more obviously correct. It
doesn't depend on PATH_MAX <= PAGE_SIZE.
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index fbb12da..e42948b 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -135,10 +135,10 @@ static void v9fs_parse_options(struct v9fs_session_info
*v9ses)
v9ses->trans = v9fs_match_trans([0]);
break;
case Opt_uname:
- match_strcpy(v9ses->uname, [0]);
+ match_strlcpy(v9ses->uname, [0], PATH_MAX);
break;
case Opt_remotename:
- match_strcpy(v9ses->aname, [0]);
+ match_strlcpy(v9ses->aname, [0], PATH_MAX);
break;
case Opt_legacy:
v9ses->flags &= ~V9FS_EXTENDED;
diff --git a/include/linux/parser.h b/include/linux/parser.h
index 26b2bdf..7dcd050 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t
args[]);
int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result);
int match_hex(substring_t *, int *result);
-void match_strcpy(char *, const substring_t *);
+size_t match_strlcpy(char *, const substring_t *, size_t);
char *match_strdup(const substring_t *);
diff --git a/lib/parser.c b/lib/parser.c
index 703c8c1..4f0cbc0 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result)
}
/**
- * match_strcpy: - copies the characters from a substring_t to a string
- * @to: string to copy characters to.
- * @s: _t to copy
+ * match_strlcpy: - Copy the characters from a substring_t to a sized buffer
+ * @dest: where to copy to
+ * @src: _t to copy
+ * @size: size of destination buffer
*
- * Description: Copies the set of characters represented by the given
- * _t @s to the c-style string @to. Caller guarantees that @to is
- * large enough to hold the characters of @s.
+ * Description: Copy the characters in _t @src to the
+ * c-style string @dest. Copy no more than @size - 1 characters, plus
+ * the terminating NUL. Return length of @src.
*/
-void match_strcpy(char *to, const substring_t *s)
+size_t match_strlcpy(char *dest, const substring_t *src, size_t size)
{
- memcpy(to, s->from, s->to - s->from);
- to[s->to - s->from] = '\0';
+ size_t ret = src->to - src->from;
+
+ if (size) {
+ size_t len = ret >= size ? size - 1 : ret;
+ memcpy(dest, src->from, len);
+ dest[len] = '\0';
+ }
+ return ret;
}
/**
@@ -206,9 +213,10 @@ void match_strcpy(char *to, const substring_t *s)
*/
char *match_strdup(const substring_t *s)
{
- char *p = kmalloc(s->to - s->from + 1, GFP_KERNEL);
+ size_t sz = s->to - s->from + 1;
+ char *p = kmalloc(sz, GFP_KERNEL);
if (p)
- match_strcpy(p, s);
+ match_strlcpy(p, s, sz);
return p;
}
@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex);
-EXPORT_SYMBOL(match_strcpy);
+EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] 9p: Make uname and remotename parsing more robust
match_strcpy() is a somewhat creepy function: the caller needs to make
sure that the destination buffer is big enough, and when he screws up
or forgets, match_strcpy() happily overruns the buffer.
There's exactly one customer: v9fs_parse_options(). I believe it
currently can't overflow its buffer, but that's not exactly obvious.
The source string is a substring of the mount options. The kernel
silently truncates those to PAGE_SIZE bytes, including the terminating
zero. See compat_sys_mount() and do_mount().
The destination buffer is obtained from __getname(), which allocates
from name_cachep, which is initialized by vfs_caches_init() for size
PATH_MAX.
We're safe as long as PATH_MAX = PAGE_SIZE. PATH_MAX is 4096. As
far as I know, the smallest PAGE_SIZE is also 4096.
Here's a patch that makes the code a bit more obviously correct. It
doesn't depend on PATH_MAX = PAGE_SIZE.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index fbb12da..e42948b 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -135,10 +135,10 @@ static void v9fs_parse_options(struct v9fs_session_info
*v9ses)
v9ses-trans = v9fs_match_trans(args[0]);
break;
case Opt_uname:
- match_strcpy(v9ses-uname, args[0]);
+ match_strlcpy(v9ses-uname, args[0], PATH_MAX);
break;
case Opt_remotename:
- match_strcpy(v9ses-aname, args[0]);
+ match_strlcpy(v9ses-aname, args[0], PATH_MAX);
break;
case Opt_legacy:
v9ses-flags = ~V9FS_EXTENDED;
diff --git a/include/linux/parser.h b/include/linux/parser.h
index 26b2bdf..7dcd050 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t
args[]);
int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result);
int match_hex(substring_t *, int *result);
-void match_strcpy(char *, const substring_t *);
+size_t match_strlcpy(char *, const substring_t *, size_t);
char *match_strdup(const substring_t *);
diff --git a/lib/parser.c b/lib/parser.c
index 703c8c1..4f0cbc0 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result)
}
/**
- * match_strcpy: - copies the characters from a substring_t to a string
- * @to: string to copy characters to.
- * @s: substring_t to copy
+ * match_strlcpy: - Copy the characters from a substring_t to a sized buffer
+ * @dest: where to copy to
+ * @src: substring_t to copy
+ * @size: size of destination buffer
*
- * Description: Copies the set of characters represented by the given
- * substring_t @s to the c-style string @to. Caller guarantees that @to is
- * large enough to hold the characters of @s.
+ * Description: Copy the characters in substring_t @src to the
+ * c-style string @dest. Copy no more than @size - 1 characters, plus
+ * the terminating NUL. Return length of @src.
*/
-void match_strcpy(char *to, const substring_t *s)
+size_t match_strlcpy(char *dest, const substring_t *src, size_t size)
{
- memcpy(to, s-from, s-to - s-from);
- to[s-to - s-from] = '\0';
+ size_t ret = src-to - src-from;
+
+ if (size) {
+ size_t len = ret = size ? size - 1 : ret;
+ memcpy(dest, src-from, len);
+ dest[len] = '\0';
+ }
+ return ret;
}
/**
@@ -206,9 +213,10 @@ void match_strcpy(char *to, const substring_t *s)
*/
char *match_strdup(const substring_t *s)
{
- char *p = kmalloc(s-to - s-from + 1, GFP_KERNEL);
+ size_t sz = s-to - s-from + 1;
+ char *p = kmalloc(sz, GFP_KERNEL);
if (p)
- match_strcpy(p, s);
+ match_strlcpy(p, s, sz);
return p;
}
@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex);
-EXPORT_SYMBOL(match_strcpy);
+EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer driver
Jeremy Fitzhardinge <[EMAIL PROTECTED]> writes:
> Markus Armbruster wrote:
>> This is a pair of Xen para-virtual frontend device drivers:
>> drivers/video/xen-fbfront.c provides a framebuffer, and
>> drivers/input/xen-kbdfront provides keyboard and mouse.
>>
>
> Unless they're actually inter-dependent, could you post this as two
> separate patches? I don't know anything about these parts of the
> kernel, so it would be nice to make it very obvious which changes are
> fb vs mouse/keyboard.
I could do that do that, but the intermediate step (one driver, not
the other) is somewhat problematic: the backend in dom0 needs both
drivers, and will refuse to complete device initialization unless
they're both present.
> (I guess input/* vs video/* should make it obvious, but it looks like
> input has a config dependency on fb, so I'll avoid making too many
> presumptions...)
Framebuffer: fbif.h xen-fbfront.c
Keyboard/mouse: kbdif.h xen-kbdfront.h
I added the config dependency because having one without the other
doesn't make sense, as explained above.
Still want it split into two separate patches?
> (Couple of comments below)
>
> J
>
>> The backends run in dom0 user space.
>>
>> Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
>>
>> ---
[...]
>> diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
>> new file mode 100644
>> index 000..84f65cf
>> --- /dev/null
>> +++ b/drivers/input/xen-kbdfront.c
>> @@ -0,0 +1,337 @@
[...]
>> +static int __devinit xenkbd_probe(struct xenbus_device *dev,
>> + const struct xenbus_device_id *id)
>> +{
[...]
>> +if (ret < 0)
>> +goto error;
>> +
>> +return 0;
>> +
>> + error_nomem:
>> +ret = -ENOMEM;
>> +xenbus_dev_fatal(dev, ret, "allocating device memory");
>> + error:
>> +xenkbd_remove(dev);
>>
>
> This is happy if dev->info is only partially initialized?
It's designed that way. dev->info is initialized so that
xenkbd_remove() does nothing. Then stuff is stored into dev->info
only when it's sufficiently initialized for xenkbd_remove() to clean
it up.
>> +return ret;
>> +}
>> +
>> +static int xenkbd_resume(struct xenbus_device *dev)
>> +{
>> +struct xenkbd_info *info = dev->dev.driver_data;
>> +
>> +xenkbd_disconnect_backend(info);
>> +memset(info->page, 0, PAGE_SIZE);
>> +return xenkbd_connect_backend(dev, info);
>> +}
>> +
>> +static int xenkbd_remove(struct xenbus_device *dev)
>> +{
>> +struct xenkbd_info *info = dev->dev.driver_data;
>> +
>> +xenkbd_disconnect_backend(info);
>> +input_unregister_device(info->kbd);
>> +input_unregister_device(info->ptr);
>>
>
> Does this free kdb and ptr?
Yes. xenkbd_probe() initializes info->kbd and info->ptr to null, and
changes that to the device only after input_register_device()
succeeds. If something goes wrong between input_allocate_device() and
input_register_device(), xenkbd_probe() frees the device with
input_free_device(). This is how input_register_device() wants to be
used according to its function comment:
/**
* input_register_device - register device with input core
* @dev: device to be registered
*
* This function registers device with input core. The device must be
* allocated with input_allocate_device() and all it's capabilities
* set up before registering.
* If function fails the device must be freed with input_free_device().
* Once device has been successfully registered it can be unregistered
* with input_unregister_device(); input_free_device() should not be
* called in this case.
*/
There's another bug here: must not call input_unregister_device() when
the device is still null. Man, I remember checking cleanup multiple
times when this stuff went into Xen (i.e. quite some time ago), and I
still missed this one. Going to check cleanup *again*.
>> +free_page((unsigned long)info->page);
>> +kfree(info);
>> +return 0;
>> +}
[...]
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer
Jeremy Fitzhardinge <[EMAIL PROTECTED]> writes: > Markus Armbruster wrote: >> Forgot to mention: This patch depends on >> >> Subject: [PATCH] xen: Make xen-blkfront write its protocol ABI to >> xenstore >> From: Markus Armbruster <> >> Date: Thu, 06 Dec 2007 14:45:53 +0100 >> >> http://lkml.org/lkml/2007/12/6/132 >> >> Sorry! > > Sorry, I haven't pushed this upstream yet, since there didn't seem to > be any particular urgency. What's the dependency? > >J Here's the description again: Frontends are expected to write their protocol ABI to xenstore. Since the protocol ABI defaults to the backend's native ABI, things work fine without that as long as the frontend's native ABI is identical to the backend's native ABI. This is not the case for xen-blkfront running 32-on-64, because its ABI differs between 32 and 64 bit, and thus needs this fix. I can break the dependency by putting the same bug that is now in xen-blkfront into xen-fbfront and xen-kbdfront. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fbdev: Make deferred I/O work as advertized
"Jaya Kumar" <[EMAIL PROTECTED]> writes: > On Thu, Feb 21, 2008 at 5:43 PM, Markus Armbruster <[EMAIL PROTECTED]> wrote: >> Fix extracted from this linux-fbdev-devel message: > > Hi Markus, > > Yes, this was discussed back in November on linux-mm and hence my > patch. I didn't push for it to be merged by itself because I don't > think it makes sense to merge it separately from the full metronomefb > patch. As far as I can tell, only hecubafb and metronomefb seem to be > the consumers. > > Out of curiosity, are you using defio or planning to use it? I would > love to hear back from people who are using it. > > Thanks, > jaya Converting to fb_defio made xen-fbfront quite a bit simpler and much more maintainable. Excellent match. It took me a bit of time to find your fix, mostly because I assumed my own code was broken, not fb_defio. It's best not to make assumptions about who uses your code; if it's any good, chances are somebody will find a use you never imagined. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer
Forgot to mention: This patch depends on Subject: [PATCH] xen: Make xen-blkfront write its protocol ABI to xenstore From: Markus Armbruster <> Date: Thu, 06 Dec 2007 14:45:53 +0100 http://lkml.org/lkml/2007/12/6/132 Sorry! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] fbdev: Make deferred I/O work as advertized
Deferred I/O was utterly broken. Reading the mmap()ed framebuffer
worked, but writing it made the VM endlessly invoke
vm_ops.page_mkwrite(). That happened because we failed to set
page->mapping and page->index.
The fix is to set them, and clean up properly before the framebuffer
gets released.
Fix extracted from this linux-fbdev-devel message:
Subject: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb
From: Jaya Kumar <[EMAIL PROTECTED]>
Date: 2008-02-18 13:41:26
Signed-off-by: Jaya Kumar <[EMAIL PROTECTED]>
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
drivers/video/fb_defio.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fb_defio.c b/drivers/video/fb_defio.c
index 0f8cfb9..24843fd 100644
--- a/drivers/video/fb_defio.c
+++ b/drivers/video/fb_defio.c
@@ -4,7 +4,7 @@
* Copyright (C) 2006 Jaya Kumar
*
* This file is subject to the terms and conditions of the GNU General Public
- * License. See the file COPYING in the main directory of this archive
+ * License. See the file COPYING in the main directory of this archive
* for more details.
*/
@@ -31,7 +31,7 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
unsigned long offset;
struct page *page;
struct fb_info *info = vma->vm_private_data;
- /* info->screen_base is in System RAM */
+ /* info->screen_base is virtual memory */
void *screen_base = (void __force *) info->screen_base;
offset = vmf->pgoff << PAGE_SHIFT;
@@ -43,6 +43,15 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
return VM_FAULT_SIGBUS;
get_page(page);
+
+ if (vma->vm_file)
+ page->mapping = vma->vm_file->f_mapping;
+ else
+ printk(KERN_ERR "no mapping available\n");
+
+ BUG_ON(!page->mapping);
+ page->index = vmf->pgoff;
+
vmf->page = page;
return 0;
}
@@ -138,11 +147,20 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_init);
void fb_deferred_io_cleanup(struct fb_info *info)
{
+ void *screen_base = (void __force *) info->screen_base;
struct fb_deferred_io *fbdefio = info->fbdefio;
+ struct page *page;
+ int i;
BUG_ON(!fbdefio);
cancel_delayed_work(>deferred_work);
flush_scheduled_work();
+
+ /* clear out the mapping that we setup */
+ for (i = 0 ; i < info->fix.smem_len; i += PAGE_SIZE) {
+ page = vmalloc_to_page(screen_base + i);
+ page->mapping = NULL;
+ }
}
EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer driver
This is a pair of Xen para-virtual frontend device drivers:
drivers/video/xen-fbfront.c provides a framebuffer, and
drivers/input/xen-kbdfront provides keyboard and mouse.
The backends run in dom0 user space.
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
drivers/input/Kconfig|9
drivers/input/Makefile |2
drivers/input/xen-kbdfront.c | 337 +++
drivers/video/Kconfig| 14
drivers/video/Makefile |1
drivers/video/xen-fbfront.c | 550 +++
include/xen/interface/io/fbif.h | 124
include/xen/interface/io/kbdif.h | 114
8 files changed, 1151 insertions(+)
diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 9dea14d..5f9d860 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -149,6 +149,15 @@ config INPUT_APMPOWER
To compile this driver as a module, choose M here: the
module will be called apm-power.
+config XEN_KBDDEV_FRONTEND
+ tristate "Xen virtual keyboard and mouse support"
+ depends on XEN_FBDEV_FRONTEND
+ default y
+ help
+ This driver implements the front-end of the Xen virtual
+ keyboard and mouse device driver. It communicates with a back-end
+ in another domain.
+
comment "Input Device Drivers"
source "drivers/input/keyboard/Kconfig"
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 2ae87b1..98c4f9a 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -23,3 +23,5 @@ obj-$(CONFIG_INPUT_TOUCHSCREEN) += touchscreen/
obj-$(CONFIG_INPUT_MISC) += misc/
obj-$(CONFIG_INPUT_APMPOWER) += apm-power.o
+
+obj-$(CONFIG_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
new file mode 100644
index 000..84f65cf
--- /dev/null
+++ b/drivers/input/xen-kbdfront.c
@@ -0,0 +1,337 @@
+/*
+ * Xen para-virtual input device
+ *
+ * Copyright (C) 2005 Anthony Liguori <[EMAIL PROTECTED]>
+ * Copyright (C) 2006-2008 Red Hat, Inc., Markus Armbruster <[EMAIL PROTECTED]>
+ *
+ * Based on linux/drivers/input/mouse/sermouse.c
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive for
+ * more details.
+ */
+
+/*
+ * TODO:
+ *
+ * Switch to grant tables together with xen-fbfront.c.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct xenkbd_info {
+ struct input_dev *kbd;
+ struct input_dev *ptr;
+ struct xenkbd_page *page;
+ int evtchn, irq;
+ struct xenbus_device *xbdev;
+ char phys[32];
+};
+
+static int xenkbd_remove(struct xenbus_device *);
+static int xenkbd_connect_backend(struct xenbus_device *, struct xenkbd_info
*);
+static void xenkbd_disconnect_backend(struct xenkbd_info *);
+
+/*
+ * Note: if you need to send out events, see xenfb_do_update() for how
+ * to do that.
+ */
+
+static irqreturn_t input_handler(int rq, void *dev_id)
+{
+ struct xenkbd_info *info = dev_id;
+ struct xenkbd_page *page = info->page;
+ __u32 cons, prod;
+
+ prod = page->in_prod;
+ if (prod == page->in_cons)
+ return IRQ_HANDLED;
+ rmb(); /* ensure we see ring contents up to prod */
+ for (cons = page->in_cons; cons != prod; cons++) {
+ union xenkbd_in_event *event;
+ struct input_dev *dev;
+ event = _IN_RING_REF(page, cons);
+
+ dev = info->ptr;
+ switch (event->type) {
+ case XENKBD_TYPE_MOTION:
+ input_report_rel(dev, REL_X, event->motion.rel_x);
+ input_report_rel(dev, REL_Y, event->motion.rel_y);
+ break;
+ case XENKBD_TYPE_KEY:
+ dev = NULL;
+ if (test_bit(event->key.keycode, info->kbd->keybit))
+ dev = info->kbd;
+ if (test_bit(event->key.keycode, info->ptr->keybit))
+ dev = info->ptr;
+ if (dev)
+ input_report_key(dev, event->key.keycode,
+event->key.pressed);
+ else
+ printk(KERN_WARNING
+ "xenkbd: unhandled keycode 0x%x\n",
+ event->key.keycode);
+ break;
+ case XENKBD_TYPE_POS:
+ input_report_abs(dev, ABS_X, event->pos.abs_x);
+ input_report_abs(dev, ABS_Y, event->pos
[PATCH 0/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer
This is a pair of Xen para-virtual frontend device drivers: drivers/video/xen-fbfront.c provides a framebuffer, and drivers/input/xen-kbdfront provides keyboard and mouse. The backends run in dom0 user space. I started with the Xen version at http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/ca05cf1a9bdc Differences to that Xen version, for those who care: * Rewritten on top of fb deferred I/O * IRQ handler names visible in /proc and /sys match the driver names. * Use framebuffer helper functions appropriate for framebuffer in system RAM. * write() refreshes the framebuffer properly. * off-by-one height of some screen refreshs fixed. * Crash when register_framebuffer() fails fixed. * Test for empty ring in input_handler() fixed. * Deadlock in xen-kbdfront resume fixed. * General clean up. I have a step-by-step patch series from that Xen version to my version, if anybody is interested. Might be useful for reviewers familiar with the Xen version. The patch consists of two parts: 1. fbdev: Make deferred I/O work as advertized I need fb deferred I/O, but is utterly broken. A fix has been floating around on linux-fbdev-devel as part of a larger patch, which as far as I know has not been merged anywhere, yet. This is just the fix. 2. xen pvfb: Para-virtual framebuffer, keyboard and pointer driver The actual drivers. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer
This is a pair of Xen para-virtual frontend device drivers: drivers/video/xen-fbfront.c provides a framebuffer, and drivers/input/xen-kbdfront provides keyboard and mouse. The backends run in dom0 user space. I started with the Xen version at http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/ca05cf1a9bdc Differences to that Xen version, for those who care: * Rewritten on top of fb deferred I/O * IRQ handler names visible in /proc and /sys match the driver names. * Use framebuffer helper functions appropriate for framebuffer in system RAM. * write() refreshes the framebuffer properly. * off-by-one height of some screen refreshs fixed. * Crash when register_framebuffer() fails fixed. * Test for empty ring in input_handler() fixed. * Deadlock in xen-kbdfront resume fixed. * General clean up. I have a step-by-step patch series from that Xen version to my version, if anybody is interested. Might be useful for reviewers familiar with the Xen version. The patch consists of two parts: 1. fbdev: Make deferred I/O work as advertized I need fb deferred I/O, but is utterly broken. A fix has been floating around on linux-fbdev-devel as part of a larger patch, which as far as I know has not been merged anywhere, yet. This is just the fix. 2. xen pvfb: Para-virtual framebuffer, keyboard and pointer driver The actual drivers. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer driver
This is a pair of Xen para-virtual frontend device drivers:
drivers/video/xen-fbfront.c provides a framebuffer, and
drivers/input/xen-kbdfront provides keyboard and mouse.
The backends run in dom0 user space.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
drivers/input/Kconfig|9
drivers/input/Makefile |2
drivers/input/xen-kbdfront.c | 337 +++
drivers/video/Kconfig| 14
drivers/video/Makefile |1
drivers/video/xen-fbfront.c | 550 +++
include/xen/interface/io/fbif.h | 124
include/xen/interface/io/kbdif.h | 114
8 files changed, 1151 insertions(+)
diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 9dea14d..5f9d860 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -149,6 +149,15 @@ config INPUT_APMPOWER
To compile this driver as a module, choose M here: the
module will be called apm-power.
+config XEN_KBDDEV_FRONTEND
+ tristate Xen virtual keyboard and mouse support
+ depends on XEN_FBDEV_FRONTEND
+ default y
+ help
+ This driver implements the front-end of the Xen virtual
+ keyboard and mouse device driver. It communicates with a back-end
+ in another domain.
+
comment Input Device Drivers
source drivers/input/keyboard/Kconfig
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 2ae87b1..98c4f9a 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -23,3 +23,5 @@ obj-$(CONFIG_INPUT_TOUCHSCREEN) += touchscreen/
obj-$(CONFIG_INPUT_MISC) += misc/
obj-$(CONFIG_INPUT_APMPOWER) += apm-power.o
+
+obj-$(CONFIG_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
new file mode 100644
index 000..84f65cf
--- /dev/null
+++ b/drivers/input/xen-kbdfront.c
@@ -0,0 +1,337 @@
+/*
+ * Xen para-virtual input device
+ *
+ * Copyright (C) 2005 Anthony Liguori [EMAIL PROTECTED]
+ * Copyright (C) 2006-2008 Red Hat, Inc., Markus Armbruster [EMAIL PROTECTED]
+ *
+ * Based on linux/drivers/input/mouse/sermouse.c
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive for
+ * more details.
+ */
+
+/*
+ * TODO:
+ *
+ * Switch to grant tables together with xen-fbfront.c.
+ */
+
+#include linux/kernel.h
+#include linux/errno.h
+#include linux/module.h
+#include linux/input.h
+#include asm/xen/hypervisor.h
+#include xen/events.h
+#include xen/page.h
+#include xen/interface/io/fbif.h
+#include xen/interface/io/kbdif.h
+#include xen/xenbus.h
+
+struct xenkbd_info {
+ struct input_dev *kbd;
+ struct input_dev *ptr;
+ struct xenkbd_page *page;
+ int evtchn, irq;
+ struct xenbus_device *xbdev;
+ char phys[32];
+};
+
+static int xenkbd_remove(struct xenbus_device *);
+static int xenkbd_connect_backend(struct xenbus_device *, struct xenkbd_info
*);
+static void xenkbd_disconnect_backend(struct xenkbd_info *);
+
+/*
+ * Note: if you need to send out events, see xenfb_do_update() for how
+ * to do that.
+ */
+
+static irqreturn_t input_handler(int rq, void *dev_id)
+{
+ struct xenkbd_info *info = dev_id;
+ struct xenkbd_page *page = info-page;
+ __u32 cons, prod;
+
+ prod = page-in_prod;
+ if (prod == page-in_cons)
+ return IRQ_HANDLED;
+ rmb(); /* ensure we see ring contents up to prod */
+ for (cons = page-in_cons; cons != prod; cons++) {
+ union xenkbd_in_event *event;
+ struct input_dev *dev;
+ event = XENKBD_IN_RING_REF(page, cons);
+
+ dev = info-ptr;
+ switch (event-type) {
+ case XENKBD_TYPE_MOTION:
+ input_report_rel(dev, REL_X, event-motion.rel_x);
+ input_report_rel(dev, REL_Y, event-motion.rel_y);
+ break;
+ case XENKBD_TYPE_KEY:
+ dev = NULL;
+ if (test_bit(event-key.keycode, info-kbd-keybit))
+ dev = info-kbd;
+ if (test_bit(event-key.keycode, info-ptr-keybit))
+ dev = info-ptr;
+ if (dev)
+ input_report_key(dev, event-key.keycode,
+event-key.pressed);
+ else
+ printk(KERN_WARNING
+ xenkbd: unhandled keycode 0x%x\n,
+ event-key.keycode);
+ break;
+ case XENKBD_TYPE_POS:
+ input_report_abs(dev, ABS_X, event-pos.abs_x);
+ input_report_abs(dev, ABS_Y, event
[PATCH 1/2] fbdev: Make deferred I/O work as advertized
Deferred I/O was utterly broken. Reading the mmap()ed framebuffer
worked, but writing it made the VM endlessly invoke
vm_ops.page_mkwrite(). That happened because we failed to set
page-mapping and page-index.
The fix is to set them, and clean up properly before the framebuffer
gets released.
Fix extracted from this linux-fbdev-devel message:
Subject: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb
From: Jaya Kumar [EMAIL PROTECTED]
Date: 2008-02-18 13:41:26
Signed-off-by: Jaya Kumar [EMAIL PROTECTED]
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
drivers/video/fb_defio.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fb_defio.c b/drivers/video/fb_defio.c
index 0f8cfb9..24843fd 100644
--- a/drivers/video/fb_defio.c
+++ b/drivers/video/fb_defio.c
@@ -4,7 +4,7 @@
* Copyright (C) 2006 Jaya Kumar
*
* This file is subject to the terms and conditions of the GNU General Public
- * License. See the file COPYING in the main directory of this archive
+ * License. See the file COPYING in the main directory of this archive
* for more details.
*/
@@ -31,7 +31,7 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
unsigned long offset;
struct page *page;
struct fb_info *info = vma-vm_private_data;
- /* info-screen_base is in System RAM */
+ /* info-screen_base is virtual memory */
void *screen_base = (void __force *) info-screen_base;
offset = vmf-pgoff PAGE_SHIFT;
@@ -43,6 +43,15 @@ static int fb_deferred_io_fault(struct vm_area_struct *vma,
return VM_FAULT_SIGBUS;
get_page(page);
+
+ if (vma-vm_file)
+ page-mapping = vma-vm_file-f_mapping;
+ else
+ printk(KERN_ERR no mapping available\n);
+
+ BUG_ON(!page-mapping);
+ page-index = vmf-pgoff;
+
vmf-page = page;
return 0;
}
@@ -138,11 +147,20 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_init);
void fb_deferred_io_cleanup(struct fb_info *info)
{
+ void *screen_base = (void __force *) info-screen_base;
struct fb_deferred_io *fbdefio = info-fbdefio;
+ struct page *page;
+ int i;
BUG_ON(!fbdefio);
cancel_delayed_work(info-deferred_work);
flush_scheduled_work();
+
+ /* clear out the mapping that we setup */
+ for (i = 0 ; i info-fix.smem_len; i += PAGE_SIZE) {
+ page = vmalloc_to_page(screen_base + i);
+ page-mapping = NULL;
+ }
}
EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer
Forgot to mention: This patch depends on Subject: [PATCH] xen: Make xen-blkfront write its protocol ABI to xenstore From: Markus Armbruster Date: Thu, 06 Dec 2007 14:45:53 +0100 http://lkml.org/lkml/2007/12/6/132 Sorry! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] fbdev: Make deferred I/O work as advertized
Jaya Kumar [EMAIL PROTECTED] writes: On Thu, Feb 21, 2008 at 5:43 PM, Markus Armbruster [EMAIL PROTECTED] wrote: Fix extracted from this linux-fbdev-devel message: Hi Markus, Yes, this was discussed back in November on linux-mm and hence my patch. I didn't push for it to be merged by itself because I don't think it makes sense to merge it separately from the full metronomefb patch. As far as I can tell, only hecubafb and metronomefb seem to be the consumers. Out of curiosity, are you using defio or planning to use it? I would love to hear back from people who are using it. Thanks, jaya Converting to fb_defio made xen-fbfront quite a bit simpler and much more maintainable. Excellent match. It took me a bit of time to find your fix, mostly because I assumed my own code was broken, not fb_defio. It's best not to make assumptions about who uses your code; if it's any good, chances are somebody will find a use you never imagined. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer
Jeremy Fitzhardinge [EMAIL PROTECTED] writes: Markus Armbruster wrote: Forgot to mention: This patch depends on Subject: [PATCH] xen: Make xen-blkfront write its protocol ABI to xenstore From: Markus Armbruster Date: Thu, 06 Dec 2007 14:45:53 +0100 http://lkml.org/lkml/2007/12/6/132 Sorry! Sorry, I haven't pushed this upstream yet, since there didn't seem to be any particular urgency. What's the dependency? J Here's the description again: Frontends are expected to write their protocol ABI to xenstore. Since the protocol ABI defaults to the backend's native ABI, things work fine without that as long as the frontend's native ABI is identical to the backend's native ABI. This is not the case for xen-blkfront running 32-on-64, because its ABI differs between 32 and 64 bit, and thus needs this fix. I can break the dependency by putting the same bug that is now in xen-blkfront into xen-fbfront and xen-kbdfront. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] xen pvfb: Para-virtual framebuffer, keyboard and pointer driver
Jeremy Fitzhardinge [EMAIL PROTECTED] writes:
Markus Armbruster wrote:
This is a pair of Xen para-virtual frontend device drivers:
drivers/video/xen-fbfront.c provides a framebuffer, and
drivers/input/xen-kbdfront provides keyboard and mouse.
Unless they're actually inter-dependent, could you post this as two
separate patches? I don't know anything about these parts of the
kernel, so it would be nice to make it very obvious which changes are
fb vs mouse/keyboard.
I could do that do that, but the intermediate step (one driver, not
the other) is somewhat problematic: the backend in dom0 needs both
drivers, and will refuse to complete device initialization unless
they're both present.
(I guess input/* vs video/* should make it obvious, but it looks like
input has a config dependency on fb, so I'll avoid making too many
presumptions...)
Framebuffer: fbif.h xen-fbfront.c
Keyboard/mouse: kbdif.h xen-kbdfront.h
I added the config dependency because having one without the other
doesn't make sense, as explained above.
Still want it split into two separate patches?
(Couple of comments below)
J
The backends run in dom0 user space.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
[...]
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
new file mode 100644
index 000..84f65cf
--- /dev/null
+++ b/drivers/input/xen-kbdfront.c
@@ -0,0 +1,337 @@
[...]
+static int __devinit xenkbd_probe(struct xenbus_device *dev,
+ const struct xenbus_device_id *id)
+{
[...]
+if (ret 0)
+goto error;
+
+return 0;
+
+ error_nomem:
+ret = -ENOMEM;
+xenbus_dev_fatal(dev, ret, allocating device memory);
+ error:
+xenkbd_remove(dev);
This is happy if dev-info is only partially initialized?
It's designed that way. dev-info is initialized so that
xenkbd_remove() does nothing. Then stuff is stored into dev-info
only when it's sufficiently initialized for xenkbd_remove() to clean
it up.
+return ret;
+}
+
+static int xenkbd_resume(struct xenbus_device *dev)
+{
+struct xenkbd_info *info = dev-dev.driver_data;
+
+xenkbd_disconnect_backend(info);
+memset(info-page, 0, PAGE_SIZE);
+return xenkbd_connect_backend(dev, info);
+}
+
+static int xenkbd_remove(struct xenbus_device *dev)
+{
+struct xenkbd_info *info = dev-dev.driver_data;
+
+xenkbd_disconnect_backend(info);
+input_unregister_device(info-kbd);
+input_unregister_device(info-ptr);
Does this free kdb and ptr?
Yes. xenkbd_probe() initializes info-kbd and info-ptr to null, and
changes that to the device only after input_register_device()
succeeds. If something goes wrong between input_allocate_device() and
input_register_device(), xenkbd_probe() frees the device with
input_free_device(). This is how input_register_device() wants to be
used according to its function comment:
/**
* input_register_device - register device with input core
* @dev: device to be registered
*
* This function registers device with input core. The device must be
* allocated with input_allocate_device() and all it's capabilities
* set up before registering.
* If function fails the device must be freed with input_free_device().
* Once device has been successfully registered it can be unregistered
* with input_unregister_device(); input_free_device() should not be
* called in this case.
*/
There's another bug here: must not call input_unregister_device() when
the device is still null. Man, I remember checking cleanup multiple
times when this stuff went into Xen (i.e. quite some time ago), and I
still missed this one. Going to check cleanup *again*.
+free_page((unsigned long)info-page);
+kfree(info);
+return 0;
+}
[...]
Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Make v9fs uname and remotename parsing more robust
match_strcpy() is a somewhat creepy function: the caller needs to make
sure that the destination buffer is big enough, and when he screws up
or forgets, match_strcpy() happily overruns the buffer.
There's exactly one customer: v9fs_parse_options(). I believe it
currently can't overflow its buffer, but that's not exactly obvious.
The source string is a substing of the mount options. The kernel
silently truncates those to PAGE_SIZE bytes, including the terminating
zero. See compat_sys_mount() and do_mount().
The destination buffer is obtained from __getname(), which allocates
from name_cachep, which is initialized by vfs_caches_init() for size
PATH_MAX.
We're safe as long as PATH_MAX <= PAGE_SIZE. PATH_MAX is 4096. As
far as I know, the smallest PAGE_SIZE is also 4096.
Here's a patch that makes the code a bit more obviously correct. It
doesn't depend on PATH_MAX <= PAGE_SIZE.
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index fbb12da..e42948b 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -135,10 +135,10 @@ static void v9fs_parse_options(struct v9fs_session_info
*v9ses)
v9ses->trans = v9fs_match_trans([0]);
break;
case Opt_uname:
- match_strcpy(v9ses->uname, [0]);
+ match_strlcpy(v9ses->uname, [0], PATH_MAX);
break;
case Opt_remotename:
- match_strcpy(v9ses->aname, [0]);
+ match_strlcpy(v9ses->aname, [0], PATH_MAX);
break;
case Opt_legacy:
v9ses->flags &= ~V9FS_EXTENDED;
diff --git a/include/linux/parser.h b/include/linux/parser.h
index 26b2bdf..7dcd050 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t
args[]);
int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result);
int match_hex(substring_t *, int *result);
-void match_strcpy(char *, const substring_t *);
+size_t match_strlcpy(char *, const substring_t *, size_t);
char *match_strdup(const substring_t *);
diff --git a/lib/parser.c b/lib/parser.c
index 703c8c1..4f0cbc0 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result)
}
/**
- * match_strcpy: - copies the characters from a substring_t to a string
- * @to: string to copy characters to.
- * @s: _t to copy
+ * match_strlcpy: - Copy the characters from a substring_t to a sized buffer
+ * @dest: where to copy to
+ * @src: _t to copy
+ * @size: size of destination buffer
*
- * Description: Copies the set of characters represented by the given
- * _t @s to the c-style string @to. Caller guarantees that @to is
- * large enough to hold the characters of @s.
+ * Description: Copy the characters in _t @src to the
+ * c-style string @dest. Copy no more than @size - 1 characters, plus
+ * the terminating NUL. Return length of @src.
*/
-void match_strcpy(char *to, const substring_t *s)
+size_t match_strlcpy(char *dest, const substring_t *src, size_t size)
{
- memcpy(to, s->from, s->to - s->from);
- to[s->to - s->from] = '\0';
+ size_t ret = src->to - src->from;
+
+ if (size) {
+ size_t len = ret >= size ? size - 1 : ret;
+ memcpy(dest, src->from, len);
+ dest[len] = '\0';
+ }
+ return ret;
}
/**
@@ -206,9 +213,10 @@ void match_strcpy(char *to, const substring_t *s)
*/
char *match_strdup(const substring_t *s)
{
- char *p = kmalloc(s->to - s->from + 1, GFP_KERNEL);
+ size_t sz = s->to - s->from + 1;
+ char *p = kmalloc(sz, GFP_KERNEL);
if (p)
- match_strcpy(p, s);
+ match_strlcpy(p, s, sz);
return p;
}
@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex);
-EXPORT_SYMBOL(match_strcpy);
+EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Make v9fs uname and remotename parsing more robust
match_strcpy() is a somewhat creepy function: the caller needs to make
sure that the destination buffer is big enough, and when he screws up
or forgets, match_strcpy() happily overruns the buffer.
There's exactly one customer: v9fs_parse_options(). I believe it
currently can't overflow its buffer, but that's not exactly obvious.
The source string is a substing of the mount options. The kernel
silently truncates those to PAGE_SIZE bytes, including the terminating
zero. See compat_sys_mount() and do_mount().
The destination buffer is obtained from __getname(), which allocates
from name_cachep, which is initialized by vfs_caches_init() for size
PATH_MAX.
We're safe as long as PATH_MAX = PAGE_SIZE. PATH_MAX is 4096. As
far as I know, the smallest PAGE_SIZE is also 4096.
Here's a patch that makes the code a bit more obviously correct. It
doesn't depend on PATH_MAX = PAGE_SIZE.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index fbb12da..e42948b 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -135,10 +135,10 @@ static void v9fs_parse_options(struct v9fs_session_info
*v9ses)
v9ses-trans = v9fs_match_trans(args[0]);
break;
case Opt_uname:
- match_strcpy(v9ses-uname, args[0]);
+ match_strlcpy(v9ses-uname, args[0], PATH_MAX);
break;
case Opt_remotename:
- match_strcpy(v9ses-aname, args[0]);
+ match_strlcpy(v9ses-aname, args[0], PATH_MAX);
break;
case Opt_legacy:
v9ses-flags = ~V9FS_EXTENDED;
diff --git a/include/linux/parser.h b/include/linux/parser.h
index 26b2bdf..7dcd050 100644
--- a/include/linux/parser.h
+++ b/include/linux/parser.h
@@ -29,5 +29,5 @@ int match_token(char *, match_table_t table, substring_t
args[]);
int match_int(substring_t *, int *result);
int match_octal(substring_t *, int *result);
int match_hex(substring_t *, int *result);
-void match_strcpy(char *, const substring_t *);
+size_t match_strlcpy(char *, const substring_t *, size_t);
char *match_strdup(const substring_t *);
diff --git a/lib/parser.c b/lib/parser.c
index 703c8c1..4f0cbc0 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -182,18 +182,25 @@ int match_hex(substring_t *s, int *result)
}
/**
- * match_strcpy: - copies the characters from a substring_t to a string
- * @to: string to copy characters to.
- * @s: substring_t to copy
+ * match_strlcpy: - Copy the characters from a substring_t to a sized buffer
+ * @dest: where to copy to
+ * @src: substring_t to copy
+ * @size: size of destination buffer
*
- * Description: Copies the set of characters represented by the given
- * substring_t @s to the c-style string @to. Caller guarantees that @to is
- * large enough to hold the characters of @s.
+ * Description: Copy the characters in substring_t @src to the
+ * c-style string @dest. Copy no more than @size - 1 characters, plus
+ * the terminating NUL. Return length of @src.
*/
-void match_strcpy(char *to, const substring_t *s)
+size_t match_strlcpy(char *dest, const substring_t *src, size_t size)
{
- memcpy(to, s-from, s-to - s-from);
- to[s-to - s-from] = '\0';
+ size_t ret = src-to - src-from;
+
+ if (size) {
+ size_t len = ret = size ? size - 1 : ret;
+ memcpy(dest, src-from, len);
+ dest[len] = '\0';
+ }
+ return ret;
}
/**
@@ -206,9 +213,10 @@ void match_strcpy(char *to, const substring_t *s)
*/
char *match_strdup(const substring_t *s)
{
- char *p = kmalloc(s-to - s-from + 1, GFP_KERNEL);
+ size_t sz = s-to - s-from + 1;
+ char *p = kmalloc(sz, GFP_KERNEL);
if (p)
- match_strcpy(p, s);
+ match_strlcpy(p, s, sz);
return p;
}
@@ -216,5 +224,5 @@ EXPORT_SYMBOL(match_token);
EXPORT_SYMBOL(match_int);
EXPORT_SYMBOL(match_octal);
EXPORT_SYMBOL(match_hex);
-EXPORT_SYMBOL(match_strcpy);
+EXPORT_SYMBOL(match_strlcpy);
EXPORT_SYMBOL(match_strdup);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] xen: Make xen-blkfront write its protocol ABI to xenstore
Frontends are expected to write their protocol ABI to xenstore. Since
the protocol ABI defaults to the backend's native ABI, things work
fine without that as long as the frontend's native ABI is identical to
the backend's native ABI. This is not the case for xen-blkfront
running 32-on-64, because its ABI differs between 32 and 64 bit, and
thus needs this fix.
Based on http://xenbits.xensource.com/xen-unstable.hg?rev/c545932a18f3
and http://xenbits.xensource.com/xen-unstable.hg?rev/ffe52263b430 by
Gerd Hoffmann <[EMAIL PROTECTED]>
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
drivers/block/xen-blkfront.c |7 +++
include/xen/interface/io/protocols.h | 21 +
2 files changed, 28 insertions(+), 0 deletions(-)
create mode 100644 include/xen/interface/io/protocols.h
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2bdebcb..70ddb8c 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -46,6 +46,7 @@
#include
#include
+#include
#include
@@ -599,6 +600,12 @@ again:
message = "writing event-channel";
goto abort_transaction;
}
+ err = xenbus_printf(xbt, dev->nodename, "protocol", "%s",
+ XEN_IO_PROTO_ABI_NATIVE);
+ if (err) {
+ message = "writing protocol";
+ goto abort_transaction;
+ }
err = xenbus_transaction_end(xbt, 0);
if (err) {
diff --git a/include/xen/interface/io/protocols.h
b/include/xen/interface/io/protocols.h
new file mode 100644
index 000..01fc8ae
--- /dev/null
+++ b/include/xen/interface/io/protocols.h
@@ -0,0 +1,21 @@
+#ifndef __XEN_PROTOCOLS_H__
+#define __XEN_PROTOCOLS_H__
+
+#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi"
+#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi"
+#define XEN_IO_PROTO_ABI_IA64 "ia64-abi"
+#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi"
+
+#if defined(__i386__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
+#elif defined(__x86_64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64
+#elif defined(__ia64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
+#elif defined(__powerpc64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
+#else
+# error arch fixup needed here
+#endif
+
+#endif
--
1.5.3.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] xen: Make xen-blkfront write its protocol ABI to xenstore
Frontends are expected to write their protocol ABI to xenstore. Since
the protocol ABI defaults to the backend's native ABI, things work
fine without that as long as the frontend's native ABI is identical to
the backend's native ABI. This is not the case for xen-blkfront
running 32-on-64, because its ABI differs between 32 and 64 bit, and
thus needs this fix.
Based on http://xenbits.xensource.com/xen-unstable.hg?rev/c545932a18f3
and http://xenbits.xensource.com/xen-unstable.hg?rev/ffe52263b430 by
Gerd Hoffmann [EMAIL PROTECTED]
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
drivers/block/xen-blkfront.c |7 +++
include/xen/interface/io/protocols.h | 21 +
2 files changed, 28 insertions(+), 0 deletions(-)
create mode 100644 include/xen/interface/io/protocols.h
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2bdebcb..70ddb8c 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -46,6 +46,7 @@
#include xen/interface/grant_table.h
#include xen/interface/io/blkif.h
+#include xen/interface/io/protocols.h
#include asm/xen/hypervisor.h
@@ -599,6 +600,12 @@ again:
message = writing event-channel;
goto abort_transaction;
}
+ err = xenbus_printf(xbt, dev-nodename, protocol, %s,
+ XEN_IO_PROTO_ABI_NATIVE);
+ if (err) {
+ message = writing protocol;
+ goto abort_transaction;
+ }
err = xenbus_transaction_end(xbt, 0);
if (err) {
diff --git a/include/xen/interface/io/protocols.h
b/include/xen/interface/io/protocols.h
new file mode 100644
index 000..01fc8ae
--- /dev/null
+++ b/include/xen/interface/io/protocols.h
@@ -0,0 +1,21 @@
+#ifndef __XEN_PROTOCOLS_H__
+#define __XEN_PROTOCOLS_H__
+
+#define XEN_IO_PROTO_ABI_X86_32 x86_32-abi
+#define XEN_IO_PROTO_ABI_X86_64 x86_64-abi
+#define XEN_IO_PROTO_ABI_IA64 ia64-abi
+#define XEN_IO_PROTO_ABI_POWERPC64 powerpc64-abi
+
+#if defined(__i386__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
+#elif defined(__x86_64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64
+#elif defined(__ia64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64
+#elif defined(__powerpc64__)
+# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64
+#else
+# error arch fixup needed here
+#endif
+
+#endif
--
1.5.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] input: Fix interrupt enable in i8042_ctr when enabling interrupt fails
When enabling interrupts fails, the interrupt enable bit remains set
in i8042_ctr. Later writes of i8042_ctr to the hardware could
accidentally retry enabling interrupts. Clear the bit on failure.
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
Acked-by: Steven Rostedt <[EMAIL PROTECTED]>
---
Some time ago Steven Rostedt and I went over this changeset:
commit de9ce703c6b807b1dfef5942df4f2fadd0fdb67a
Author: Dmitry Torokhov <[EMAIL PROTECTED]>
Date: Sun Sep 10 21:57:21 2006 -0400
Input: i8042 - get rid of polling timer
Remove polling timer that was used to detect keybord/mice hotplug and
register both IRQs right away instead of waiting for a driver to
attach to a port.
Signed-off-by: Dmitry Torokhov <[EMAIL PROTECTED]>
Steven pointed out to me that it changes behavior when enabling IRQ
fails.
The old code enabled IRQs this way:
i8042_ctr |= port->irqen;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
i8042_ctr &= ~port->irqen;
return -1;
}
i8042_ctr shadows the 8042's CTR. So, when enabling fails, the bit is
cleared in the shadow.
The new code does not clear the bit on the error path:
static int i8042_enable_kbd_port(void)
{
i8042_ctr &= ~I8042_CTR_KBDDIS;
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
printk(KERN_ERR "i8042.c: Failed to enable KBD port.\n");
return -EIO;
}
return 0;
}
Same for i8042_enable_aux_port().
This leads to the question whether there are later writes of i8042_ctr
(possibly with other bits altered) to the hardware, which could
accidentally retry enabling interrupts.
I believe this possible, but unlikely (perhaps not so unlikely on
virtual machines). Scenarios involve enable succeeding the first
time, failing the second time, and succeeding the third time. I can
provide details, but the point I'd like to make is not that this is
broken (although it is, strictly speaking), but that it is not
obviously correct where it easily could be: just clear the interrupt
enable bits when writing them to the hardware failed, like the old
code did.
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index db9cca3..71a7e39 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -385,6 +385,7 @@ static int i8042_enable_kbd_port(void)
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr &= ~I8042_CTR_KBDINT;
printk(KERN_ERR "i8042.c: Failed to enable KBD port.\n");
return -EIO;
}
@@ -402,6 +403,7 @@ static int i8042_enable_aux_port(void)
i8042_ctr |= I8042_CTR_AUXINT;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr &= ~I8042_CTR_AUXINT;
printk(KERN_ERR "i8042.c: Failed to enable AUX port.\n");
return -EIO;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] input: Fix interrupt enable in i8042_ctr when enabling interrupt fails
When enabling interrupts fails, the interrupt enable bit remains set
in i8042_ctr. Later writes of i8042_ctr to the hardware could
accidentally retry enabling interrupts. Clear the bit on failure.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
Acked-by: Steven Rostedt [EMAIL PROTECTED]
---
Some time ago Steven Rostedt and I went over this changeset:
commit de9ce703c6b807b1dfef5942df4f2fadd0fdb67a
Author: Dmitry Torokhov [EMAIL PROTECTED]
Date: Sun Sep 10 21:57:21 2006 -0400
Input: i8042 - get rid of polling timer
Remove polling timer that was used to detect keybord/mice hotplug and
register both IRQs right away instead of waiting for a driver to
attach to a port.
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
Steven pointed out to me that it changes behavior when enabling IRQ
fails.
The old code enabled IRQs this way:
i8042_ctr |= port-irqen;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
i8042_ctr = ~port-irqen;
return -1;
}
i8042_ctr shadows the 8042's CTR. So, when enabling fails, the bit is
cleared in the shadow.
The new code does not clear the bit on the error path:
static int i8042_enable_kbd_port(void)
{
i8042_ctr = ~I8042_CTR_KBDDIS;
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
printk(KERN_ERR i8042.c: Failed to enable KBD port.\n);
return -EIO;
}
return 0;
}
Same for i8042_enable_aux_port().
This leads to the question whether there are later writes of i8042_ctr
(possibly with other bits altered) to the hardware, which could
accidentally retry enabling interrupts.
I believe this possible, but unlikely (perhaps not so unlikely on
virtual machines). Scenarios involve enable succeeding the first
time, failing the second time, and succeeding the third time. I can
provide details, but the point I'd like to make is not that this is
broken (although it is, strictly speaking), but that it is not
obviously correct where it easily could be: just clear the interrupt
enable bits when writing them to the hardware failed, like the old
code did.
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index db9cca3..71a7e39 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -385,6 +385,7 @@ static int i8042_enable_kbd_port(void)
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr = ~I8042_CTR_KBDINT;
printk(KERN_ERR i8042.c: Failed to enable KBD port.\n);
return -EIO;
}
@@ -402,6 +403,7 @@ static int i8042_enable_aux_port(void)
i8042_ctr |= I8042_CTR_AUXINT;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr = ~I8042_CTR_AUXINT;
printk(KERN_ERR i8042.c: Failed to enable AUX port.\n);
return -EIO;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] input: Fix interrupt enable in i8042_ctr when enabling interrupt fails
When enabling interrupts fails, the interrupt enable bit remains set
in i8042_ctr. Later writes of i8042_ctr to the hardware could
accidentally retry enabling interrupts. Clear the bit on failure.
Signed-off-by: Markus Armbruster <[EMAIL PROTECTED]>
---
Some time ago Steven Rostedt and I went over this changeset:
commit de9ce703c6b807b1dfef5942df4f2fadd0fdb67a
Author: Dmitry Torokhov <[EMAIL PROTECTED]>
Date: Sun Sep 10 21:57:21 2006 -0400
Input: i8042 - get rid of polling timer
Remove polling timer that was used to detect keybord/mice hotplug and
register both IRQs right away instead of waiting for a driver to
attach to a port.
Signed-off-by: Dmitry Torokhov <[EMAIL PROTECTED]>
Steven pointed out to me that it changes behavior when enabling IRQ
fails.
The old code enabled IRQs this way:
i8042_ctr |= port->irqen;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
i8042_ctr &= ~port->irqen;
return -1;
}
i8042_ctr shadows the 8042's CTR. So, when enabling fails, the bit is
cleared in the shadow.
The new code does not clear the bit on the error path:
static int i8042_enable_kbd_port(void)
{
i8042_ctr &= ~I8042_CTR_KBDDIS;
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
printk(KERN_ERR "i8042.c: Failed to enable KBD port.\n");
return -EIO;
}
return 0;
}
Same for i8042_enable_aux_port().
This leads to the question whether there are later writes of i8042_ctr
(possibly with other bits altered) to the hardware, which could
accidentally retry enabling interrupts.
I believe this possible, but unlikely. Scenarios involve enable
succeeding the first time, failing the second time, and succeeding the
third time. I can provide details, but the point I'd like to make is
not that this is broken (although it is, strictly speaking), but that
it is not obviously correct where it easily could be: just clear the
interrupt enable bits when writing them to the hardware failed, like
the old code did.
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index db9cca3..71a7e39 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -385,6 +385,7 @@ static int i8042_enable_kbd_port(void)
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr &= ~I8042_CTR_KBDINT;
printk(KERN_ERR "i8042.c: Failed to enable KBD port.\n");
return -EIO;
}
@@ -402,6 +403,7 @@ static int i8042_enable_aux_port(void)
i8042_ctr |= I8042_CTR_AUXINT;
if (i8042_command(_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr &= ~I8042_CTR_AUXINT;
printk(KERN_ERR "i8042.c: Failed to enable AUX port.\n");
return -EIO;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] input: Fix interrupt enable in i8042_ctr when enabling interrupt fails
When enabling interrupts fails, the interrupt enable bit remains set
in i8042_ctr. Later writes of i8042_ctr to the hardware could
accidentally retry enabling interrupts. Clear the bit on failure.
Signed-off-by: Markus Armbruster [EMAIL PROTECTED]
---
Some time ago Steven Rostedt and I went over this changeset:
commit de9ce703c6b807b1dfef5942df4f2fadd0fdb67a
Author: Dmitry Torokhov [EMAIL PROTECTED]
Date: Sun Sep 10 21:57:21 2006 -0400
Input: i8042 - get rid of polling timer
Remove polling timer that was used to detect keybord/mice hotplug and
register both IRQs right away instead of waiting for a driver to
attach to a port.
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
Steven pointed out to me that it changes behavior when enabling IRQ
fails.
The old code enabled IRQs this way:
i8042_ctr |= port-irqen;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
i8042_ctr = ~port-irqen;
return -1;
}
i8042_ctr shadows the 8042's CTR. So, when enabling fails, the bit is
cleared in the shadow.
The new code does not clear the bit on the error path:
static int i8042_enable_kbd_port(void)
{
i8042_ctr = ~I8042_CTR_KBDDIS;
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
printk(KERN_ERR i8042.c: Failed to enable KBD port.\n);
return -EIO;
}
return 0;
}
Same for i8042_enable_aux_port().
This leads to the question whether there are later writes of i8042_ctr
(possibly with other bits altered) to the hardware, which could
accidentally retry enabling interrupts.
I believe this possible, but unlikely. Scenarios involve enable
succeeding the first time, failing the second time, and succeeding the
third time. I can provide details, but the point I'd like to make is
not that this is broken (although it is, strictly speaking), but that
it is not obviously correct where it easily could be: just clear the
interrupt enable bits when writing them to the hardware failed, like
the old code did.
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index db9cca3..71a7e39 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -385,6 +385,7 @@ static int i8042_enable_kbd_port(void)
i8042_ctr |= I8042_CTR_KBDINT;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr = ~I8042_CTR_KBDINT;
printk(KERN_ERR i8042.c: Failed to enable KBD port.\n);
return -EIO;
}
@@ -402,6 +403,7 @@ static int i8042_enable_aux_port(void)
i8042_ctr |= I8042_CTR_AUXINT;
if (i8042_command(i8042_ctr, I8042_CMD_CTL_WCTR)) {
+ i8042_ctr = ~I8042_CTR_AUXINT;
printk(KERN_ERR i8042.c: Failed to enable AUX port.\n);
return -EIO;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
