diff mbox series

[06/15] ipmi: Add a UUID device property

Message ID 20190919213924.31852-7-minyard@acm.org (mailing list archive)
State New, archived
Headers show
Series [01/15] ipmi: Fix watchdog NMI handling | expand

Commit Message

Corey Minyard Sept. 19, 2019, 9:39 p.m. UTC 
From: Corey Minyard <cminyard@mvista.com>

Using the UUID that qemu generates probably isn't the best thing
to do, allow it to be passed in via properties, and use QemuUUID
for the type.

If the UUID is not set, return an unsupported command error.  This
way we are not providing an all-zero (or randomly generated) GUID
to the IPMI user.  This lets the host fall back to the other
method of using the get device id command to determind the BMC
being accessed.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: Cédric Le Goater <clg@kaod.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 22 ++++++++++++++--------
 qemu-options.hx        | 10 +++++++---
 2 files changed, 21 insertions(+), 11 deletions(-)

Comments

Cédric Le Goater Sept. 23, 2019, 5:51 a.m. UTC | #1 
On 19/09/2019 23:39, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Using the UUID that qemu generates probably isn't the best thing
> to do, allow it to be passed in via properties, and use QemuUUID
> for the type.
> 
> If the UUID is not set, return an unsupported command error.  This
> way we are not providing an all-zero (or randomly generated) GUID
> to the IPMI user.  This lets the host fall back to the other
> method of using the get device id command to determind the BMC
> being accessed.


Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.


> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/ipmi/ipmi_bmc_sim.c | 22 ++++++++++++++--------
>  qemu-options.hx        | 10 +++++++---
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 6e6cd1b47d..71e56f3b13 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -223,7 +223,7 @@ struct IPMIBmcSim {
>      uint8_t restart_cause;
>  
>      uint8_t acpi_power_state[2];
> -    uint8_t uuid[16];
> +    QemuUUID uuid;
>  
>      IPMISel sel;
>      IPMISdr sdr;
> @@ -941,8 +941,19 @@ static void get_device_guid(IPMIBmcSim *ibs,
>  {
>      unsigned int i;
>  
> +    /* An uninitialized uuid is all zeros, use that to know if it is set. */
>      for (i = 0; i < 16; i++) {
> -        rsp_buffer_push(rsp, ibs->uuid[i]);
> +        if (ibs->uuid.data[i]) {
> +            goto uuid_set;
> +        }
> +    }
> +    /* No uuid is set, return an error. */
> +    rsp_buffer_set_error(rsp, IPMI_CC_INVALID_CMD);
> +    return;
> +
> + uuid_set:
> +    for (i = 0; i < 16; i++) {
> +        rsp_buffer_push(rsp, ibs->uuid.data[i]);
>      }
>  }
>  
> @@ -1986,12 +1997,6 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp)
>      ibs->acpi_power_state[0] = 0;
>      ibs->acpi_power_state[1] = 0;
>  
> -    if (qemu_uuid_set) {
> -        memcpy(&ibs->uuid, &qemu_uuid, 16);
> -    } else {
> -        memset(&ibs->uuid, 0, 16);
> -    }
> -
>      ipmi_init_sensors_from_sdrs(ibs);
>      register_cmds(ibs);
>  
> @@ -2011,6 +2016,7 @@ static Property ipmi_sim_properties[] = {
>      DEFINE_PROP_UINT8("fwrev2", IPMIBmcSim, fwrev2, 0),
>      DEFINE_PROP_UINT32("mfg_id", IPMIBmcSim, mfg_id, 0),
>      DEFINE_PROP_UINT16("product_id", IPMIBmcSim, product_id, 0),
> +    DEFINE_PROP_UUID_NODEFAULT("guid", IPMIBmcSim, uuid),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index bbfd936d29..ed9292f65e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -701,7 +701,7 @@ possible drivers and properties, use @code{-device help} and
>  @code{-device @var{driver},help}.
>  
>  Some drivers are:
> -@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}]
> +@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}][,guid=@var{uuid}]
>  
>  Add an IPMI BMC.  This is a simulation of a hardware management
>  interface processor that normally sits on a system.  It provides
> @@ -714,8 +714,8 @@ controllers.  If you don't know what this means, it is safe to ignore
>  it.
>  
>  @table @option
> -@item bmc=@var{id}
> -The BMC to connect to, one of ipmi-bmc-sim or ipmi-bmc-extern above.
> +@item id=@var{id}
> +The BMC id for interfaces to use this device.
>  @item slave_addr=@var{val}
>  Define slave address to use for the BMC.  The default is 0x20.
>  @item sdrfile=@var{file}
> @@ -724,6 +724,10 @@ file containing raw Sensor Data Records (SDR) data. The default is none.
>  size of a Field Replaceable Unit (FRU) area.  The default is 1024.
>  @item frudatafile=@var{file}
>  file containing raw Field Replaceable Unit (FRU) inventory data. The default is none.
> +@item guid=@var{uuid}
> +value for the GUID for the BMC, in standard UUID format.  If this is set,
> +get "Get GUID" command to the BMC will return it.  Otherwise "Get GUID"
> +will return an error.
>  @end table
>  
>  @item -device ipmi-bmc-extern,id=@var{id},chardev=@var{id}[,slave_addr=@var{val}]
>
diff mbox series

Patch

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 6e6cd1b47d..71e56f3b13 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -223,7 +223,7 @@  struct IPMIBmcSim {
     uint8_t restart_cause;
 
     uint8_t acpi_power_state[2];
-    uint8_t uuid[16];
+    QemuUUID uuid;
 
     IPMISel sel;
     IPMISdr sdr;
@@ -941,8 +941,19 @@  static void get_device_guid(IPMIBmcSim *ibs,
 {
     unsigned int i;
 
+    /* An uninitialized uuid is all zeros, use that to know if it is set. */
     for (i = 0; i < 16; i++) {
-        rsp_buffer_push(rsp, ibs->uuid[i]);
+        if (ibs->uuid.data[i]) {
+            goto uuid_set;
+        }
+    }
+    /* No uuid is set, return an error. */
+    rsp_buffer_set_error(rsp, IPMI_CC_INVALID_CMD);
+    return;
+
+ uuid_set:
+    for (i = 0; i < 16; i++) {
+        rsp_buffer_push(rsp, ibs->uuid.data[i]);
     }
 }
 
@@ -1986,12 +1997,6 @@  static void ipmi_sim_realize(DeviceState *dev, Error **errp)
     ibs->acpi_power_state[0] = 0;
     ibs->acpi_power_state[1] = 0;
 
-    if (qemu_uuid_set) {
-        memcpy(&ibs->uuid, &qemu_uuid, 16);
-    } else {
-        memset(&ibs->uuid, 0, 16);
-    }
-
     ipmi_init_sensors_from_sdrs(ibs);
     register_cmds(ibs);
 
@@ -2011,6 +2016,7 @@  static Property ipmi_sim_properties[] = {
     DEFINE_PROP_UINT8("fwrev2", IPMIBmcSim, fwrev2, 0),
     DEFINE_PROP_UINT32("mfg_id", IPMIBmcSim, mfg_id, 0),
     DEFINE_PROP_UINT16("product_id", IPMIBmcSim, product_id, 0),
+    DEFINE_PROP_UUID_NODEFAULT("guid", IPMIBmcSim, uuid),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index bbfd936d29..ed9292f65e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -701,7 +701,7 @@  possible drivers and properties, use @code{-device help} and
 @code{-device @var{driver},help}.
 
 Some drivers are:
-@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}]
+@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}][,guid=@var{uuid}]
 
 Add an IPMI BMC.  This is a simulation of a hardware management
 interface processor that normally sits on a system.  It provides
@@ -714,8 +714,8 @@  controllers.  If you don't know what this means, it is safe to ignore
 it.
 
 @table @option
-@item bmc=@var{id}
-The BMC to connect to, one of ipmi-bmc-sim or ipmi-bmc-extern above.
+@item id=@var{id}
+The BMC id for interfaces to use this device.
 @item slave_addr=@var{val}
 Define slave address to use for the BMC.  The default is 0x20.
 @item sdrfile=@var{file}
@@ -724,6 +724,10 @@  file containing raw Sensor Data Records (SDR) data. The default is none.
 size of a Field Replaceable Unit (FRU) area.  The default is 1024.
 @item frudatafile=@var{file}
 file containing raw Field Replaceable Unit (FRU) inventory data. The default is none.
+@item guid=@var{uuid}
+value for the GUID for the BMC, in standard UUID format.  If this is set,
+get "Get GUID" command to the BMC will return it.  Otherwise "Get GUID"
+will return an error.
 @end table
 
 @item -device ipmi-bmc-extern,id=@var{id},chardev=@var{id}[,slave_addr=@var{val}]