Re: [Qemu-devel] [PATCH] Better support for dma_addr_t variables

[David Gibson]

On Fri, Mar 30, 2012 at 11:34:25AM +0200, Andreas Färber wrote:
> Am 30.03.2012 11:32, schrieb Andreas Färber:
> > Am 27.03.2012 04:43, schrieb David Gibson:
> >> A while back, we introduced the dma_addr_t type, which is supposed to
> >> be used for bus visible memory addresses.  At present, this is an
> >> alias for target_phys_addr_t, but this will change when we eventually
> >> add support for guest visible IOMMUs.
> >>
> >> There are some instances of target_phys_addr_t in the code now which
> >> should really be dma_addr_t, but can't be trivially converted due to
> >> missing features which this patch corrects.
> >>
> >>  * We add DMA_ADDR_BITS analagous to TARGET_PHYS_ADDR_BITS.  This is
> >>    important where we need to make a compile-time (#if) based on the
> >>    size of dma_addr_t.
> >>
> >>  * We add a new helper macro to create device properties which take a
> >>    dma_addr_t, currently an alias to DEFINE_PROP_TADDR().
> >>
> >> Signed-off-by: David Gibson <address@hidden>
> >> ---
> >>  dma.h         |    1 +
> >>  hw/qdev-dma.h |    4 ++++
> >>  2 files changed, 5 insertions(+), 0 deletions(-)
> >>  create mode 100644 hw/qdev-dma.h
> >>
> >> diff --git a/dma.h b/dma.h
> >> index 05ac325..463095c 100644
> >> --- a/dma.h
> >> +++ b/dma.h
> >> @@ -32,6 +32,7 @@ struct QEMUSGList {
> >>  #if defined(TARGET_PHYS_ADDR_BITS)
> >>  typedef target_phys_addr_t dma_addr_t;
> >>  
> >> +#define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
> >>  #define DMA_ADDR_FMT TARGET_FMT_plx
> >>  
> >>  struct ScatterGatherEntry {
> >> diff --git a/hw/qdev-dma.h b/hw/qdev-dma.h
> >> new file mode 100644
> >> index 0000000..e407771
> >> --- /dev/null
> >> +++ b/hw/qdev-dma.h
> >> @@ -0,0 +1,4 @@
> >> +#include "qdev-addr.h"
> >> +
> >> +#define DEFINE_PROP_DMAADDR(_n, _s, _f, _d)                               
> >> \
> >> +    DEFINE_PROP_TADDR(_n, _s, _f, _d)
> > 
> > Is a new header just for this really needed? It's not being used in this
> > patch, so its necessity is hard to judge. ;)
> 
> Additionally it's missing a license notice.

Just like qdev-addr.h.  And qdev.h for that matter.

You seriously want a license notice for two lines of trivial macro?

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson