[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 09/10] rust/timer/hpet: add qom and qdev APIs support
From: |
Zhao Liu |
Subject: |
Re: [PATCH 09/10] rust/timer/hpet: add qom and qdev APIs support |
Date: |
Sun, 9 Feb 2025 02:06:35 +0800 |
> > > This needs to be "match addr & !4".
> >
> > I understand it's not necessary:
> >
> > In timer_and_addr(), I've masked the address with 0x18.
> >
> > fn timer_and_addr(&self, addr: hwaddr) ->
> > Option<(&BqlRefCell<HPETTimer>, hwaddr)> {
> > let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
> >
> > if timer_id > self.num_timers.get() {
> > None
> > } else {
> > Some((&self.timers[timer_id], addr & 0x18))
> >
>
> Ah, this should be 0x1C (or perhaps 0x1F). Otherwise there is a bug in
> accessing the high 32 bits of a 64-bit register; shift will always be 0 in
> HPETTimer::read and write.
Yes, you're right. Then we should use 0x1F so that invalid access could
detected (or loged in future) and ignored.
Based on the similar reason, C side also need to use "addr & (0x1f | ~4)"
instead of 0x18 to catch invalid access. If I'm right, I can submit a
fix.
Thanks,
Zhao