[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/8] ipmi: add ACPI power and GUID commands
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH 5/8] ipmi: add ACPI power and GUID commands |
Date: |
Mon, 18 Jan 2016 13:04:11 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 |
On 01/17/2016 01:08 PM, Marcel Apfelbaum wrote:
> 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 <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.
>
> I meant the correlation between the macros and the "out" label.
Yes. I agree these gotos are a little odd. There is a slight difference
with the routine ipmi_sim_handle_command() and the use of the macro
IPMI_ADD_RSP_DATA(). I will see what I can do.
Thanks,
C.
> 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);
>>>
>>>
>>
>