[PATCH 2/5] hw/timer/hpet: Adjust num_timers in hpet_init()

Zhao Liu posted 5 patches 5 months, 4 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
[PATCH 2/5] hw/timer/hpet: Adjust num_timers in hpet_init()
Posted by Zhao Liu 5 months, 4 weeks ago
Currently, HPET adjusts num_timers in hpet_realize(), and doesn't change
it in any other place. And this field is initialized as a property.

Therefore, it's possible to move such adjustments to hept_init(), so
that Rust side can synchronize this change.

Adjust num_timers in hpet_init().

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/timer/hpet.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 0fd1337a1564..48b1a9289f83 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -682,6 +682,12 @@ static void hpet_init(Object *obj)
     /* HPET Area */
     memory_region_init_io(&s->iomem, obj, &hpet_ram_ops, s, "hpet", HPET_LEN);
     sysbus_init_mmio(sbd, &s->iomem);
+
+    if (s->num_timers < HPET_MIN_TIMERS) {
+        s->num_timers = HPET_MIN_TIMERS;
+    } else if (s->num_timers > HPET_MAX_TIMERS) {
+        s->num_timers = HPET_MAX_TIMERS;
+    }
 }
 
 static void hpet_realize(DeviceState *dev, Error **errp)
@@ -710,11 +716,6 @@ static void hpet_realize(DeviceState *dev, Error **errp)
         sysbus_init_irq(sbd, &s->irqs[i]);
     }
 
-    if (s->num_timers < HPET_MIN_TIMERS) {
-        s->num_timers = HPET_MIN_TIMERS;
-    } else if (s->num_timers > HPET_MAX_TIMERS) {
-        s->num_timers = HPET_MAX_TIMERS;
-    }
     for (i = 0; i < HPET_MAX_TIMERS; i++) {
         timer = &s->timer[i];
         timer->qemu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, hpet_timer, timer);
-- 
2.34.1
Re: [PATCH 2/5] hw/timer/hpet: Adjust num_timers in hpet_init()
Posted by Paolo Bonzini 5 months, 4 weeks ago
On 5/20/25 17:27, Zhao Liu wrote:
> Currently, HPET adjusts num_timers in hpet_realize(), and doesn't change
> it in any other place. And this field is initialized as a property.

Properties are initialized *after* hpet_init.  For hw/timer/hpet you can 
check s->num_timers and return an error if it's out of bounds, but for 
the Rust version we don't have Error** support yet. :(

Queued 1-4-5 for now.

Paolo

> Therefore, it's possible to move such adjustments to hept_init(), so
> that Rust side can synchronize this change.
> 
> Adjust num_timers in hpet_init().
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   hw/timer/hpet.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 0fd1337a1564..48b1a9289f83 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -682,6 +682,12 @@ static void hpet_init(Object *obj)
>       /* HPET Area */
>       memory_region_init_io(&s->iomem, obj, &hpet_ram_ops, s, "hpet", HPET_LEN);
>       sysbus_init_mmio(sbd, &s->iomem);
> +
> +    if (s->num_timers < HPET_MIN_TIMERS) {
> +        s->num_timers = HPET_MIN_TIMERS;
> +    } else if (s->num_timers > HPET_MAX_TIMERS) {
> +        s->num_timers = HPET_MAX_TIMERS;
> +    }
>   }
>   
>   static void hpet_realize(DeviceState *dev, Error **errp)
> @@ -710,11 +716,6 @@ static void hpet_realize(DeviceState *dev, Error **errp)
>           sysbus_init_irq(sbd, &s->irqs[i]);
>       }
>   
> -    if (s->num_timers < HPET_MIN_TIMERS) {
> -        s->num_timers = HPET_MIN_TIMERS;
> -    } else if (s->num_timers > HPET_MAX_TIMERS) {
> -        s->num_timers = HPET_MAX_TIMERS;
> -    }
>       for (i = 0; i < HPET_MAX_TIMERS; i++) {
>           timer = &s->timer[i];
>           timer->qemu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, hpet_timer, timer);
Re: [PATCH 2/5] hw/timer/hpet: Adjust num_timers in hpet_init()
Posted by Zhao Liu 5 months, 4 weeks ago
On Tue, May 20, 2025 at 05:16:22PM +0200, Paolo Bonzini wrote:
> Date: Tue, 20 May 2025 17:16:22 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 2/5] hw/timer/hpet: Adjust num_timers in hpet_init()
> 
> On 5/20/25 17:27, Zhao Liu wrote:
> > Currently, HPET adjusts num_timers in hpet_realize(), and doesn't change
> > it in any other place. And this field is initialized as a property.
> 
> Properties are initialized *after* hpet_init.  For hw/timer/hpet you can
> check s->num_timers and return an error if it's out of bounds, but for the
> Rust version we don't have Error** support yet. :(

Oops, yes.

(Note for myself,) the default property value is set before hpet_init(),
but the subsequent adjustments to property (via object_property_set_uint8())
need to take boundaries into account, which is why the num_timers adjustment
is placed in realize().

> Queued 1-4-5 for now.

Thanks!

Zhao