[PATCH] Add macros for acessing lguest fields
The assumption that we have an overall irqs_pending flags,
and a one-to-one lguest <-> task mapping fails to hold on x86_64,
where we can have multiple puppies, aka vcpus.
Although ifdefs could be used, it makes the code much more
unreadable, and other ports are on the way, anyway. So some sort
of acessor is preferred anyway.
Signed-off-by: Glauber de Oliveira Costa <[EMAIL PROTECTED]>
---
drivers/lguest/io.c | 10 +-
drivers/lguest/lg.h |3 +++
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/lguest/io.c b/drivers/lguest/io.c
index ea68613..70bab38 100644
--- a/drivers/lguest/io.c
+++ b/drivers/lguest/io.c
@@ -247,7 +247,7 @@ static int lgread_other(struct lguest *lg,
void *buf, u32 addr, unsigned bytes)
{
if (!lguest_address_ok(lg, addr, bytes)
- || access_process_vm(lg->tsk, addr, buf, bytes, 0) != bytes) {
+ || access_process_vm(lguest_task(lg), addr, buf, bytes, 0) !=
bytes) {
memset(buf, 0, bytes);
kill_guest(lg, "bad address in registered DMA struct");
return 0;
@@ -261,7 +261,7 @@ static int lgwrite_other(struct lguest *lg, u32 addr,
const void *buf, unsigned bytes)
{
if (!lguest_address_ok(lg, addr, bytes)
- || (access_process_vm(lg->tsk, addr, (void *)buf, bytes, 1)
+ || (access_process_vm(lguest_task(lg), addr, (void *)buf, bytes, 1)
!= bytes)) {
kill_guest(lg, "bad address writing to registered DMA");
return 0;
@@ -376,7 +376,7 @@ static u32 do_dma(struct lguest *srclg, const struct
lguest_dma *src,
* we only want a single page. But it works, and returns the
* number of pages. Note that we're holding the destination's
* mmap_sem, as get_user_pages() requires. */
- if (get_user_pages(dstlg->tsk, dstlg->mm,
+ if (get_user_pages(lguest_task(dstlg), dstlg->mm,
dst->addr[i], 1, 1, 1, pages+i, NULL)
!= 1) {
/* This means the destination gave us a bogus buffer */
@@ -469,9 +469,9 @@ static int dma_transfer(struct lguest *srclg,
/* We trigger the destination interrupt, even if the destination was
* empty and we didn't transfer anything: this gives them a chance to
* wake up and refill. */
- set_bit(dst->interrupt, dstlg->irqs_pending);
+ set_bit(dst->interrupt, lguest_irqs_pending(dstlg));
/* Wake up the destination process. */
- wake_up_process(dstlg->tsk);
+ wake_up_process(lguest_task(dstlg));
/* If we passed the last "struct lguest_dma", the receive had no
* buffers left. */
return i == dst->num_dmas;
diff --git a/drivers/lguest/lg.h b/drivers/lguest/lg.h
index 64f0abe..b1ed671 100644
--- a/drivers/lguest/lg.h
+++ b/drivers/lguest/lg.h
@@ -258,6 +258,9 @@ unsigned long get_dma_buffer(struct lguest *lg, unsigned
long key,
void do_hypercalls(struct lguest *lg);
void write_timestamp(struct lguest *lg);
+#define lguest_task(__lg) __lg->tsk
+#define lguest_irqs_pending(__lg) __lg->irqs_pending
+
/*L:035
* Let's step aside for the moment, to study one important routine that's used
* widely in the Host code.
--
1.4.4.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Bug #126 OVERDRIVE CPU
Exquisite Replica Watches All the top Brands... Visit our online Shop! www.iruirueh.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH] input: Fix interrupt enable in i8042_ctr when enabling interrupt fails
Hi Steven, Markus, On 9/10/07, Steven Rostedt <[EMAIL PROTECTED]> wrote: > > -- > On Mon, 10 Sep 2007, Markus Armbruster wrote: > > > > 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. > > > > I also want to stress that this is more of a clean up for "technically > correct" code than a bug fix. This bug probably would never happen on > baremetal unless it was running on broken hardware. > > BUT!!! > > With more and more systems going to a virtual environment, having a bug or > some other anomaly can trigger the error that this patch prevents. The > patch will also trigger a print in the case of running on a buggy virtual > machine, which would help out the developers of that virtual machine to > fix their code. > The patch is in my tree and will be merged in the next window. -- Dmitry ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH] input: Fix interrupt enable in i8042_ctr when enabling interrupt fails
-- On Mon, 10 Sep 2007, Markus Armbruster wrote: > > 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. > I also want to stress that this is more of a clean up for "technically correct" code than a bug fix. This bug probably would never happen on baremetal unless it was running on broken hardware. BUT!!! With more and more systems going to a virtual environment, having a bug or some other anomaly can trigger the error that this patch prevents. The patch will also trigger a print in the case of running on a buggy virtual machine, which would help out the developers of that virtual machine to fix their code. Please apply. -- Steve ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
[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;
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
