Re: [Qemu-devel] [PATCH 2/2] ppc_prep: convert ioport to memory API

["Andreas Färber" <andreas.faerber@web.de>]

Benoît,

Just spotted this.

Am 01.12.2011 17:57, schrieb Benoît Canet:
> Signed-off-by: Benoît Canet <benoit.canet@gmail.com>

You should've cc'ed me here as maintainer. As indicated to you, I have
pending patches for PReP (e.g., moving io800 and dropping speaker in
favor of qdev), which this conflicts with once again. I'm in favor of
merging your conversions as a short-term solution though, so that it can
go through Avi's tree.

The patch itself looks okay on brief look, except that io_fake and
prep-fake should better be io_fakeio and prep-fakeio respectively (still
ugly as hell, but the real Super I/O we prepared for 40P turned out
incompatible somehow).

I'll test later. Did you in some way?

Regards,
Andreas

> ---
>  hw/ppc_prep.c |   80 ++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 62 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index f22d5b9..299305b 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -94,7 +94,8 @@ static int speaker_data_on;
>  static int dummy_refresh_clock;
>  #endif
>  
> -static void speaker_ioport_write (void *opaque, uint32_t addr, uint32_t val)
> +static void speaker_ioport_write(void *opaque, target_phys_addr_t addr,
> +                                 uint64_t val, unsigned size)
>  {
>  #if 0
>      speaker_data_on = (val >> 1) & 1;
> @@ -102,7 +103,8 @@ static void speaker_ioport_write (void *opaque, uint32_t addr, uint32_t val)
>  #endif
>  }
>  
> -static uint32_t speaker_ioport_read (void *opaque, uint32_t addr)
> +static uint64_t speaker_ioport_read(void *opaque, target_phys_addr_t addr,
> +                                    unsigned size)
>  {
>  #if 0
>      int out;
> @@ -114,6 +116,16 @@ static uint32_t speaker_ioport_read (void *opaque, uint32_t addr)
>      return 0;
>  }
>  
> +static const MemoryRegionOps speaker_ioport_ops = {
> +    .read = speaker_ioport_read,
> +    .write = speaker_ioport_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
>  /* PCI intack register */
>  /* Read-only register (?) */
>  static void PPC_intack_write (void *opaque, target_phys_addr_t addr,
> @@ -251,7 +263,8 @@ enum {
>  
>  static sysctrl_t *sysctrl;
>  
> -static void PREP_io_write (void *opaque, uint32_t addr, uint32_t val)
> +static void PREP_io_write(void *opaque, target_phys_addr_t addr,
> +                          uint64_t val, unsigned size)
>  {
>      sysctrl_t *sysctrl = opaque;
>  
> @@ -260,7 +273,8 @@ static void PREP_io_write (void *opaque, uint32_t addr, uint32_t val)
>      sysctrl->fake_io[addr - 0x0398] = val;
>  }
>  
> -static uint32_t PREP_io_read (void *opaque, uint32_t addr)
> +static uint64_t PREP_io_read(void *opaque, target_phys_addr_t addr,
> +                             unsigned size)
>  {
>      sysctrl_t *sysctrl = opaque;
>  
> @@ -269,7 +283,18 @@ static uint32_t PREP_io_read (void *opaque, uint32_t addr)
>      return sysctrl->fake_io[addr - 0x0398];
>  }
>  
> -static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
> +static const MemoryRegionOps PREP_io_ops = {
> +    .read = PREP_io_read,
> +    .write = PREP_io_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
> +static void PREP_io_800_writeb(void *opaque, target_phys_addr_t addr,
> +                               uint64_t val, unsigned size)
>  {
>      sysctrl_t *sysctrl = opaque;
>  
> @@ -330,13 +355,14 @@ static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
>          sysctrl->contiguous_map = val & 0x01;
>          break;
>      default:
> -        printf("ERROR: unaffected IO port write: %04" PRIx32
> -               " => %02" PRIx32"\n", addr, val);
> +        printf("ERROR: unaffected IO port write: %04" PRIx64
> +               " => %02" PRIx64 "\n", addr, val);
>          break;
>      }
>  }
>  
> -static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
> +static uint64_t PREP_io_800_readb(void *opaque, target_phys_addr_t addr,
> +                                  unsigned size)
>  {
>      sysctrl_t *sysctrl = opaque;
>      uint32_t retval = 0xFF;
> @@ -393,15 +419,25 @@ static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
>          retval = sysctrl->contiguous_map;
>          break;
>      default:
> -        printf("ERROR: unaffected IO port: %04" PRIx32 " read\n", addr);
> +        printf("ERROR: unaffected IO port: %04" PRIx64 " read\n", addr);
>          break;
>      }
> -    PPC_IO_DPRINTF("0x%08" PRIx32 " <= 0x%02" PRIx32 "\n",
> +    PPC_IO_DPRINTF("0x%08" PRIx64 " <= 0x%02" PRIx64 "\n",
>                     addr - PPC_IO_BASE, retval);
>  
>      return retval;
>  }
>  
> +static const MemoryRegionOps PREP_io_800_ops = {
> +    .read = PREP_io_800_readb,
> +    .write = PREP_io_800_writeb,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
>  static inline target_phys_addr_t prep_IO_address(sysctrl_t *sysctrl,
>                                                   target_phys_addr_t addr)
>  {
> @@ -514,6 +550,10 @@ static void ppc_prep_init (ram_addr_t ram_size,
>      M48t59State *m48t59;
>      MemoryRegion *PPC_io_memory = g_new(MemoryRegion, 1);
>      MemoryRegion *intack = g_new(MemoryRegion, 1);
> +    MemoryRegion *io_speaker = g_new(MemoryRegion, 1);
> +    MemoryRegion *io_fake = g_new(MemoryRegion, 1);
> +    MemoryRegion *io_control_092 = g_new(MemoryRegion, 1);
> +    MemoryRegion *io_control_800 = g_new(MemoryRegion, 1);
>  #if 0
>      MemoryRegion *xcsr = g_new(MemoryRegion, 1);
>  #endif
> @@ -679,17 +719,21 @@ static void ppc_prep_init (ram_addr_t ram_size,
>      fdctrl_init_isa(fd);
>  
>      /* Register speaker port */
> -    register_ioport_read(0x61, 1, 1, speaker_ioport_read, NULL);
> -    register_ioport_write(0x61, 1, 1, speaker_ioport_write, NULL);
> +    memory_region_init_io(io_speaker, &speaker_ioport_ops, NULL,
> +                          "speaker", 1);
> +    memory_region_add_subregion(sysmem, 0x61, io_speaker);
>      /* Register fake IO ports for PREP */
>      sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET];
> -    register_ioport_read(0x398, 2, 1, &PREP_io_read, sysctrl);
> -    register_ioport_write(0x398, 2, 1, &PREP_io_write, sysctrl);
> +    memory_region_init_io(io_fake, &PREP_io_ops, sysctrl, "prep-fake", 2);
> +    memory_region_add_subregion(sysmem, 0x398, io_fake);
>      /* System control ports */
> -    register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
> -    register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
> -    register_ioport_read(0x0800, 0x52, 1, &PREP_io_800_readb, sysctrl);
> -    register_ioport_write(0x0800, 0x52, 1, &PREP_io_800_writeb, sysctrl);
> +    memory_region_init_io(io_control_092, &PREP_io_800_ops, sysctrl,
> +                          "prep-control-0x92", 1);
> +    memory_region_add_subregion(sysmem, 0x0092, io_control_092);
> +
> +    memory_region_init_io(io_control_800, &PREP_io_800_ops, sysctrl,
> +                          "prep-control-0x800", 52);
> +    memory_region_add_subregion(sysmem, 0x0800, io_control_800);
>      /* PCI intack location */
>      memory_region_init_io(intack, &PPC_intack_ops, NULL, "ppc-intack", 4);
>      memory_region_add_subregion(sysmem, 0xBFFFFFF0, intack);