> > > 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


Reply via email to