Il sab 8 feb 2025, 11:36 Zhao Liu <zhao1....@intel.com> ha scritto:
> On Wed, Jan 29, 2025 at 11:58:14AM +0100, Paolo Bonzini wrote:
> > Date: Wed, 29 Jan 2025 11:58:14 +0100
> > From: Paolo Bonzini <pbonz...@redhat.com>
> > Subject: Re: [PATCH 09/10] rust/timer/hpet: add qom and qdev APIs support
> >
> >
> >
> > On Sat, Jan 25, 2025 at 1:32 PM Zhao Liu <zhao1....@intel.com> wrote:
> > > fn read(&mut self, addr: hwaddr, _size: u32) -> u64 {
> >
> > This can be &self.
>
> Done.
>
> > > let shift: u64 = (addr & 4) * 8;
> > >
> > > + match addr {
> > > + HPET_TN_CFG_REG => self.config >> shift, // including
> interrupt capabilities
> >
> > 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.
// No need for HPETClass. Just like OBJECT_DECLARE_SIMPLE_TYPE in C.
>
> Then this can be "grep", as a reference.
>
Sure.
Thanks,
Paolo
> > type Class = <SysBusDevice as ObjectType>::Class;
> > const TYPE_NAME: &'static CStr = crate::TYPE_HPET;
> > }
> >
> > which is indeed more similar to OBJECT_DECLARE_SIMPLE_TYPE().
>
> Awesome! Thanks.
>
> Regards,
> Zhao
>
>
