* [PATCH v2] hw/misc: use extract64 instead of 1 << i
@ 2024-12-27 10:46 Tigran Sogomonian
2024-12-27 12:37 ` Alex Bennée
2024-12-27 15:16 ` Richard Henderson
0 siblings, 2 replies; 6+ messages in thread
From: Tigran Sogomonian @ 2024-12-27 10:46 UTC (permalink / raw)
To: peter.maydell, qemu-arm, qemu-devel, sdl.qemu; +Cc: Tigran Sogomonian
1 << i is casted to uint64_t while bitwise and with val.
So this value may become 0xffffffff80000000 but only
31th "start" bit is required.
Use the bitfield extract() API instead.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
---
hw/misc/mps2-fpgaio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
index d07568248d..04a3da5db0 100644
--- a/hw/misc/mps2-fpgaio.c
+++ b/hw/misc/mps2-fpgaio.c
@@ -198,7 +198,7 @@ static void mps2_fpgaio_write(void *opaque, hwaddr offset, uint64_t value,
s->led0 = value & MAKE_64BIT_MASK(0, s->num_leds);
for (i = 0; i < s->num_leds; i++) {
- led_set_state(s->led[i], value & (1 << i));
+ led_set_state(s->led[i], extract64(value, i, 1));
}
}
break;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/misc: use extract64 instead of 1 << i
2024-12-27 10:46 [PATCH v2] hw/misc: use extract64 instead of 1 << i Tigran Sogomonian
@ 2024-12-27 12:37 ` Alex Bennée
2024-12-27 15:16 ` Richard Henderson
1 sibling, 0 replies; 6+ messages in thread
From: Alex Bennée @ 2024-12-27 12:37 UTC (permalink / raw)
To: Tigran Sogomonian; +Cc: peter.maydell, qemu-arm, qemu-devel, sdl.qemu
Tigran Sogomonian <tsogomonian@astralinux.ru> writes:
> 1 << i is casted to uint64_t while bitwise and with val.
> So this value may become 0xffffffff80000000 but only
> 31th "start" bit is required.
> Use the bitfield extract() API instead.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] hw/misc: use extract64 instead of 1 << i
2024-12-27 10:46 [PATCH v2] hw/misc: use extract64 instead of 1 << i Tigran Sogomonian
2024-12-27 12:37 ` Alex Bennée
@ 2024-12-27 15:16 ` Richard Henderson
2025-02-04 12:20 ` Тигран Согомонян
2025-02-04 12:24 ` Тигран Согомонян
1 sibling, 2 replies; 6+ messages in thread
From: Richard Henderson @ 2024-12-27 15:16 UTC (permalink / raw)
To: Tigran Sogomonian, peter.maydell, qemu-arm, qemu-devel, sdl.qemu
On 12/27/24 02:46, Tigran Sogomonian wrote:
> 1 << i is casted to uint64_t while bitwise and with val.
> So this value may become 0xffffffff80000000 but only
> 31th "start" bit is required.
> Use the bitfield extract() API instead.
Again, I < 32. There is no overflow. The type of value is irrelevant.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
> ---
> hw/misc/mps2-fpgaio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
> index d07568248d..04a3da5db0 100644
> --- a/hw/misc/mps2-fpgaio.c
> +++ b/hw/misc/mps2-fpgaio.c
> @@ -198,7 +198,7 @@ static void mps2_fpgaio_write(void *opaque, hwaddr offset, uint64_t value,
>
> s->led0 = value & MAKE_64BIT_MASK(0, s->num_leds);
> for (i = 0; i < s->num_leds; i++) {
> - led_set_state(s->led[i], value & (1 << i));
> + led_set_state(s->led[i], extract64(value, i, 1));
> }
> }
> break;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/misc: use extract64 instead of 1 << i
2024-12-27 15:16 ` Richard Henderson
@ 2025-02-04 12:20 ` Тигран Согомонян
2025-02-04 12:24 ` Тигран Согомонян
1 sibling, 0 replies; 6+ messages in thread
From: Тигран Согомонян @ 2025-02-04 12:20 UTC (permalink / raw)
To: Richard Henderson, peter.maydell, qemu-arm, qemu-devel, sdl.qemu
27/12/24 18:16, Richard Henderson пишет:
> On 12/27/24 02:46, Tigran Sogomonian wrote:
>> 1 << i is casted to uint64_t while bitwise and with val.
>> So this value may become 0xffffffff80000000 but only
>> 31th "start" bit is required.
>> Use the bitfield extract() API instead.
>
> Again, I < 32. There is no overflow. The type of value is irrelevant.
>
s->num_leds maximum MPS2FPGAIO_MAX_LEDS (32). For bitwise AND, the
result of shift 1 << i will be converted to uint64_t because value is of
type uint64_t (integer promotion) and for i = 31 instead of the expected
0b10000000000000000000000000000000 we get
0b1111111111111111111111111111111100000000000000000000000000000000. Need
to be corrected to value & (1U << i).
>
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
>> ---
>> hw/misc/mps2-fpgaio.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
>> index d07568248d..04a3da5db0 100644
>> --- a/hw/misc/mps2-fpgaio.c
>> +++ b/hw/misc/mps2-fpgaio.c
>> @@ -198,7 +198,7 @@ static void mps2_fpgaio_write(void *opaque,
>> hwaddr offset, uint64_t value,
>> s->led0 = value & MAKE_64BIT_MASK(0, s->num_leds);
>> for (i = 0; i < s->num_leds; i++) {
>> - led_set_state(s->led[i], value & (1 << i));
>> + led_set_state(s->led[i], extract64(value, i, 1));
>> }
>> }
>> break;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/misc: use extract64 instead of 1 << i
2024-12-27 15:16 ` Richard Henderson
2025-02-04 12:20 ` Тигран Согомонян
@ 2025-02-04 12:24 ` Тигран Согомонян
2025-03-11 12:56 ` Тигран Согомонян
1 sibling, 1 reply; 6+ messages in thread
From: Тигран Согомонян @ 2025-02-04 12:24 UTC (permalink / raw)
To: Richard Henderson, peter.maydell, qemu-arm, qemu-devel, sdl.qemu
27/12/24 18:16, Richard Henderson пишет:
> On 12/27/24 02:46, Tigran Sogomonian wrote:
>> 1 << i is casted to uint64_t while bitwise and with val.
>> So this value may become 0xffffffff80000000 but only
>> 31th "start" bit is required.
>> Use the bitfield extract() API instead.
>
> Again, I < 32. There is no overflow. The type of value is irrelevant.
>
s->num_leds maximum MPS2FPGAIO_MAX_LEDS (32). For bitwise AND,
the result of shift 1 << i will be converted to uint64_t because
value is of type uint64_t (integer promotion) and for i = 31 instead of
the expected 0b10000000000000000000000000000000 we get
0b1111111111111111111111111111111100000000000000000000000000000000.
Need to be corrected to value & (1U << i).
>
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>
>> Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
>> ---
>> hw/misc/mps2-fpgaio.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
>> index d07568248d..04a3da5db0 100644
>> --- a/hw/misc/mps2-fpgaio.c
>> +++ b/hw/misc/mps2-fpgaio.c
>> @@ -198,7 +198,7 @@ static void mps2_fpgaio_write(void *opaque,
>> hwaddr offset, uint64_t value,
>> s->led0 = value & MAKE_64BIT_MASK(0, s->num_leds);
>> for (i = 0; i < s->num_leds; i++) {
>> - led_set_state(s->led[i], value & (1 << i));
>> + led_set_state(s->led[i], extract64(value, i, 1));
>> }
>> }
>> break;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] hw/misc: use extract64 instead of 1 << i
2025-02-04 12:24 ` Тигран Согомонян
@ 2025-03-11 12:56 ` Тигран Согомонян
0 siblings, 0 replies; 6+ messages in thread
From: Тигран Согомонян @ 2025-03-11 12:56 UTC (permalink / raw)
To: Richard Henderson, peter.maydell, qemu-arm, qemu-devel, sdl.qemu
04/02/25 15:24, Тигран Согомонян пишет:
> 27/12/24 18:16, Richard Henderson пишет:
>> On 12/27/24 02:46, Tigran Sogomonian wrote:
>>> 1 << i is casted to uint64_t while bitwise and with val.
>>> So this value may become 0xffffffff80000000 but only
>>> 31th "start" bit is required.
>>> Use the bitfield extract() API instead.
>>
>> Again, I < 32. There is no overflow. The type of value is irrelevant.
>>
> s->num_leds maximum MPS2FPGAIO_MAX_LEDS (32). For bitwise AND,
> the result of shift 1 << i will be converted to uint64_t because
> value is of type uint64_t (integer promotion) and for i = 31 instead of
> the expected 0b10000000000000000000000000000000 we get
> 0b1111111111111111111111111111111100000000000000000000000000000000.
> Need to be corrected to value & (1U << i).
>>
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>
>>> Signed-off-by: Tigran Sogomonian <tsogomonian@astralinux.ru>
>>> ---
>>> hw/misc/mps2-fpgaio.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
>>> index d07568248d..04a3da5db0 100644
>>> --- a/hw/misc/mps2-fpgaio.c
>>> +++ b/hw/misc/mps2-fpgaio.c
>>> @@ -198,7 +198,7 @@ static void mps2_fpgaio_write(void *opaque,
>>> hwaddr offset, uint64_t value,
>>> s->led0 = value & MAKE_64BIT_MASK(0, s->num_leds);
>>> for (i = 0; i < s->num_leds; i++) {
>>> - led_set_state(s->led[i], value & (1 << i));
>>> + led_set_state(s->led[i], extract64(value, i, 1));
>>> }
>>> }
>>> break;
>>
>>
>
>
Just a friendly reminder)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-11 12:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-27 10:46 [PATCH v2] hw/misc: use extract64 instead of 1 << i Tigran Sogomonian
2024-12-27 12:37 ` Alex Bennée
2024-12-27 15:16 ` Richard Henderson
2025-02-04 12:20 ` Тигран Согомонян
2025-02-04 12:24 ` Тигран Согомонян
2025-03-11 12:56 ` Тигран Согомонян
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.