qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: hpet: fix decoding of timer registers
@ 2025-03-21 13:33 Paolo Bonzini
  2025-03-21 14:26 ` Peter Maydell
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2025-03-21 13:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-rust, Peter Maydell

Due to a missing "& 0x18", timer registers are not decoded correctly.
This breaks the tests/functional/test_x86_64_tuxrun.py functional
test.

Fixes: 519088b7cf6 ("rust: hpet: decode HPET registers into enums", 2025-03-06)
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/timer/hpet/src/hpet.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 63c1971f0b5..3ae3ec25f17 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -776,7 +776,7 @@ fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode {
             let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
             if timer_id <= self.num_timers.get() {
                 // TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id)
-                TimerRegister::try_from(addr)
+                TimerRegister::try_from(addr & 0x18)
                     .map(|reg| HPETRegister::Timer(&self.timers[timer_id], reg))
             } else {
                 // TODO: Add trace point -  trace_hpet_timer_id_out_of_range(timer_id)
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: hpet: fix decoding of timer registers
  2025-03-21 13:33 [PATCH] rust: hpet: fix decoding of timer registers Paolo Bonzini
@ 2025-03-21 14:26 ` Peter Maydell
  2025-03-21 18:38   ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2025-03-21 14:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust

On Fri, 21 Mar 2025 at 13:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Due to a missing "& 0x18", timer registers are not decoded correctly.
> This breaks the tests/functional/test_x86_64_tuxrun.py functional
> test.
>
> Fixes: 519088b7cf6 ("rust: hpet: decode HPET registers into enums", 2025-03-06)
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/timer/hpet/src/hpet.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
> index 63c1971f0b5..3ae3ec25f17 100644
> --- a/rust/hw/timer/hpet/src/hpet.rs
> +++ b/rust/hw/timer/hpet/src/hpet.rs
> @@ -776,7 +776,7 @@ fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode {
>              let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
>              if timer_id <= self.num_timers.get() {
>                  // TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id)
> -                TimerRegister::try_from(addr)
> +                TimerRegister::try_from(addr & 0x18)
>                      .map(|reg| HPETRegister::Timer(&self.timers[timer_id], reg))
>              } else {
>                  // TODO: Add trace point -  trace_hpet_timer_id_out_of_range(timer_id)
> --

Tested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

If I understand the code correctly I think you could also
write this as "addr & 0x1f" which might be a little nicer
as it then lines up with the "/ 0x20".

thanks
-- PMM


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rust: hpet: fix decoding of timer registers
  2025-03-21 14:26 ` Peter Maydell
@ 2025-03-21 18:38   ` Paolo Bonzini
  0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:38 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, qemu-rust

On Fri, Mar 21, 2025 at 3:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> Tested-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> If I understand the code correctly I think you could also
> write this as "addr & 0x1f" which might be a little nicer
> as it then lines up with the "/ 0x20".

Yeah, I was undecided between that and 0x18... in the end I went with
0x18 because bits 0-1 are clear (due to alignment) and bit 2 is
cleared above, and the TimerRegister enum also has "0x18" in the
comment.

Paolo



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-03-21 18:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-21 13:33 [PATCH] rust: hpet: fix decoding of timer registers Paolo Bonzini
2025-03-21 14:26 ` Peter Maydell
2025-03-21 18:38   ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).