On 25.11.20 09:48, Andrea Bastoni wrote:
> On 25/11/2020 07:07, Jan Kiszka wrote:
>> On 23.11.20 21:46, Andrea Bastoni wrote:
>>> Signed-off-by: Andrea Bastoni <[email protected]>
>>> ---
>>>  hypervisor/arch/arm-common/include/asm/bitops.h | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/hypervisor/arch/arm-common/include/asm/bitops.h 
>>> b/hypervisor/arch/arm-common/include/asm/bitops.h
>>> index 808c9a0f..a726862f 100644
>>> --- a/hypervisor/arch/arm-common/include/asm/bitops.h
>>> +++ b/hypervisor/arch/arm-common/include/asm/bitops.h
>>> @@ -31,6 +31,7 @@ static inline unsigned long clz(unsigned long word)
>>>  /* Returns the position of the least significant 1, MSB=31, LSB=0*/
>>>  static inline unsigned long ffsl(unsigned long word)
>>>  {
>>> +   // FIXME: the ffsl on x86 isn't robust.
>>
>> Can you elaborate?
> 
> There's an imbalance between the ffsl on arm and the ffsl on x86. The one on 
> x86
> doesn't check whether word is 0, this one does.
> 
> I haven't yet had time to check the callers and make sure that they will never
> use 0.
> 
> I put the FIXME there before I implemented the assert(). Actually it could be
> now possibile to do
> 
> unsigned long ffsl(unsigned long word)
> {
>     assert(word != 0);
>     ...
> }
> 
> And remove the check on arm.

Indeed, that check on arm looks dangerous.

> 
> Furthermore it is maybe possible to get rid of the __bultin_ffsl() and use
> jailhouse own ffsl() implementation.

I do not recall anymore - and our beloved git history isn't helpful on
this either - why we migrated in 2014 from __builtin_ff* to the assembly
versions. Maybe compiler issues? We still have __builtin_ffs[l] in
hypervisor/include/jailhouse/mmio.h, which makes more sense for
constants. So there is room for a cleanup.

All unrelated to this patch, though.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/be564959-e20a-65c8-0d9e-e9481380559d%40siemens.com.

Reply via email to