* [Qemu-devel] [PATCH 0/6] ioport related clean ups
@ 2009-07-02 10:32 Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 1/6] split out ioport related stuffs from vl.c into ioport.c Isaku Yamahata
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
This patch series cleans up io port emulation
sliming down bloated vl.c a bit and consolidated
ones used for user emulation.
Isaku Yamahata (6):
split out ioport related stuffs from vl.c into ioport.c.
use constant IOPORTS_MASK instead of 0xffff.
ioport: consolidate duplicated logic in register_ioport_{read,
write}().
ioport: remove some #ifdef DEBUG_UNUSED_IOPORT.
consolidate user cpu_{in, out}[bwl] into ioport-user.c
use uint32_t for ioport port and value instead of int.
Makefile.target | 9 +-
bsd-user/main.c | 33 -------
cpu-all.h | 12 +--
darwin-user/main.c | 33 -------
hw/apb_pci.c | 12 +-
hw/hw.h | 5 +-
hw/isa.h | 8 +-
hw/isa_mmio.c | 12 +-
ioport-user.c | 59 ++++++++++++
ioport.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++++++++
ioport.h | 55 +++++++++++
linux-user/main.c | 33 -------
monitor.c | 2 +-
tests/Makefile | 4 +-
tests/qruncom.c | 33 -------
vl.c | 226 ---------------------------------------------
16 files changed, 399 insertions(+), 398 deletions(-)
create mode 100644 ioport-user.c
create mode 100644 ioport.c
create mode 100644 ioport.h
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 1/6] split out ioport related stuffs from vl.c into ioport.c.
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
@ 2009-07-02 10:32 ` Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 2/6] use constant IOPORTS_MASK instead of 0xffff Isaku Yamahata
` (4 subsequent siblings)
5 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
split out ioport related stuffs from vl.c into ioport.c.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Makefile.target | 2 +-
cpu-all.h | 12 +---
hw/hw.h | 5 +-
hw/isa.h | 8 +--
ioport.c | 258 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
ioport.h | 54 ++++++++++++
vl.c | 226 ------------------------------------------------
7 files changed, 317 insertions(+), 248 deletions(-)
create mode 100644 ioport.c
create mode 100644 ioport.h
diff --git a/Makefile.target b/Makefile.target
index a593503..7f7c167 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -489,7 +489,7 @@ endif #CONFIG_BSD_USER
ifndef CONFIG_USER_ONLY
obj-y = vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o \
- gdbstub.o gdbstub-xml.o msix.o
+ gdbstub.o gdbstub-xml.o msix.o ioport.o
# virtio has to be here due to weird dependency between PCI and virtio-net.
# need to fix this properly
obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
diff --git a/cpu-all.h b/cpu-all.h
index 97a224d..bb27830 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -836,17 +836,7 @@ void cpu_set_log_filename(const char *filename);
int cpu_str_to_log_mask(const char *str);
/* IO ports API */
-
-/* NOTE: as these functions may be even used when there is an isa
- brige on non x86 targets, we always defined them */
-#ifndef NO_CPU_IO_DEFS
-void cpu_outb(CPUState *env, int addr, int val);
-void cpu_outw(CPUState *env, int addr, int val);
-void cpu_outl(CPUState *env, int addr, int val);
-int cpu_inb(CPUState *env, int addr);
-int cpu_inw(CPUState *env, int addr);
-int cpu_inl(CPUState *env, int addr);
-#endif
+#include "ioport.h"
/* memory API */
diff --git a/hw/hw.h b/hw/hw.h
index 2e43c1f..a424d28 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -10,6 +10,7 @@
#include "cpu-common.h"
#endif
+#include "ioport.h"
#include "irq.h"
/* VM Load/Save */
@@ -266,8 +267,4 @@ void qemu_register_reset(QEMUResetHandler *func, void *opaque);
typedef int QEMUBootSetHandler(void *opaque, const char *boot_device);
void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-/* These should really be in isa.h, but are here to make pc.h happy. */
-typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
-typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
-
#endif
diff --git a/hw/isa.h b/hw/isa.h
index a8c1a56..f126ecc 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -2,13 +2,9 @@
#define HW_ISA_H
/* ISA bus */
-extern target_phys_addr_t isa_mem_base;
+#include "ioport.h"
-int register_ioport_read(int start, int length, int size,
- IOPortReadFunc *func, void *opaque);
-int register_ioport_write(int start, int length, int size,
- IOPortWriteFunc *func, void *opaque);
-void isa_unassign_ioport(int start, int length);
+extern target_phys_addr_t isa_mem_base;
void isa_mmio_init(target_phys_addr_t base, target_phys_addr_t size);
diff --git a/ioport.c b/ioport.c
new file mode 100644
index 0000000..d875209
--- /dev/null
+++ b/ioport.c
@@ -0,0 +1,258 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+/*
+ * splitted out ioport related stuffs from vl.c.
+ */
+
+#include "ioport.h"
+
+/***********************************************************/
+/* IO Port */
+
+//#define DEBUG_UNUSED_IOPORT
+//#define DEBUG_IOPORT
+
+#ifdef DEBUG_IOPORT
+# define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__)
+#else
+# define LOG_IOPORT(...) do { } while (0)
+#endif
+
+/* XXX: use a two level table to limit memory usage */
+
+static void *ioport_opaque[MAX_IOPORTS];
+static IOPortReadFunc *ioport_read_table[3][MAX_IOPORTS];
+static IOPortWriteFunc *ioport_write_table[3][MAX_IOPORTS];
+
+static IOPortReadFunc default_ioport_readb, default_ioport_readw, default_ioport_readl;
+static IOPortWriteFunc default_ioport_writeb, default_ioport_writew, default_ioport_writel;
+
+static uint32_t ioport_read(int index, uint32_t address)
+{
+ static IOPortReadFunc *default_func[3] = {
+ default_ioport_readb,
+ default_ioport_readw,
+ default_ioport_readl
+ };
+ IOPortReadFunc *func = ioport_read_table[index][address];
+ if (!func)
+ func = default_func[index];
+ return func(ioport_opaque[address], address);
+}
+
+static void ioport_write(int index, uint32_t address, uint32_t data)
+{
+ static IOPortWriteFunc *default_func[3] = {
+ default_ioport_writeb,
+ default_ioport_writew,
+ default_ioport_writel
+ };
+ IOPortWriteFunc *func = ioport_write_table[index][address];
+ if (!func)
+ func = default_func[index];
+ func(ioport_opaque[address], address, data);
+}
+
+static uint32_t default_ioport_readb(void *opaque, uint32_t address)
+{
+#ifdef DEBUG_UNUSED_IOPORT
+ fprintf(stderr, "unused inb: port=0x%04x\n", address);
+#endif
+ return 0xff;
+}
+
+static void default_ioport_writeb(void *opaque, uint32_t address, uint32_t data)
+{
+#ifdef DEBUG_UNUSED_IOPORT
+ fprintf(stderr, "unused outb: port=0x%04x data=0x%02x\n", address, data);
+#endif
+}
+
+/* default is to make two byte accesses */
+static uint32_t default_ioport_readw(void *opaque, uint32_t address)
+{
+ uint32_t data;
+ data = ioport_read(0, address);
+ address = (address + 1) & (MAX_IOPORTS - 1);
+ data |= ioport_read(0, address) << 8;
+ return data;
+}
+
+static void default_ioport_writew(void *opaque, uint32_t address, uint32_t data)
+{
+ ioport_write(0, address, data & 0xff);
+ address = (address + 1) & (MAX_IOPORTS - 1);
+ ioport_write(0, address, (data >> 8) & 0xff);
+}
+
+static uint32_t default_ioport_readl(void *opaque, uint32_t address)
+{
+#ifdef DEBUG_UNUSED_IOPORT
+ fprintf(stderr, "unused inl: port=0x%04x\n", address);
+#endif
+ return 0xffffffff;
+}
+
+static void default_ioport_writel(void *opaque, uint32_t address, uint32_t data)
+{
+#ifdef DEBUG_UNUSED_IOPORT
+ fprintf(stderr, "unused outl: port=0x%04x data=0x%02x\n", address, data);
+#endif
+}
+
+/* size is the word size in byte */
+int register_ioport_read(int start, int length, int size,
+ IOPortReadFunc *func, void *opaque)
+{
+ int i, bsize;
+
+ if (size == 1) {
+ bsize = 0;
+ } else if (size == 2) {
+ bsize = 1;
+ } else if (size == 4) {
+ bsize = 2;
+ } else {
+ hw_error("register_ioport_read: invalid size");
+ return -1;
+ }
+ for(i = start; i < start + length; i += size) {
+ ioport_read_table[bsize][i] = func;
+ if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
+ hw_error("register_ioport_read: invalid opaque");
+ ioport_opaque[i] = opaque;
+ }
+ return 0;
+}
+
+/* size is the word size in byte */
+int register_ioport_write(int start, int length, int size,
+ IOPortWriteFunc *func, void *opaque)
+{
+ int i, bsize;
+
+ if (size == 1) {
+ bsize = 0;
+ } else if (size == 2) {
+ bsize = 1;
+ } else if (size == 4) {
+ bsize = 2;
+ } else {
+ hw_error("register_ioport_write: invalid size");
+ return -1;
+ }
+ for(i = start; i < start + length; i += size) {
+ ioport_write_table[bsize][i] = func;
+ if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
+ hw_error("register_ioport_write: invalid opaque");
+ ioport_opaque[i] = opaque;
+ }
+ return 0;
+}
+
+void isa_unassign_ioport(int start, int length)
+{
+ int i;
+
+ for(i = start; i < start + length; i++) {
+ ioport_read_table[0][i] = default_ioport_readb;
+ ioport_read_table[1][i] = default_ioport_readw;
+ ioport_read_table[2][i] = default_ioport_readl;
+
+ ioport_write_table[0][i] = default_ioport_writeb;
+ ioport_write_table[1][i] = default_ioport_writew;
+ ioport_write_table[2][i] = default_ioport_writel;
+
+ ioport_opaque[i] = NULL;
+ }
+}
+
+/***********************************************************/
+
+void cpu_outb(CPUState *env, int addr, int val)
+{
+ LOG_IOPORT("outb: %04x %02x\n", addr, val);
+ ioport_write(0, addr, val);
+#ifdef CONFIG_KQEMU
+ if (env)
+ env->last_io_time = cpu_get_time_fast();
+#endif
+}
+
+void cpu_outw(CPUState *env, int addr, int val)
+{
+ LOG_IOPORT("outw: %04x %04x\n", addr, val);
+ ioport_write(1, addr, val);
+#ifdef CONFIG_KQEMU
+ if (env)
+ env->last_io_time = cpu_get_time_fast();
+#endif
+}
+
+void cpu_outl(CPUState *env, int addr, int val)
+{
+ LOG_IOPORT("outl: %04x %08x\n", addr, val);
+ ioport_write(2, addr, val);
+#ifdef CONFIG_KQEMU
+ if (env)
+ env->last_io_time = cpu_get_time_fast();
+#endif
+}
+
+int cpu_inb(CPUState *env, int addr)
+{
+ int val;
+ val = ioport_read(0, addr);
+ LOG_IOPORT("inb : %04x %02x\n", addr, val);
+#ifdef CONFIG_KQEMU
+ if (env)
+ env->last_io_time = cpu_get_time_fast();
+#endif
+ return val;
+}
+
+int cpu_inw(CPUState *env, int addr)
+{
+ int val;
+ val = ioport_read(1, addr);
+ LOG_IOPORT("inw : %04x %04x\n", addr, val);
+#ifdef CONFIG_KQEMU
+ if (env)
+ env->last_io_time = cpu_get_time_fast();
+#endif
+ return val;
+}
+
+int cpu_inl(CPUState *env, int addr)
+{
+ int val;
+ val = ioport_read(2, addr);
+ LOG_IOPORT("inl : %04x %08x\n", addr, val);
+#ifdef CONFIG_KQEMU
+ if (env)
+ env->last_io_time = cpu_get_time_fast();
+#endif
+ return val;
+}
+
diff --git a/ioport.h b/ioport.h
new file mode 100644
index 0000000..309a402
--- /dev/null
+++ b/ioport.h
@@ -0,0 +1,54 @@
+/*
+ * defines ioport related functions
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301 USA
+ */
+
+/**************************************************************************
+ * IO ports API
+ */
+
+#ifndef IOPORT_H
+#define IOPORT_H
+
+#include "qemu-common.h"
+
+#define MAX_IOPORTS (64 * 1024)
+
+/* These should really be in isa.h, but are here to make pc.h happy. */
+typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
+typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
+
+int register_ioport_read(int start, int length, int size,
+ IOPortReadFunc *func, void *opaque);
+int register_ioport_write(int start, int length, int size,
+ IOPortWriteFunc *func, void *opaque);
+void isa_unassign_ioport(int start, int length);
+
+
+/* NOTE: as these functions may be even used when there is an isa
+ brige on non x86 targets, we always defined them */
+#if !defined(NO_CPU_IO_DEFS) && defined(NEED_CPU_H)
+void cpu_outb(CPUState *env, int addr, int val);
+void cpu_outw(CPUState *env, int addr, int val);
+void cpu_outl(CPUState *env, int addr, int val);
+int cpu_inb(CPUState *env, int addr);
+int cpu_inw(CPUState *env, int addr);
+int cpu_inl(CPUState *env, int addr);
+#endif
+
+#endif /* IOPORT_H */
diff --git a/vl.c b/vl.c
index 7b7489c..7ae3a2c 100644
--- a/vl.c
+++ b/vl.c
@@ -167,18 +167,9 @@ int main(int argc, char **argv)
#include "slirp/libslirp.h"
-//#define DEBUG_UNUSED_IOPORT
-//#define DEBUG_IOPORT
//#define DEBUG_NET
//#define DEBUG_SLIRP
-
-#ifdef DEBUG_IOPORT
-# define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__)
-#else
-# define LOG_IOPORT(...) do { } while (0)
-#endif
-
#define DEFAULT_RAM_SIZE 128
/* Max number of USB devices that can be specified on the commandline. */
@@ -187,14 +178,8 @@ int main(int argc, char **argv)
/* Max number of bluetooth switches on the commandline. */
#define MAX_BT_CMDLINE 10
-/* XXX: use a two level table to limit memory usage */
-#define MAX_IOPORTS 65536
-
static const char *data_dir;
const char *bios_name = NULL;
-static void *ioport_opaque[MAX_IOPORTS];
-static IOPortReadFunc *ioport_read_table[3][MAX_IOPORTS];
-static IOPortWriteFunc *ioport_write_table[3][MAX_IOPORTS];
/* Note: drives_table[MAX_DRIVES] is a dummy block driver if none available
to store the VM snapshots */
DriveInfo drives_table[MAX_DRIVES+1];
@@ -294,217 +279,6 @@ uint8_t qemu_uuid[16];
target_phys_addr_t isa_mem_base = 0;
PicState2 *isa_pic;
-static IOPortReadFunc default_ioport_readb, default_ioport_readw, default_ioport_readl;
-static IOPortWriteFunc default_ioport_writeb, default_ioport_writew, default_ioport_writel;
-
-static uint32_t ioport_read(int index, uint32_t address)
-{
- static IOPortReadFunc *default_func[3] = {
- default_ioport_readb,
- default_ioport_readw,
- default_ioport_readl
- };
- IOPortReadFunc *func = ioport_read_table[index][address];
- if (!func)
- func = default_func[index];
- return func(ioport_opaque[address], address);
-}
-
-static void ioport_write(int index, uint32_t address, uint32_t data)
-{
- static IOPortWriteFunc *default_func[3] = {
- default_ioport_writeb,
- default_ioport_writew,
- default_ioport_writel
- };
- IOPortWriteFunc *func = ioport_write_table[index][address];
- if (!func)
- func = default_func[index];
- func(ioport_opaque[address], address, data);
-}
-
-static uint32_t default_ioport_readb(void *opaque, uint32_t address)
-{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused inb: port=0x%04x\n", address);
-#endif
- return 0xff;
-}
-
-static void default_ioport_writeb(void *opaque, uint32_t address, uint32_t data)
-{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused outb: port=0x%04x data=0x%02x\n", address, data);
-#endif
-}
-
-/* default is to make two byte accesses */
-static uint32_t default_ioport_readw(void *opaque, uint32_t address)
-{
- uint32_t data;
- data = ioport_read(0, address);
- address = (address + 1) & (MAX_IOPORTS - 1);
- data |= ioport_read(0, address) << 8;
- return data;
-}
-
-static void default_ioport_writew(void *opaque, uint32_t address, uint32_t data)
-{
- ioport_write(0, address, data & 0xff);
- address = (address + 1) & (MAX_IOPORTS - 1);
- ioport_write(0, address, (data >> 8) & 0xff);
-}
-
-static uint32_t default_ioport_readl(void *opaque, uint32_t address)
-{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused inl: port=0x%04x\n", address);
-#endif
- return 0xffffffff;
-}
-
-static void default_ioport_writel(void *opaque, uint32_t address, uint32_t data)
-{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused outl: port=0x%04x data=0x%02x\n", address, data);
-#endif
-}
-
-/* size is the word size in byte */
-int register_ioport_read(int start, int length, int size,
- IOPortReadFunc *func, void *opaque)
-{
- int i, bsize;
-
- if (size == 1) {
- bsize = 0;
- } else if (size == 2) {
- bsize = 1;
- } else if (size == 4) {
- bsize = 2;
- } else {
- hw_error("register_ioport_read: invalid size");
- return -1;
- }
- for(i = start; i < start + length; i += size) {
- ioport_read_table[bsize][i] = func;
- if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
- hw_error("register_ioport_read: invalid opaque");
- ioport_opaque[i] = opaque;
- }
- return 0;
-}
-
-/* size is the word size in byte */
-int register_ioport_write(int start, int length, int size,
- IOPortWriteFunc *func, void *opaque)
-{
- int i, bsize;
-
- if (size == 1) {
- bsize = 0;
- } else if (size == 2) {
- bsize = 1;
- } else if (size == 4) {
- bsize = 2;
- } else {
- hw_error("register_ioport_write: invalid size");
- return -1;
- }
- for(i = start; i < start + length; i += size) {
- ioport_write_table[bsize][i] = func;
- if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
- hw_error("register_ioport_write: invalid opaque");
- ioport_opaque[i] = opaque;
- }
- return 0;
-}
-
-void isa_unassign_ioport(int start, int length)
-{
- int i;
-
- for(i = start; i < start + length; i++) {
- ioport_read_table[0][i] = default_ioport_readb;
- ioport_read_table[1][i] = default_ioport_readw;
- ioport_read_table[2][i] = default_ioport_readl;
-
- ioport_write_table[0][i] = default_ioport_writeb;
- ioport_write_table[1][i] = default_ioport_writew;
- ioport_write_table[2][i] = default_ioport_writel;
-
- ioport_opaque[i] = NULL;
- }
-}
-
-/***********************************************************/
-
-void cpu_outb(CPUState *env, int addr, int val)
-{
- LOG_IOPORT("outb: %04x %02x\n", addr, val);
- ioport_write(0, addr, val);
-#ifdef CONFIG_KQEMU
- if (env)
- env->last_io_time = cpu_get_time_fast();
-#endif
-}
-
-void cpu_outw(CPUState *env, int addr, int val)
-{
- LOG_IOPORT("outw: %04x %04x\n", addr, val);
- ioport_write(1, addr, val);
-#ifdef CONFIG_KQEMU
- if (env)
- env->last_io_time = cpu_get_time_fast();
-#endif
-}
-
-void cpu_outl(CPUState *env, int addr, int val)
-{
- LOG_IOPORT("outl: %04x %08x\n", addr, val);
- ioport_write(2, addr, val);
-#ifdef CONFIG_KQEMU
- if (env)
- env->last_io_time = cpu_get_time_fast();
-#endif
-}
-
-int cpu_inb(CPUState *env, int addr)
-{
- int val;
- val = ioport_read(0, addr);
- LOG_IOPORT("inb : %04x %02x\n", addr, val);
-#ifdef CONFIG_KQEMU
- if (env)
- env->last_io_time = cpu_get_time_fast();
-#endif
- return val;
-}
-
-int cpu_inw(CPUState *env, int addr)
-{
- int val;
- val = ioport_read(1, addr);
- LOG_IOPORT("inw : %04x %04x\n", addr, val);
-#ifdef CONFIG_KQEMU
- if (env)
- env->last_io_time = cpu_get_time_fast();
-#endif
- return val;
-}
-
-int cpu_inl(CPUState *env, int addr)
-{
- int val;
- val = ioport_read(2, addr);
- LOG_IOPORT("inl : %04x %08x\n", addr, val);
-#ifdef CONFIG_KQEMU
- if (env)
- env->last_io_time = cpu_get_time_fast();
-#endif
- return val;
-}
-
/***********************************************************/
void hw_error(const char *fmt, ...)
{
--
1.6.0.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 2/6] use constant IOPORTS_MASK instead of 0xffff.
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 1/6] split out ioport related stuffs from vl.c into ioport.c Isaku Yamahata
@ 2009-07-02 10:32 ` Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 3/6] ioport: consolidate duplicated logic in register_ioport_{read, write}() Isaku Yamahata
` (3 subsequent siblings)
5 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
use constant IOPORTS_MASK instead of 0xffff.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/apb_pci.c | 12 ++++++------
hw/isa_mmio.c | 12 ++++++------
ioport.c | 4 ++--
ioport.h | 1 +
monitor.c | 2 +-
5 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index b63ccd1..9f2a44d 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -148,26 +148,26 @@ static CPUReadMemoryFunc *pci_apb_read[] = {
static void pci_apb_iowriteb (void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- cpu_outb(NULL, addr & 0xffff, val);
+ cpu_outb(NULL, addr & IOPORTS_MASK, val);
}
static void pci_apb_iowritew (void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- cpu_outw(NULL, addr & 0xffff, val);
+ cpu_outw(NULL, addr & IOPORTS_MASK, val);
}
static void pci_apb_iowritel (void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- cpu_outl(NULL, addr & 0xffff, val);
+ cpu_outl(NULL, addr & IOPORTS_MASK, val);
}
static uint32_t pci_apb_ioreadb (void *opaque, target_phys_addr_t addr)
{
uint32_t val;
- val = cpu_inb(NULL, addr & 0xffff);
+ val = cpu_inb(NULL, addr & IOPORTS_MASK);
return val;
}
@@ -175,7 +175,7 @@ static uint32_t pci_apb_ioreadw (void *opaque, target_phys_addr_t addr)
{
uint32_t val;
- val = cpu_inw(NULL, addr & 0xffff);
+ val = cpu_inw(NULL, addr & IOPORTS_MASK);
return val;
}
@@ -183,7 +183,7 @@ static uint32_t pci_apb_ioreadl (void *opaque, target_phys_addr_t addr)
{
uint32_t val;
- val = cpu_inl(NULL, addr & 0xffff);
+ val = cpu_inl(NULL, addr & IOPORTS_MASK);
return val;
}
diff --git a/hw/isa_mmio.c b/hw/isa_mmio.c
index 1d5e8dc..868c7b0 100644
--- a/hw/isa_mmio.c
+++ b/hw/isa_mmio.c
@@ -28,7 +28,7 @@
static void isa_mmio_writeb (void *opaque, target_phys_addr_t addr,
uint32_t val)
{
- cpu_outb(NULL, addr & 0xffff, val);
+ cpu_outb(NULL, addr & IOPORTS_MASK, val);
}
static void isa_mmio_writew (void *opaque, target_phys_addr_t addr,
@@ -37,7 +37,7 @@ static void isa_mmio_writew (void *opaque, target_phys_addr_t addr,
#ifdef TARGET_WORDS_BIGENDIAN
val = bswap16(val);
#endif
- cpu_outw(NULL, addr & 0xffff, val);
+ cpu_outw(NULL, addr & IOPORTS_MASK, val);
}
static void isa_mmio_writel (void *opaque, target_phys_addr_t addr,
@@ -46,14 +46,14 @@ static void isa_mmio_writel (void *opaque, target_phys_addr_t addr,
#ifdef TARGET_WORDS_BIGENDIAN
val = bswap32(val);
#endif
- cpu_outl(NULL, addr & 0xffff, val);
+ cpu_outl(NULL, addr & IOPORTS_MASK, val);
}
static uint32_t isa_mmio_readb (void *opaque, target_phys_addr_t addr)
{
uint32_t val;
- val = cpu_inb(NULL, addr & 0xffff);
+ val = cpu_inb(NULL, addr & IOPORTS_MASK);
return val;
}
@@ -61,7 +61,7 @@ static uint32_t isa_mmio_readw (void *opaque, target_phys_addr_t addr)
{
uint32_t val;
- val = cpu_inw(NULL, addr & 0xffff);
+ val = cpu_inw(NULL, addr & IOPORTS_MASK);
#ifdef TARGET_WORDS_BIGENDIAN
val = bswap16(val);
#endif
@@ -72,7 +72,7 @@ static uint32_t isa_mmio_readl (void *opaque, target_phys_addr_t addr)
{
uint32_t val;
- val = cpu_inl(NULL, addr & 0xffff);
+ val = cpu_inl(NULL, addr & IOPORTS_MASK);
#ifdef TARGET_WORDS_BIGENDIAN
val = bswap32(val);
#endif
diff --git a/ioport.c b/ioport.c
index d875209..04bffc4 100644
--- a/ioport.c
+++ b/ioport.c
@@ -94,7 +94,7 @@ static uint32_t default_ioport_readw(void *opaque, uint32_t address)
{
uint32_t data;
data = ioport_read(0, address);
- address = (address + 1) & (MAX_IOPORTS - 1);
+ address = (address + 1) & IOPORTS_MASK;
data |= ioport_read(0, address) << 8;
return data;
}
@@ -102,7 +102,7 @@ static uint32_t default_ioport_readw(void *opaque, uint32_t address)
static void default_ioport_writew(void *opaque, uint32_t address, uint32_t data)
{
ioport_write(0, address, data & 0xff);
- address = (address + 1) & (MAX_IOPORTS - 1);
+ address = (address + 1) & IOPORTS_MASK;
ioport_write(0, address, (data >> 8) & 0xff);
}
diff --git a/ioport.h b/ioport.h
index 309a402..4cb59e9 100644
--- a/ioport.h
+++ b/ioport.h
@@ -28,6 +28,7 @@
#include "qemu-common.h"
#define MAX_IOPORTS (64 * 1024)
+#define IOPORTS_MASK (MAX_IOPORTS - 1)
/* These should really be in isa.h, but are here to make pc.h happy. */
typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
diff --git a/monitor.c b/monitor.c
index bad79fe..eaceee5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1161,7 +1161,7 @@ static void do_ioport_read(Monitor *mon, int count, int format, int size,
int suffix;
if (has_index) {
- cpu_outb(NULL, addr & 0xffff, index & 0xff);
+ cpu_outb(NULL, addr & IOPORTS_MASK, index & 0xff);
addr++;
}
addr &= 0xffff;
--
1.6.0.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 3/6] ioport: consolidate duplicated logic in register_ioport_{read, write}().
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 1/6] split out ioport related stuffs from vl.c into ioport.c Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 2/6] use constant IOPORTS_MASK instead of 0xffff Isaku Yamahata
@ 2009-07-02 10:32 ` Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 4/6] ioport: remove some #ifdef DEBUG_UNUSED_IOPORT Isaku Yamahata
` (2 subsequent siblings)
5 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
consolidate duplicated logic in register_ioport_{read, write}().
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
ioport.c | 30 ++++++++++++++++--------------
1 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/ioport.c b/ioport.c
index 04bffc4..01cfaf7 100644
--- a/ioport.c
+++ b/ioport.c
@@ -121,19 +121,27 @@ static void default_ioport_writel(void *opaque, uint32_t address, uint32_t data)
#endif
}
+static int ioport_bsize(int size, int *bsize)
+{
+ if (size == 1) {
+ *bsize = 0;
+ } else if (size == 2) {
+ *bsize = 1;
+ } else if (size == 4) {
+ *bsize = 2;
+ } else {
+ return -1;
+ }
+ return 0;
+}
+
/* size is the word size in byte */
int register_ioport_read(int start, int length, int size,
IOPortReadFunc *func, void *opaque)
{
int i, bsize;
- if (size == 1) {
- bsize = 0;
- } else if (size == 2) {
- bsize = 1;
- } else if (size == 4) {
- bsize = 2;
- } else {
+ if (ioport_bsize(size, &bsize)) {
hw_error("register_ioport_read: invalid size");
return -1;
}
@@ -152,13 +160,7 @@ int register_ioport_write(int start, int length, int size,
{
int i, bsize;
- if (size == 1) {
- bsize = 0;
- } else if (size == 2) {
- bsize = 1;
- } else if (size == 4) {
- bsize = 2;
- } else {
+ if (ioport_bsize(size, &bsize)) {
hw_error("register_ioport_write: invalid size");
return -1;
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 4/6] ioport: remove some #ifdef DEBUG_UNUSED_IOPORT.
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
` (2 preceding siblings ...)
2009-07-02 10:32 ` [Qemu-devel] [PATCH 3/6] ioport: consolidate duplicated logic in register_ioport_{read, write}() Isaku Yamahata
@ 2009-07-02 10:32 ` Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 5/6] consolidate user cpu_{in, out}[bwl] into ioport-user.c Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int Isaku Yamahata
5 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
remove some #ifdef DEBUG_UNUSED_IOPORT in ioport.c
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
ioport.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/ioport.c b/ioport.c
index 01cfaf7..87c01b4 100644
--- a/ioport.c
+++ b/ioport.c
@@ -33,6 +33,12 @@
//#define DEBUG_UNUSED_IOPORT
//#define DEBUG_IOPORT
+#ifdef DEBUG_UNUSED_IOPORT
+# define LOG_UNUSED_IOPORT(...) fprintf(stderr, __VA_ARGS__)
+#else
+# define LOG_UNUSED_IOPORT(...) do{ } while (0)
+#endif
+
#ifdef DEBUG_IOPORT
# define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__)
#else
@@ -76,17 +82,14 @@ static void ioport_write(int index, uint32_t address, uint32_t data)
static uint32_t default_ioport_readb(void *opaque, uint32_t address)
{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused inb: port=0x%04x\n", address);
-#endif
+ LOG_UNUSED_IOPORT("unused inb: port=0x%04x\n", address);
return 0xff;
}
static void default_ioport_writeb(void *opaque, uint32_t address, uint32_t data)
{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused outb: port=0x%04x data=0x%02x\n", address, data);
-#endif
+ LOG_UNUSED_IOPORT("unused outb: port=0x%04x data=0x%02x\n",
+ address, data);
}
/* default is to make two byte accesses */
@@ -108,17 +111,14 @@ static void default_ioport_writew(void *opaque, uint32_t address, uint32_t data)
static uint32_t default_ioport_readl(void *opaque, uint32_t address)
{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused inl: port=0x%04x\n", address);
-#endif
+ LOG_UNUSED_IOPORT("unused inl: port=0x%04x\n", address);
return 0xffffffff;
}
static void default_ioport_writel(void *opaque, uint32_t address, uint32_t data)
{
-#ifdef DEBUG_UNUSED_IOPORT
- fprintf(stderr, "unused outl: port=0x%04x data=0x%02x\n", address, data);
-#endif
+ LOG_UNUSED_IOPORT("unused outl: port=0x%04x data=0x%02x\n",
+ address, data);
}
static int ioport_bsize(int size, int *bsize)
--
1.6.0.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 5/6] consolidate user cpu_{in, out}[bwl] into ioport-user.c
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
` (3 preceding siblings ...)
2009-07-02 10:32 ` [Qemu-devel] [PATCH 4/6] ioport: remove some #ifdef DEBUG_UNUSED_IOPORT Isaku Yamahata
@ 2009-07-02 10:32 ` Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int Isaku Yamahata
5 siblings, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
consolidate user cpu_{in, out}[bwl] into ioport-user.c
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Makefile.target | 7 +++--
bsd-user/main.c | 33 -----------------------------
darwin-user/main.c | 33 -----------------------------
ioport-user.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
linux-user/main.c | 33 -----------------------------
tests/Makefile | 4 +-
tests/qruncom.c | 33 -----------------------------
7 files changed, 65 insertions(+), 137 deletions(-)
create mode 100644 ioport-user.c
diff --git a/Makefile.target b/Makefile.target
index 7f7c167..1a71f3a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -321,7 +321,8 @@ CFLAGS+=-p
endif
obj-y = main.o syscall.o strace.o mmap.o signal.o path.o thunk.o \
- elfload.o linuxload.o uaccess.o envlist.o gdbstub.o gdbstub-xml.o
+ elfload.o linuxload.o uaccess.o envlist.o gdbstub.o gdbstub-xml.o \
+ ioport-user.o
LIBS+= $(PTHREADLIBS)
LIBS+= $(CLOCKLIBS)
obj-$(TARGET_HAS_BFLT) += flatload.o
@@ -372,7 +373,7 @@ LDFLAGS+=-Wl,-segaddr,__STD_PROG_ZONE,0x1000 -image_base 0x0e000000
LIBS+=-lmx
obj-y = main.o commpage.o machload.o mmap.o signal.o syscall.o thunk.o \
- gdbstub.o gdbstub-xml.o
+ gdbstub.o gdbstub-xml.o ioport-user.o
# Note: this is a workaround. The real fix is to avoid compiling
# cpu_signal_handler() in cpu-exec.c.
@@ -471,7 +472,7 @@ endif
endif
obj-y = main.o bsdload.o elfload.o mmap.o path.o signal.o strace.o syscall.o \
- gdbstub.o gdbstub-xml.o
+ gdbstub.o gdbstub-xml.o ioport-user.o
obj-y += uaccess.o
# Note: this is a workaround. The real fix is to avoid compiling
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 827c9c3..e4a6255 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -55,39 +55,6 @@ void gemu_log(const char *fmt, ...)
va_end(ap);
}
-void cpu_outb(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outb: port=0x%04x, data=%02x\n", addr, val);
-}
-
-void cpu_outw(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outw: port=0x%04x, data=%04x\n", addr, val);
-}
-
-void cpu_outl(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outl: port=0x%04x, data=%08x\n", addr, val);
-}
-
-int cpu_inb(CPUState *env, int addr)
-{
- fprintf(stderr, "inb: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inw(CPUState *env, int addr)
-{
- fprintf(stderr, "inw: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inl(CPUState *env, int addr)
-{
- fprintf(stderr, "inl: port=0x%04x\n", addr);
- return 0;
-}
-
#if defined(TARGET_I386)
int cpu_get_pic_interrupt(CPUState *env)
{
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 5e3c48d..27c7284 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -72,39 +72,6 @@ void gemu_log(const char *fmt, ...)
va_end(ap);
}
-void cpu_outb(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outb: port=0x%04x, data=%02x\n", addr, val);
-}
-
-void cpu_outw(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outw: port=0x%04x, data=%04x\n", addr, val);
-}
-
-void cpu_outl(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outl: port=0x%04x, data=%08x\n", addr, val);
-}
-
-int cpu_inb(CPUState *env, int addr)
-{
- fprintf(stderr, "inb: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inw(CPUState *env, int addr)
-{
- fprintf(stderr, "inw: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inl(CPUState *env, int addr)
-{
- fprintf(stderr, "inl: port=0x%04x\n", addr);
- return 0;
-}
-
int cpu_get_pic_interrupt(CPUState *env)
{
return -1;
diff --git a/ioport-user.c b/ioport-user.c
new file mode 100644
index 0000000..fe8567f
--- /dev/null
+++ b/ioport-user.c
@@ -0,0 +1,59 @@
+/*
+ * qemu user ioport functions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include <stdio.h>
+
+#include "qemu.h"
+#include "qemu-common.h"
+#include "ioport.h"
+
+void cpu_outb(CPUState *env, int addr, int val)
+{
+ fprintf(stderr, "outb: port=0x%04x, data=%02x\n", addr, val);
+}
+
+void cpu_outw(CPUState *env, int addr, int val)
+{
+ fprintf(stderr, "outw: port=0x%04x, data=%04x\n", addr, val);
+}
+
+void cpu_outl(CPUState *env, int addr, int val)
+{
+ fprintf(stderr, "outl: port=0x%04x, data=%08x\n", addr, val);
+}
+
+int cpu_inb(CPUState *env, int addr)
+{
+ fprintf(stderr, "inb: port=0x%04x\n", addr);
+ return 0;
+}
+
+int cpu_inw(CPUState *env, int addr)
+{
+ fprintf(stderr, "inw: port=0x%04x\n", addr);
+ return 0;
+}
+
+int cpu_inl(CPUState *env, int addr)
+{
+ fprintf(stderr, "inl: port=0x%04x\n", addr);
+ return 0;
+}
diff --git a/linux-user/main.c b/linux-user/main.c
index 7eabd0c..ef448a4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -85,39 +85,6 @@ void gemu_log(const char *fmt, ...)
va_end(ap);
}
-void cpu_outb(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outb: port=0x%04x, data=%02x\n", addr, val);
-}
-
-void cpu_outw(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outw: port=0x%04x, data=%04x\n", addr, val);
-}
-
-void cpu_outl(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outl: port=0x%04x, data=%08x\n", addr, val);
-}
-
-int cpu_inb(CPUState *env, int addr)
-{
- fprintf(stderr, "inb: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inw(CPUState *env, int addr)
-{
- fprintf(stderr, "inw: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inl(CPUState *env, int addr)
-{
- fprintf(stderr, "inl: port=0x%04x\n", addr);
- return 0;
-}
-
#if defined(TARGET_I386)
int cpu_get_pic_interrupt(CPUState *env)
{
diff --git a/tests/Makefile b/tests/Makefile
index 326b733..69092e5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -78,9 +78,9 @@ runcom: runcom.c
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $<
# NOTE: -fomit-frame-pointer is currently needed : this is a bug in libqemu
-qruncom: qruncom.c ../i386-user/libqemu.a
+qruncom: qruncom.c ../ioport-user.c ../i386-user/libqemu.a
$(CC) $(CFLAGS) -fomit-frame-pointer $(LDFLAGS) -I../target-i386 -I.. -I../i386-user -I../fpu \
- -o $@ $< -L../i386-user -lqemu -lm
+ -o $@ $(filter %.c, $^) -L../i386-user -lqemu -lm
# arm test
hello-arm: hello-arm.o
diff --git a/tests/qruncom.c b/tests/qruncom.c
index 5e503bc..a8d0ef6 100644
--- a/tests/qruncom.c
+++ b/tests/qruncom.c
@@ -16,39 +16,6 @@
//#define SIGTEST
-void cpu_outb(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outb: port=0x%04x, data=%02x\n", addr, val);
-}
-
-void cpu_outw(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outw: port=0x%04x, data=%04x\n", addr, val);
-}
-
-void cpu_outl(CPUState *env, int addr, int val)
-{
- fprintf(stderr, "outl: port=0x%04x, data=%08x\n", addr, val);
-}
-
-int cpu_inb(CPUState *env, int addr)
-{
- fprintf(stderr, "inb: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inw(CPUState *env, int addr)
-{
- fprintf(stderr, "inw: port=0x%04x\n", addr);
- return 0;
-}
-
-int cpu_inl(CPUState *env, int addr)
-{
- fprintf(stderr, "inl: port=0x%04x\n", addr);
- return 0;
-}
-
int cpu_get_pic_interrupt(CPUState *env)
{
return -1;
--
1.6.0.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
` (4 preceding siblings ...)
2009-07-02 10:32 ` [Qemu-devel] [PATCH 5/6] consolidate user cpu_{in, out}[bwl] into ioport-user.c Isaku Yamahata
@ 2009-07-02 10:32 ` Isaku Yamahata
2009-07-02 18:11 ` Stuart Brady
2009-07-09 19:07 ` Anthony Liguori
5 siblings, 2 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-02 10:32 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata
use uint32_t for ioport port and value instead of int.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
ioport-user.c | 24 ++++++++++++------------
ioport.c | 47 ++++++++++++++++++++++++-----------------------
ioport.h | 18 +++++++++---------
3 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/ioport-user.c b/ioport-user.c
index fe8567f..017423b 100644
--- a/ioport-user.c
+++ b/ioport-user.c
@@ -25,35 +25,35 @@
#include "qemu-common.h"
#include "ioport.h"
-void cpu_outb(CPUState *env, int addr, int val)
+void cpu_outb(CPUState *env, uint32_t addr, uint32_t val)
{
- fprintf(stderr, "outb: port=0x%04x, data=%02x\n", addr, val);
+ fprintf(stderr, "outb: port=0x%04"PRIx32", data=%02"PRIx32"\n", addr, val);
}
-void cpu_outw(CPUState *env, int addr, int val)
+void cpu_outw(CPUState *env, uint32_t addr, uint32_t val)
{
- fprintf(stderr, "outw: port=0x%04x, data=%04x\n", addr, val);
+ fprintf(stderr, "outw: port=0x%04"PRIx32", data=%04"PRIx32"\n", addr, val);
}
-void cpu_outl(CPUState *env, int addr, int val)
+void cpu_outl(CPUState *env, uint32_t addr, uint32_t val)
{
- fprintf(stderr, "outl: port=0x%04x, data=%08x\n", addr, val);
+ fprintf(stderr, "outl: port=0x%04"PRIx32", data=%08"PRIx32"\n", addr, val);
}
-int cpu_inb(CPUState *env, int addr)
+uint32_t cpu_inb(CPUState *env, uint32_t addr)
{
- fprintf(stderr, "inb: port=0x%04x\n", addr);
+ fprintf(stderr, "inb: port=0x%04"PRIx32"\n", addr);
return 0;
}
-int cpu_inw(CPUState *env, int addr)
+uint32_t cpu_inw(CPUState *env, uint32_t addr)
{
- fprintf(stderr, "inw: port=0x%04x\n", addr);
+ fprintf(stderr, "inw: port=0x%04"PRIx32"\n", addr);
return 0;
}
-int cpu_inl(CPUState *env, int addr)
+uint32_t cpu_inl(CPUState *env, uint32_t addr)
{
- fprintf(stderr, "inl: port=0x%04x\n", addr);
+ fprintf(stderr, "inl: port=0x%04"PRIx32"\n", addr);
return 0;
}
diff --git a/ioport.c b/ioport.c
index 87c01b4..7e88279 100644
--- a/ioport.c
+++ b/ioport.c
@@ -82,13 +82,14 @@ static void ioport_write(int index, uint32_t address, uint32_t data)
static uint32_t default_ioport_readb(void *opaque, uint32_t address)
{
- LOG_UNUSED_IOPORT("unused inb: port=0x%04x\n", address);
+ LOG_UNUSED_IOPORT(stderr, "unused inb: port=0x%04"PRIx32"\n", address);
return 0xff;
}
static void default_ioport_writeb(void *opaque, uint32_t address, uint32_t data)
{
- LOG_UNUSED_IOPORT("unused outb: port=0x%04x data=0x%02x\n",
+ LOG_UNUSED_IOPORT(stderr,
+ "unused outb: port=0x%04"PRIx32" data=0x%02"PRIx32"\n",
address, data);
}
@@ -111,13 +112,14 @@ static void default_ioport_writew(void *opaque, uint32_t address, uint32_t data)
static uint32_t default_ioport_readl(void *opaque, uint32_t address)
{
- LOG_UNUSED_IOPORT("unused inl: port=0x%04x\n", address);
+ LOG_UNUSED_IOPORT(stderr, "unused inl: port=0x%04"PRIx32"\n", address);
return 0xffffffff;
}
static void default_ioport_writel(void *opaque, uint32_t address, uint32_t data)
{
- LOG_UNUSED_IOPORT("unused outl: port=0x%04x data=0x%02x\n",
+ LOG_UNUSED_IOPORT(stderr,
+ "unused outl: port=0x%04"PRIx32" data=0x%02"PRIx32"\n",
address, data);
}
@@ -136,7 +138,7 @@ static int ioport_bsize(int size, int *bsize)
}
/* size is the word size in byte */
-int register_ioport_read(int start, int length, int size,
+int register_ioport_read(uint32_t start, int length, int size,
IOPortReadFunc *func, void *opaque)
{
int i, bsize;
@@ -155,7 +157,7 @@ int register_ioport_read(int start, int length, int size,
}
/* size is the word size in byte */
-int register_ioport_write(int start, int length, int size,
+int register_ioport_write(uint32_t start, int length, int size,
IOPortWriteFunc *func, void *opaque)
{
int i, bsize;
@@ -173,7 +175,7 @@ int register_ioport_write(int start, int length, int size,
return 0;
}
-void isa_unassign_ioport(int start, int length)
+void isa_unassign_ioport(uint32_t start, int length)
{
int i;
@@ -192,9 +194,9 @@ void isa_unassign_ioport(int start, int length)
/***********************************************************/
-void cpu_outb(CPUState *env, int addr, int val)
+void cpu_outb(CPUState *env, uint32_t addr, uint32_t val)
{
- LOG_IOPORT("outb: %04x %02x\n", addr, val);
+ LOG_IOPORT("outb: %04"PRIx32" %02"PRIx32"\n", addr, val);
ioport_write(0, addr, val);
#ifdef CONFIG_KQEMU
if (env)
@@ -202,9 +204,9 @@ void cpu_outb(CPUState *env, int addr, int val)
#endif
}
-void cpu_outw(CPUState *env, int addr, int val)
+void cpu_outw(CPUState *env, uint32_t addr, uint32_t val)
{
- LOG_IOPORT("outw: %04x %04x\n", addr, val);
+ LOG_IOPORT("outw: %04"PRIx32" %04"PRIx32"\n", addr, val);
ioport_write(1, addr, val);
#ifdef CONFIG_KQEMU
if (env)
@@ -212,9 +214,9 @@ void cpu_outw(CPUState *env, int addr, int val)
#endif
}
-void cpu_outl(CPUState *env, int addr, int val)
+void cpu_outl(CPUState *env, uint32_t addr, uint32_t val)
{
- LOG_IOPORT("outl: %04x %08x\n", addr, val);
+ LOG_IOPORT("outl: %04"PRIx32" %08"PRIx32"\n", addr, val);
ioport_write(2, addr, val);
#ifdef CONFIG_KQEMU
if (env)
@@ -222,11 +224,11 @@ void cpu_outl(CPUState *env, int addr, int val)
#endif
}
-int cpu_inb(CPUState *env, int addr)
+uint32_t cpu_inb(CPUState *env, uint32_t addr)
{
- int val;
+ uint32_t val;
val = ioport_read(0, addr);
- LOG_IOPORT("inb : %04x %02x\n", addr, val);
+ LOG_IOPORT("inb : %04"PRIx32" %02"PRIx32"\n", addr, val);
#ifdef CONFIG_KQEMU
if (env)
env->last_io_time = cpu_get_time_fast();
@@ -234,11 +236,11 @@ int cpu_inb(CPUState *env, int addr)
return val;
}
-int cpu_inw(CPUState *env, int addr)
+uint32_t cpu_inw(CPUState *env, uint32_t addr)
{
- int val;
+ uint32_t val;
val = ioport_read(1, addr);
- LOG_IOPORT("inw : %04x %04x\n", addr, val);
+ LOG_IOPORT("inw : %04"PRIx32" %04"PRIx32"\n", addr, val);
#ifdef CONFIG_KQEMU
if (env)
env->last_io_time = cpu_get_time_fast();
@@ -246,15 +248,14 @@ int cpu_inw(CPUState *env, int addr)
return val;
}
-int cpu_inl(CPUState *env, int addr)
+uint32_t cpu_inl(CPUState *env, uint32_t addr)
{
- int val;
+ uint32_t val;
val = ioport_read(2, addr);
- LOG_IOPORT("inl : %04x %08x\n", addr, val);
+ LOG_IOPORT("inl : %04"PRIx32" %08"PRIx32"\n", addr, val);
#ifdef CONFIG_KQEMU
if (env)
env->last_io_time = cpu_get_time_fast();
#endif
return val;
}
-
diff --git a/ioport.h b/ioport.h
index 4cb59e9..b3bf741 100644
--- a/ioport.h
+++ b/ioport.h
@@ -34,22 +34,22 @@
typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
-int register_ioport_read(int start, int length, int size,
+int register_ioport_read(uint32_t start, int length, int size,
IOPortReadFunc *func, void *opaque);
-int register_ioport_write(int start, int length, int size,
+int register_ioport_write(uint32_t start, int length, int size,
IOPortWriteFunc *func, void *opaque);
-void isa_unassign_ioport(int start, int length);
+void isa_unassign_ioport(uint32_t start, int length);
/* NOTE: as these functions may be even used when there is an isa
brige on non x86 targets, we always defined them */
#if !defined(NO_CPU_IO_DEFS) && defined(NEED_CPU_H)
-void cpu_outb(CPUState *env, int addr, int val);
-void cpu_outw(CPUState *env, int addr, int val);
-void cpu_outl(CPUState *env, int addr, int val);
-int cpu_inb(CPUState *env, int addr);
-int cpu_inw(CPUState *env, int addr);
-int cpu_inl(CPUState *env, int addr);
+void cpu_outb(CPUState *env, uint32_t addr, uint32_t val);
+void cpu_outw(CPUState *env, uint32_t addr, uint32_t val);
+void cpu_outl(CPUState *env, uint32_t addr, uint32_t val);
+uint32_t cpu_inb(CPUState *env, uint32_t addr);
+uint32_t cpu_inw(CPUState *env, uint32_t addr);
+uint32_t cpu_inl(CPUState *env, uint32_t addr);
#endif
#endif /* IOPORT_H */
--
1.6.0.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-02 10:32 ` [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int Isaku Yamahata
@ 2009-07-02 18:11 ` Stuart Brady
2009-07-02 18:30 ` Stuart Brady
2009-07-09 19:07 ` Anthony Liguori
1 sibling, 1 reply; 25+ messages in thread
From: Stuart Brady @ 2009-07-02 18:11 UTC (permalink / raw)
To: qemu-devel
On Thu, Jul 02, 2009 at 07:32:11PM +0900, Isaku Yamahata wrote:
> -void cpu_outb(CPUState *env, int addr, int val)
> +void cpu_outb(CPUState *env, uint32_t addr, uint32_t val)
uint8_t for 'b' and uint16_t for 'w' might be slightly better, in that
it would allow compiler warnings regarding truncations to be generated.
> - LOG_UNUSED_IOPORT("unused inb: port=0x%04x\n", address);
> + LOG_UNUSED_IOPORT(stderr, "unused inb: port=0x%04"PRIx32"\n", address);
These don't look intentional...
--
Stuart Brady
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-02 18:11 ` Stuart Brady
@ 2009-07-02 18:30 ` Stuart Brady
0 siblings, 0 replies; 25+ messages in thread
From: Stuart Brady @ 2009-07-02 18:30 UTC (permalink / raw)
To: qemu-devel
On Thu, Jul 02, 2009 at 07:11:13PM +0100, Stuart Brady wrote:
> On Thu, Jul 02, 2009 at 07:32:11PM +0900, Isaku Yamahata wrote:
> > - LOG_UNUSED_IOPORT("unused inb: port=0x%04x\n", address);
> > + LOG_UNUSED_IOPORT(stderr, "unused inb: port=0x%04"PRIx32"\n", address);
>
> These don't look intentional...
To clarify, I was referring to the 'stderr' part, and missed the 'PRIx32'.
Cheers,
--
Stuart Brady
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-02 10:32 ` [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int Isaku Yamahata
2009-07-02 18:11 ` Stuart Brady
@ 2009-07-09 19:07 ` Anthony Liguori
2009-07-10 8:21 ` Samuel Thibault
1 sibling, 1 reply; 25+ messages in thread
From: Anthony Liguori @ 2009-07-09 19:07 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
Isaku Yamahata wrote:
> use uint32_t for ioport port and value instead of int.
>
Why?
It's not obvious to me.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-09 19:07 ` Anthony Liguori
@ 2009-07-10 8:21 ` Samuel Thibault
2009-07-10 8:45 ` Isaku Yamahata
2009-07-10 12:53 ` Anthony Liguori
0 siblings, 2 replies; 25+ messages in thread
From: Samuel Thibault @ 2009-07-10 8:21 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Isaku Yamahata, qemu-devel
Anthony Liguori, le Thu 09 Jul 2009 14:07:47 -0500, a écrit :
> Isaku Yamahata wrote:
> >use uint32_t for ioport port and value instead of int.
> >
>
> Why?
In theory int could be only signed 16bit, so at least unsigned int could
be needed for the port. uint16_t should be fine. For value, as said
uint8/16/32 should probably be better.
Samuel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 8:21 ` Samuel Thibault
@ 2009-07-10 8:45 ` Isaku Yamahata
2009-07-10 8:54 ` Samuel Thibault
2009-07-10 12:55 ` Anthony Liguori
2009-07-10 12:53 ` Anthony Liguori
1 sibling, 2 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-10 8:45 UTC (permalink / raw)
To: Samuel Thibault; +Cc: qemu-devel
On Fri, Jul 10, 2009 at 10:21:21AM +0200, Samuel Thibault wrote:
> Anthony Liguori, le Thu 09 Jul 2009 14:07:47 -0500, a écrit :
> > Isaku Yamahata wrote:
> > >use uint32_t for ioport port and value instead of int.
> > >
> >
> > Why?
>
> In theory int could be only signed 16bit, so at least unsigned int could
> be needed for the port. uint16_t should be fine. For value, as said
> uint8/16/32 should probably be better.
Then, the signatures should like the followings?
void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
--
yamahata
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 8:45 ` Isaku Yamahata
@ 2009-07-10 8:54 ` Samuel Thibault
2009-07-10 9:09 ` Tristan Gingold
2009-07-10 12:55 ` Anthony Liguori
1 sibling, 1 reply; 25+ messages in thread
From: Samuel Thibault @ 2009-07-10 8:54 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
Isaku Yamahata, le Fri 10 Jul 2009 17:45:08 +0900, a écrit :
> void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
uint16_t addr, and uint* for val
> uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
uint16_t addr
Samuel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 8:54 ` Samuel Thibault
@ 2009-07-10 9:09 ` Tristan Gingold
2009-07-10 12:59 ` Anthony Liguori
0 siblings, 1 reply; 25+ messages in thread
From: Tristan Gingold @ 2009-07-10 9:09 UTC (permalink / raw)
To: Samuel Thibault; +Cc: Isaku Yamahata, qemu-devel
On Jul 10, 2009, at 10:54 AM, Samuel Thibault wrote:
> Isaku Yamahata, le Fri 10 Jul 2009 17:45:08 +0900, a écrit :
>> void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
>
> uint16_t addr, and uint* for val
>
>> uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
>
> uint16_t addr
Some machines allow more than 2**16 ports (guess which :-)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 8:21 ` Samuel Thibault
2009-07-10 8:45 ` Isaku Yamahata
@ 2009-07-10 12:53 ` Anthony Liguori
1 sibling, 0 replies; 25+ messages in thread
From: Anthony Liguori @ 2009-07-10 12:53 UTC (permalink / raw)
To: Samuel Thibault; +Cc: Isaku Yamahata, qemu-devel
Samuel Thibault wrote:
> Anthony Liguori, le Thu 09 Jul 2009 14:07:47 -0500, a écrit :
>
>> Isaku Yamahata wrote:
>>
>>> use uint32_t for ioport port and value instead of int.
>>>
>>>
>> Why?
>>
>
> In theory int could be only signed 16bit, so at least unsigned int could
> be needed for the port.
There's no platform that QEMU runs on that has a 16-bit integer and it's
extraordinarily unlikely that there ever will be.
But the point is, patches need descriptive changelogs saying why there
making a change, not just what the change is.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 8:45 ` Isaku Yamahata
2009-07-10 8:54 ` Samuel Thibault
@ 2009-07-10 12:55 ` Anthony Liguori
2009-07-13 3:14 ` Isaku Yamahata
1 sibling, 1 reply; 25+ messages in thread
From: Anthony Liguori @ 2009-07-10 12:55 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Samuel Thibault, qemu-devel
Isaku Yamahata wrote:
> Then, the signatures should like the followings?
>
> void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
> uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
>
If anything, it ought to be:
void cpu_out[bwl](CPUState *env, uint16_t addr, int{8, 16, 32}_t val);
But it's int today because the assumption is that most architectures can
more efficiently pass int than other types (because it's the native
type) and that int is adequate to contain all of the necessary types.
But my original question still remains, what's the motivation for this
change?
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 9:09 ` Tristan Gingold
@ 2009-07-10 12:59 ` Anthony Liguori
2009-07-10 13:12 ` Tristan Gingold
0 siblings, 1 reply; 25+ messages in thread
From: Anthony Liguori @ 2009-07-10 12:59 UTC (permalink / raw)
To: Tristan Gingold; +Cc: Isaku Yamahata, Samuel Thibault, qemu-devel
Tristan Gingold wrote:
>
> On Jul 10, 2009, at 10:54 AM, Samuel Thibault wrote:
>
>> Isaku Yamahata, le Fri 10 Jul 2009 17:45:08 +0900, a écrit :
>>> void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
>>
>> uint16_t addr, and uint* for val
>>
>>> uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
>>
>> uint16_t addr
>
> Some machines allow more than 2**16 ports (guess which :-)
But I thought that PIO on ia64 ended up being MMIO or something funky
like that? Would it really end up using this code path?
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 12:59 ` Anthony Liguori
@ 2009-07-10 13:12 ` Tristan Gingold
2009-07-10 13:16 ` Anthony Liguori
0 siblings, 1 reply; 25+ messages in thread
From: Tristan Gingold @ 2009-07-10 13:12 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Isaku Yamahata, Samuel Thibault, qemu-devel
On Jul 10, 2009, at 2:59 PM, Anthony Liguori wrote:
> Tristan Gingold wrote:
>>
>> On Jul 10, 2009, at 10:54 AM, Samuel Thibault wrote:
>>
>>> Isaku Yamahata, le Fri 10 Jul 2009 17:45:08 +0900, a écrit :
>>>> void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t
>>>> val);
>>>
>>> uint16_t addr, and uint* for val
>>>
>>>> uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
>>>
>>> uint16_t addr
>>
>> Some machines allow more than 2**16 ports (guess which :-)
>
> But I thought that PIO on ia64 ended up being MMIO or something
> funky like that? Would it really end up using this code path?
Yes.
Port IO is done through usual memory access, but either the processor
or the north-bridge can recognize this
address and convert the memory access to a port access. After all PCI
cards knows about IO vs Memory.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 13:12 ` Tristan Gingold
@ 2009-07-10 13:16 ` Anthony Liguori
2009-07-10 13:28 ` Tristan Gingold
0 siblings, 1 reply; 25+ messages in thread
From: Anthony Liguori @ 2009-07-10 13:16 UTC (permalink / raw)
To: Tristan Gingold; +Cc: Isaku Yamahata, Samuel Thibault, qemu-devel
Tristan Gingold wrote:
> Yes.
>
> Port IO is done through usual memory access, but either the processor
> or the north-bridge can recognize this
> address and convert the memory access to a port access. After all PCI
> cards knows about IO vs Memory.
So would we actually model this in QEMU through cpu_in/out?
Can PCI IO regions that are marked as PIO actually be > 16-bit in size?
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 13:16 ` Anthony Liguori
@ 2009-07-10 13:28 ` Tristan Gingold
0 siblings, 0 replies; 25+ messages in thread
From: Tristan Gingold @ 2009-07-10 13:28 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Isaku Yamahata, Samuel Thibault, qemu-devel
On Jul 10, 2009, at 3:16 PM, Anthony Liguori wrote:
> Tristan Gingold wrote:
>> Yes.
>>
>> Port IO is done through usual memory access, but either the
>> processor or the north-bridge can recognize this
>> address and convert the memory access to a port access. After all
>> PCI cards knows about IO vs Memory.
>
> So would we actually model this in QEMU through cpu_in/out?
Yes, as the cpu can generate port IO cycles for them.
> Can PCI IO regions that are marked as PIO actually be > 16-bit in
> size?
yes: pci 2.2 6.2.5.1 allows this explicitely.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-10 12:55 ` Anthony Liguori
@ 2009-07-13 3:14 ` Isaku Yamahata
2009-07-13 18:59 ` Anthony Liguori
0 siblings, 1 reply; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-13 3:14 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Samuel Thibault, qemu-devel
On Fri, Jul 10, 2009 at 07:55:44AM -0500, Anthony Liguori wrote:
> Isaku Yamahata wrote:
> > Then, the signatures should like the followings?
> >
> > void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
> > uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
> >
>
> If anything, it ought to be:
>
> void cpu_out[bwl](CPUState *env, uint16_t addr, int{8, 16, 32}_t val);
>
> But it's int today because the assumption is that most architectures can
> more efficiently pass int than other types (because it's the native
> type) and that int is adequate to contain all of the necessary types.
>
> But my original question still remains, what's the motivation for this
> change?
Yes, the patch description too terse.
The motivation is to remove inconsistency with the other part of
qemu.
- Using int for cpu_{in, out}[bwl] is inconsistent with other part.
For address or value, uintN_t is used by other qemu part.
At least I can confirm, softmmu, CPU{Read, Write}MemoryFunc, pci,
target_phys_addr_t and the callers of cpu_{in, out}[bwl]().
- It is a bad idea to mix signed with unsigned unnecessary.
So at least unsigned int should be used instead of int.
- (u)int_fastN_t is more appropriate than (u)int.
It is claimed that int is used for optimization assuming that
sizeof(int or uint) >= sizeof(uint32_t).
(u)int_fast32_t is more appropriate than (u)int with such a assumption.
The other part of qemu like softmmu which is much more performance
critical uses uint32_t.
So what do you think?
I'll add more patch description and update/send the patch
with whatever types we agree on. (or leave it as int)
Thanks,
--
yamahata
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-13 3:14 ` Isaku Yamahata
@ 2009-07-13 18:59 ` Anthony Liguori
2009-07-13 21:19 ` Stuart Brady
2009-07-14 1:58 ` Isaku Yamahata
0 siblings, 2 replies; 25+ messages in thread
From: Anthony Liguori @ 2009-07-13 18:59 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Tristan Gingold, Samuel Thibault, qemu-devel
Isaku Yamahata wrote:
> On Fri, Jul 10, 2009 at 07:55:44AM -0500, Anthony Liguori wrote:
>
>> Isaku Yamahata wrote:
>>
>>> Then, the signatures should like the followings?
>>>
>>> void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
>>> uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
>>>
>>>
>> If anything, it ought to be:
>>
>> void cpu_out[bwl](CPUState *env, uint16_t addr, int{8, 16, 32}_t val);
>>
>> But it's int today because the assumption is that most architectures can
>> more efficiently pass int than other types (because it's the native
>> type) and that int is adequate to contain all of the necessary types.
>>
>> But my original question still remains, what's the motivation for this
>> change?
>>
>
> Yes, the patch description too terse.
> The motivation is to remove inconsistency with the other part of
> qemu.
>
> - Using int for cpu_{in, out}[bwl] is inconsistent with other part.
> For address or value, uintN_t is used by other qemu part.
> At least I can confirm, softmmu, CPU{Read, Write}MemoryFunc, pci,
> target_phys_addr_t and the callers of cpu_{in, out}[bwl]().
>
What does the PCI spec say about the size of PIO IO regions? Can they
be as large as MEM IO regions?
If so, I would think that we should use ram_addr_t for addr and then we
can use the appropriate uintN_t type for value. Switching value like
that though could have some subtle consequences. For instance,
cpu_outb(env, ..., 128) would have worked properly before as would
cpu_outb(env, ..., -32).
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-13 18:59 ` Anthony Liguori
@ 2009-07-13 21:19 ` Stuart Brady
2009-07-13 21:28 ` Anthony Liguori
2009-07-14 1:58 ` Isaku Yamahata
1 sibling, 1 reply; 25+ messages in thread
From: Stuart Brady @ 2009-07-13 21:19 UTC (permalink / raw)
To: qemu-devel
On Mon, Jul 13, 2009 at 01:59:31PM -0500, Anthony Liguori wrote:
> If so, I would think that we should use ram_addr_t for addr and then we
> can use the appropriate uintN_t type for value. Switching value like
> that though could have some subtle consequences. For instance,
> cpu_outb(env, ..., 128) would have worked properly before as would
> cpu_outb(env, ..., -32).
Isn't ram_addr_t solely for return values from (and internals of)
qemu_ram_alloc() and friends?
Perhaps port IO addresses should have their own type?
TBH, I'm not so sure that port IO is particularly 'special' in any way.
I suppose that generally speaking, code can't be executed from port IO
addresses (although that's not true on machines where 'port IO' is
actually memory mapped). I'm not sure how PCI fits into this, though.
Cheers,
--
Stuart Brady
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-13 21:19 ` Stuart Brady
@ 2009-07-13 21:28 ` Anthony Liguori
0 siblings, 0 replies; 25+ messages in thread
From: Anthony Liguori @ 2009-07-13 21:28 UTC (permalink / raw)
To: Stuart Brady; +Cc: qemu-devel
Stuart Brady wrote:
> On Mon, Jul 13, 2009 at 01:59:31PM -0500, Anthony Liguori wrote:
>
>> If so, I would think that we should use ram_addr_t for addr and then we
>> can use the appropriate uintN_t type for value. Switching value like
>> that though could have some subtle consequences. For instance,
>> cpu_outb(env, ..., 128) would have worked properly before as would
>> cpu_outb(env, ..., -32).
>>
>
> Isn't ram_addr_t solely for return values from (and internals of)
> qemu_ram_alloc() and friends?
>
Well it's sort of our internal address type.
> Perhaps port IO addresses should have their own type?
>
Certainly not a bad idea but I'd still recommend typedef ram_addr_t
pio_addr_t.
> TBH, I'm not so sure that port IO is particularly 'special' in any way.
>
Right, that's why I suggested ram_addr_t. ram_addr_t is just an
internal address and what we map it to is up to us.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int.
2009-07-13 18:59 ` Anthony Liguori
2009-07-13 21:19 ` Stuart Brady
@ 2009-07-14 1:58 ` Isaku Yamahata
1 sibling, 0 replies; 25+ messages in thread
From: Isaku Yamahata @ 2009-07-14 1:58 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Tristan Gingold, Samuel Thibault, qemu-devel
On Mon, Jul 13, 2009 at 01:59:31PM -0500, Anthony Liguori wrote:
> Isaku Yamahata wrote:
> >On Fri, Jul 10, 2009 at 07:55:44AM -0500, Anthony Liguori wrote:
> >
> >>Isaku Yamahata wrote:
> >>
> >>>Then, the signatures should like the followings?
> >>>
> >>>void cpu_out[bwl](CPUState *env, int64_t addr, int{8, 16, 32}_t val);
> >>>uint{8, 16, 32}_t cpu_inw(CPUState *env, int16_t addr);
> >>>
> >>>
> >>If anything, it ought to be:
> >>
> >>void cpu_out[bwl](CPUState *env, uint16_t addr, int{8, 16, 32}_t val);
> >>
> >>But it's int today because the assumption is that most architectures can
> >>more efficiently pass int than other types (because it's the native
> >>type) and that int is adequate to contain all of the necessary types.
> >>
> >>But my original question still remains, what's the motivation for this
> >>change?
> >>
> >
> >Yes, the patch description too terse.
> >The motivation is to remove inconsistency with the other part of
> >qemu.
> >
> >- Using int for cpu_{in, out}[bwl] is inconsistent with other part.
> > For address or value, uintN_t is used by other qemu part.
> > At least I can confirm, softmmu, CPU{Read, Write}MemoryFunc, pci,
> > target_phys_addr_t and the callers of cpu_{in, out}[bwl]().
> >
>
> What does the PCI spec say about the size of PIO IO regions? Can they
> be as large as MEM IO regions?
> If so, I would think that we should use ram_addr_t for addr and then we
> can use the appropriate uintN_t type for value.
PIO IO regions address is 32 bit width. Unlike MEM IO, 64 bit isn't allowed.
So uint32_t for address?
> Switching value like
> that though could have some subtle consequences. For instance,
> cpu_outb(env, ..., 128) would have worked properly before as would
> cpu_outb(env, ..., -32).
All the callers except kvm_handle_io() passes uint32_t as value.
So it wouldn't matter.
--
yamahata
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2009-07-14 1:58 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-02 10:32 [Qemu-devel] [PATCH 0/6] ioport related clean ups Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 1/6] split out ioport related stuffs from vl.c into ioport.c Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 2/6] use constant IOPORTS_MASK instead of 0xffff Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 3/6] ioport: consolidate duplicated logic in register_ioport_{read, write}() Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 4/6] ioport: remove some #ifdef DEBUG_UNUSED_IOPORT Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 5/6] consolidate user cpu_{in, out}[bwl] into ioport-user.c Isaku Yamahata
2009-07-02 10:32 ` [Qemu-devel] [PATCH 6/6] use uint32_t for ioport port and value instead of int Isaku Yamahata
2009-07-02 18:11 ` Stuart Brady
2009-07-02 18:30 ` Stuart Brady
2009-07-09 19:07 ` Anthony Liguori
2009-07-10 8:21 ` Samuel Thibault
2009-07-10 8:45 ` Isaku Yamahata
2009-07-10 8:54 ` Samuel Thibault
2009-07-10 9:09 ` Tristan Gingold
2009-07-10 12:59 ` Anthony Liguori
2009-07-10 13:12 ` Tristan Gingold
2009-07-10 13:16 ` Anthony Liguori
2009-07-10 13:28 ` Tristan Gingold
2009-07-10 12:55 ` Anthony Liguori
2009-07-13 3:14 ` Isaku Yamahata
2009-07-13 18:59 ` Anthony Liguori
2009-07-13 21:19 ` Stuart Brady
2009-07-13 21:28 ` Anthony Liguori
2009-07-14 1:58 ` Isaku Yamahata
2009-07-10 12:53 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).