* [Qemu-devel] [PATCH v3] Two-Level IOPort Lookup
@ 2009-04-13 14:11 Brian Wheeler
2009-04-13 14:40 ` malc
0 siblings, 1 reply; 3+ messages in thread
From: Brian Wheeler @ 2009-04-13 14:11 UTC (permalink / raw)
To: qemu-devel
I've fixed a few things:
* my pointer arithmetic was gross looking, so I fixed it
* added an allocated page count for stats when debugging
On my platform (x86-64) the in-tree implementation allocates 3.5M for
the PC target. With this implementation the PC target consumes 2K of
pointer table + 256K of malloc'd memory.
For the Alpha target, statically allocating all 24-bits worth of ioport
tables is about 1G of ram. With this patch, it uses 32K for the pointer
table and 512K of malloc'd memory
Is there anything else I need to look at before this patch is worthy of
inclusion?
Signed-off-by: Brian Wheeler <bdwheele@indiana.edu>
Index: vl.c
===================================================================
--- vl.c (revision 7097)
+++ vl.c (working copy)
@@ -168,8 +168,8 @@
//#define DEBUG_IOPORT
//#define DEBUG_NET
//#define DEBUG_SLIRP
+//#define DEBUG_IOPORT_FIND
-
#ifdef DEBUG_IOPORT
# define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__)
#else
@@ -184,14 +184,31 @@
/* 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
-
const char *bios_dir = CONFIG_QEMU_SHAREDIR;
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];
+
+struct ioport {
+ void *opaque;
+ IOPortReadFunc *read[3];
+ IOPortWriteFunc *write[3];
+ void *pad;
+};
+typedef struct ioport ioport_t;
+
+#ifdef TARGET_ALPHA
+#define IOPORT_MAXBITS 24
+#define IOPORT_PAGEBITS 12
+#else
+#define IOPORT_MAXBITS 16
+#define IOPORT_PAGEBITS 8
+#endif
+
+#define IOPORT_ENTRYMASK ((1<<IOPORT_PAGEBITS)-1)
+#define IOPORT_PAGEMASK ~IOPORT_ENTRYMASK
+#define MAX_IOPORTS (1<<IOPORT_MAXBITS)
+
+ioport_t *ioport_table[1<<(IOPORT_MAXBITS-IOPORT_PAGEBITS)];
+
/* Note: drives_table[MAX_DRIVES] is a dummy block driver if none available
to store the VM snapshots */
DriveInfo drives_table[MAX_DRIVES+1];
@@ -288,30 +305,78 @@
static IOPortReadFunc default_ioport_readb, default_ioport_readw, default_ioport_readl;
static IOPortWriteFunc default_ioport_writeb, default_ioport_writew, default_ioport_writel;
+
+static ioport_t ioport_default_func = {
+ NULL,
+ {default_ioport_readb, default_ioport_readw, default_ioport_readl},
+ {default_ioport_writeb, default_ioport_writew, default_ioport_writel},
+ NULL
+};
+
+static inline ioport_t *ioport_find(uint32_t address, int allocate)
+{
+ uint32_t page = (address & IOPORT_PAGEMASK) >> IOPORT_PAGEBITS;
+ uint32_t entry = address & IOPORT_ENTRYMASK;
+#ifdef DEBUG_IOPORT_FIND
+ static int page_count = 0;
+ if (address >= (1<<IOPORT_MAXBITS)) {
+ hw_error("Maximum port # for this architecture is %d. "
+ "Port %d requested.", (1<<IOPORT_MAXBITS)-1, address);
+ }
+#endif
+ if (ioport_table[page] == NULL) {
+ if (allocate) {
+ ioport_table[page]=qemu_mallocz((1<<IOPORT_PAGEBITS) *
+ sizeof(ioport_t));
+#ifdef DEBUG_IOPORT_FIND
+ printf("Initializing ioport page %d to: %p (0x%lx bytes)\n",
+ page, ioport_table[page],
+ (1<<IOPORT_PAGEBITS)*sizeof(ioport_t));
+ page_count++;
+#endif
+ } else {
+ return &ioport_default_func;
+ }
+ }
+ ioport_t *p=&(ioport_table[page][entry]);
+
+#ifdef DEBUG_IOPORT_FIND
+ if (allocate) {
+ printf("port find %d: page=%d, address=%p, entry=%d, address=%p, "
+ "offset=0x%lx\n", address, page, ioport_table[page],
+ entry, p, p-ioport_table[page]);
+ printf(" data: %p\n", p->opaque);
+ printf(" read: %p, %p, %p\n", p->read[0], p->read[1], p->read[2]);
+ printf(" write: %p, %p, %p\n", p->write[0], p->write[1], p->write[2]);
+
+ printf(" --- %d pages allocated, %ld bytes used. --- \n",
+ page_count,
+ page_count * (sizeof(ioport_t) * (1<<IOPORT_PAGEBITS)));
+ }
+#endif
+ return p;
+}
+
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);
+ ioport_t *p = ioport_find(address, 0);
+ IOPortReadFunc *func = p->read[index];
+ void *opaque = p->opaque;
+ if (!func) {
+ func = ioport_default_func.read[index];
+ }
+ return func(opaque, 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);
+ ioport_t *p = ioport_find(address, 0);
+ IOPortWriteFunc *func = p->write[index];
+ void *opaque = p->opaque;
+ if (!func) {
+ func = ioport_default_func.write[index];
+ }
+ func(opaque, address, data);
}
static uint32_t default_ioport_readb(void *opaque, uint32_t address)
@@ -378,10 +443,11 @@
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)
+ ioport_t *p = ioport_find(i, 1);
+ p->read[bsize] = func;
+ if (p->opaque != NULL && p->opaque != opaque)
hw_error("register_ioport_read: invalid opaque");
- ioport_opaque[i] = opaque;
+ p->opaque = opaque;
}
return 0;
}
@@ -403,10 +469,11 @@
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)
+ ioport_t *p = ioport_find(i, 1);
+ p->write[bsize] = func;
+ if (p->opaque != NULL && p->opaque != opaque)
hw_error("register_ioport_write: invalid opaque");
- ioport_opaque[i] = opaque;
+ p->opaque = opaque;
}
return 0;
}
@@ -416,15 +483,16 @@
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_t *p = ioport_find(i, 1);
+ p->read[0] = default_ioport_readb;
+ p->read[1] = default_ioport_readw;
+ p->read[2] = 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;
+ p->write[0] = default_ioport_writeb;
+ p->write[1] = default_ioport_writew;
+ p->write[2] = default_ioport_writel;
- ioport_opaque[i] = NULL;
+ p->opaque = NULL;
}
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Two-Level IOPort Lookup
2009-04-13 14:11 [Qemu-devel] [PATCH v3] Two-Level IOPort Lookup Brian Wheeler
@ 2009-04-13 14:40 ` malc
2009-04-13 14:57 ` Brian Wheeler
0 siblings, 1 reply; 3+ messages in thread
From: malc @ 2009-04-13 14:40 UTC (permalink / raw)
To: qemu-devel
On Mon, 13 Apr 2009, Brian Wheeler wrote:
> I've fixed a few things:
> * my pointer arithmetic was gross looking, so I fixed it
> * added an allocated page count for stats when debugging
>
>
> On my platform (x86-64) the in-tree implementation allocates 3.5M for
> the PC target. With this implementation the PC target consumes 2K of
> pointer table + 256K of malloc'd memory.
>
> For the Alpha target, statically allocating all 24-bits worth of ioport
> tables is about 1G of ram. With this patch, it uses 32K for the pointer
> table and 512K of malloc'd memory
>
> Is there anything else I need to look at before this patch is worthy of
> inclusion?
>
>
>
> Signed-off-by: Brian Wheeler <bdwheele@indiana.edu>
>
> Index: vl.c
> ===================================================================
> --- vl.c (revision 7097)
> +++ vl.c (working copy)
> @@ -168,8 +168,8 @@
> //#define DEBUG_IOPORT
> //#define DEBUG_NET
> //#define DEBUG_SLIRP
> +//#define DEBUG_IOPORT_FIND
>
> -
> #ifdef DEBUG_IOPORT
> # define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__)
> #else
> @@ -184,14 +184,31 @@
> /* 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
> -
> const char *bios_dir = CONFIG_QEMU_SHAREDIR;
> 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];
> +
> +struct ioport {
> + void *opaque;
> + IOPortReadFunc *read[3];
> + IOPortWriteFunc *write[3];
> + void *pad;
> +};
> +typedef struct ioport ioport_t;
Please do not use _t.
[..snip..]
> +static inline ioport_t *ioport_find(uint32_t address, int allocate)
> +{
> + uint32_t page = (address & IOPORT_PAGEMASK) >> IOPORT_PAGEBITS;
> + uint32_t entry = address & IOPORT_ENTRYMASK;
> +#ifdef DEBUG_IOPORT_FIND
> + static int page_count = 0;
> + if (address >= (1<<IOPORT_MAXBITS)) {
> + hw_error("Maximum port # for this architecture is %d. "
^ extra space?
[..snip..]
> for(i = start; i < start + length; i += size) {
> - ioport_read_table[bsize][i] = func;
> - if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
> + ioport_t *p = ioport_find(i, 1);
Indentation appears messed up.
[..snip..]
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] Two-Level IOPort Lookup
2009-04-13 14:40 ` malc
@ 2009-04-13 14:57 ` Brian Wheeler
0 siblings, 0 replies; 3+ messages in thread
From: Brian Wheeler @ 2009-04-13 14:57 UTC (permalink / raw)
To: qemu-devel
On Mon, 2009-04-13 at 18:40 +0400, malc wrote:
> On Mon, 13 Apr 2009, Brian Wheeler wrote:
>
> > I've fixed a few things:
> > * my pointer arithmetic was gross looking, so I fixed it
> > * added an allocated page count for stats when debugging
> >
> >
> > On my platform (x86-64) the in-tree implementation allocates 3.5M for
> > the PC target. With this implementation the PC target consumes 2K of
> > pointer table + 256K of malloc'd memory.
> >
> > For the Alpha target, statically allocating all 24-bits worth of ioport
> > tables is about 1G of ram. With this patch, it uses 32K for the pointer
> > table and 512K of malloc'd memory
> >
> > Is there anything else I need to look at before this patch is worthy of
> > inclusion?
> >
> >
> >
> > Signed-off-by: Brian Wheeler <bdwheele@indiana.edu>
> >
> > Index: vl.c
> > ===================================================================
> > --- vl.c (revision 7097)
> > +++ vl.c (working copy)
> > @@ -168,8 +168,8 @@
> > //#define DEBUG_IOPORT
> > //#define DEBUG_NET
> > //#define DEBUG_SLIRP
> > +//#define DEBUG_IOPORT_FIND
> >
> > -
> > #ifdef DEBUG_IOPORT
> > # define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__)
> > #else
> > @@ -184,14 +184,31 @@
> > /* 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
> > -
> > const char *bios_dir = CONFIG_QEMU_SHAREDIR;
> > 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];
> > +
> > +struct ioport {
> > + void *opaque;
> > + IOPortReadFunc *read[3];
> > + IOPortWriteFunc *write[3];
> > + void *pad;
> > +};
> > +typedef struct ioport ioport_t;
>
> Please do not use _t.
>
Ah, I missed that part in the CODING_STYLE document. I've now called it
IOPort
> [..snip..]
>
> > +static inline ioport_t *ioport_find(uint32_t address, int allocate)
> > +{
> > + uint32_t page = (address & IOPORT_PAGEMASK) >> IOPORT_PAGEBITS;
> > + uint32_t entry = address & IOPORT_ENTRYMASK;
> > +#ifdef DEBUG_IOPORT_FIND
> > + static int page_count = 0;
> > + if (address >= (1<<IOPORT_MAXBITS)) {
> > + hw_error("Maximum port # for this architecture is %d. "
> ^ extra space?
>
> [..snip..]
>
Not really, since the next line is a continuation of the string (to meet
the 80-char-wide rule) and that ends a sentence.
> > for(i = start; i < start + length; i += size) {
> > - ioport_read_table[bsize][i] = func;
> > - if (ioport_opaque[i] != NULL && ioport_opaque[i] != opaque)
> > + ioport_t *p = ioport_find(i, 1);
>
> Indentation appears messed up.
>
> [..snip..]
>
Sigh, yeah.
A new patch is forthcoming.
Brian
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-04-13 14:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-13 14:11 [Qemu-devel] [PATCH v3] Two-Level IOPort Lookup Brian Wheeler
2009-04-13 14:40 ` malc
2009-04-13 14:57 ` Brian Wheeler
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).