[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 03/17] ipmi: Add a local BMC simulation
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH v4 03/17] ipmi: Add a local BMC simulation |
Date: |
Thu, 26 Nov 2015 19:07:31 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 |
On 11/24/2015 08:46 PM, Corey Minyard wrote:
> On 11/24/2015 07:31 AM, Cédric Le Goater wrote:
>> A few comments below,
>
> Thanks a bunch for the review. As you probably have guessed, this was
> not really intended as a fully functional BMC, though it has most of the
> trappings of what you would need to implement one.
>
> I assume you are working on the power systems, and it makes sense to
> extend this for that application.
Yes. I am trying to add the few extra commands the opal firmware and the
kernel are using when running on an open power platform.
It is currently booting fine but there are a few errors and still a few
gaps to fill to make it look like a BMC was in charge. FRU would be nice
also but we will see how it goes.
>> On 11/12/2015 08:02 PM, address@hidden wrote:
>>> From: Corey Minyard <address@hidden>
>>>
>>> This provides a minimal local BMC, basically enough to comply with the
>>> spec and provide a complete watchdog timer (including a sensor, SDR,
>>> and event).
>>>
>>> Signed-off-by: Corey Minyard <address@hidden>
>>> ---
>>> default-configs/i386-softmmu.mak | 1 +
>>> default-configs/x86_64-softmmu.mak | 1 +
>>> hw/ipmi/Makefile.objs | 1 +
>>> hw/ipmi/ipmi_bmc_sim.c | 1731
>>> ++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 1734 insertions(+)
>>> create mode 100644 hw/ipmi/ipmi_bmc_sim.c
>>>
>>> diff --git a/default-configs/i386-softmmu.mak
>>> b/default-configs/i386-softmmu.mak
>>> index 8fa751a..00a0346 100644
>>> --- a/default-configs/i386-softmmu.mak
>>> +++ b/default-configs/i386-softmmu.mak
>>> @@ -10,6 +10,7 @@ CONFIG_VMWARE_VGA=y
>>> CONFIG_VIRTIO_VGA=y
>>> CONFIG_VMMOUSE=y
>>> CONFIG_IPMI=y
>>> +CONFIG_IPMI_LOCAL=y
>>> CONFIG_SERIAL=y
>>> CONFIG_PARALLEL=y
>>> CONFIG_I8254=y
>>> diff --git a/default-configs/x86_64-softmmu.mak
>>> b/default-configs/x86_64-softmmu.mak
>>> index 6767f4f..39a619f 100644
>>> --- a/default-configs/x86_64-softmmu.mak
>>> +++ b/default-configs/x86_64-softmmu.mak
>>> @@ -10,6 +10,7 @@ CONFIG_VMWARE_VGA=y
>>> CONFIG_VIRTIO_VGA=y
>>> CONFIG_VMMOUSE=y
>>> CONFIG_IPMI=y
>>> +CONFIG_IPMI_LOCAL=y
>>> CONFIG_SERIAL=y
>>> CONFIG_PARALLEL=y
>>> CONFIG_I8254=y
>>> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
>>> index 65bde11..875271c 100644
>>> --- a/hw/ipmi/Makefile.objs
>>> +++ b/hw/ipmi/Makefile.objs
>>> @@ -1 +1,2 @@
>>> common-obj-$(CONFIG_IPMI) += ipmi.o
>>> +common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_bmc_sim.o
>>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>>> new file mode 100644
>>> index 0000000..d246029
>>> --- /dev/null
>>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>>> @@ -0,0 +1,1731 @@
>>> +/*
>>> + * IPMI BMC emulation
>>> + *
>>> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining a
>>> copy
>>> + * of this software and associated documentation files (the "Software"),
>>> to deal
>>> + * in the Software without restriction, including without limitation the
>>> rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>>> sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be included
>>> in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>>> OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>>> FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>>> IN
>>> + * THE SOFTWARE.
>>> + */
>>> +
>>> +#include <stdio.h>
>>> +#include <string.h>
>>> +#include <stdint.h>
>>> +#include "qemu/timer.h"
>>> +#include "hw/ipmi/ipmi.h"
>>> +#include "qemu/error-report.h"
>>> +
>>> +#define IPMI_NETFN_CHASSIS 0x00
>>> +#define IPMI_NETFN_CHASSIS_MAXCMD 0x03
>>> +
>>> +#define IPMI_CMD_GET_CHASSIS_CAPABILITIES 0x00
>>> +#define IPMI_CMD_GET_CHASSIS_STATUS 0x01
>>> +#define IPMI_CMD_CHASSIS_CONTROL 0x02
>>> +
>>> +#define IPMI_NETFN_SENSOR_EVENT 0x04
>>> +#define IPMI_NETFN_SENSOR_EVENT_MAXCMD 0x2e
>>> +
>>> +#define IPMI_CMD_SET_SENSOR_EVT_ENABLE 0x28
>>> +#define IPMI_CMD_GET_SENSOR_EVT_ENABLE 0x29
>>> +#define IPMI_CMD_REARM_SENSOR_EVTS 0x2a
>>> +#define IPMI_CMD_GET_SENSOR_EVT_STATUS 0x2b
>>> +#define IPMI_CMD_GET_SENSOR_READING 0x2d
>> I have started adding a few commands and got scared by
>> IPMI_CMD_SET_SENSOR_READING spec. By any chance, would
>> you have one in stock ?
>
> I'm not sure what you mean. That was added recently and is optional.
> If your target system doesn't implement it, you don't really need to
> implement it, either.
>
> The IPMI spec is short on rationale, but I'm guessing that particular
> command has two purposes:
>
> * Sensors whose value is provided by software running on the system.
> * Testing for situations that you cannot reasonably reproduce on a
> real system.
>
> Is there a reason you would need this particular command?
Yes. The Open Power systems use this command to set the "System Progress
Sensor (0F)" and another one "Boot Count (C3) " which is an OEM one.
Looking at the code, I think the message we send as an issue with the
spec, or the receiver has non specified expectations :
https://github.com/open-power/skiboot/blob/master/hw/ipmi/ipmi-sensor.c
request.sensor_number = fw_sensor_num;
request.operation = 0xa0; /* Set event data bytes, assertion bits */
request.assertion_mask[0] = 0x04; /* Firmware progress offset */
request.event_data[1] = state;
Should not that be :
request.sensor_number = fw_sensor_num;
request.operation = 0xa0; /* Set event data bytes, assertion bits */
request.assertion_mask[0] = 0x04; /* Firmware progress offset (2) */
request.event_data[1] = 0xc2; /* event extension in byte2, event offset
2 */
request.event_data[2] = state;
?
> If so, it will take a little work but won't be too hard.
Well, the event data operations should be interesting ! Should we
generate an event for each command SET_SENSOR_READING we receive ?
Thanks,
C.
- Re: [Qemu-devel] [PATCH v4 02/17] Add a base IPMI interface, (continued)
- [Qemu-devel] [PATCH v4 13/17] pc: Postpone SMBIOS table installation to post machine init, minyard, 2015/11/12
- [Qemu-devel] [PATCH v4 09/17] ipmi: Add migration capability to the IPMI devices., minyard, 2015/11/12
- [Qemu-devel] [PATCH v4 12/17] smbios: Move table build tools into an include file., minyard, 2015/11/12
- [Qemu-devel] [PATCH v4 14/17] ipmi: Add SMBIOS table entry, minyard, 2015/11/12
- [Qemu-devel] [PATCH v4 06/17] ipmi: Add a BT low-level interface, minyard, 2015/11/12
- [Qemu-devel] [PATCH v4 03/17] ipmi: Add a local BMC simulation, minyard, 2015/11/12
- [Qemu-devel] [PATCH v4 17/17] ipmi: Add a force off function, minyard, 2015/11/12
[Qemu-devel] [PATCH v4 15/17] acpi: Add IPMI table entries, minyard, 2015/11/12
Re: [Qemu-devel] [PATCH v4 00/17] Add an IPMI device to QEMU, Cédric Le Goater, 2015/11/15
Re: [Qemu-devel] [PATCH v4 00/17] Add an IPMI device to QEMU, Benjamin Herrenschmidt, 2015/11/16