[SeaBIOS] [PATCH 07/18] virtio: add version 1.0 read/write macros

[Kevin O'Connor]

On Mon, Jun 29, 2015 at 03:46:54PM +0200, Gerd Hoffmann wrote:
> On Mo, 2015-06-29 at 09:02 -0400, Kevin O'Connor wrote:
> > On Mon, Jun 29, 2015 at 10:53:29AM +0200, Gerd Hoffmann wrote:
> > > Add macros to read/write registers of virtio-1.0 regions.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
> > > ---
> > >  src/hw/virtio-pci.h | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 76 insertions(+)
> > > 
> > > diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h
> > > index 893a7dd..e1d8b3e 100644
> > > --- a/src/hw/virtio-pci.h
> > > +++ b/src/hw/virtio-pci.h
> > > @@ -111,6 +111,82 @@ struct vp_device {
> > >      struct vp_cap common, notify, isr, device;
> > >  };
> > >  
> > > +#define vp_modern_read(_cap, _struct, _field, _var) {                   \
> > > +        u32 addr = _cap.addr;                                           \
> > > +        addr += offsetof(_struct, _field);                              \
> > 
> > Wouldn't this make more sense if the bulk of the code was in a
> > function?
> 
> The idea is that 'sizeof((_struct *)0)->_field)' evaluates to a
> compile-time constant, so gcc can optimize away the bulk of the #define.

Would it matter though?  Are any of these accesses performance
sensitive?  Also, if "_vp_modern_read()" was marked as inline wouldn't
gcc ultimately produce the same thing?

> Also we don't have to use u64 for _var then (except when it actually is
> a 64bit value).
> 
> But having 'vp_modern_read(..., var)' in the code instead of 'var =
> vp_modern_read(...)' isn't that nice indeed.

Hrmm.  That's an interesting question - how well would gcc do if
_vp_modern_read() was inline'd and returned a 64bit value that was
then immediately truncated in the majority of cases.

-Kevin