Hi Yunpeng,

On 28/10/25 19:01, Yunpeng Yang wrote:
The following IPMI commands are added or modified to support getting
fake LAN channel configurations from the `ipmi_bmc_sim` device:
* Get Channel Access;
* Get Channel Info Command;
* Get LAN Configuration Parameters.

The fake LAN channel configurations can be specified from QEMU
commandline options for device `ipmi_bmc_sim`. Inside the guest OS, the
configurations can be retrieved from the device using some IPMI tools,
e.g., `ipmitool lan print`.
Note, there is not a real LAN channel. The fake LAN channel is suitable
for testing purposes.

Signed-off-by: Yunpeng Yang <[email protected]>
---
  hw/ipmi/ipmi_bmc_sim.c      | 297 ++++++++++++++++++++++++++++++++++--
  include/hw/ipmi/ipmi.h      |   1 +
  qemu-options.hx             |  26 ++++
  tests/qtest/ipmi-kcs-test.c |  60 ++++++++
  4 files changed, 374 insertions(+), 10 deletions(-)


  /* Same as a timespec struct. */
  struct ipmi_time {
@@ -170,6 +177,23 @@ typedef struct IPMISensor {
  #define MAX_SENSORS 20
  #define IPMI_WATCHDOG_SENSOR 0
+#define NBYTES_IP 4
+#define NBYTES_MAC 6
+
+typedef struct IPMILan {
+    uint8_t channel;
+    uint8_t ipaddr[NBYTES_IP];
+    uint8_t ipsrc;
+    MACAddr macaddr;
+    uint8_t netmask[NBYTES_IP];
+    uint8_t defgw_ipaddr[NBYTES_IP];
+    MACAddr defgw_macaddr;
+
+    char *arg_ipaddr;
+    char *arg_netmask;
+    char *arg_defgw_ipaddr;
+} IPMILan;
+
  #define MAX_NETFNS 64
typedef struct IPMIRcvBufEntry {
@@ -215,6 +239,7 @@ struct IPMIBmcSim {
      IPMIFru fru;
      IPMISensor sensors[MAX_SENSORS];
      char *sdr_filename;
+    IPMILan lan;

[...]

+static inline bool is_valid_netmask(const uint8_t *netmask)
+{
+    uint32_t mask = netmask[3];
+    uint32_t inverted;
+
+    mask |= (uint32_t) netmask[2] << 8;
+    mask |= (uint32_t) netmask[1] << 16;
+    mask |= (uint32_t) netmask[0] << 24;

Why manually byteswap?

I'd expect such helper already available in include/net/ somewhere,
otherwise what about:

  static bool is_ipv4_netmask_valid(const void *buf)
  {
      uint32_t netmask = ldl_be_p(buf);

      return clo32(netmask) + ctz32(netmask) == 32;
  }

+    inverted = ~mask;
+    return mask != 0 && (inverted & (inverted + 1)) == 0;
+}


@@ -2176,7 +2395,7 @@ static void ipmi_sdr_init(IPMIBmcSim *ibs)
static const VMStateDescription vmstate_ipmi_sim = {
      .name = TYPE_IPMI_BMC_SIMULATOR,
-    .version_id = 1,
+    .version_id = 2,
      .minimum_version_id = 1,
      .fields = (const VMStateField[]) {
          VMSTATE_UINT8(bmc_global_enables, IPMIBmcSim),
@@ -2198,6 +2417,13 @@ static const VMStateDescription vmstate_ipmi_sim = {
          VMSTATE_UINT16(sensors[IPMI_WATCHDOG_SENSOR].deassert_states,
                         IPMIBmcSim),
          VMSTATE_UINT16(sensors[IPMI_WATCHDOG_SENSOR].assert_enable, 
IPMIBmcSim),
+        VMSTATE_UINT8_V(lan.channel, IPMIBmcSim, 2),
+        VMSTATE_UINT8_ARRAY_V(lan.ipaddr, IPMIBmcSim, NBYTES_IP, 2),
+        VMSTATE_UINT8_V(lan.ipsrc, IPMIBmcSim, 2),
+        VMSTATE_UINT8_ARRAY_V(lan.macaddr.a, IPMIBmcSim, NBYTES_MAC, 2),
+        VMSTATE_UINT8_ARRAY_V(lan.netmask, IPMIBmcSim, NBYTES_IP, 2),
+        VMSTATE_UINT8_ARRAY_V(lan.defgw_ipaddr, IPMIBmcSim, NBYTES_IP, 2),
+        VMSTATE_UINT8_ARRAY_V(lan.defgw_macaddr.a, IPMIBmcSim, NBYTES_MAC, 2),

Safer would be to add a VMStateDescription for the new IPMILan
structure and link it as subsections, so other code using could just
link as subsection, not duplicating the VMSTATE_FOO() macros.

          VMSTATE_END_OF_LIST()
      }
  };

Regards,

Phil.

Reply via email to