Re: [RFC 09/13] i386/fw_cfg: move hpet_cfg definition to hpet.c

[Zhao Liu]

On Sat, Dec 07, 2024 at 10:16:03AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Sat, 7 Dec 2024 10:16:03 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: Re: [RFC 09/13] i386/fw_cfg: move hpet_cfg definition to hpet.c
> 
> On 5/12/24 22:19, Paolo Bonzini wrote:
> > On Thu, Dec 5, 2024 at 10:18 PM Philippe Mathieu-Daudé
> > <philmd@linaro.org> wrote:
> > > 
> > > On 5/12/24 13:46, Zhao Liu wrote:
> > > > Hi Philippe,
> > > > 
> > > > On Thu, Dec 05, 2024 at 01:04:58PM +0100, Philippe Mathieu-Daudé wrote:
> > > > > Date: Thu, 5 Dec 2024 13:04:58 +0100
> > > > > From: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > > Subject: Re: [RFC 09/13] i386/fw_cfg: move hpet_cfg definition to 
> > > > > hpet.c
> > > > > 
> > > > > On 5/12/24 07:07, Zhao Liu wrote:
> > > > > > HPET device needs to access and update hpet_cfg variable, but now 
> > > > > > it is
> > > > > > defined in hw/i386/fw_cfg.c and Rust code can't access it.
> > > > > > 
> > > > > > Move hpet_cfg definition to hpet.c (and rename it to hpet_fw_cfg). 
> > > > > > This
> > > > > > allows Rust HPET device implements its own global hpet_fw_cfg 
> > > > > > variable,
> > > > > > and will further reduce the use of unsafe C code access and calls 
> > > > > > in the
> > > > > > Rust HPET implementation.
> > > > > > 
> > > > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > > > > > ---
> > > > > >     hw/i386/fw_cfg.c        |  4 +---
> > > > > >     hw/timer/hpet.c         | 16 +++++++++-------
> > > > > >     include/hw/timer/hpet.h |  2 +-
> > > > > >     3 files changed, 11 insertions(+), 11 deletions(-)
> > > > > 
> > > > > 
> > > > > > diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
> > > > > > index d17a8d43199e..dbf709251a8f 100644
> > > > > > --- a/include/hw/timer/hpet.h
> > > > > > +++ b/include/hw/timer/hpet.h
> > > > > > @@ -74,7 +74,7 @@ struct hpet_fw_config
> > > > > >         struct hpet_fw_entry hpet[8];
> > > > > >     } QEMU_PACKED;
> > > > > > -extern struct hpet_fw_config hpet_cfg;
> > > > > > +extern struct hpet_fw_config hpet_fw_cfg;
> > > > > 
> > > > > Could this field belong to the (yet unexisting) HPETClass?
> > > > 
> > > > Several instances would share the same class, so HPETClass could manage
> > > > multiple HPETState info.
> > > > 
> > > > But in fw_cfg.c, do you have idea about how to get the HPETClass?
> > > 
> > > Have hpet_find() return an Object and call object_get_class()?
> 
> Implemented as
> 20241206191124.9195-1-philmd@linaro.org/">https://lore.kernel.org/qemu-devel/20241206191124.9195-1-philmd@linaro.org/,
> hoping it simplifies the Rust model integration.
> 

Hi Philli, thank you very much!

This approach is very helpful for more “rich” QOM class attempts, and if
Paolo feels the same way, I'll move to your approach for the next
version!

Regards,
Zhao