[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/8] ipmi: introduce an ipmi_bmc_init_sensor() A
|
From: |
Corey Minyard |
|
Subject: |
Re: [Qemu-devel] [PATCH 7/8] ipmi: introduce an ipmi_bmc_init_sensor() API |
|
Date: |
Fri, 8 Jan 2016 14:18:40 -0600 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 |
The way the SDR and sensors are handled currently in the code I wrote is
far from ideal, it's not scalable. In my mind, the BMC in qemu would
never be a very elaborate one, you would use an external BMC for that.
There are a couple of issues to deal with here:
We need support for SDRs besides just sensor type 2, there are sensor
type 1 and 3, management device locator, FRU device locator, entity
association, and a few others. Those are not important for the BMC, but
they are important for management software using the BMC. If we need to
add all those, we probably need something more sophisticated, like using
the openipmi library SDR compiler and loading the SDRs externally. But
if your needs are basic, then this is ok.
It would be nice if the SDR repository time did not change every time
you restarted the system. It's not a big deal for a few SDRs, but it
could be for a larger system.
I'm ok with the patch as-is assuming your needs are simple, but if you
need something more extensive we probably should think about something else.
A few comments inline, too.
On 01/05/2016 11:30 AM, Cédric Le Goater wrote:
This routine will let qemu platforms populate the sdr/sensor tables of
the IPMI BMC simulator with their customs needs.
The patch adds a compact sensor record typedef to ease definition of
sdrs. To be used in the code the following way:
static ipmi_sdr_compact_buffer my_init_sdrs[] = {
{ /* Firmware Progress Sensor */
0xff, 0xff, 0x51, 0x02, 43, 0x20, 0x00, 0xff,
0x22, 0x00, 0xff, 0x40, 0x0f, 0x6f, 0x07, 0x00,
0x00, 0x00, 0xff, 0xff, 0xc0, 0x00, 0x00, 0x01,
0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd0,
'F', 'W', ' ', 'B', 'o', 'o', 't', ' ',
'P', 'r', 'o', 'g', 'r', 'e', 's', 's',
},
...
I assume the idea is that you use struct ipmi_sdr_compact to define
these so you can get names associated with the values. Is that the case?
};
struct ipmi_sdr_compact *sdr =
(struct ipmi_sdr_compact *) &my_init_sdrs[0];
ipmi_bmc_init_sensor(IPMI_BMC(obj), my_init_sdrs[0],
sdr->rec_length + 5, &sdr->sensor_owner_number);
Signed-off-by: Cédric Le Goater <address@hidden>
---
hw/ipmi/ipmi_bmc_sim.c | 61 +++++++++++++++++++++++++++++++++++++++++---------
include/hw/ipmi/ipmi.h | 37 ++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 4f7c74da4b6b..9618db44ce69 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -527,6 +527,22 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs,
unsigned int sensor,
}
}
+static void ipmi_init_sensor(IPMISensor *sens, const uint8_t *sdr)
+{
+ IPMI_SENSOR_SET_PRESENT(sens, 1);
+ IPMI_SENSOR_SET_SCAN_ON(sens, (sdr[10] >> 6) & 1);
+ IPMI_SENSOR_SET_EVENTS_ON(sens, (sdr[10] >> 5) & 1);
+ sens->assert_suppt = sdr[14] | (sdr[15] << 8);
+ sens->deassert_suppt = sdr[16] | (sdr[17] << 8);
+ sens->states_suppt = sdr[18] | (sdr[19] << 8);
+ sens->sensor_type = sdr[12];
+ sens->evt_reading_type_code = sdr[13] & 0x7f;
Can you use struct ipmi_sdr_compact to extract these?
+
+ /* Enable all the events that are supported. */
+ sens->assert_enable = sens->assert_suppt;
+ sens->deassert_enable = sens->deassert_suppt;
+}
+
static void ipmi_init_sensors_from_sdrs(IPMIBmcSim *s)
{
unsigned int i, pos;
@@ -553,19 +569,42 @@ static void ipmi_init_sensors_from_sdrs(IPMIBmcSim *s)
}
sens = s->sensors + sdr[7];
- IPMI_SENSOR_SET_PRESENT(sens, 1);
- IPMI_SENSOR_SET_SCAN_ON(sens, (sdr[10] >> 6) & 1);
- IPMI_SENSOR_SET_EVENTS_ON(sens, (sdr[10] >> 5) & 1);
- sens->assert_suppt = sdr[14] | (sdr[15] << 8);
- sens->deassert_suppt = sdr[16] | (sdr[17] << 8);
- sens->states_suppt = sdr[18] | (sdr[19] << 8);
- sens->sensor_type = sdr[12];
- sens->evt_reading_type_code = sdr[13] & 0x7f;
+ ipmi_init_sensor(sens, sdr);
+ }
+}
+
+int ipmi_bmc_init_sensor(IPMIBmc *b, const uint8_t *sdr,
+ unsigned int len, uint8_t *sensor_num)
+{
+ IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
+ int ret;
+ unsigned int i;
+ IPMISensor *sens;
- /* Enable all the events that are supported. */
- sens->assert_enable = sens->assert_suppt;
- sens->deassert_enable = sens->deassert_suppt;
+ for (i = 0; i < MAX_SENSORS; i++) {
+ sens = ibs->sensors + i;
+ if (!IPMI_SENSOR_GET_PRESENT(sens)) {
+ break;
+ }
+ }
+
+ if (i == MAX_SENSORS) {
+ return 1;
}
+
+ ret = sdr_add_entry(ibs, sdr, len, NULL);
+ if (ret) {
+ return ret;
+ }
+
+ ipmi_init_sensor(sens, sdr);
+ if (sensor_num) {
+ *sensor_num = i;
+ }
+
+ /* patch sensor in sdr table. This is a little hacky. */
+ ibs->sdr.sdr[ibs->sdr.next_free - len + 7] = i;
+ return 0;
}
static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn,
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 32bac0fa8877..ce1f539754be 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -210,4 +210,41 @@ IPMIFwInfo *ipmi_next_fwinfo(IPMIFwInfo *current);
#define ipmi_debug(fs, ...)
#endif
+/*
+ * 43.2 SDR Type 02h. Compact Sensor Record
+ */
+struct ipmi_sdr_compact {
+ uint16_t rec_id;
+ uint8_t sdr_version; /* 0x51 */
+ uint8_t rec_type; /* 0x02 Compact Sensor Record */
+ uint8_t rec_length;
+ uint8_t sensor_owner_id;
+ uint8_t sensor_owner_lun;
+ uint8_t sensor_owner_number; /* byte 8 */
+ uint8_t entity_id;
+ uint8_t entity_instance;
+ uint8_t sensor_init;
+ uint8_t sensor_caps;
+ uint8_t sensor_type;
+ uint8_t reading_type;
+ uint8_t assert_mask[2]; /* byte 16 */
+ uint8_t deassert_mask[2];
+ uint8_t discrete_mask[2];
+ uint8_t sensor_unit1;
+ uint8_t sensor_unit2;
+ uint8_t sensor_unit3;
+ uint8_t sensor_direction[2]; /* byte 24 */
+ uint8_t positive_threshold;
+ uint8_t negative_threshold;
+ uint8_t reserved[3];
+ uint8_t oem;
+ uint8_t id_str_len; /* byte 32 */
+ uint8_t id_string[16];
+};
+
+typedef uint8_t ipmi_sdr_compact_buffer[sizeof(struct ipmi_sdr_compact)];
+
+int ipmi_bmc_init_sensor(IPMIBmc *b, const uint8_t *entry,
+ unsigned int len, uint8_t *sensor_num);
+
#endif