Migration relies on having the same device configuration on the source
and destination.  Therefore, there is no need to modify flags,
timer capabilities and the fw_cfg HPET block id on migration;
it was set to exactly the same values by realize.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/timer/hpet.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 5399f1b2a3f..18c8ce26e0d 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -275,16 +275,6 @@ static int hpet_post_load(void *opaque, int version_id)
                         - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     }
 
-    /* Push number of timers into capability returned via HPET_ID */
-    s->capability &= ~HPET_ID_NUM_TIM_MASK;
-    s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
-    hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
-
-    /* Derive HPET_MSI_SUPPORT from the capability of the first timer. */
-    s->flags &= ~(1 << HPET_MSI_SUPPORT);
-    if (s->timer[0].config & HPET_TN_FSB_CAP) {
-        s->flags |= 1 << HPET_MSI_SUPPORT;
-    }
     return 0;
 }
 
-- 
2.47.1

On Thu, Dec 05, 2024 at 09:37:21PM +0100, Paolo Bonzini wrote:
> Date: Thu,  5 Dec 2024 21:37:21 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] hpet: do not overwrite properties on post_load
> X-Mailer: git-send-email 2.47.1
> 
> Migration relies on having the same device configuration on the source
> and destination.  Therefore, there is no need to modify flags,
> timer capabilities and the fw_cfg HPET block id on migration;
> it was set to exactly the same values by realize.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/timer/hpet.c | 10 ----------
>  1 file changed, 10 deletions(-)

Indeed, thanks!

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 5399f1b2a3f..18c8ce26e0d 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -275,16 +275,6 @@ static int hpet_post_load(void *opaque, int version_id)
>                          - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>      }
>  
> -    /* Push number of timers into capability returned via HPET_ID */
> -    s->capability &= ~HPET_ID_NUM_TIM_MASK;
> -    s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
> -    hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
> -
> -    /* Derive HPET_MSI_SUPPORT from the capability of the first timer. */
> -    s->flags &= ~(1 << HPET_MSI_SUPPORT);
> -    if (s->timer[0].config & HPET_TN_FSB_CAP) {
> -        s->flags |= 1 << HPET_MSI_SUPPORT;

About the MSI (FSB) support, I haven't seen it being used anywhere. Is it the
dead code?

> -    }
>      return 0;
>  }
>  
> -- 
> 2.47.1
>

This patch was missed :-) which could free HPETState.flags from a
BqlCell.

> > -    /* Push number of timers into capability returned via HPET_ID */
> > -    s->capability &= ~HPET_ID_NUM_TIM_MASK;
> > -    s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
> > -    hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
> > -
> > -    /* Derive HPET_MSI_SUPPORT from the capability of the first timer. */
> > -    s->flags &= ~(1 << HPET_MSI_SUPPORT);
> > -    if (s->timer[0].config & HPET_TN_FSB_CAP) {
> > -        s->flags |= 1 << HPET_MSI_SUPPORT;
> 
> About the MSI (FSB) support, I haven't seen it being used anywhere. Is it the
> dead code?
> 

I have this question since I find it seems no way to set "msi" and
"timers" properties by user, and no code sets them.

    DEFINE_PROP_UINT8("timers", HPETState, num_timers, HPET_MIN_TIMERS),
    DEFINE_PROP_BIT("msi", HPETState, flags, HPET_MSI_SUPPORT, false),

Thanks,
Zhao