[Qemu-devel] [PATCH] net: rtl8139: do not use old_mmio accesses

Matt Parker posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170812213330.20539-1-mtparkr@gmail.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
hw/net/rtl8139.c | 60 +++++++++++++++-----------------------------------------
1 file changed, 16 insertions(+), 44 deletions(-)
[Qemu-devel] [PATCH] net: rtl8139: do not use old_mmio accesses
Posted by Matt Parker 6 years, 7 months ago
This updates the current MemoryRegionOps for the bar 1 memory region
from using the old_mmio accessors to the .read and .write accessors.

Signed-off-by: Matt Parker <mtparkr@gmail.com>
---
 hw/net/rtl8139.c | 60 +++++++++++++++-----------------------------------------
 1 file changed, 16 insertions(+), 44 deletions(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 671c7e48c6..b3aefda291 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -3131,39 +3131,6 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
 }
 
 /* */
-
-static void rtl8139_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
-{
-    rtl8139_io_writeb(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_mmio_writew(void *opaque, hwaddr addr, uint32_t val)
-{
-    rtl8139_io_writew(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_mmio_writel(void *opaque, hwaddr addr, uint32_t val)
-{
-    rtl8139_io_writel(opaque, addr & 0xFF, val);
-}
-
-static uint32_t rtl8139_mmio_readb(void *opaque, hwaddr addr)
-{
-    return rtl8139_io_readb(opaque, addr & 0xFF);
-}
-
-static uint32_t rtl8139_mmio_readw(void *opaque, hwaddr addr)
-{
-    uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
-    return val;
-}
-
-static uint32_t rtl8139_mmio_readl(void *opaque, hwaddr addr)
-{
-    uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
-    return val;
-}
-
 static int rtl8139_post_load(void *opaque, int version_id)
 {
     RTL8139State* s = opaque;
@@ -3344,18 +3311,23 @@ static const MemoryRegionOps rtl8139_io_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static uint64_t rtl8139_mmio_read(void *opaque, hwaddr addr, unsigned size)
+{
+   return rtl8139_ioport_read(opaque, addr & 0xFF, size);
+}
+
+static void rtl8139_mmio_write(void *opaque, hwaddr addr, uint64_t val,
+                               unsigned size)
+{
+    return rtl8139_ioport_write(opaque, addr & 0xFF, val, size);
+}
+
 static const MemoryRegionOps rtl8139_mmio_ops = {
-    .old_mmio = {
-        .read = {
-            rtl8139_mmio_readb,
-            rtl8139_mmio_readw,
-            rtl8139_mmio_readl,
-        },
-        .write = {
-            rtl8139_mmio_writeb,
-            rtl8139_mmio_writew,
-            rtl8139_mmio_writel,
-        },
+    .read = rtl8139_mmio_read,
+    .write = rtl8139_mmio_write,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 4,
     },
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
-- 
2.13.2


Re: [Qemu-devel] [PATCH] net: rtl8139: do not use old_mmio accesses
Posted by Paolo Bonzini 6 years, 7 months ago
On 12/08/2017 23:33, Matt Parker wrote:
> This updates the current MemoryRegionOps for the bar 1 memory region
> from using the old_mmio accessors to the .read and .write accessors.
> 
> Signed-off-by: Matt Parker <mtparkr@gmail.com>
> ---
>  hw/net/rtl8139.c | 60 +++++++++++++++-----------------------------------------
>  1 file changed, 16 insertions(+), 44 deletions(-)
> 
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 671c7e48c6..b3aefda291 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -3131,39 +3131,6 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
>  }
>  
>  /* */
> -
> -static void rtl8139_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    rtl8139_io_writeb(opaque, addr & 0xFF, val);
> -}
> -
> -static void rtl8139_mmio_writew(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    rtl8139_io_writew(opaque, addr & 0xFF, val);
> -}
> -
> -static void rtl8139_mmio_writel(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    rtl8139_io_writel(opaque, addr & 0xFF, val);
> -}
> -
> -static uint32_t rtl8139_mmio_readb(void *opaque, hwaddr addr)
> -{
> -    return rtl8139_io_readb(opaque, addr & 0xFF);
> -}
> -
> -static uint32_t rtl8139_mmio_readw(void *opaque, hwaddr addr)
> -{
> -    uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
> -    return val;
> -}
> -
> -static uint32_t rtl8139_mmio_readl(void *opaque, hwaddr addr)
> -{
> -    uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
> -    return val;
> -}
> -
>  static int rtl8139_post_load(void *opaque, int version_id)
>  {
>      RTL8139State* s = opaque;
> @@ -3344,18 +3311,23 @@ static const MemoryRegionOps rtl8139_io_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> +static uint64_t rtl8139_mmio_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +   return rtl8139_ioport_read(opaque, addr & 0xFF, size);
> +}
> +
> +static void rtl8139_mmio_write(void *opaque, hwaddr addr, uint64_t val,
> +                               unsigned size)
> +{
> +    return rtl8139_ioport_write(opaque, addr & 0xFF, val, size);
> +}
> +
>  static const MemoryRegionOps rtl8139_mmio_ops = {

I think you don't even need to declare a separate MemoryRegionOps, and
can just recycle rtl8139_io_ops?

Paolo

> -    .old_mmio = {
> -        .read = {
> -            rtl8139_mmio_readb,
> -            rtl8139_mmio_readw,
> -            rtl8139_mmio_readl,
> -        },
> -        .write = {
> -            rtl8139_mmio_writeb,
> -            rtl8139_mmio_writew,
> -            rtl8139_mmio_writel,
> -        },
> +    .read = rtl8139_mmio_read,
> +    .write = rtl8139_mmio_write,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
>      },
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
> 


Re: [Qemu-devel] [PATCH] net: rtl8139: do not use old_mmio accesses
Posted by Matt Parker 6 years, 7 months ago
On Mon, Aug 14, 2017 at 10:40:27AM +0200, Paolo Bonzini wrote:
> On 12/08/2017 23:33, Matt Parker wrote:
> > This updates the current MemoryRegionOps for the bar 1 memory region
> > from using the old_mmio accessors to the .read and .write accessors.
> > 
> > Signed-off-by: Matt Parker <mtparkr@gmail.com>
> > ---
> >  hw/net/rtl8139.c | 60 +++++++++++++++-----------------------------------------
> >  1 file changed, 16 insertions(+), 44 deletions(-)
> > 
> > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> > index 671c7e48c6..b3aefda291 100644
> > --- a/hw/net/rtl8139.c
> > +++ b/hw/net/rtl8139.c
> > @@ -3131,39 +3131,6 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
> >  }
> >  
> >  /* */
> > -
> > -static void rtl8139_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
> > -{
> > -    rtl8139_io_writeb(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static void rtl8139_mmio_writew(void *opaque, hwaddr addr, uint32_t val)
> > -{
> > -    rtl8139_io_writew(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static void rtl8139_mmio_writel(void *opaque, hwaddr addr, uint32_t val)
> > -{
> > -    rtl8139_io_writel(opaque, addr & 0xFF, val);
> > -}
> > -
> > -static uint32_t rtl8139_mmio_readb(void *opaque, hwaddr addr)
> > -{
> > -    return rtl8139_io_readb(opaque, addr & 0xFF);
> > -}
> > -
> > -static uint32_t rtl8139_mmio_readw(void *opaque, hwaddr addr)
> > -{
> > -    uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
> > -    return val;
> > -}
> > -
> > -static uint32_t rtl8139_mmio_readl(void *opaque, hwaddr addr)
> > -{
> > -    uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
> > -    return val;
> > -}
> > -
> >  static int rtl8139_post_load(void *opaque, int version_id)
> >  {
> >      RTL8139State* s = opaque;
> > @@ -3344,18 +3311,23 @@ static const MemoryRegionOps rtl8139_io_ops = {
> >      .endianness = DEVICE_LITTLE_ENDIAN,
> >  };
> >  
> > +static uint64_t rtl8139_mmio_read(void *opaque, hwaddr addr, unsigned size)
> > +{
> > +   return rtl8139_ioport_read(opaque, addr & 0xFF, size);
> > +}
> > +
> > +static void rtl8139_mmio_write(void *opaque, hwaddr addr, uint64_t val,
> > +                               unsigned size)
> > +{
> > +    return rtl8139_ioport_write(opaque, addr & 0xFF, val, size);
> > +}
> > +
> >  static const MemoryRegionOps rtl8139_mmio_ops = {
> 
> I think you don't even need to declare a separate MemoryRegionOps, and
> can just recycle rtl8139_io_ops?

Looking at the memory region, it's only 0x100 bytes in size,
so addr & 0xFF would not be needed.

In that case your right, the bar1 memory can just reuse the MemoryRegion from IO.
I'll update the patch for this.

Thanks!
Matt
> 
> > -    .old_mmio = {
> > -        .read = {
> > -            rtl8139_mmio_readb,
> > -            rtl8139_mmio_readw,
> > -            rtl8139_mmio_readl,
> > -        },
> > -        .write = {
> > -            rtl8139_mmio_writeb,
> > -            rtl8139_mmio_writew,
> > -            rtl8139_mmio_writel,
> > -        },
> > +    .read = rtl8139_mmio_read,
> > +    .write = rtl8139_mmio_write,
> > +    .impl = {
> > +        .min_access_size = 1,
> > +        .max_access_size = 4,
> >      },
> >      .endianness = DEVICE_LITTLE_ENDIAN,
> >  };
> > 
> 

Re: [Qemu-devel] [PATCH] net: rtl8139: do not use old_mmio accesses
Posted by Paolo Bonzini 6 years, 7 months ago
On 14/08/2017 21:03, Matt Parker wrote:
>>>  static const MemoryRegionOps rtl8139_mmio_ops = {
>> I think you don't even need to declare a separate MemoryRegionOps, and
>> can just recycle rtl8139_io_ops?
> Looking at the memory region, it's only 0x100 bytes in size,
> so addr & 0xFF would not be needed.
> 
> In that case your right, the bar1 memory can just reuse the MemoryRegion from IO.
> I'll update the patch for this.

You can reuse the MemoryRegionOps; you cannot reuse the MemoryRegion,
but you can replace memory_region_init_io with memory_region_init_alias
if you prefer.

Paolo