Re: [Xen-devel] [PATCH v3 1/2] atomic: add atomic_and operations

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 24, 2020 at 10:19:44AM +0000, Julien Grall wrote:
> Hi,
> 
> On 24/02/2020 10:09, Roger Pau Monné wrote:
> > On Mon, Feb 24, 2020 at 10:02:53AM +0000, Julien Grall wrote:
> > > Hi Roger,
> > > 
> > > The logic for Arm64 and Arm32 looks good to me. I just have one question
> > > below.
> > > 
> > > On 24/02/2020 08:43, Roger Pau Monne wrote:
> > > > To x86 and Arm. This performs an atomic AND operation against an
> > > > atomic_t variable with the provided mask.
> > > > 
> > > > Requested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > > ---
> > > >    xen/include/asm-arm/arm32/atomic.h | 17 +++++++++++++++++
> > > >    xen/include/asm-arm/arm64/atomic.h | 14 ++++++++++++++
> > > >    xen/include/asm-x86/atomic.h       |  8 ++++++++
> > > >    3 files changed, 39 insertions(+)
> > > > 
> > > > diff --git a/xen/include/asm-arm/arm32/atomic.h 
> > > > b/xen/include/asm-arm/arm32/atomic.h
> > > > index c03eb684cd..4637381bcc 100644
> > > > --- a/xen/include/asm-arm/arm32/atomic.h
> > > > +++ b/xen/include/asm-arm/arm32/atomic.h
> > > > @@ -96,6 +96,23 @@ static inline int atomic_sub_return(int i, atomic_t 
> > > > *v)
> > > >         return result;
> > > >    }
> > > > +static inline void atomic_and(unsigned int m, atomic_t *v)
> > > 
> > > All the atomic helpers have taken a signed int so far because the counter 
> > > is
> > > an int. Any reason to diverge from that?
> > 
> > Since this is not an arithmetic operation I felt unsigned int was a
> > more suitable type to describe a bitmask: it felt weird to pass a
> > bitmask with type int, because signedness doesn't make sense when
> > referring to a mask.
> 
> At some point I would like to have macro generating all the atomics in on
> Arm in the same way a Linux (see asm-generic/atomic.h). This is to avoid
> duplication and make easy to introduce Armv8.1 LSE atomics. So I would like
> to avoid introducing difference in the prototype unless it is stricly
> necessary.

Why not have the macro generator function get passed the type of the
parameter?

Easily enough you could even have a second wrapper around that
generator macro, ie: GEN_ATOMIC_{ARITHMETIC/BITMASK} or some such that
hides the parameter type underneath.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel