From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44811) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMISJ-0006wi-Rp for qemu-devel@nongnu.org; Thu, 21 Jan 2016 11:51:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMISG-0007jG-Lf for qemu-devel@nongnu.org; Thu, 21 Jan 2016 11:51:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMISG-0007iY-Dp for qemu-devel@nongnu.org; Thu, 21 Jan 2016 11:51:28 -0500 References: <1452015002-28493-6-git-send-email-clg@fr.ibm.com> <569B8350.2040305@gmail.com> <20160117161425-mutt-send-email-mst@redhat.com> <56A10948.8020306@gmail.com> <56A10A33.9010705@fr.ibm.com> From: Marcel Apfelbaum Message-ID: <56A10C8B.7060508@redhat.com> Date: Thu, 21 Jan 2016 18:51:23 +0200 MIME-Version: 1.0 In-Reply-To: <56A10A33.9010705@fr.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 5/8] ipmi: add ACPI power and GUID commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= , minyard@acm.org, "Michael S. Tsirkin" Cc: Corey Minyard , qemu-devel@nongnu.org On 01/21/2016 06:41 PM, C=E9dric Le Goater wrote: > On 01/21/2016 05:37 PM, Corey Minyard wrote: >> 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=E9dric Le Goater wrote: >>>>> Signed-off-by: C=E9dric 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] =3D cmd[2]; >>>>> + ibs->power_state[1] =3D 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 cod= e >>>> simply returns. >>>> Also the correlation between those macros is a little odd. >>>> >>>> Thanks, >>>> Marcel >>> >>> Yes - 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? > > I have the patch ready (and more). I will send this one to start with. Thanks! Marcel > > C. >