From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42604) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aKm8W-0007PA-JA for qemu-devel@nongnu.org; Sun, 17 Jan 2016 07:08:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aKm8T-0005g7-7w for qemu-devel@nongnu.org; Sun, 17 Jan 2016 07:08:48 -0500 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:33820) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aKm8S-0005g3-Ti for qemu-devel@nongnu.org; Sun, 17 Jan 2016 07:08:45 -0500 Received: by mail-wm0-x244.google.com with SMTP id b14so11339018wmb.1 for ; Sun, 17 Jan 2016 04:08:44 -0800 (PST) References: <1452015002-28493-6-git-send-email-clg@fr.ibm.com> <569B8350.2040305@gmail.com> From: Marcel Apfelbaum Message-ID: <569B8449.3090501@gmail.com> Date: Sun, 17 Jan 2016 14:08:41 +0200 MIME-Version: 1.0 In-Reply-To: <569B8350.2040305@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 5/8] ipmi: add ACPI power and GUID commands Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= , Corey Minyard Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" On 01/17/2016 02:04 PM, Marcel Apfelbaum wrote: > On 01/05/2016 07:29 PM, Cédric Le Goater wrote: >> Signed-off-by: Cédric Le Goater >> --- >> 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 >> #include >> #include >> +#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. I meant the correlation between the macros and the "out" label. Thanks, Marcel > > Thanks, > Marcel > > >> +} >> + >> +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); >> >> >