Re: [Qemu-devel] [PATCH v5 17/24] util: add linux bit ordering reversal functions

[Kuo-Jung Su]

2013/3/1 Peter Maydell <address@hidden>:
> On 27 February 2013 07:15, Kuo-Jung Su <address@hidden> wrote:
>> From: Kuo-Jung Su <address@hidden>
>>
>> Some ethernet mac relies on the bit ordering reversal functions
>> to performance the multicast address hash code calculation.
>> So I've ported the bitrev.[ch] from linux kernel into QEMU.
>>
>> Signed-off-by: Kuo-Jung Su <address@hidden>
>> ---
>>  include/qemu/bitrev.h |   25 +++++++++++++++++++++
>>  util/Makefile.objs    |    2 +-
>>  util/bitrev.c         |   59 
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 85 insertions(+), 1 deletion(-)
>>  create mode 100644 include/qemu/bitrev.h
>>  create mode 100644 util/bitrev.c
>>
>> diff --git a/include/qemu/bitrev.h b/include/qemu/bitrev.h
>> new file mode 100644
>> index 0000000..7d570c2
>> --- /dev/null
>> +++ b/include/qemu/bitrev.h
>
> There's no need for a new header just for these three functions:
> put them in include/qemu/bitops.h.
>
> Similarly, the implementations should go in utils/bitops.c.
> There is a minor snag that bitops.[ch] are LGPL2.1+ and this code
> is GPL2, but since LGPL lets you "upgrade" the LGPL code to GPL,
> we can just mark the whole of bitops.[ch] as GPL by updating the
> license statement at the top.
>
> I've cc'd Anthony to advise on the specific mechanics of doing that.
>

I've rewrite the bit ordering functions with byte_rev_table[] removed,
so there is no any update going to utils/bitops.c.
However I have no idea if the license should be update or not,
please check the patch bellow, and giev me some comments please.

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index affcc96..326bdf6 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -3,7 +3,8 @@
  *
  * Copyright (C) 2010 Corentin Chary <address@hidden>
  *
- * Mostly inspired by (stolen from) linux/bitmap.h and linux/bitops.h
+ * Mostly inspired by (stolen from) linux/bitmap.h, linux/bitops.h
+ * and linux/bitrev.h
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
  * See the COPYING.LIB file in the top-level directory.
@@ -273,4 +274,50 @@ static inline uint64_t deposit64(uint64_t value,
int start, int length,
     return (value & ~mask) | ((fieldval << start) & mask);
 }

+/**
+ * bitrev8:
+ * @value: the value to reverse bit ordering from
+ *
+ * Reverse the 8 bit input @value
+ *
+ * Returns: the input @value with reversed bit ordering
+ */
+static inline uint8_t bitrev8(uint8_t value)
+{
+    uint8_t ret = 0;
+    int i;
+    for (i = 0; i < 8; ++i) {
+        if (value & BIT(i)) {
+            ret |= BIT(7 - i);
+        }
+    }
+    return ret;
+}
+
+/**
+ * bitrev16:
+ * @value: the value to reverse bit ordering from
+ *
+ * Reverse the 16 bit input @value
+ *
+ * Returns: the input @value with reversed bit ordering
+ */
+static inline uint16_t bitrev16(uint16_t value)
+{
+    return (bitrev8(value & 0xff) << 8) | bitrev8(value >> 8);
+}
+
+/**
+ * bitrev32:
+ * @value: the value to reverse bit ordering from
+ *
+ * Reverse the 32 bit input @value
+ *
+ * Returns: the input @value with reversed bit ordering
+ */
+static inline uint32_t bitrev32(uint32_t value)
+{
+    return (bitrev16(value & 0xffff) << 16) | bitrev16(value >> 16);
+}
+
 #endif

>> @@ -0,0 +1,25 @@
>> +/*
>> + * Bit ordering reversal functions (From 
>> linux-kernel/include/linux/bitrev.h)
>> + *
>> + * Written by Akinobu Mita <address@hidden>
>> + * Ported to QEMU by Kuo-Jung Su <address@hidden>
>> + *
>> + * This code is licensed under GNU GPL
>> + */
>> +
>> +#ifndef BITREV_H
>> +#define BITREV_H
>> +
>> +#include "qemu-common.h"
>> +
>> +extern uint8_t const byte_rev_table[256];
>> +
>> +static inline uint8_t bitrev8(uint8_t byte)
>> +{
>> +    return byte_rev_table[byte];
>> +}
>> +
>> +extern uint16_t bitrev16(uint16_t in);
>> +extern uint32_t bitrev32(uint32_t in);
>
> Please provide documentation comments for all these functions
> (in the header file, not in the .c file).
>

Got it, thanks.

> thanks
> -- PMM



--
Best wishes,
Kuo-Jung Su