Patchwork [RFC,v3,30/56] rtl8139: convert to memory API

login
register
mail settings
Submitter Avi Kivity
Date July 10, 2011, 6:14 p.m.
Message ID <1310321709-30770-31-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/104121/
State New
Headers show

Comments

Avi Kivity - July 10, 2011, 6:14 p.m.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/rtl8139.c |  172 +++++++++++++++++-----------------------------------------
 1 files changed, 51 insertions(+), 121 deletions(-)
Alex Williamson - July 12, 2011, 10:41 p.m.
On Sun, 2011-07-10 at 21:14 +0300, Avi Kivity wrote:
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  hw/rtl8139.c |  172 +++++++++++++++++-----------------------------------------
>  1 files changed, 51 insertions(+), 121 deletions(-)
> 
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index 5214b8c..fa661fc 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -474,7 +474,6 @@ typedef struct RTL8139State {
>  
>      NICState *nic;
>      NICConf conf;
> -    int rtl8139_mmio_io_addr;
>  
>      /* C ring mode */
>      uint32_t   currTxDesc;
> @@ -506,6 +505,9 @@ typedef struct RTL8139State {
>      QEMUTimer *timer;
>      int64_t TimerExpire;
>  
> +    MemoryRegion bar_io;
> +    MemoryRegion bar_mem;
> +
>      /* Support migration to/from old versions */
>      int rtl8139_mmio_io_addr_dummy;
>  } RTL8139State;
> @@ -2705,12 +2707,8 @@ static uint32_t rtl8139_MultiIntr_read(RTL8139State *s)
>      return ret;
>  }
>  
> -static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
> +static void rtl8139_io_writeb(RTL8139State *s, uint8_t addr, uint32_t val)
>  {
> -    RTL8139State *s = opaque;
> -
> -    addr &= 0xff;
> -
>      switch (addr)
>      {
>          case MAC0 ... MAC0+5:
> @@ -2792,10 +2790,8 @@ static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
>      }
>  }
>  
> -static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
> +static void rtl8139_io_writew(RTL8139State *s, uint8_t addr, uint32_t val)
>  {
> -    RTL8139State *s = opaque;
> -
>      addr &= 0xfe;
>  
>      switch (addr)
> @@ -2846,8 +2842,8 @@ static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
>              DPRINTF("ioport write(w) addr=0x%x val=0x%04x via write(b)\n",
>                  addr, val);
>  
> -            rtl8139_io_writeb(opaque, addr, val & 0xff);
> -            rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
> +            rtl8139_io_writeb(s, addr, val & 0xff);
> +            rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
>              break;
>      }
>  }
> @@ -2892,10 +2888,8 @@ static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time)
>      }
>  }
>  
> -static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
> +static void rtl8139_io_writel(RTL8139State *s, uint8_t addr, uint32_t val)
>  {
> -    RTL8139State *s = opaque;
> -
>      addr &= 0xfc;
>  
>      switch (addr)
> @@ -2952,21 +2946,18 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
>          default:
>              DPRINTF("ioport write(l) addr=0x%x val=0x%08x via write(b)\n",
>                  addr, val);
> -            rtl8139_io_writeb(opaque, addr, val & 0xff);
> -            rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
> -            rtl8139_io_writeb(opaque, addr + 2, (val >> 16) & 0xff);
> -            rtl8139_io_writeb(opaque, addr + 3, (val >> 24) & 0xff);
> +            rtl8139_io_writeb(s, addr, val & 0xff);
> +            rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
> +            rtl8139_io_writeb(s, addr + 2, (val >> 16) & 0xff);
> +            rtl8139_io_writeb(s, addr + 3, (val >> 24) & 0xff);
>              break;
>      }
>  }
>  
> -static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
> +static uint32_t rtl8139_io_readb(RTL8139State *s, uint8_t addr)
>  {
> -    RTL8139State *s = opaque;
>      int ret;
>  
> -    addr &= 0xff;
> -
>      switch (addr)
>      {
>          case MAC0 ... MAC0+5:
> @@ -3034,9 +3025,8 @@ static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
>      return ret;
>  }
>  
> -static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
> +static uint32_t rtl8139_io_readw(RTL8139State *s, uint8_t addr)
>  {
> -    RTL8139State *s = opaque;
>      uint32_t ret;
>  
>      addr &= 0xfe; /* mask lower bit */
> @@ -3101,8 +3091,8 @@ static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
>          default:
>              DPRINTF("ioport read(w) addr=0x%x via read(b)\n", addr);
>  
> -            ret  = rtl8139_io_readb(opaque, addr);
> -            ret |= rtl8139_io_readb(opaque, addr + 1) << 8;
> +            ret  = rtl8139_io_readb(s, addr);
> +            ret |= rtl8139_io_readb(s, addr + 1) << 8;
>  
>              DPRINTF("ioport read(w) addr=0x%x val=0x%04x\n", addr, ret);
>              break;
> @@ -3182,71 +3172,40 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
>      return ret;
>  }
>  
> -/* */
> -
> -static void rtl8139_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
> -{
> -    rtl8139_io_writeb(opaque, addr & 0xFF, val);
> -}
> -
> -static void rtl8139_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
> -{
> -    rtl8139_io_writew(opaque, addr & 0xFF, val);
> -}
> -
> -static void rtl8139_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
> -{
> -    rtl8139_io_writel(opaque, addr & 0xFF, val);
> -}
> -
> -static uint32_t rtl8139_ioport_readb(void *opaque, uint32_t addr)
> +static uint64_t rtl8139_io_read(void *opaque,
> +                                target_phys_addr_t addr,
> +                                unsigned size)
>  {
> -    return rtl8139_io_readb(opaque, addr & 0xFF);
> -}
> -
> -static uint32_t rtl8139_ioport_readw(void *opaque, uint32_t addr)
> -{
> -    return rtl8139_io_readw(opaque, addr & 0xFF);
> -}
> -
> -static uint32_t rtl8139_ioport_readl(void *opaque, uint32_t addr)
> -{
> -    return rtl8139_io_readl(opaque, addr & 0xFF);
> -}
> -
> -/* */
> -
> -static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    rtl8139_io_writeb(opaque, addr & 0xFF, val);
> -}
> -
> -static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    rtl8139_io_writew(opaque, addr & 0xFF, val);
> -}
> +    RTL8139State *s = opaque;
>  
> -static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    rtl8139_io_writel(opaque, addr & 0xFF, val);
> +    switch (size) {
> +    case 1: return rtl8139_io_readb(s, addr);
> +    case 2: return rtl8139_io_readw(s, addr);
> +    case 4: return rtl8139_io_readl(s, addr);
> +    default: abort();
> +    }
>  }
>  
> -static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr)
> +static void rtl8139_io_write(void *opaque,
> +                             target_phys_addr_t addr,
> +                             uint64_t data,
> +                             unsigned size)
>  {
> -    return rtl8139_io_readb(opaque, addr & 0xFF);
> -}
> +    RTL8139State *s = opaque;
>  
> -static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr)
> -{
> -    uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
> -    return val;
> +    switch (size) {
> +    case 1: return rtl8139_io_writeb(s, addr, data);
> +    case 2: return rtl8139_io_writew(s, addr, data);
> +    case 4: return rtl8139_io_writel(s, addr, data);
> +    default: abort();
> +    }
>  }
>  
> -static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
> -{
> -    uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
> -    return val;
> -}
> +static MemoryRegionOps rtl8139_io_ops = {
> +    .read = rtl8139_io_read,
> +    .write = rtl8139_io_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
>  
>  static int rtl8139_post_load(void *opaque, int version_id)
>  {
> @@ -3283,7 +3242,7 @@ static void rtl8139_pre_save(void *opaque)
>      rtl8139_set_next_tctr_time(s, current_time);
>      s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
>                         get_ticks_per_sec());
> -    s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr;
> +    s->rtl8139_mmio_io_addr_dummy = 0;

This makes the dummy value fairly useless for preserving new->old
migration.  Drop it altogether and bump the version or add a subsection
to prevent migration to old versions that consume this?

Alex


>  }
>  
>  static const VMStateDescription vmstate_rtl8139 = {
> @@ -3379,33 +3338,6 @@ static const VMStateDescription vmstate_rtl8139 = {
>  /***********************************************************/
>  /* PCI RTL8139 definitions */
>  
> -static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num,
> -                       pcibus_t addr, pcibus_t size, int type)
> -{
> -    RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev);
> -
> -    register_ioport_write(addr, 0x100, 1, rtl8139_ioport_writeb, s);
> -    register_ioport_read( addr, 0x100, 1, rtl8139_ioport_readb,  s);
> -
> -    register_ioport_write(addr, 0x100, 2, rtl8139_ioport_writew, s);
> -    register_ioport_read( addr, 0x100, 2, rtl8139_ioport_readw,  s);
> -
> -    register_ioport_write(addr, 0x100, 4, rtl8139_ioport_writel, s);
> -    register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl,  s);
> -}
> -
> -static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = {
> -    rtl8139_mmio_readb,
> -    rtl8139_mmio_readw,
> -    rtl8139_mmio_readl,
> -};
> -
> -static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
> -    rtl8139_mmio_writeb,
> -    rtl8139_mmio_writew,
> -    rtl8139_mmio_writel,
> -};
> -
>  static void rtl8139_timer(void *opaque)
>  {
>      RTL8139State *s = opaque;
> @@ -3432,7 +3364,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
>  {
>      RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
>  
> -    cpu_unregister_io_memory(s->rtl8139_mmio_io_addr);
> +    memory_region_destroy(&s->bar_io);
> +    memory_region_destroy(&s->bar_mem);
>      if (s->cplus_txbuffer) {
>          qemu_free(s->cplus_txbuffer);
>          s->cplus_txbuffer = NULL;
> @@ -3462,15 +3395,12 @@ static int pci_rtl8139_init(PCIDevice *dev)
>       * list bit in status register, and offset 0xdc seems unused. */
>      pci_conf[PCI_CAPABILITY_LIST] = 0xdc;
>  
> -    /* I/O handler for memory-mapped I/O */
> -    s->rtl8139_mmio_io_addr =
> -        cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s,
> -                               DEVICE_LITTLE_ENDIAN);
> -
> -    pci_register_bar(&s->dev, 0, 0x100,
> -                           PCI_BASE_ADDRESS_SPACE_IO,  rtl8139_ioport_map);
> -
> -    pci_register_bar_simple(&s->dev, 1, 0x100, 0, s->rtl8139_mmio_io_addr);
> +    memory_region_init_io(&s->bar_io, &rtl8139_io_ops, s, "rtl8139", 0x100);
> +    memory_region_init_io(&s->bar_mem, &rtl8139_io_ops, s, "rtl8139", 0x100);
> +    pci_register_bar_region(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
> +                            &s->bar_io);
> +    pci_register_bar_region(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
> +                            &s->bar_mem);
>  
>      qemu_macaddr_default_if_unset(&s->conf.macaddr);
>
Alex Williamson - July 12, 2011, 10:47 p.m.
On Tue, 2011-07-12 at 16:41 -0600, Alex Williamson wrote:
> On Sun, 2011-07-10 at 21:14 +0300, Avi Kivity wrote:
> > Signed-off-by: Avi Kivity <avi@redhat.com>
> > ---
> >  hw/rtl8139.c |  172 +++++++++++++++++-----------------------------------------
> >  1 files changed, 51 insertions(+), 121 deletions(-)
> > 
> > diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> > index 5214b8c..fa661fc 100644
> > --- a/hw/rtl8139.c
> > +++ b/hw/rtl8139.c
> > @@ -474,7 +474,6 @@ typedef struct RTL8139State {
> >  
> >      NICState *nic;
> >      NICConf conf;
> > -    int rtl8139_mmio_io_addr;
> >  
> >      /* C ring mode */
> >      uint32_t   currTxDesc;
> > @@ -506,6 +505,9 @@ typedef struct RTL8139State {
> >      QEMUTimer *timer;
> >      int64_t TimerExpire;
> >  
> > +    MemoryRegion bar_io;
> > +    MemoryRegion bar_mem;
> > +
> >      /* Support migration to/from old versions */
> >      int rtl8139_mmio_io_addr_dummy;
> >  } RTL8139State;
> > @@ -2705,12 +2707,8 @@ static uint32_t rtl8139_MultiIntr_read(RTL8139State *s)
> >      return ret;
> >  }
> >  
> > -static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
> > +static void rtl8139_io_writeb(RTL8139State *s, uint8_t addr, uint32_t val)
> >  {
> > -    RTL8139State *s = opaque;
> > -
> > -    addr &= 0xff;
> > -
> >      switch (addr)
> >      {
> >          case MAC0 ... MAC0+5:
> > @@ -2792,10 +2790,8 @@ static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
> >      }
> >  }
> >  
> > -static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
> > +static void rtl8139_io_writew(RTL8139State *s, uint8_t addr, uint32_t val)
> >  {
> > -    RTL8139State *s = opaque;
> > -
> >      addr &= 0xfe;
> >  
> >      switch (addr)
> > @@ -2846,8 +2842,8 @@ static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
> >              DPRINTF("ioport write(w) addr=0x%x val=0x%04x via write(b)\n",
> >                  addr, val);
> >  
> > -            rtl8139_io_writeb(opaque, addr, val & 0xff);
> > -            rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
> > +            rtl8139_io_writeb(s, addr, val & 0xff);
> > +            rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
> >              break;
> >      }
> >  }
> > @@ -2892,10 +2888,8 @@ static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time)
> >      }
> >  }
> >  
> > -static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
> > +static void rtl8139_io_writel(RTL8139State *s, uint8_t addr, uint32_t val)
> >  {
> > -    RTL8139State *s = opaque;
> > -
> >      addr &= 0xfc;
> >  
> >      switch (addr)
> > @@ -2952,21 +2946,18 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
> >          default:
> >              DPRINTF("ioport write(l) addr=0x%x val=0x%08x via write(b)\n",
> >                  addr, val);
> > -            rtl8139_io_writeb(opaque, addr, val & 0xff);
> > -            rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
> > -            rtl8139_io_writeb(opaque, addr + 2, (val >> 16) & 0xff);
> > -            rtl8139_io_writeb(opaque, addr + 3, (val >> 24) & 0xff);
> > +            rtl8139_io_writeb(s, addr, val & 0xff);
> > +            rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
> > +            rtl8139_io_writeb(s, addr + 2, (val >> 16) & 0xff);
> > +            rtl8139_io_writeb(s, addr + 3, (val >> 24) & 0xff);
> >              break;
> >      }
> >  }
> >  
> > -static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
> > +static uint32_t rtl8139_io_readb(RTL8139State *s, uint8_t addr)
> >  {
> > -    RTL8139State *s = opaque;
> >      int ret;
> >  
> > -    addr &= 0xff;
> > -
> >      switch (addr)
> >      {
> >          case MAC0 ... MAC0+5:
> > @@ -3034,9 +3025,8 @@ static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
> >      return ret;
> >  }
> >  
> > -static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
> > +static uint32_t rtl8139_io_readw(RTL8139State *s, uint8_t addr)
> >  {
> > -    RTL8139State *s = opaque;
> >      uint32_t ret;
> >  
> >      addr &= 0xfe; /* mask lower bit */
> > @@ -3101,8 +3091,8 @@ static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
> >          default:
> >              DPRINTF("ioport read(w) addr=0x%x via read(b)\n", addr);
> >  
> > -            ret  = rtl8139_io_readb(opaque, addr);
> > -            ret |= rtl8139_io_readb(opaque, addr + 1) << 8;
> > +            ret  = rtl8139_io_readb(s, addr);
> > +            ret |= rtl8139_io_readb(s, addr + 1) << 8;
> >  
> >              DPRINTF("ioport read(w) addr=0x%x val=0x%04x\n", addr, ret);
> >              break;
> > @@ -3182,71 +3172,40 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
> >      return ret;
> >  }
> >  
> > -/* */
> > -
> > -static void rtl8139_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > -    rtl8139_io_writeb(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static void rtl8139_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > -    rtl8139_io_writew(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static void rtl8139_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > -    rtl8139_io_writel(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static uint32_t rtl8139_ioport_readb(void *opaque, uint32_t addr)
> > +static uint64_t rtl8139_io_read(void *opaque,
> > +                                target_phys_addr_t addr,
> > +                                unsigned size)
> >  {
> > -    return rtl8139_io_readb(opaque, addr & 0xFF);
> > -}
> > -
> > -static uint32_t rtl8139_ioport_readw(void *opaque, uint32_t addr)
> > -{
> > -    return rtl8139_io_readw(opaque, addr & 0xFF);
> > -}
> > -
> > -static uint32_t rtl8139_ioport_readl(void *opaque, uint32_t addr)
> > -{
> > -    return rtl8139_io_readl(opaque, addr & 0xFF);
> > -}
> > -
> > -/* */
> > -
> > -static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
> > -{
> > -    rtl8139_io_writeb(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
> > -{
> > -    rtl8139_io_writew(opaque, addr & 0xFF, val);
> > -}
> > +    RTL8139State *s = opaque;
> >  
> > -static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
> > -{
> > -    rtl8139_io_writel(opaque, addr & 0xFF, val);
> > +    switch (size) {
> > +    case 1: return rtl8139_io_readb(s, addr);
> > +    case 2: return rtl8139_io_readw(s, addr);
> > +    case 4: return rtl8139_io_readl(s, addr);
> > +    default: abort();
> > +    }
> >  }
> >  
> > -static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr)
> > +static void rtl8139_io_write(void *opaque,
> > +                             target_phys_addr_t addr,
> > +                             uint64_t data,
> > +                             unsigned size)
> >  {
> > -    return rtl8139_io_readb(opaque, addr & 0xFF);
> > -}
> > +    RTL8139State *s = opaque;
> >  
> > -static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr)
> > -{
> > -    uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
> > -    return val;
> > +    switch (size) {
> > +    case 1: return rtl8139_io_writeb(s, addr, data);
> > +    case 2: return rtl8139_io_writew(s, addr, data);
> > +    case 4: return rtl8139_io_writel(s, addr, data);
> > +    default: abort();
> > +    }
> >  }
> >  
> > -static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
> > -{
> > -    uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
> > -    return val;
> > -}
> > +static MemoryRegionOps rtl8139_io_ops = {
> > +    .read = rtl8139_io_read,
> > +    .write = rtl8139_io_write,
> > +    .endianness = DEVICE_LITTLE_ENDIAN,
> > +};
> >  
> >  static int rtl8139_post_load(void *opaque, int version_id)
> >  {
> > @@ -3283,7 +3242,7 @@ static void rtl8139_pre_save(void *opaque)
> >      rtl8139_set_next_tctr_time(s, current_time);
> >      s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> >                         get_ticks_per_sec());
> > -    s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr;
> > +    s->rtl8139_mmio_io_addr_dummy = 0;
> 
> This makes the dummy value fairly useless for preserving new->old
> migration.  Drop it altogether and bump the version or add a subsection
> to prevent migration to old versions that consume this?

I guess to preserve as much as we can, rtl8139_hotplug_ready_needed()
would just return 1 and we'd stuff VMSTATE_UNUSED to fill the gap from
dummy in the vmstate.

Alex

> >  }
> >  
> >  static const VMStateDescription vmstate_rtl8139 = {
> > @@ -3379,33 +3338,6 @@ static const VMStateDescription vmstate_rtl8139 = {
> >  /***********************************************************/
> >  /* PCI RTL8139 definitions */
> >  
> > -static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num,
> > -                       pcibus_t addr, pcibus_t size, int type)
> > -{
> > -    RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev);
> > -
> > -    register_ioport_write(addr, 0x100, 1, rtl8139_ioport_writeb, s);
> > -    register_ioport_read( addr, 0x100, 1, rtl8139_ioport_readb,  s);
> > -
> > -    register_ioport_write(addr, 0x100, 2, rtl8139_ioport_writew, s);
> > -    register_ioport_read( addr, 0x100, 2, rtl8139_ioport_readw,  s);
> > -
> > -    register_ioport_write(addr, 0x100, 4, rtl8139_ioport_writel, s);
> > -    register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl,  s);
> > -}
> > -
> > -static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = {
> > -    rtl8139_mmio_readb,
> > -    rtl8139_mmio_readw,
> > -    rtl8139_mmio_readl,
> > -};
> > -
> > -static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
> > -    rtl8139_mmio_writeb,
> > -    rtl8139_mmio_writew,
> > -    rtl8139_mmio_writel,
> > -};
> > -
> >  static void rtl8139_timer(void *opaque)
> >  {
> >      RTL8139State *s = opaque;
> > @@ -3432,7 +3364,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
> >  {
> >      RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
> >  
> > -    cpu_unregister_io_memory(s->rtl8139_mmio_io_addr);
> > +    memory_region_destroy(&s->bar_io);
> > +    memory_region_destroy(&s->bar_mem);
> >      if (s->cplus_txbuffer) {
> >          qemu_free(s->cplus_txbuffer);
> >          s->cplus_txbuffer = NULL;
> > @@ -3462,15 +3395,12 @@ static int pci_rtl8139_init(PCIDevice *dev)
> >       * list bit in status register, and offset 0xdc seems unused. */
> >      pci_conf[PCI_CAPABILITY_LIST] = 0xdc;
> >  
> > -    /* I/O handler for memory-mapped I/O */
> > -    s->rtl8139_mmio_io_addr =
> > -        cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s,
> > -                               DEVICE_LITTLE_ENDIAN);
> > -
> > -    pci_register_bar(&s->dev, 0, 0x100,
> > -                           PCI_BASE_ADDRESS_SPACE_IO,  rtl8139_ioport_map);
> > -
> > -    pci_register_bar_simple(&s->dev, 1, 0x100, 0, s->rtl8139_mmio_io_addr);
> > +    memory_region_init_io(&s->bar_io, &rtl8139_io_ops, s, "rtl8139", 0x100);
> > +    memory_region_init_io(&s->bar_mem, &rtl8139_io_ops, s, "rtl8139", 0x100);
> > +    pci_register_bar_region(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
> > +                            &s->bar_io);
> > +    pci_register_bar_region(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
> > +                            &s->bar_mem);
> >  
> >      qemu_macaddr_default_if_unset(&s->conf.macaddr);
> >  
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity - July 13, 2011, 6:52 a.m.
On 07/13/2011 01:41 AM, Alex Williamson wrote:
> >   static int rtl8139_post_load(void *opaque, int version_id)
> >   {
> >  @@ -3283,7 +3242,7 @@ static void rtl8139_pre_save(void *opaque)
> >       rtl8139_set_next_tctr_time(s, current_time);
> >       s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> >                          get_ticks_per_sec());
> >  -    s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr;
> >  +    s->rtl8139_mmio_io_addr_dummy = 0;
>
> This makes the dummy value fairly useless for preserving new->old
> migration.

> Drop it altogether and bump the version or add a subsection
> to prevent migration to old versions that consume this?

That means we can't migrate to 0.14, even though 0.14 is safe.

How about we fix 0.13.blah?  And make the rule that we only support 
backwards migration to fully patched releases.  There's no problem 
requiring an updated target; only an updated source is an issue.

Patch

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 5214b8c..fa661fc 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -474,7 +474,6 @@  typedef struct RTL8139State {
 
     NICState *nic;
     NICConf conf;
-    int rtl8139_mmio_io_addr;
 
     /* C ring mode */
     uint32_t   currTxDesc;
@@ -506,6 +505,9 @@  typedef struct RTL8139State {
     QEMUTimer *timer;
     int64_t TimerExpire;
 
+    MemoryRegion bar_io;
+    MemoryRegion bar_mem;
+
     /* Support migration to/from old versions */
     int rtl8139_mmio_io_addr_dummy;
 } RTL8139State;
@@ -2705,12 +2707,8 @@  static uint32_t rtl8139_MultiIntr_read(RTL8139State *s)
     return ret;
 }
 
-static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
+static void rtl8139_io_writeb(RTL8139State *s, uint8_t addr, uint32_t val)
 {
-    RTL8139State *s = opaque;
-
-    addr &= 0xff;
-
     switch (addr)
     {
         case MAC0 ... MAC0+5:
@@ -2792,10 +2790,8 @@  static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
     }
 }
 
-static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
+static void rtl8139_io_writew(RTL8139State *s, uint8_t addr, uint32_t val)
 {
-    RTL8139State *s = opaque;
-
     addr &= 0xfe;
 
     switch (addr)
@@ -2846,8 +2842,8 @@  static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
             DPRINTF("ioport write(w) addr=0x%x val=0x%04x via write(b)\n",
                 addr, val);
 
-            rtl8139_io_writeb(opaque, addr, val & 0xff);
-            rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
+            rtl8139_io_writeb(s, addr, val & 0xff);
+            rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
             break;
     }
 }
@@ -2892,10 +2888,8 @@  static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time)
     }
 }
 
-static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
+static void rtl8139_io_writel(RTL8139State *s, uint8_t addr, uint32_t val)
 {
-    RTL8139State *s = opaque;
-
     addr &= 0xfc;
 
     switch (addr)
@@ -2952,21 +2946,18 @@  static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
         default:
             DPRINTF("ioport write(l) addr=0x%x val=0x%08x via write(b)\n",
                 addr, val);
-            rtl8139_io_writeb(opaque, addr, val & 0xff);
-            rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
-            rtl8139_io_writeb(opaque, addr + 2, (val >> 16) & 0xff);
-            rtl8139_io_writeb(opaque, addr + 3, (val >> 24) & 0xff);
+            rtl8139_io_writeb(s, addr, val & 0xff);
+            rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
+            rtl8139_io_writeb(s, addr + 2, (val >> 16) & 0xff);
+            rtl8139_io_writeb(s, addr + 3, (val >> 24) & 0xff);
             break;
     }
 }
 
-static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
+static uint32_t rtl8139_io_readb(RTL8139State *s, uint8_t addr)
 {
-    RTL8139State *s = opaque;
     int ret;
 
-    addr &= 0xff;
-
     switch (addr)
     {
         case MAC0 ... MAC0+5:
@@ -3034,9 +3025,8 @@  static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
     return ret;
 }
 
-static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
+static uint32_t rtl8139_io_readw(RTL8139State *s, uint8_t addr)
 {
-    RTL8139State *s = opaque;
     uint32_t ret;
 
     addr &= 0xfe; /* mask lower bit */
@@ -3101,8 +3091,8 @@  static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
         default:
             DPRINTF("ioport read(w) addr=0x%x via read(b)\n", addr);
 
-            ret  = rtl8139_io_readb(opaque, addr);
-            ret |= rtl8139_io_readb(opaque, addr + 1) << 8;
+            ret  = rtl8139_io_readb(s, addr);
+            ret |= rtl8139_io_readb(s, addr + 1) << 8;
 
             DPRINTF("ioport read(w) addr=0x%x val=0x%04x\n", addr, ret);
             break;
@@ -3182,71 +3172,40 @@  static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
     return ret;
 }
 
-/* */
-
-static void rtl8139_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
-{
-    rtl8139_io_writeb(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
-{
-    rtl8139_io_writew(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
-{
-    rtl8139_io_writel(opaque, addr & 0xFF, val);
-}
-
-static uint32_t rtl8139_ioport_readb(void *opaque, uint32_t addr)
+static uint64_t rtl8139_io_read(void *opaque,
+                                target_phys_addr_t addr,
+                                unsigned size)
 {
-    return rtl8139_io_readb(opaque, addr & 0xFF);
-}
-
-static uint32_t rtl8139_ioport_readw(void *opaque, uint32_t addr)
-{
-    return rtl8139_io_readw(opaque, addr & 0xFF);
-}
-
-static uint32_t rtl8139_ioport_readl(void *opaque, uint32_t addr)
-{
-    return rtl8139_io_readl(opaque, addr & 0xFF);
-}
-
-/* */
-
-static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    rtl8139_io_writeb(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    rtl8139_io_writew(opaque, addr & 0xFF, val);
-}
+    RTL8139State *s = opaque;
 
-static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    rtl8139_io_writel(opaque, addr & 0xFF, val);
+    switch (size) {
+    case 1: return rtl8139_io_readb(s, addr);
+    case 2: return rtl8139_io_readw(s, addr);
+    case 4: return rtl8139_io_readl(s, addr);
+    default: abort();
+    }
 }
 
-static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr)
+static void rtl8139_io_write(void *opaque,
+                             target_phys_addr_t addr,
+                             uint64_t data,
+                             unsigned size)
 {
-    return rtl8139_io_readb(opaque, addr & 0xFF);
-}
+    RTL8139State *s = opaque;
 
-static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr)
-{
-    uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
-    return val;
+    switch (size) {
+    case 1: return rtl8139_io_writeb(s, addr, data);
+    case 2: return rtl8139_io_writew(s, addr, data);
+    case 4: return rtl8139_io_writel(s, addr, data);
+    default: abort();
+    }
 }
 
-static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
-{
-    uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
-    return val;
-}
+static MemoryRegionOps rtl8139_io_ops = {
+    .read = rtl8139_io_read,
+    .write = rtl8139_io_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
 
 static int rtl8139_post_load(void *opaque, int version_id)
 {
@@ -3283,7 +3242,7 @@  static void rtl8139_pre_save(void *opaque)
     rtl8139_set_next_tctr_time(s, current_time);
     s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
                        get_ticks_per_sec());
-    s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr;
+    s->rtl8139_mmio_io_addr_dummy = 0;
 }
 
 static const VMStateDescription vmstate_rtl8139 = {
@@ -3379,33 +3338,6 @@  static const VMStateDescription vmstate_rtl8139 = {
 /***********************************************************/
 /* PCI RTL8139 definitions */
 
-static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num,
-                       pcibus_t addr, pcibus_t size, int type)
-{
-    RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev);
-
-    register_ioport_write(addr, 0x100, 1, rtl8139_ioport_writeb, s);
-    register_ioport_read( addr, 0x100, 1, rtl8139_ioport_readb,  s);
-
-    register_ioport_write(addr, 0x100, 2, rtl8139_ioport_writew, s);
-    register_ioport_read( addr, 0x100, 2, rtl8139_ioport_readw,  s);
-
-    register_ioport_write(addr, 0x100, 4, rtl8139_ioport_writel, s);
-    register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl,  s);
-}
-
-static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = {
-    rtl8139_mmio_readb,
-    rtl8139_mmio_readw,
-    rtl8139_mmio_readl,
-};
-
-static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
-    rtl8139_mmio_writeb,
-    rtl8139_mmio_writew,
-    rtl8139_mmio_writel,
-};
-
 static void rtl8139_timer(void *opaque)
 {
     RTL8139State *s = opaque;
@@ -3432,7 +3364,8 @@  static int pci_rtl8139_uninit(PCIDevice *dev)
 {
     RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
 
-    cpu_unregister_io_memory(s->rtl8139_mmio_io_addr);
+    memory_region_destroy(&s->bar_io);
+    memory_region_destroy(&s->bar_mem);
     if (s->cplus_txbuffer) {
         qemu_free(s->cplus_txbuffer);
         s->cplus_txbuffer = NULL;
@@ -3462,15 +3395,12 @@  static int pci_rtl8139_init(PCIDevice *dev)
      * list bit in status register, and offset 0xdc seems unused. */
     pci_conf[PCI_CAPABILITY_LIST] = 0xdc;
 
-    /* I/O handler for memory-mapped I/O */
-    s->rtl8139_mmio_io_addr =
-        cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s,
-                               DEVICE_LITTLE_ENDIAN);
-
-    pci_register_bar(&s->dev, 0, 0x100,
-                           PCI_BASE_ADDRESS_SPACE_IO,  rtl8139_ioport_map);
-
-    pci_register_bar_simple(&s->dev, 1, 0x100, 0, s->rtl8139_mmio_io_addr);
+    memory_region_init_io(&s->bar_io, &rtl8139_io_ops, s, "rtl8139", 0x100);
+    memory_region_init_io(&s->bar_mem, &rtl8139_io_ops, s, "rtl8139", 0x100);
+    pci_register_bar_region(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
+                            &s->bar_io);
+    pci_register_bar_region(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                            &s->bar_mem);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);