Re: [Qemu-devel] [PATCH 21/24] isa: add isa_address_space()

[Richard Henderson <rth@twiddle.net>]

On 08/08/2011 10:07 AM, Avi Kivity wrote:
> A helper that returns the address space used by ISA devices.  Useful
> for getting rid of isa_mem_base, multiple ISA buses, or ISA buses behind
> bridges.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  hw/isa-bus.c |    6 ++++++
>  hw/isa.h     |    1 +
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 2765543..1cb497f 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -20,6 +20,7 @@
>  #include "monitor.h"
>  #include "sysbus.h"
>  #include "isa.h"
> +#include "exec-memory.h"
>  
>  struct ISABus {
>      BusState qbus;
> @@ -202,4 +203,9 @@ static char *isabus_get_fw_dev_path(DeviceState *dev)
>      return strdup(path);
>  }
>  
> +MemoryRegion *isa_address_space(ISADevice *dev)
> +{
> +    return get_system_memory();
> +}
> +

This does not help get rid of isa_mem_base, as far as I can see.
All this is going to do is the equivalent of isa_mem_base == 0.

Did you have a plan beyond this?

The simplest replacement, as far as I can see, is to replace one
global variable with another:

  MemoryRegion *isa_address_space;

Define that this variable *must* be set by any platform that 
wants to use ISA.  It can be set to the address_space_mem 
parameter (i.e. the copy of get_system_memory() that the
platforms receive as a parameter).

For Alpha, I can set this to the sub-region that I pass off to
the PCI subsystem.

For those other non-x86 platforms that currently set isa_mem_base
to a non-zero value, they either re-use an otherwise existing
memory region or create a new sub-region properly placed.

Of course, as far as I can see, this variable is only used by
the VGA devices.  Surely we can arrange to pass down some address
space during setup of the VGA?



r~