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

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/5/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/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index 0e4494627c21..965e6306838a 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -26,8 +26,6 @@
>   #include CONFIG_DEVICES
>   #include "target/i386/cpu.h"
>   
> -struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};

This breaks if you disable HPET, which is why fw_cfg.c defines it.

You can do something like

diff --git a/include/hw/timer/hpet-fw-cfg.h b/include/hw/timer/hpet-fw-cfg.h
new file mode 100644
index 00000000000..234a49fc92e
--- /dev/null
+++ b/include/hw/timer/hpet-fw-cfg.h
@@ -0,0 +1,16 @@
+struct hpet_fw_entry
+{
+    uint32_t event_timer_block_id;
+    uint64_t address;
+    uint16_t min_tick;
+    uint8_t page_prot;
+} QEMU_PACKED;
+
+struct hpet_fw_config
+{
+    uint8_t count;
+    struct hpet_fw_entry hpet[8];
+} QEMU_PACKED;
+
+extern struct hpet_fw_config hpet_fw_cfg;
+
diff --git a/include/hw/timer/hpet.h b/include/hw/timer/hpet.h
index d17a8d43199..6f7fcbc3c60 100644
--- a/include/hw/timer/hpet.h
+++ b/include/hw/timer/hpet.h
@@ -60,26 +60,12 @@
  #define HPET_TN_INT_ROUTE_CAP_SHIFT 32
  #define HPET_TN_CFG_BITS_READONLY_OR_RESERVED 0xffff80b1U
  
-struct hpet_fw_entry
-{
-    uint32_t event_timer_block_id;
-    uint64_t address;
-    uint16_t min_tick;
-    uint8_t page_prot;
-} QEMU_PACKED;
-
-struct hpet_fw_config
-{
-    uint8_t count;
-    struct hpet_fw_entry hpet[8];
-} QEMU_PACKED;
-
-extern struct hpet_fw_config hpet_cfg;
-
  #define TYPE_HPET "hpet"
  
  #define HPET_INTCAP "hpet-intcap"
  
+#include "hw/timer/hpet-fw-cfg.h"
+
  static inline bool hpet_find(void)
  {
      return object_resolve_path_type("", TYPE_HPET, NULL);
diff --git a/rust/wrapper.h b/rust/wrapper.h
index 285d0eb6ad0..82381e43472 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -62,3 +62,4 @@ typedef enum memory_order {
  #include "qapi/error.h"
  #include "migration/vmstate.h"
  #include "chardev/char-serial.h"
+#include "hw/timer/hpet-fw-cfg.h"


but you will have to use unsafe to access it since it's a "static mut".

Paolo