* [RFC 1/1 linux-next] tools/lguest: update variables to avoid setreg(eax, eax)
@ 2015-06-16 20:27 Fabian Frederick
2015-06-23 11:45 ` Rusty Russell
0 siblings, 1 reply; 2+ messages in thread
From: Fabian Frederick @ 2015-06-16 20:27 UTC (permalink / raw)
To: linux-kernel; +Cc: Julia Lawall, Fabian Frederick, Rusty Russell, lguest
eax is used for offset calculation but is also a local
variable used to store register value.
Use val for value and defval for default value to remove
this ambiguity.
Thanks to Julia Lawall for Coccinelle scripting support.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
tools/lguest/lguest.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
index e440524..d466c79 100644
--- a/tools/lguest/lguest.c
+++ b/tools/lguest/lguest.c
@@ -1611,12 +1611,12 @@ static void emulate_insn(const u8 insn[])
{
unsigned long args[] = { LHREQ_TRAP, 13 };
unsigned int insnlen = 0, in = 0, small_operand = 0, byte_access;
- unsigned int eax, port, mask;
+ unsigned int val, port, mask;
/*
* Default is to return all-ones on IO port reads, which traditionally
* means "there's nothing there".
*/
- u32 val = 0xFFFFFFFF;
+ u32 defval = 0xFFFFFFFF;
/*
* This must be the Guest kernel trying to do something, not userspace!
@@ -1692,29 +1692,29 @@ static void emulate_insn(const u8 insn[])
* If it was an "IN" instruction, they expect the result to be read
* into %eax, so we change %eax.
*/
- eax = getreg(eax);
+ val = getreg(eax);
if (in) {
/* This is the PS/2 keyboard status; 1 means ready for output */
if (port == 0x64)
- val = 1;
+ defval = 1;
else if (is_pci_addr_port(port))
- pci_addr_ioread(port, mask, &val);
+ pci_addr_ioread(port, mask, &defval);
else if (is_pci_data_port(port))
- pci_data_ioread(port, mask, &val);
+ pci_data_ioread(port, mask, &defval);
/* Clear the bits we're about to read */
- eax &= ~mask;
- /* Copy bits in from val. */
- eax |= val & mask;
+ val &= ~mask;
+ /* Copy bits in from defval. */
+ val |= defval & mask;
/* Now update the register. */
- setreg(eax, eax);
+ setreg(eax, val);
} else {
if (is_pci_addr_port(port)) {
- if (!pci_addr_iowrite(port, mask, eax))
+ if (!pci_addr_iowrite(port, mask, val))
goto bad_io;
} else if (is_pci_data_port(port)) {
- if (!pci_data_iowrite(port, mask, eax))
+ if (!pci_data_iowrite(port, mask, val))
goto bad_io;
}
/* There are many other ports, eg. CMOS clock, serial
@@ -1722,7 +1722,7 @@ static void emulate_insn(const u8 insn[])
}
verbose("IO %s of %x to %u: %#08x\n",
- in ? "IN" : "OUT", mask, port, eax);
+ in ? "IN" : "OUT", mask, port, val);
skip_insn:
/* Finally, we've "done" the instruction, so move past it. */
setreg(eip, getreg(eip) + insnlen);
--
2.4.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC 1/1 linux-next] tools/lguest: update variables to avoid setreg(eax, eax)
2015-06-16 20:27 [RFC 1/1 linux-next] tools/lguest: update variables to avoid setreg(eax, eax) Fabian Frederick
@ 2015-06-23 11:45 ` Rusty Russell
0 siblings, 0 replies; 2+ messages in thread
From: Rusty Russell @ 2015-06-23 11:45 UTC (permalink / raw)
To: Fabian Frederick, linux-kernel; +Cc: Julia Lawall, Fabian Frederick, lguest
Fabian Frederick <fabf@skynet.be> writes:
> eax is used for offset calculation but is also a local
> variable used to store register value.
Well, it's the value we're going to put back in eax. And it came from
eax.
Renaming val to def_val is weird, since it's not the default. You can
tell it's set in various places.
I don't think this patch adds clarity. What are you trying to do?
Thanks,
Rusty.
> tools/lguest/lguest.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
> index e440524..d466c79 100644
> --- a/tools/lguest/lguest.c
> +++ b/tools/lguest/lguest.c
> @@ -1611,12 +1611,12 @@ static void emulate_insn(const u8 insn[])
> {
> unsigned long args[] = { LHREQ_TRAP, 13 };
> unsigned int insnlen = 0, in = 0, small_operand = 0, byte_access;
> - unsigned int eax, port, mask;
> + unsigned int val, port, mask;
> /*
> * Default is to return all-ones on IO port reads, which traditionally
> * means "there's nothing there".
> */
> - u32 val = 0xFFFFFFFF;
> + u32 defval = 0xFFFFFFFF;
>
> /*
> * This must be the Guest kernel trying to do something, not userspace!
> @@ -1692,29 +1692,29 @@ static void emulate_insn(const u8 insn[])
> * If it was an "IN" instruction, they expect the result to be read
> * into %eax, so we change %eax.
> */
> - eax = getreg(eax);
> + val = getreg(eax);
>
> if (in) {
> /* This is the PS/2 keyboard status; 1 means ready for output */
> if (port == 0x64)
> - val = 1;
> + defval = 1;
> else if (is_pci_addr_port(port))
> - pci_addr_ioread(port, mask, &val);
> + pci_addr_ioread(port, mask, &defval);
> else if (is_pci_data_port(port))
> - pci_data_ioread(port, mask, &val);
> + pci_data_ioread(port, mask, &defval);
>
> /* Clear the bits we're about to read */
> - eax &= ~mask;
> - /* Copy bits in from val. */
> - eax |= val & mask;
> + val &= ~mask;
> + /* Copy bits in from defval. */
> + val |= defval & mask;
> /* Now update the register. */
> - setreg(eax, eax);
> + setreg(eax, val);
> } else {
> if (is_pci_addr_port(port)) {
> - if (!pci_addr_iowrite(port, mask, eax))
> + if (!pci_addr_iowrite(port, mask, val))
> goto bad_io;
> } else if (is_pci_data_port(port)) {
> - if (!pci_data_iowrite(port, mask, eax))
> + if (!pci_data_iowrite(port, mask, val))
> goto bad_io;
> }
> /* There are many other ports, eg. CMOS clock, serial
> @@ -1722,7 +1722,7 @@ static void emulate_insn(const u8 insn[])
> }
>
> verbose("IO %s of %x to %u: %#08x\n",
> - in ? "IN" : "OUT", mask, port, eax);
> + in ? "IN" : "OUT", mask, port, val);
> skip_insn:
> /* Finally, we've "done" the instruction, so move past it. */
> setreg(eip, getreg(eip) + insnlen);
> --
> 2.4.2
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-06-23 11:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-16 20:27 [RFC 1/1 linux-next] tools/lguest: update variables to avoid setreg(eax, eax) Fabian Frederick
2015-06-23 11:45 ` Rusty Russell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.