|
From: | Corey Minyard |
Subject: | Re: [Qemu-devel] [PATCH 5/8] ipmi: add ACPI power and GUID commands |
Date: | Thu, 21 Jan 2016 10:37:28 -0600 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
On 01/17/2016 08:16 AM, Michael S. Tsirkin wrote:
On Sun, Jan 17, 2016 at 02:04:32PM +0200, Marcel Apfelbaum wrote:On 01/05/2016 07:29 PM, Cédric Le Goater wrote:Signed-off-by: Cédric Le Goater <address@hidden> --- hw/ipmi/ipmi_bmc_sim.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c index 60586a67104e..c3a06d0ac7e4 100644 --- a/hw/ipmi/ipmi_bmc_sim.c +++ b/hw/ipmi/ipmi_bmc_sim.c @@ -25,6 +25,7 @@ #include <stdio.h> #include <string.h> #include <stdint.h> +#include "sysemu/sysemu.h" #include "qemu/timer.h" #include "hw/ipmi/ipmi.h" #include "qemu/error-report.h" @@ -54,6 +55,9 @@ #define IPMI_CMD_GET_DEVICE_ID 0x01 #define IPMI_CMD_COLD_RESET 0x02 #define IPMI_CMD_WARM_RESET 0x03 +#define IPMI_CMD_SET_POWER_STATE 0x06 +#define IPMI_CMD_GET_POWER_STATE 0x07 +#define IPMI_CMD_GET_DEVICE_GUID 0x08 #define IPMI_CMD_RESET_WATCHDOG_TIMER 0x22 #define IPMI_CMD_SET_WATCHDOG_TIMER 0x24 #define IPMI_CMD_GET_WATCHDOG_TIMER 0x25 @@ -215,6 +219,9 @@ struct IPMIBmcSim { uint8_t restart_cause; + uint8_t power_state[2]; + uint8_t uuid[16]; + IPMISel sel; IPMISdr sdr; IPMIFru fru; @@ -842,6 +849,42 @@ static void warm_reset(IPMIBmcSim *ibs, k->reset(s, false); } } +static void set_power_state(IPMIBmcSim *ibs, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len) +{ + IPMI_CHECK_CMD_LEN(4); + ibs->power_state[0] = cmd[2]; + ibs->power_state[1] = cmd[3]; + out: + return;Hi, I am sorry for my late comment, but I find a little strange the use of the "out" label here. I understand this is because of its usage in IPMI_* macros, but I looked into every usage(I hope I didn't miss anything) and the code simply returns. Also the correlation between those macros is a little odd. Thanks, MarcelYes - these macros with goto out are confusing. Please rewrite them to return bool, and put goto out in the caller.
Marcel, do you want me to do this? -corey
+} + +static void get_power_state(IPMIBmcSim *ibs, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len) +{ + IPMI_ADD_RSP_DATA(ibs->power_state[0]); + IPMI_ADD_RSP_DATA(ibs->power_state[1]); + out: + return; +} + +static void get_device_guid(IPMIBmcSim *ibs, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len) +{ + unsigned int i; + + for (i = 0; i < 16; i++) { + IPMI_ADD_RSP_DATA(ibs->uuid[i]); + } + out: + return; +} static void set_bmc_global_enables(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len, @@ -1781,6 +1824,9 @@ static const IPMICmdHandler app_cmds[IPMI_NETFN_APP_MAXCMD] = { [IPMI_CMD_GET_DEVICE_ID] = get_device_id, [IPMI_CMD_COLD_RESET] = cold_reset, [IPMI_CMD_WARM_RESET] = warm_reset, + [IPMI_CMD_SET_POWER_STATE] = set_power_state, + [IPMI_CMD_GET_POWER_STATE] = get_power_state, + [IPMI_CMD_GET_DEVICE_GUID] = get_device_guid, [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = set_bmc_global_enables, [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = get_bmc_global_enables, [IPMI_CMD_CLR_MSG_FLAGS] = clr_msg_flags, @@ -1907,6 +1953,15 @@ static void ipmi_sim_init(Object *obj) i += len; } + ibs->power_state[0] = 0; + ibs->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);
[Prev in Thread] | Current Thread | [Next in Thread] |