Re: [Minios-devel] [UNIKRAFT PATCHv4 16/43] uk/arch: Implement ukarch_find_lsbit for Arm64

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Sharan,

> -----Original Message-----
> From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> Sent: 2018年7月12日 21:11
> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxx
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 16/43] uk/arch: Implement
> ukarch_find_lsbit for Arm64
>
> Hello Wei Chen,
>
> Please find my comment(s) inline.
>
> On 07/06/2018 11:03 AM, Wei Chen wrote:
> > This function is ported from Arm32 and is modified to work
> > on Arm64.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >   include/uk/arch/arm/arm64/atomic.h | 64 ++++++++++++++++++++++++++++++
> >   1 file changed, 64 insertions(+)
> >   create mode 100644 include/uk/arch/arm/arm64/atomic.h
> >
> > diff --git a/include/uk/arch/arm/arm64/atomic.h
> b/include/uk/arch/arm/arm64/atomic.h
> > new file mode 100644
> > index 0000000..53f010b
> > --- /dev/null
> > +++ b/include/uk/arch/arm/arm64/atomic.h
> > @@ -0,0 +1,64 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Port from Mini-OS: include/arm/os.h
> > + */
> > +/*
> > + * Copyright (c) 2009 Citrix Systems, Inc. All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> > + * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
> > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> CONSEQUENTIAL
> > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> STRICT
> > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> WAY
> > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE.
> > + */
> > +
> > +#ifndef __UKARCH_ATOMIC_H__
> > +#error Do not include this header directly
> > +#endif
> > +
>
> For finding the lsbit shouldn't we use the builtin function
> __builtin_ffs, __builtin_ffsl?
>

When I was implementing the ukarch_find_lsbit, I found both Arm32 and
X86_64 were not using the builtin-functions. X86_64 is using "bsfq",
Arm32 is using the same instructions as Arm64.

> > +/**
> > + * ukarch_find_lsbit - find first (lowest) set bit in word.
> > + * @word: The word to search
> > + *
> > + * Undefined if no bit exists, so code should check against 0 first.
> > + */
> > +static inline unsigned long ukarch_find_lsbit(unsigned long word)
> > +{
> > +int clz;
> > +
> > +/* xxxxx10000 = word
> > + * xxxxx01111 = word - 1
> > + * 0000011111 = word ^ (word - 1)
> > + *      4     = 63 - clz(word ^ (word - 1))
> > + */
> > +
> > +__asm__("sub x0, %[word], #1\n"
> > +"eor x0, x0, %[word]\n"
> > +"clz %[clz], x0\n"
> > +:
> > +/* Outputs: */
> > +[clz] "=r"(clz)
> > +:
> > +/* Inputs: */
> > +[word] "r"(word)
> > +:
> > +/* Clobbers: */
> > +"x0");
> > +
> > +return 63 - clz;
> > +}
> >
>
> Thanks & Regards
> Sharan
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel