Re: [PATCH 09/10] rust/timer/hpet: add qom and qdev APIs support

[Paolo Bonzini <pbonzini@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

Il sab 8 feb 2025, 11:36 Zhao Liu <zhao1.liu@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 <pbonzini@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.liu@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
>
>

[-- Attachment #2: Type: text/html, Size: 3191 bytes --]