Skip to content

Commit 13d354e

Browse files
wuxyintellijinxia
authored andcommitted
HV:treewide:Update return type for bit operations fls and clz
Change the return type of function fls and clz as uint16_t; When the input is zero, INVALID_BIT_INDEX is returned; Update temporary variable type and return value check of caller when it call fls or clz; When input value is zero, clz returns 32 directly. V1-->V2: INVALID_BIT_INDEX instead of INVALID_NUMBER; Add type conversion as needed; Add "U/UL" for constant value as needed; Codeing style fixing. V2-->V3: Use type conversion to remove side effect of the variable which stores fls/clz return value; fls return INVALID_BIT_INDEX directly when the input value is zero. V3-->v4: Clean up comments for fls. Note: For instruction "bsrl", destination register value is undefined when source register value is zero. Signed-off-by: Xiangyang Wu <xiangyang.wu@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
1 parent 4110f3a commit 13d354e

File tree

3 files changed

+41
-23
lines changed

3 files changed

+41
-23
lines changed

hypervisor/arch/x86/guest/vlapic.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -767,25 +767,26 @@ vlapic_process_eoi(struct vlapic *vlapic)
767767
{
768768
struct lapic_regs *lapic = vlapic->apic_page;
769769
struct lapic_reg *isrptr, *tmrptr;
770-
int i, bitpos, vector;
770+
int i, vector;
771+
uint16_t bitpos;
771772

772773
isrptr = &lapic->isr[0];
773774
tmrptr = &lapic->tmr[0];
774775

775776
for (i = 7; i >= 0; i--) {
776777
bitpos = fls(isrptr[i].val);
777-
if (bitpos >= 0) {
778+
if (bitpos != INVALID_BIT_INDEX) {
778779
if (vlapic->isrvec_stk_top <= 0) {
779780
panic("invalid vlapic isrvec_stk_top %d",
780781
vlapic->isrvec_stk_top);
781782
}
782-
isrptr[i].val &= ~(1 << bitpos);
783-
vector = i * 32 + bitpos;
783+
isrptr[i].val &= ~(1U << (uint32_t)bitpos);
784+
vector = i * 32 + (int32_t)bitpos;
784785
dev_dbg(ACRN_DBG_LAPIC, "EOI vector %d", vector);
785786
VLAPIC_CTR_ISR(vlapic, "vlapic_process_eoi");
786787
vlapic->isrvec_stk_top--;
787788
vlapic_update_ppr(vlapic);
788-
if ((tmrptr[i].val & (1 << bitpos)) != 0) {
789+
if ((tmrptr[i].val & (1U << (uint32_t)bitpos)) != 0U) {
789790
/* hook to vIOAPIC */
790791
vioapic_process_eoi(vlapic->vm, vector);
791792
}
@@ -1131,7 +1132,8 @@ int
11311132
vlapic_pending_intr(struct vlapic *vlapic, uint32_t *vecptr)
11321133
{
11331134
struct lapic_regs *lapic = vlapic->apic_page;
1134-
int i, bitpos;
1135+
int i;
1136+
uint16_t bitpos;
11351137
uint32_t vector;
11361138
uint32_t val;
11371139
struct lapic_reg *irrptr;
@@ -1144,8 +1146,8 @@ vlapic_pending_intr(struct vlapic *vlapic, uint32_t *vecptr)
11441146
for (i = 7; i >= 0; i--) {
11451147
val = atomic_load((int *)&irrptr[i].val);
11461148
bitpos = fls(val);
1147-
if (bitpos >= 0) {
1148-
vector = i * 32 + bitpos;
1149+
if (bitpos != INVALID_BIT_INDEX) {
1150+
vector = (uint32_t)(i * 32) + (uint32_t)bitpos;
11491151
if (PRIO(vector) > PRIO(lapic->ppr)) {
11501152
if (vecptr != NULL)
11511153
*vecptr = vector;

hypervisor/include/lib/bits.h

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,35 +31,47 @@
3131
#define BITS_H
3232

3333
#define BUS_LOCK "lock ; "
34+
/**
35+
*
36+
* INVALID_BIT_INDEX means when input paramter is zero,
37+
* bit operations function can't find bit set and return
38+
* the invalid bit index directly.
39+
*
40+
**/
41+
#define INVALID_BIT_INDEX 0xffffU
3442

3543
/**
3644
*
3745
* fls - Find the Last (most significant) bit Set in value and
3846
* return the bit index of that bit.
3947
*
4048
* Bits are numbered starting at 0,the least significant bit.
41-
* A return value of -1 means that the argument was zero.
49+
* A return value of INVALID_BIT_INDEX means return value is
50+
* invalid bit index when the input argument was zero.
4251
*
4352
* Examples:
44-
* fls (0x0) = -1
53+
* fls (0x0) = INVALID_BIT_INDEX
4554
* fls (0x01) = 0
46-
* fls (0xf0) = 7
55+
* fls (0x80) = 7
4756
* ...
4857
* fls (0x80000001) = 31
4958
*
50-
* @param value: 'unsigned int' type value
59+
* @param value: 'uint32_t' type value
5160
*
52-
* @return value: zero-based bit index, -1 means 'value' was zero.
61+
* @return value: zero-based bit index, INVALID_BIT_INDEX means
62+
* when 'value' was zero, bit operations function can't find bit
63+
* set and return the invalid bit index directly.
5364
*
5465
* **/
55-
static inline int fls(unsigned int value)
66+
static inline uint16_t fls(uint32_t value)
5667
{
57-
int ret;
58-
68+
uint32_t ret = 0U;
69+
if (value == 0U)
70+
return (INVALID_BIT_INDEX);
5971
asm volatile("bsrl %1,%0"
6072
: "=r" (ret)
61-
: "rm" (value), "0" (-1));
62-
return ret;
73+
: "rm" (value));
74+
return (uint16_t)ret;
6375
}
6476

6577
static inline int fls64(unsigned long value)
@@ -124,9 +136,13 @@ static inline int ffz64(unsigned long value)
124136
*
125137
* @return The number of leading zeros in 'value'.
126138
*/
127-
static inline int clz(unsigned int value)
139+
static inline uint16_t clz(uint32_t value)
128140
{
129-
return (31 - fls(value));
141+
if (value == 0U)
142+
return 32U;
143+
else{
144+
return (31U - fls(value));
145+
}
130146
}
131147

132148
/**

hypervisor/lib/div.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ static int do_udiv32(uint32_t dividend, uint32_t divisor,
1616
* are valid * clz(dividend)<=clz(divisor)
1717
*/
1818

19-
mask = clz(divisor) - clz(dividend);
19+
mask = (uint32_t)(clz(divisor) - clz(dividend));
2020
/* align divisor and dividend */
2121
divisor <<= mask;
2222
mask = 1U << mask;
@@ -26,8 +26,8 @@ static int do_udiv32(uint32_t dividend, uint32_t divisor,
2626
dividend -= divisor;
2727
res->q.dwords.low |= mask;
2828
}
29-
divisor >>= 1;
30-
} while (((mask >>= 1) != 0) && (dividend != 0));
29+
divisor >>= 1U;
30+
} while (((mask >>= 1U) != 0U) && (dividend != 0U));
3131
/* dividend now contains the reminder */
3232
res->r.dwords.low = dividend;
3333
return 0;

0 commit comments

Comments
 (0)