* [PATCH] target/ppc: Fix 64-bit decrementer
@ 2021-09-13 16:27 Cédric Le Goater
2021-09-13 17:07 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 7+ messages in thread
From: Cédric Le Goater @ 2021-09-13 16:27 UTC (permalink / raw)
To: David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
The current way the mask is built can overflow with a 64-bit decrementer.
Use MAKE_64BIT_MASK instead.
Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
This was found with the QEMU Microwatt machine which uses a 64bit
decrementer. Here is an experimental tree:
https://github.com/legoater/qemu/tree/microwatt
hw/ppc/ppc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 7375bf4fa910..a86125c50ff9 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -876,7 +876,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
bool negative;
/* Truncate value to decr_width and sign extend for simplicity */
- value &= ((1ULL << nr_bits) - 1);
+ value &= MAKE_64BIT_MASK(0, nr_bits);
negative = !!(value & (1ULL << (nr_bits - 1)));
if (negative) {
value |= (0xFFFFFFFFULL << nr_bits);
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix 64-bit decrementer
2021-09-13 16:27 [PATCH] target/ppc: Fix 64-bit decrementer Cédric Le Goater
@ 2021-09-13 17:07 ` Philippe Mathieu-Daudé
2021-09-13 17:22 ` Luis Fernando Fujita Pires
0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-13 17:07 UTC (permalink / raw)
To: Cédric Le Goater, David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel
On 9/13/21 6:27 PM, Cédric Le Goater wrote:
> The current way the mask is built can overflow with a 64-bit decrementer.
> Use MAKE_64BIT_MASK instead.
>
> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> This was found with the QEMU Microwatt machine which uses a 64bit
> decrementer. Here is an experimental tree:
>
> https://github.com/legoater/qemu/tree/microwatt
>
> hw/ppc/ppc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 7375bf4fa910..a86125c50ff9 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -876,7 +876,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
> bool negative;
>
> /* Truncate value to decr_width and sign extend for simplicity */
> - value &= ((1ULL << nr_bits) - 1);
> + value &= MAKE_64BIT_MASK(0, nr_bits);
What about:
value = extract64(value, 0, nr_bits);
if (value != sextract64(value, 0, nr_bits)) { ...
> negative = !!(value & (1ULL << (nr_bits - 1)));
> if (negative) {
> value |= (0xFFFFFFFFULL << nr_bits);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] target/ppc: Fix 64-bit decrementer
2021-09-13 17:07 ` Philippe Mathieu-Daudé
@ 2021-09-13 17:22 ` Luis Fernando Fujita Pires
2021-09-13 17:29 ` Luis Fernando Fujita Pires
0 siblings, 1 reply; 7+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-09-13 17:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Cédric Le Goater, David Gibson, Greg Kurz
Cc: qemu-ppc, qemu-devel
> > bool negative;
> >
> > /* Truncate value to decr_width and sign extend for simplicity */
> > - value &= ((1ULL << nr_bits) - 1);
> > + value &= MAKE_64BIT_MASK(0, nr_bits);
>
> What about:
>
> value = extract64(value, 0, nr_bits);
> if (value != sextract64(value, 0, nr_bits)) { ...
Or:
value = extract64(value, 0, nr_bits);
value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);
Also avoiding the problem with an invalid 64-bit shift with:
> > value |= (0xFFFFFFFFULL << nr_bits);
--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] target/ppc: Fix 64-bit decrementer
2021-09-13 17:22 ` Luis Fernando Fujita Pires
@ 2021-09-13 17:29 ` Luis Fernando Fujita Pires
2021-09-13 18:05 ` Luis Fernando Fujita Pires
0 siblings, 1 reply; 7+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-09-13 17:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Cédric Le Goater, David Gibson, Greg Kurz
Cc: qemu-ppc, qemu-devel
> value = extract64(value, 0, nr_bits);
> value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);
Oops, sorry. 64 might not be correct here. It would depend on the target being either 32 or 64.
--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] target/ppc: Fix 64-bit decrementer
2021-09-13 17:29 ` Luis Fernando Fujita Pires
@ 2021-09-13 18:05 ` Luis Fernando Fujita Pires
2021-09-13 19:55 ` Cédric Le Goater
2021-09-13 20:19 ` Peter Maydell
0 siblings, 2 replies; 7+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-09-13 18:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Cédric Le Goater, David Gibson, Greg Kurz
Cc: qemu-ppc, qemu-devel
> > value = extract64(value, 0, nr_bits);
> > value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);
>
> Oops, sorry. 64 might not be correct here. It would depend on the target being
> either 32 or 64.
In fact, sextract already does the sign extension, so this should be all that's needed, right?
value = sextract<32,64>(value, 0, nr_bits);
--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix 64-bit decrementer
2021-09-13 18:05 ` Luis Fernando Fujita Pires
@ 2021-09-13 19:55 ` Cédric Le Goater
2021-09-13 20:19 ` Peter Maydell
1 sibling, 0 replies; 7+ messages in thread
From: Cédric Le Goater @ 2021-09-13 19:55 UTC (permalink / raw)
To: Luis Fernando Fujita Pires, Philippe Mathieu-Daudé,
David Gibson, Greg Kurz
Cc: qemu-ppc, qemu-devel
On 9/13/21 8:05 PM, Luis Fernando Fujita Pires wrote:
>>> value = extract64(value, 0, nr_bits);
>>> value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);
>>
>> Oops, sorry. 64 might not be correct here. It would depend on the target being
>> either 32 or 64.
>
> In fact, sextract already does the sign extension, so this should be all that's needed, right?
> value = sextract<32,64>(value, 0, nr_bits);
I am fine with any solution ! Please give a try to this machine :
https://github.com/legoater/qemu/tree/microwatt
It's the only one with a 64 bit decrementer :)
(We should come up with a simpler test case)
Thanks,
C.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] target/ppc: Fix 64-bit decrementer
2021-09-13 18:05 ` Luis Fernando Fujita Pires
2021-09-13 19:55 ` Cédric Le Goater
@ 2021-09-13 20:19 ` Peter Maydell
1 sibling, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2021-09-13 20:19 UTC (permalink / raw)
To: Luis Fernando Fujita Pires
Cc: Philippe Mathieu-Daudé,
qemu-devel, Greg Kurz, qemu-ppc, Cédric Le Goater,
David Gibson
On Mon, 13 Sept 2021 at 19:09, Luis Fernando Fujita Pires
<luis.pires@eldorado.org.br> wrote:
>
> > > value = extract64(value, 0, nr_bits);
> > > value = ((target_long)value << (64 - nr_bits)) >> (64 - nr_bits);
> >
> > Oops, sorry. 64 might not be correct here. It would depend on the target being
> > either 32 or 64.
>
> In fact, sextract already does the sign extension, so this should be all that's needed, right?
> value = sextract<32,64>(value, 0, nr_bits);
Indeed, sextract64() is the preferred way to do a sign extension.
(The one thing to watch out for is that you mustn't try to
extract a zero-width field; it will assert if you do.
It also asserts if you specify a field whose start,length
would put either end to the left of bit 63 or the right of
bit 0, but that's less likely than the zero-width case.)
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-13 20:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 16:27 [PATCH] target/ppc: Fix 64-bit decrementer Cédric Le Goater
2021-09-13 17:07 ` Philippe Mathieu-Daudé
2021-09-13 17:22 ` Luis Fernando Fujita Pires
2021-09-13 17:29 ` Luis Fernando Fujita Pires
2021-09-13 18:05 ` Luis Fernando Fujita Pires
2021-09-13 19:55 ` Cédric Le Goater
2021-09-13 20:19 ` Peter Maydell
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.