Patchwork [1/2] Type-safe ioport callbacks

login
register
mail settings
Submitter Avi Kivity
Date Oct. 24, 2010, 3:34 p.m.
Message ID <1287934469-16624-2-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/69040/
State New
Headers show

Comments

Avi Kivity - Oct. 24, 2010, 3:34 p.m.
The current ioport callbacks are not type-safe, in that they accept an "opaque"
pointer as an argument whose type must match the argument to the registration
function; this is not checked by the compiler.

This patch adds an alternative that is type-safe.  Instead of an opaque
argument, both registation and the callback use a new IOPort type.  The
callback then uses container_of() to access its main structures.

Currently the old and new methods exist side by side; once the old way is gone,
we can also save a bunch of memory since the new method requires one pointer
per ioport instead of 6.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 ioport.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ioport.h |   16 +++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)
Avi Kivity - Oct. 24, 2010, 4:41 p.m.
On 10/24/2010 05:34 PM, Avi Kivity wrote:
> Currently the old and new methods exist side by side; once the old way is gone,
> we can also save a bunch of memory since the new method requires one pointer
> per ioport instead of 6.

Actually, 1:7, we replace 3 read callbacks, 3 write callbacks, and 1 
opaque pointer with a single IOPort pointer.
Blue Swirl - Oct. 24, 2010, 6:14 p.m.
On Sun, Oct 24, 2010 at 3:34 PM, Avi Kivity <avi@redhat.com> wrote:
> The current ioport callbacks are not type-safe, in that they accept an "opaque"
> pointer as an argument whose type must match the argument to the registration
> function; this is not checked by the compiler.
>
> This patch adds an alternative that is type-safe.  Instead of an opaque
> argument, both registation and the callback use a new IOPort type.  The
> callback then uses container_of() to access its main structures.
>
> Currently the old and new methods exist side by side; once the old way is gone,
> we can also save a bunch of memory since the new method requires one pointer
> per ioport instead of 6.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>

If we are going to change the interface, let's do it so that it's
useful for other uses too:
http://article.gmane.org/gmane.comp.emulators.qemu/76984

> ---
>  ioport.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ioport.h |   16 +++++++++++++++
>  2 files changed, 80 insertions(+), 0 deletions(-)
>
> diff --git a/ioport.c b/ioport.c
> index ec3dc65..47eafc3 100644
> --- a/ioport.c
> +++ b/ioport.c
> @@ -174,6 +174,70 @@ int register_ioport_write(pio_addr_t start, int length, int size,
>     return 0;
>  }
>
> +static uint32_t ioport_readb_thunk(void *opaque, uint32_t addr)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->readb(ioport, addr);
> +}
> +
> +static uint32_t ioport_readw_thunk(void *opaque, uint32_t addr)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->readw(ioport, addr);
> +}
> +
> +static uint32_t ioport_readl_thunk(void *opaque, uint32_t addr)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->readl(ioport, addr);
> +}
> +
> +static void ioport_writeb_thunk(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->writeb(ioport, addr, data);
> +}
> +
> +static void ioport_writew_thunk(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->writew(ioport, addr, data);
> +}
> +
> +static void ioport_writel_thunk(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->writel(ioport, addr, data);
> +}
> +
> +void ioport_register(IOPort *ioport, pio_addr_t start, pio_addr_t length)
> +{
> +    if (ioport->ops->readb) {
> +        register_ioport_read(start, length, 1, ioport_readb_thunk, ioport);
> +    }
> +    if (ioport->ops->readw) {
> +        register_ioport_read(start, length, 2, ioport_readw_thunk, ioport);
> +    }
> +    if (ioport->ops->readl) {
> +        register_ioport_read(start, length, 4, ioport_readl_thunk, ioport);
> +    }
> +    if (ioport->ops->writeb) {
> +        register_ioport_write(start, length, 1, ioport_writeb_thunk, ioport);
> +    }
> +    if (ioport->ops->writew) {
> +        register_ioport_write(start, length, 2, ioport_writew_thunk, ioport);
> +    }
> +    if (ioport->ops->writel) {
> +        register_ioport_write(start, length, 4, ioport_writel_thunk, ioport);
> +    }
> +}
> +
>  void isa_unassign_ioport(pio_addr_t start, int length)
>  {
>     int i;
> diff --git a/ioport.h b/ioport.h
> index 3d3c8a3..8036e59 100644
> --- a/ioport.h
> +++ b/ioport.h
> @@ -36,6 +36,22 @@ typedef uint32_t pio_addr_t;
>  typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
>  typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
>
> +struct IOPort;
> +
> +typedef struct IOPortOps {
> +    uint32_t (*readb)(struct IOPort *ioport, pio_addr_t addr);
> +    uint32_t (*readw)(struct IOPort *ioport, pio_addr_t addr);
> +    uint32_t (*readl)(struct IOPort *ioport, pio_addr_t addr);
> +    void (*writeb)(struct IOPort *ioport, pio_addr_t addr, uint32_t data);
> +    void (*writew)(struct IOPort *ioport, pio_addr_t addr, uint32_t data);
> +    void (*writel)(struct IOPort *ioport, pio_addr_t addr, uint32_t data);
> +} IOPortOps;
> +
> +typedef struct IOPort {
> +    IOPortOps *ops;

const
Avi Kivity - Oct. 25, 2010, 10 a.m.
On 10/24/2010 08:14 PM, Blue Swirl wrote:
> On Sun, Oct 24, 2010 at 3:34 PM, Avi Kivity<avi@redhat.com>  wrote:
> >  The current ioport callbacks are not type-safe, in that they accept an "opaque"
> >  pointer as an argument whose type must match the argument to the registration
> >  function; this is not checked by the compiler.
> >
> >  This patch adds an alternative that is type-safe.  Instead of an opaque
> >  argument, both registation and the callback use a new IOPort type.  The
> >  callback then uses container_of() to access its main structures.
> >
> >  Currently the old and new methods exist side by side; once the old way is gone,
> >  we can also save a bunch of memory since the new method requires one pointer
> >  per ioport instead of 6.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
>
> If we are going to change the interface, let's do it so that it's
> useful for other uses too:
> http://article.gmane.org/gmane.comp.emulators.qemu/76984

I don't really see why we need registration; cpu_register_io() takes 
function pointers, a size, and an opaque, and gives an integer handle in 
return.  With the IOPort object approach, you set up the IOPort with 
function pointers, size is implied, and the opaque is derived using 
container_of(); the handle is simply the address of the object.

> >  +typedef struct IOPort {
> >  +    IOPortOps *ops;
>
> const

Yup, will fix.  Will repost once we agree on the approach.
Juan Quintela - Oct. 25, 2010, 12:54 p.m.
Avi Kivity <avi@redhat.com> wrote:

> +static void ioport_writeb_thunk(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->writeb(ioport, addr, data);
> +}
> +
> +static void ioport_writew_thunk(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->writew(ioport, addr, data);
> +}
> +
> +static void ioport_writel_thunk(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    IOPort *ioport = opaque;
> +
> +    return ioport->ops->writel(ioport, addr, data);
> +}

"return" is not needed on this three functions.

Rest of aproach looks good.

Later, Juan.
Avi Kivity - Oct. 25, 2010, 12:56 p.m.
On 10/25/2010 02:54 PM, Juan Quintela wrote:
> Avi Kivity<avi@redhat.com>  wrote:
>
> >  +static void ioport_writeb_thunk(void *opaque, uint32_t addr, uint32_t data)
> >  +{
> >  +    IOPort *ioport = opaque;
> >  +
> >  +    return ioport->ops->writeb(ioport, addr, data);
> >  +}
> >  +
> >  +static void ioport_writew_thunk(void *opaque, uint32_t addr, uint32_t data)
> >  +{
> >  +    IOPort *ioport = opaque;
> >  +
> >  +    return ioport->ops->writew(ioport, addr, data);
> >  +}
> >  +
> >  +static void ioport_writel_thunk(void *opaque, uint32_t addr, uint32_t data)
> >  +{
> >  +    IOPort *ioport = opaque;
> >  +
> >  +    return ioport->ops->writel(ioport, addr, data);
> >  +}
>
> "return" is not needed on this three functions.
>

Fixed.
Blue Swirl - Oct. 25, 2010, 6:38 p.m.
On Mon, Oct 25, 2010 at 10:00 AM, Avi Kivity <avi@redhat.com> wrote:
>  On 10/24/2010 08:14 PM, Blue Swirl wrote:
>>
>> On Sun, Oct 24, 2010 at 3:34 PM, Avi Kivity<avi@redhat.com>  wrote:
>> >  The current ioport callbacks are not type-safe, in that they accept an
>> > "opaque"
>> >  pointer as an argument whose type must match the argument to the
>> > registration
>> >  function; this is not checked by the compiler.
>> >
>> >  This patch adds an alternative that is type-safe.  Instead of an opaque
>> >  argument, both registation and the callback use a new IOPort type.  The
>> >  callback then uses container_of() to access its main structures.
>> >
>> >  Currently the old and new methods exist side by side; once the old way
>> > is gone,
>> >  we can also save a bunch of memory since the new method requires one
>> > pointer
>> >  per ioport instead of 6.
>> >
>> >  Signed-off-by: Avi Kivity<avi@redhat.com>
>>
>> If we are going to change the interface, let's do it so that it's
>> useful for other uses too:
>> http://article.gmane.org/gmane.comp.emulators.qemu/76984
>
> I don't really see why we need registration; cpu_register_io() takes
> function pointers, a size, and an opaque, and gives an integer handle in
> return.  With the IOPort object approach, you set up the IOPort with
> function pointers, size is implied, and the opaque is derived using
> container_of(); the handle is simply the address of the object.

With the handle, we can separate setting up the structures at device
level, and mapping the object using only the handle at bus or other
higher level. Can this be done with the object approach?

The purpose of that patch series was to perform the separation for PCI
BARs. I wasn't so happy with the series, so I never pushed.
Avi Kivity - Oct. 26, 2010, 8:05 a.m.
On 10/25/2010 08:38 PM, Blue Swirl wrote:
> >
> >  I don't really see why we need registration; cpu_register_io() takes
> >  function pointers, a size, and an opaque, and gives an integer handle in
> >  return.  With the IOPort object approach, you set up the IOPort with
> >  function pointers, size is implied, and the opaque is derived using
> >  container_of(); the handle is simply the address of the object.
>
> With the handle, we can separate setting up the structures at device
> level, and mapping the object using only the handle at bus or other
> higher level. Can this be done with the object approach?

I believe so.  The handle is simply an indirect pointer, no?

> The purpose of that patch series was to perform the separation for PCI
> BARs. I wasn't so happy with the series, so I never pushed.

In fact I think an IOPort is even more suitable; if we need additional 
attributes we can use a derived object:

struct PCIIOPort {
     IOPort ioport;
     /* additional fields */
};
Blue Swirl - Oct. 26, 2010, 3:09 p.m.
On Tue, Oct 26, 2010 at 8:05 AM, Avi Kivity <avi@redhat.com> wrote:
>  On 10/25/2010 08:38 PM, Blue Swirl wrote:
>>
>> >
>> >  I don't really see why we need registration; cpu_register_io() takes
>> >  function pointers, a size, and an opaque, and gives an integer handle
>> > in
>> >  return.  With the IOPort object approach, you set up the IOPort with
>> >  function pointers, size is implied, and the opaque is derived using
>> >  container_of(); the handle is simply the address of the object.
>>
>> With the handle, we can separate setting up the structures at device
>> level, and mapping the object using only the handle at bus or other
>> higher level. Can this be done with the object approach?
>
> I believe so.  The handle is simply an indirect pointer, no?

Yes, but then the object should also contain size information. That
should not be needed for mapping at higher level.

>> The purpose of that patch series was to perform the separation for PCI
>> BARs. I wasn't so happy with the series, so I never pushed.
>
> In fact I think an IOPort is even more suitable; if we need additional
> attributes we can use a derived object:
>
> struct PCIIOPort {
>    IOPort ioport;
>    /* additional fields */
> };

One issue with my series was that it would be great if the devices
just had some BAR structures (used by PCI layer to map the devices)
inside PCI/qdev structures, but I invented that too late. Maybe this
can be addressed in your design?
Avi Kivity - Oct. 26, 2010, 5:18 p.m.
On 10/26/2010 05:09 PM, Blue Swirl wrote:
> On Tue, Oct 26, 2010 at 8:05 AM, Avi Kivity<avi@redhat.com>  wrote:
> >    On 10/25/2010 08:38 PM, Blue Swirl wrote:
> >>
> >>  >
> >>  >    I don't really see why we need registration; cpu_register_io() takes
> >>  >    function pointers, a size, and an opaque, and gives an integer handle
> >>  >  in
> >>  >    return.  With the IOPort object approach, you set up the IOPort with
> >>  >    function pointers, size is implied, and the opaque is derived using
> >>  >    container_of(); the handle is simply the address of the object.
> >>
> >>  With the handle, we can separate setting up the structures at device
> >>  level, and mapping the object using only the handle at bus or other
> >>  higher level. Can this be done with the object approach?
> >
> >  I believe so.  The handle is simply an indirect pointer, no?
>
> Yes, but then the object should also contain size information. That
> should not be needed for mapping at higher level.

Sorry, I don't follow your meaning.

When I said "size is implied" I meant that the IOPort object has a 
separate function pointer for sizes 1, 2, and 4, so it ioport_register() 
doesn't need a size parameter.  But I don't see how that relates to your 
comment.

> >>  The purpose of that patch series was to perform the separation for PCI
> >>  BARs. I wasn't so happy with the series, so I never pushed.
> >
> >  In fact I think an IOPort is even more suitable; if we need additional
> >  attributes we can use a derived object:
> >
> >  struct PCIIOPort {
> >      IOPort ioport;
> >      /* additional fields */
> >  };
>
> One issue with my series was that it would be great if the devices
> just had some BAR structures (used by PCI layer to map the devices)
> inside PCI/qdev structures, but I invented that too late. Maybe this
> can be addressed in your design?

It looks to be orthogonal.  It would be great to have a BAR object; that 
object can then use your API, my API, or the existing API.
Anthony Liguori - Oct. 26, 2010, 5:27 p.m.
On 10/26/2010 12:18 PM, Avi Kivity wrote:
>  On 10/26/2010 05:09 PM, Blue Swirl wrote:
>> On Tue, Oct 26, 2010 at 8:05 AM, Avi Kivity<avi@redhat.com>  wrote:
>> >    On 10/25/2010 08:38 PM, Blue Swirl wrote:
>> >>
>> >> >
>> >> >    I don't really see why we need registration; 
>> cpu_register_io() takes
>> >> >    function pointers, a size, and an opaque, and gives an 
>> integer handle
>> >> >  in
>> >> >    return.  With the IOPort object approach, you set up the 
>> IOPort with
>> >> >    function pointers, size is implied, and the opaque is derived 
>> using
>> >> >    container_of(); the handle is simply the address of the object.
>> >>
>> >>  With the handle, we can separate setting up the structures at device
>> >>  level, and mapping the object using only the handle at bus or other
>> >>  higher level. Can this be done with the object approach?
>> >
>> >  I believe so.  The handle is simply an indirect pointer, no?
>>
>> Yes, but then the object should also contain size information. That
>> should not be needed for mapping at higher level.
>
> Sorry, I don't follow your meaning.
>
> When I said "size is implied" I meant that the IOPort object has a 
> separate function pointer for sizes 1, 2, and 4, so it 
> ioport_register() doesn't need a size parameter.  But I don't see how 
> that relates to your comment.

Yeah, I don't think it makes sense to combine "this is how to dispatch 
I/O" with "this is a region of I/O address space".

I think an IORegion should contain an IOPort structure though.  I think 
the name needs rethinking.

Maybe:

struct PortIOHandler;
struct MemoryIOHandler;

And it would be good to add a memory callback to this series too.

Regards,

Anthony Liguori

>> >>  The purpose of that patch series was to perform the separation 
>> for PCI
>> >>  BARs. I wasn't so happy with the series, so I never pushed.
>> >
>> >  In fact I think an IOPort is even more suitable; if we need 
>> additional
>> >  attributes we can use a derived object:
>> >
>> >  struct PCIIOPort {
>> >      IOPort ioport;
>> >      /* additional fields */
>> >  };
>>
>> One issue with my series was that it would be great if the devices
>> just had some BAR structures (used by PCI layer to map the devices)
>> inside PCI/qdev structures, but I invented that too late. Maybe this
>> can be addressed in your design?
>
> It looks to be orthogonal.  It would be great to have a BAR object; 
> that object can then use your API, my API, or the existing API.
>
Avi Kivity - Oct. 26, 2010, 5:35 p.m.
On 10/26/2010 07:27 PM, Anthony Liguori wrote:
>> Sorry, I don't follow your meaning.
>>
>> When I said "size is implied" I meant that the IOPort object has a 
>> separate function pointer for sizes 1, 2, and 4, so it 
>> ioport_register() doesn't need a size parameter.  But I don't see how 
>> that relates to your comment.
>
>
> Yeah, I don't think it makes sense to combine "this is how to dispatch 
> I/O" with "this is a region of I/O address space".

Oh, so Blue meant the size of the region in ports, not the size of the 
individual ports.  I think that putting the range length (but not base 
address) in the IOPort structure may make sense.

>
> I think an IORegion should contain an IOPort structure though.  I 
> think the name needs rethinking.
>
> Maybe:
>
> struct PortIOHandler;
> struct MemoryIOHandler;

Why two types?  I think some devices use PIO on a PC and MMIO on other 
architectures.  Sharing the type would allow sharing code.
Blue Swirl - Oct. 26, 2010, 6:33 p.m.
On Tue, Oct 26, 2010 at 5:35 PM, Avi Kivity <avi@redhat.com> wrote:
>  On 10/26/2010 07:27 PM, Anthony Liguori wrote:
>>>
>>> Sorry, I don't follow your meaning.
>>>
>>> When I said "size is implied" I meant that the IOPort object has a
>>> separate function pointer for sizes 1, 2, and 4, so it ioport_register()
>>> doesn't need a size parameter.  But I don't see how that relates to your
>>> comment.
>>
>>
>> Yeah, I don't think it makes sense to combine "this is how to dispatch
>> I/O" with "this is a region of I/O address space".
>
> Oh, so Blue meant the size of the region in ports, not the size of the
> individual ports.  I think that putting the range length (but not base
> address) in the IOPort structure may make sense.

Yes, that's what I meant. Consider for example the handlers: they
expect that the port is within some range.

>>
>> I think an IORegion should contain an IOPort structure though.  I think
>> the name needs rethinking.
>>
>> Maybe:
>>
>> struct PortIOHandler;
>> struct MemoryIOHandler;
>
> Why two types?  I think some devices use PIO on a PC and MMIO on other
> architectures.  Sharing the type would allow sharing code.

Then there are the functions provided by rwhandler.c. I think that
interface makes even more sense compared to 8/16/32 (and 64?) bit
handlers in many cases.
Avi Kivity - Oct. 27, 2010, 9:26 a.m.
On 10/26/2010 08:33 PM, Blue Swirl wrote:
> >
> >  Why two types?  I think some devices use PIO on a PC and MMIO on other
> >  architectures.  Sharing the type would allow sharing code.
>
> Then there are the functions provided by rwhandler.c. I think that
> interface makes even more sense compared to 8/16/32 (and 64?) bit
> handlers in many cases.

On the other hand, that makes the transition harder.

Perhaps we can have a type with {read,write}(addr, width), and another 
built on top that provides the traditional {read,write}[bwl](addr) to 
ease the transition.
Blue Swirl - Oct. 27, 2010, 8:23 p.m.
On Wed, Oct 27, 2010 at 9:26 AM, Avi Kivity <avi@redhat.com> wrote:
>  On 10/26/2010 08:33 PM, Blue Swirl wrote:
>>
>> >
>> >  Why two types?  I think some devices use PIO on a PC and MMIO on other
>> >  architectures.  Sharing the type would allow sharing code.
>>
>> Then there are the functions provided by rwhandler.c. I think that
>> interface makes even more sense compared to 8/16/32 (and 64?) bit
>> handlers in many cases.
>
> On the other hand, that makes the transition harder.
>
> Perhaps we can have a type with {read,write}(addr, width), and another built
> on top that provides the traditional {read,write}[bwl](addr) to ease the
> transition.

That should work.

Patch

diff --git a/ioport.c b/ioport.c
index ec3dc65..47eafc3 100644
--- a/ioport.c
+++ b/ioport.c
@@ -174,6 +174,70 @@  int register_ioport_write(pio_addr_t start, int length, int size,
     return 0;
 }
 
+static uint32_t ioport_readb_thunk(void *opaque, uint32_t addr)
+{
+    IOPort *ioport = opaque;
+
+    return ioport->ops->readb(ioport, addr);
+}
+
+static uint32_t ioport_readw_thunk(void *opaque, uint32_t addr)
+{
+    IOPort *ioport = opaque;
+
+    return ioport->ops->readw(ioport, addr);
+}
+
+static uint32_t ioport_readl_thunk(void *opaque, uint32_t addr)
+{
+    IOPort *ioport = opaque;
+
+    return ioport->ops->readl(ioport, addr);
+}
+
+static void ioport_writeb_thunk(void *opaque, uint32_t addr, uint32_t data)
+{
+    IOPort *ioport = opaque;
+
+    return ioport->ops->writeb(ioport, addr, data);
+}
+
+static void ioport_writew_thunk(void *opaque, uint32_t addr, uint32_t data)
+{
+    IOPort *ioport = opaque;
+
+    return ioport->ops->writew(ioport, addr, data);
+}
+
+static void ioport_writel_thunk(void *opaque, uint32_t addr, uint32_t data)
+{
+    IOPort *ioport = opaque;
+
+    return ioport->ops->writel(ioport, addr, data);
+}
+
+void ioport_register(IOPort *ioport, pio_addr_t start, pio_addr_t length)
+{
+    if (ioport->ops->readb) {
+        register_ioport_read(start, length, 1, ioport_readb_thunk, ioport);
+    }
+    if (ioport->ops->readw) {
+        register_ioport_read(start, length, 2, ioport_readw_thunk, ioport);
+    }
+    if (ioport->ops->readl) {
+        register_ioport_read(start, length, 4, ioport_readl_thunk, ioport);
+    }
+    if (ioport->ops->writeb) {
+        register_ioport_write(start, length, 1, ioport_writeb_thunk, ioport);
+    }
+    if (ioport->ops->writew) {
+        register_ioport_write(start, length, 2, ioport_writew_thunk, ioport);
+    }
+    if (ioport->ops->writel) {
+        register_ioport_write(start, length, 4, ioport_writel_thunk, ioport);
+    }
+}
+
 void isa_unassign_ioport(pio_addr_t start, int length)
 {
     int i;
diff --git a/ioport.h b/ioport.h
index 3d3c8a3..8036e59 100644
--- a/ioport.h
+++ b/ioport.h
@@ -36,6 +36,22 @@  typedef uint32_t pio_addr_t;
 typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
 typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
 
+struct IOPort;
+
+typedef struct IOPortOps {
+    uint32_t (*readb)(struct IOPort *ioport, pio_addr_t addr);
+    uint32_t (*readw)(struct IOPort *ioport, pio_addr_t addr);
+    uint32_t (*readl)(struct IOPort *ioport, pio_addr_t addr);
+    void (*writeb)(struct IOPort *ioport, pio_addr_t addr, uint32_t data);
+    void (*writew)(struct IOPort *ioport, pio_addr_t addr, uint32_t data);
+    void (*writel)(struct IOPort *ioport, pio_addr_t addr, uint32_t data);
+} IOPortOps;
+
+typedef struct IOPort {
+    IOPortOps *ops;
+} IOPort;
+
+void ioport_register(IOPort *ioport, pio_addr_t start, pio_addr_t length);
 int register_ioport_read(pio_addr_t start, int length, int size,
                          IOPortReadFunc *func, void *opaque);
 int register_ioport_write(pio_addr_t start, int length, int size,