Re: [Qemu-devel] [PATCH v3] Two-Level IOPort Lookup

[malc <av1474@comtv.ru>]

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