* [PATCH v3 1/5] ipmi/pci-ipmi-bt: Rename copy-paste variables
2025-04-01 14:01 [PATCH v3 0/5] ipmi: bmc-sim improvements Nicholas Piggin
@ 2025-04-01 14:01 ` Nicholas Piggin
2025-04-01 14:01 ` [PATCH v3 2/5] ipmi: add fwinfo to pci ipmi devices Nicholas Piggin
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-01 14:01 UTC (permalink / raw)
To: Corey Minyard; +Cc: Nicholas Piggin, qemu-devel, Philippe Mathieu-Daudé
IPMI drivers use p/k suffix in variable names depending on bt or kcs.
The pci bt driver must have come from the kcs driver because it's
still using k suffixes in some cases. Rename.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ipmi/pci_ipmi_bt.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
index afeea6f3031..a3b742d22c9 100644
--- a/hw/ipmi/pci_ipmi_bt.c
+++ b/hw/ipmi/pci_ipmi_bt.c
@@ -38,49 +38,49 @@ struct PCIIPMIBTDevice {
uint32_t uuid;
};
-static void pci_ipmi_raise_irq(IPMIBT *ik)
+static void pci_ipmi_raise_irq(IPMIBT *ib)
{
- PCIIPMIBTDevice *pik = ik->opaque;
+ PCIIPMIBTDevice *pib = ib->opaque;
- pci_set_irq(&pik->dev, true);
+ pci_set_irq(&pib->dev, true);
}
-static void pci_ipmi_lower_irq(IPMIBT *ik)
+static void pci_ipmi_lower_irq(IPMIBT *ib)
{
- PCIIPMIBTDevice *pik = ik->opaque;
+ PCIIPMIBTDevice *pib = ib->opaque;
- pci_set_irq(&pik->dev, false);
+ pci_set_irq(&pib->dev, false);
}
static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
{
Error *err = NULL;
- PCIIPMIBTDevice *pik = PCI_IPMI_BT(pd);
+ PCIIPMIBTDevice *pib = PCI_IPMI_BT(pd);
IPMIInterface *ii = IPMI_INTERFACE(pd);
IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
- if (!pik->bt.bmc) {
+ if (!pib->bt.bmc) {
error_setg(errp, "IPMI device requires a bmc attribute to be set");
return;
}
- pik->uuid = ipmi_next_uuid();
+ pib->uuid = ipmi_next_uuid();
- pik->bt.bmc->intf = ii;
- pik->bt.opaque = pik;
+ pib->bt.bmc->intf = ii;
+ pib->bt.opaque = pib;
pci_config_set_prog_interface(pd->config, 0x02); /* BT */
pci_config_set_interrupt_pin(pd->config, 0x01);
- pik->bt.use_irq = 1;
- pik->bt.raise_irq = pci_ipmi_raise_irq;
- pik->bt.lower_irq = pci_ipmi_lower_irq;
+ pib->bt.use_irq = 1;
+ pib->bt.raise_irq = pci_ipmi_raise_irq;
+ pib->bt.lower_irq = pci_ipmi_lower_irq;
iic->init(ii, 8, &err);
if (err) {
error_propagate(errp, err);
return;
}
- pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->bt.io);
+ pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pib->bt.io);
}
const VMStateDescription vmstate_PCIIPMIBTDevice = {
@@ -96,16 +96,16 @@ const VMStateDescription vmstate_PCIIPMIBTDevice = {
static void pci_ipmi_bt_instance_init(Object *obj)
{
- PCIIPMIBTDevice *pik = PCI_IPMI_BT(obj);
+ PCIIPMIBTDevice *pib = PCI_IPMI_BT(obj);
- ipmi_bmc_find_and_link(obj, (Object **) &pik->bt.bmc);
+ ipmi_bmc_find_and_link(obj, (Object **) &pib->bt.bmc);
}
static void *pci_ipmi_bt_get_backend_data(IPMIInterface *ii)
{
- PCIIPMIBTDevice *pik = PCI_IPMI_BT(ii);
+ PCIIPMIBTDevice *pib = PCI_IPMI_BT(ii);
- return &pik->bt;
+ return &pib->bt;
}
static void pci_ipmi_bt_class_init(ObjectClass *oc, void *data)
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 2/5] ipmi: add fwinfo to pci ipmi devices
2025-04-01 14:01 [PATCH v3 0/5] ipmi: bmc-sim improvements Nicholas Piggin
2025-04-01 14:01 ` [PATCH v3 1/5] ipmi/pci-ipmi-bt: Rename copy-paste variables Nicholas Piggin
@ 2025-04-01 14:01 ` Nicholas Piggin
2025-04-01 14:58 ` Philippe Mathieu-Daudé
2025-04-01 14:01 ` [PATCH v3 3/5] ipmi/bmc-sim: Add 'Get Channel Info' command Nicholas Piggin
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-01 14:01 UTC (permalink / raw)
To: Corey Minyard; +Cc: Nicholas Piggin, qemu-devel
This requires some adjustments to callers to avoid possible behaviour
changes for PCI devices.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
include/hw/ipmi/ipmi.h | 5 +++++
hw/acpi/ipmi.c | 3 ++-
hw/ipmi/isa_ipmi_bt.c | 1 +
hw/ipmi/isa_ipmi_kcs.c | 1 +
hw/ipmi/pci_ipmi_bt.c | 12 ++++++++++++
hw/ipmi/pci_ipmi_kcs.c | 11 +++++++++++
hw/smbios/smbios_type_38.c | 7 ++++++-
7 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 77a7213ed93..c8ef04856e1 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -90,6 +90,11 @@ typedef struct IPMIFwInfo {
} memspace;
int interrupt_number;
+ enum {
+ IPMI_NO_IRQ = 0,
+ IPMI_ISA_IRQ,
+ IPMI_PCI_IRQ,
+ } irq_source;
enum {
IPMI_LEVEL_IRQ,
IPMI_EDGE_IRQ
diff --git a/hw/acpi/ipmi.c b/hw/acpi/ipmi.c
index a20e57d465c..39f8f2f6d68 100644
--- a/hw/acpi/ipmi.c
+++ b/hw/acpi/ipmi.c
@@ -55,7 +55,8 @@ static Aml *aml_ipmi_crs(IPMIFwInfo *info)
abort();
}
- if (info->interrupt_number) {
+ /* Should PCI interrupts also be appended? */
+ if (info->irq_source == IPMI_ISA_IRQ && info->interrupt_number) {
aml_append(crs, aml_irq_no_flags(info->interrupt_number));
}
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index a1b66d5ee82..4a6496b5980 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -49,6 +49,7 @@ static void isa_ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
ISAIPMIBTDevice *iib = ISA_IPMI_BT(ii);
ipmi_bt_get_fwinfo(&iib->bt, info);
+ info->irq_source = IPMI_ISA_IRQ;
info->interrupt_number = iib->isairq;
info->i2c_slave_address = iib->bt.bmc->slave_addr;
info->uuid = iib->uuid;
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index d9ebdd5371f..31d16aba74e 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -49,6 +49,7 @@ static void isa_ipmi_kcs_get_fwinfo(IPMIInterface *ii, IPMIFwInfo *info)
ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(ii);
ipmi_kcs_get_fwinfo(&iik->kcs, info);
+ info->irq_source = IPMI_ISA_IRQ;
info->interrupt_number = iik->isairq;
info->uuid = iik->uuid;
}
diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
index a3b742d22c9..7ba8b3ab965 100644
--- a/hw/ipmi/pci_ipmi_bt.c
+++ b/hw/ipmi/pci_ipmi_bt.c
@@ -38,6 +38,17 @@ struct PCIIPMIBTDevice {
uint32_t uuid;
};
+static void pci_ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
+{
+ PCIIPMIBTDevice *pib = PCI_IPMI_BT(ii);
+
+ ipmi_bt_get_fwinfo(&pib->bt, info);
+ info->irq_source = IPMI_PCI_IRQ;
+ info->interrupt_number = pci_intx(&pib->dev);
+ info->i2c_slave_address = pib->bt.bmc->slave_addr;
+ info->uuid = pib->uuid;
+}
+
static void pci_ipmi_raise_irq(IPMIBT *ib)
{
PCIIPMIBTDevice *pib = ib->opaque;
@@ -125,6 +136,7 @@ static void pci_ipmi_bt_class_init(ObjectClass *oc, void *data)
iic->get_backend_data = pci_ipmi_bt_get_backend_data;
ipmi_bt_class_init(iic);
+ iic->get_fwinfo = pci_ipmi_bt_get_fwinfo;
}
static const TypeInfo pci_ipmi_bt_info = {
diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
index 05ba97ec58f..c817f1e8808 100644
--- a/hw/ipmi/pci_ipmi_kcs.c
+++ b/hw/ipmi/pci_ipmi_kcs.c
@@ -38,6 +38,16 @@ struct PCIIPMIKCSDevice {
uint32_t uuid;
};
+static void pci_ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
+{
+ PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(ii);
+
+ ipmi_kcs_get_fwinfo(&pik->kcs, info);
+ info->irq_source = IPMI_PCI_IRQ;
+ info->interrupt_number = pci_intx(&pik->dev);
+ info->uuid = pik->uuid;
+}
+
static void pci_ipmi_raise_irq(IPMIKCS *ik)
{
PCIIPMIKCSDevice *pik = ik->opaque;
@@ -125,6 +135,7 @@ static void pci_ipmi_kcs_class_init(ObjectClass *oc, void *data)
iic->get_backend_data = pci_ipmi_kcs_get_backend_data;
ipmi_kcs_class_init(iic);
+ iic->get_fwinfo = pci_ipmi_kcs_get_fwinfo;
}
static const TypeInfo pci_ipmi_kcs_info = {
diff --git a/hw/smbios/smbios_type_38.c b/hw/smbios/smbios_type_38.c
index 168b886647d..e9b856fcd96 100644
--- a/hw/smbios/smbios_type_38.c
+++ b/hw/smbios/smbios_type_38.c
@@ -72,7 +72,12 @@ static void smbios_build_one_type_38(IPMIFwInfo *info)
" SMBIOS, ignoring this entry.", info->register_spacing);
return;
}
- t->interrupt_number = info->interrupt_number;
+ if (info->irq_source == IPMI_ISA_IRQ) {
+ t->interrupt_number = info->interrupt_number;
+ } else {
+ /* TODO: How to handle PCI? */
+ t->interrupt_number = 0;
+ }
SMBIOS_BUILD_TABLE_POST;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 2/5] ipmi: add fwinfo to pci ipmi devices
2025-04-01 14:01 ` [PATCH v3 2/5] ipmi: add fwinfo to pci ipmi devices Nicholas Piggin
@ 2025-04-01 14:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-01 14:58 UTC (permalink / raw)
To: Nicholas Piggin, Corey Minyard; +Cc: qemu-devel
On 1/4/25 16:01, Nicholas Piggin wrote:
> This requires some adjustments to callers to avoid possible behaviour
> changes for PCI devices.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> include/hw/ipmi/ipmi.h | 5 +++++
> hw/acpi/ipmi.c | 3 ++-
> hw/ipmi/isa_ipmi_bt.c | 1 +
> hw/ipmi/isa_ipmi_kcs.c | 1 +
> hw/ipmi/pci_ipmi_bt.c | 12 ++++++++++++
> hw/ipmi/pci_ipmi_kcs.c | 11 +++++++++++
> hw/smbios/smbios_type_38.c | 7 ++++++-
> 7 files changed, 38 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/5] ipmi/bmc-sim: Add 'Get Channel Info' command
2025-04-01 14:01 [PATCH v3 0/5] ipmi: bmc-sim improvements Nicholas Piggin
2025-04-01 14:01 ` [PATCH v3 1/5] ipmi/pci-ipmi-bt: Rename copy-paste variables Nicholas Piggin
2025-04-01 14:01 ` [PATCH v3 2/5] ipmi: add fwinfo to pci ipmi devices Nicholas Piggin
@ 2025-04-01 14:01 ` Nicholas Piggin
2025-04-01 14:01 ` [PATCH v3 4/5] ipmi/bmc-sim: implement watchdog dont log flag Nicholas Piggin
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-01 14:01 UTC (permalink / raw)
To: Corey Minyard; +Cc: Nicholas Piggin, qemu-devel
Linux issues this command when booting a powernv machine.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
include/hw/ipmi/ipmi.h | 10 +++++++
hw/ipmi/ipmi_bmc_sim.c | 68 ++++++++++++++++++++++++++++++++++++++++--
hw/ipmi/ipmi_bt.c | 2 ++
hw/ipmi/ipmi_kcs.c | 1 +
4 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index c8ef04856e1..802a2febb07 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -41,6 +41,15 @@ enum ipmi_op {
IPMI_SEND_NMI
};
+/* Channel properties */
+#define IPMI_CHANNEL_IPMB 0x00
+#define IPMI_CHANNEL_SYSTEM 0x0f
+#define IPMI_CHANNEL_MEDIUM_IPMB 0x01
+#define IPMI_CHANNEL_MEDIUM_SYSTEM 0x0c
+#define IPMI_CHANNEL_PROTOCOL_IPMB 0x01
+#define IPMI_CHANNEL_PROTOCOL_KCS 0x05
+#define IPMI_CHANNEL_PROTOCOL_BT_15 0x08
+
#define IPMI_CC_INVALID_CMD 0xc1
#define IPMI_CC_COMMAND_INVALID_FOR_LUN 0xc2
#define IPMI_CC_TIMEOUT 0xc3
@@ -76,6 +85,7 @@ typedef struct IPMIFwInfo {
int interface_type;
uint8_t ipmi_spec_major_revision;
uint8_t ipmi_spec_minor_revision;
+ uint8_t ipmi_channel_protocol;
uint8_t i2c_slave_address;
uint32_t uuid;
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 6157ac71201..d63f2348ba1 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -70,6 +70,7 @@
#define IPMI_CMD_GET_MSG 0x33
#define IPMI_CMD_SEND_MSG 0x34
#define IPMI_CMD_READ_EVT_MSG_BUF 0x35
+#define IPMI_CMD_GET_CHANNEL_INFO 0x42
#define IPMI_NETFN_STORAGE 0x0a
@@ -1020,8 +1021,8 @@ static void send_msg(IPMIBmcSim *ibs,
uint8_t *buf;
uint8_t netfn, rqLun, rsLun, rqSeq;
- if (cmd[2] != 0) {
- /* We only handle channel 0 with no options */
+ if (cmd[2] != IPMI_CHANNEL_IPMB) {
+ /* We only handle channel 0h (IPMB) with no options */
rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
return;
}
@@ -1219,6 +1220,68 @@ static void get_watchdog_timer(IPMIBmcSim *ibs,
}
}
+static void get_channel_info(IPMIBmcSim *ibs,
+ uint8_t *cmd, unsigned int cmd_len,
+ RspBuffer *rsp)
+{
+ IPMIInterface *s = ibs->parent.intf;
+ IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
+ IPMIFwInfo info = {};
+ uint8_t ch = cmd[2] & 0x0f;
+
+ /* Only define channel 0h (IPMB) and Fh (system interface) */
+
+ if (ch == 0x0e) { /* "This channel" */
+ ch = IPMI_CHANNEL_SYSTEM;
+ }
+ rsp_buffer_push(rsp, ch);
+
+ if (ch != IPMI_CHANNEL_IPMB && ch != IPMI_CHANNEL_SYSTEM) {
+ /* Not a supported channel */
+ rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
+ return;
+ }
+
+ if (k->get_fwinfo) {
+ k->get_fwinfo(s, &info);
+ }
+
+ if (ch == IPMI_CHANNEL_IPMB) {
+ rsp_buffer_push(rsp, IPMI_CHANNEL_MEDIUM_IPMB);
+ rsp_buffer_push(rsp, IPMI_CHANNEL_PROTOCOL_IPMB);
+ } else { /* IPMI_CHANNEL_SYSTEM */
+ rsp_buffer_push(rsp, IPMI_CHANNEL_MEDIUM_SYSTEM);
+ rsp_buffer_push(rsp, info.ipmi_channel_protocol);
+ }
+
+ rsp_buffer_push(rsp, 0x00); /* Session-less */
+
+ /* IPMI Enterprise Number for Vendor ID */
+ rsp_buffer_push(rsp, 0xf2);
+ rsp_buffer_push(rsp, 0x1b);
+ rsp_buffer_push(rsp, 0x00);
+
+ if (ch == IPMI_CHANNEL_SYSTEM) {
+ uint8_t irq;
+
+ if (info.irq_source == IPMI_ISA_IRQ) {
+ irq = info.interrupt_number;
+ } else if (info.irq_source == IPMI_PCI_IRQ) {
+ irq = 0x10 + info.interrupt_number;
+ } else {
+ irq = 0xff; /* no interrupt / unspecified */
+ }
+
+ /* Both interrupts use the same irq number */
+ rsp_buffer_push(rsp, irq);
+ rsp_buffer_push(rsp, irq);
+ } else {
+ /* Reserved */
+ rsp_buffer_push(rsp, 0x00);
+ rsp_buffer_push(rsp, 0x00);
+ }
+}
+
static void get_sdr_rep_info(IPMIBmcSim *ibs,
uint8_t *cmd, unsigned int cmd_len,
RspBuffer *rsp)
@@ -2015,6 +2078,7 @@ static const IPMICmdHandler app_cmds[] = {
[IPMI_CMD_RESET_WATCHDOG_TIMER] = { reset_watchdog_timer },
[IPMI_CMD_SET_WATCHDOG_TIMER] = { set_watchdog_timer, 8 },
[IPMI_CMD_GET_WATCHDOG_TIMER] = { get_watchdog_timer },
+ [IPMI_CMD_GET_CHANNEL_INFO] = { get_channel_info, 3 },
};
static const IPMINetfn app_netfn = {
.cmd_nums = ARRAY_SIZE(app_cmds),
diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
index 583fc64730c..28cf6ab2185 100644
--- a/hw/ipmi/ipmi_bt.c
+++ b/hw/ipmi/ipmi_bt.c
@@ -419,6 +419,8 @@ void ipmi_bt_get_fwinfo(struct IPMIBT *ib, IPMIFwInfo *info)
info->interface_type = IPMI_SMBIOS_BT;
info->ipmi_spec_major_revision = 2;
info->ipmi_spec_minor_revision = 0;
+ /* BT System Interface Format, IPMI v1.5 */
+ info->ipmi_channel_protocol = IPMI_CHANNEL_PROTOCOL_BT_15;
info->base_address = ib->io_base;
info->register_length = ib->io_length;
info->register_spacing = 1;
diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
index c15977cab4c..578dd7cef34 100644
--- a/hw/ipmi/ipmi_kcs.c
+++ b/hw/ipmi/ipmi_kcs.c
@@ -405,6 +405,7 @@ void ipmi_kcs_get_fwinfo(IPMIKCS *ik, IPMIFwInfo *info)
info->interface_type = IPMI_SMBIOS_KCS;
info->ipmi_spec_major_revision = 2;
info->ipmi_spec_minor_revision = 0;
+ info->ipmi_channel_protocol = IPMI_CHANNEL_PROTOCOL_KCS;
info->base_address = ik->io_base;
info->i2c_slave_address = ik->bmc->slave_addr;
info->register_length = ik->io_length;
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 4/5] ipmi/bmc-sim: implement watchdog dont log flag
2025-04-01 14:01 [PATCH v3 0/5] ipmi: bmc-sim improvements Nicholas Piggin
` (2 preceding siblings ...)
2025-04-01 14:01 ` [PATCH v3 3/5] ipmi/bmc-sim: Add 'Get Channel Info' command Nicholas Piggin
@ 2025-04-01 14:01 ` Nicholas Piggin
2025-04-01 14:01 ` [PATCH v3 5/5] ipmi/bmc-sim: add error handling for 'Set BMC Global Enables' command Nicholas Piggin
2025-04-01 19:17 ` [PATCH v3 0/5] ipmi: bmc-sim improvements Corey Minyard
5 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-01 14:01 UTC (permalink / raw)
To: Corey Minyard; +Cc: Nicholas Piggin, qemu-devel
If the dont-log flag is set in the 'timer use' field for the
'set watchdog' command, a watchdog timeout will not get logged as
a timer use expiration.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ipmi/ipmi_bmc_sim.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index d63f2348ba1..98cfab6a455 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -514,7 +514,8 @@ static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
unsigned int bit, unsigned int val,
- uint8_t evd1, uint8_t evd2, uint8_t evd3)
+ uint8_t evd1, uint8_t evd2, uint8_t evd3,
+ bool do_log)
{
IPMISensor *sens;
uint16_t mask;
@@ -534,7 +535,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
return; /* Already asserted */
}
sens->assert_states |= mask & sens->assert_suppt;
- if (sens->assert_enable & mask & sens->assert_states) {
+ if (do_log && (sens->assert_enable & mask & sens->assert_states)) {
/* Send an event on assert */
gen_event(ibs, sensor, 0, evd1, evd2, evd3);
}
@@ -544,7 +545,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
return; /* Already deasserted */
}
sens->deassert_states |= mask & sens->deassert_suppt;
- if (sens->deassert_enable & mask & sens->deassert_states) {
+ if (do_log && (sens->deassert_enable & mask & sens->deassert_states)) {
/* Send an event on deassert */
gen_event(ibs, sensor, 1, evd1, evd2, evd3);
}
@@ -700,6 +701,7 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
{
IPMIInterface *s = ibs->parent.intf;
IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
+ bool do_log = !IPMI_BMC_WATCHDOG_GET_DONT_LOG(ibs);
if (!ibs->watchdog_running) {
goto out;
@@ -711,14 +713,16 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
k->do_hw_op(s, IPMI_SEND_NMI, 0);
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
- 0xc8, (2 << 4) | 0xf, 0xff);
+ 0xc8, (2 << 4) | 0xf, 0xff,
+ do_log);
break;
case IPMI_BMC_WATCHDOG_PRE_MSG_INT:
ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
k->set_atn(s, 1, attn_irq_enabled(ibs));
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
- 0xc8, (3 << 4) | 0xf, 0xff);
+ 0xc8, (3 << 4) | 0xf, 0xff,
+ do_log);
break;
default:
@@ -738,24 +742,28 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
switch (IPMI_BMC_WATCHDOG_GET_ACTION(ibs)) {
case IPMI_BMC_WATCHDOG_ACTION_NONE:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 0, 1,
- 0xc0, ibs->watchdog_use & 0xf, 0xff);
+ 0xc0, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
break;
case IPMI_BMC_WATCHDOG_ACTION_RESET:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 1, 1,
- 0xc1, ibs->watchdog_use & 0xf, 0xff);
+ 0xc1, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
break;
case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 2, 1,
- 0xc2, ibs->watchdog_use & 0xf, 0xff);
+ 0xc2, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
break;
case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 2, 1,
- 0xc3, ibs->watchdog_use & 0xf, 0xff);
+ 0xc3, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
break;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 5/5] ipmi/bmc-sim: add error handling for 'Set BMC Global Enables' command
2025-04-01 14:01 [PATCH v3 0/5] ipmi: bmc-sim improvements Nicholas Piggin
` (3 preceding siblings ...)
2025-04-01 14:01 ` [PATCH v3 4/5] ipmi/bmc-sim: implement watchdog dont log flag Nicholas Piggin
@ 2025-04-01 14:01 ` Nicholas Piggin
2025-04-01 19:17 ` [PATCH v3 0/5] ipmi: bmc-sim improvements Corey Minyard
5 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-01 14:01 UTC (permalink / raw)
To: Corey Minyard; +Cc: Nicholas Piggin, qemu-devel
Mask out unsupported bits and return failure if attempting to set
any. This is not required by the IPMI spec, but it does require that
system software not change bits it isn't aware of.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ipmi/ipmi_bmc_sim.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 98cfab6a455..3c008a3a603 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -235,6 +235,7 @@ struct IPMIBmcSim {
#define IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE_SET(s) \
(IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE & (s)->msg_flags)
+#define IPMI_BMC_GLOBAL_ENABLES_SUPPORTED 0x0f
#define IPMI_BMC_RCV_MSG_QUEUE_INT_BIT 0
#define IPMI_BMC_EVBUF_FULL_INT_BIT 1
#define IPMI_BMC_EVENT_MSG_BUF_BIT 2
@@ -934,7 +935,14 @@ static void set_bmc_global_enables(IPMIBmcSim *ibs,
uint8_t *cmd, unsigned int cmd_len,
RspBuffer *rsp)
{
- set_global_enables(ibs, cmd[2]);
+ uint8_t val = cmd[2];
+
+ if (val & ~IPMI_BMC_GLOBAL_ENABLES_SUPPORTED) {
+ rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
+ return;
+ }
+
+ set_global_enables(ibs, val);
}
static void get_bmc_global_enables(IPMIBmcSim *ibs,
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/5] ipmi: bmc-sim improvements
2025-04-01 14:01 [PATCH v3 0/5] ipmi: bmc-sim improvements Nicholas Piggin
` (4 preceding siblings ...)
2025-04-01 14:01 ` [PATCH v3 5/5] ipmi/bmc-sim: add error handling for 'Set BMC Global Enables' command Nicholas Piggin
@ 2025-04-01 19:17 ` Corey Minyard
2025-04-11 6:25 ` Nicholas Piggin
5 siblings, 1 reply; 11+ messages in thread
From: Corey Minyard @ 2025-04-01 19:17 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Corey Minyard, qemu-devel
On Wed, Apr 02, 2025 at 12:01:47AM +1000, Nicholas Piggin wrote:
> These little things came up when looking at behaviour of IPMI with
> the bmc-sim implementation running the ppc powernv machine, and
> trying to clean up error messages and missing features.
This all looks good to me. Thanks to Philippe for reviewing, too.
I can give you an
Acked-by: Corey Minyard <cminyard@mvista.com>
if you are working on this for your own tree, or I can take it into
mine.
-corey
>
> Since v1 (thanks to Corey for review and suggestions):
> - Added fwinfo to PCI devices
> - Report interrupt number in Get Channel Info for ISA, PCI, and
> unknown/unassigned.
> - Fix error reporting for Get Channel Info unsupported channels.
> Verify it is the correct error code that ipmitool looks for
> https://github.com/ipmitool/ipmitool/blob/master/lib/ipmi_channel.c#L256C16-L256C45
> - Change _CH_ to _CHANNEL_ in some defines names.
> - Also avoid adding event logs with watchdog don't log flag.
>
> Since v2:
> - Don't log watchog flag should not apply to watchdog expiry
> field.
> - Moved protocol type field from class to IPMIFwInfo.
> - Rename new FwInfo member irq to irq_source.
> - Add comments about handling PCI devices to existing callers
> of ->fwinfo
>
> Thanks,
> Nick
>
>
> Nicholas Piggin (5):
> ipmi/pci-ipmi-bt: Rename copy-paste variables
> ipmi: add fwinfo to pci ipmi devices
> ipmi/bmc-sim: Add 'Get Channel Info' command
> ipmi/bmc-sim: implement watchdog dont log flag
> ipmi/bmc-sim: add error handling for 'Set BMC Global Enables' command
>
> include/hw/ipmi/ipmi.h | 15 ++++++
> hw/acpi/ipmi.c | 3 +-
> hw/ipmi/ipmi_bmc_sim.c | 104 ++++++++++++++++++++++++++++++++-----
> hw/ipmi/ipmi_bt.c | 2 +
> hw/ipmi/ipmi_kcs.c | 1 +
> hw/ipmi/isa_ipmi_bt.c | 1 +
> hw/ipmi/isa_ipmi_kcs.c | 1 +
> hw/ipmi/pci_ipmi_bt.c | 50 +++++++++++-------
> hw/ipmi/pci_ipmi_kcs.c | 11 ++++
> hw/smbios/smbios_type_38.c | 7 ++-
> 10 files changed, 162 insertions(+), 33 deletions(-)
>
> --
> 2.47.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/5] ipmi: bmc-sim improvements
2025-04-01 19:17 ` [PATCH v3 0/5] ipmi: bmc-sim improvements Corey Minyard
@ 2025-04-11 6:25 ` Nicholas Piggin
2025-04-11 18:20 ` Corey Minyard
0 siblings, 1 reply; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-11 6:25 UTC (permalink / raw)
To: corey; +Cc: Corey Minyard, qemu-devel
On Wed Apr 2, 2025 at 5:17 AM AEST, Corey Minyard wrote:
> On Wed, Apr 02, 2025 at 12:01:47AM +1000, Nicholas Piggin wrote:
>> These little things came up when looking at behaviour of IPMI with
>> the bmc-sim implementation running the ppc powernv machine, and
>> trying to clean up error messages and missing features.
>
> This all looks good to me. Thanks to Philippe for reviewing, too.
>
> I can give you an
>
> Acked-by: Corey Minyard <cminyard@mvista.com>
>
> if you are working on this for your own tree, or I can take it into
> mine.
Hey Corey,
Thanks for all the review, and sorry I missed your question...
I don't have anything further in my tree, I don't have an
immediate need for it, it was just tidying up a few errors
and warnings I noticed.
I'd be happy for you to take it in your tree and send it up
when it suits you.
Thanks,
Nick
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/5] ipmi: bmc-sim improvements
2025-04-11 6:25 ` Nicholas Piggin
@ 2025-04-11 18:20 ` Corey Minyard
2025-04-12 5:19 ` Nicholas Piggin
0 siblings, 1 reply; 11+ messages in thread
From: Corey Minyard @ 2025-04-11 18:20 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Corey Minyard, qemu-devel
On Fri, Apr 11, 2025 at 04:25:10PM +1000, Nicholas Piggin wrote:
> On Wed Apr 2, 2025 at 5:17 AM AEST, Corey Minyard wrote:
> > On Wed, Apr 02, 2025 at 12:01:47AM +1000, Nicholas Piggin wrote:
> >> These little things came up when looking at behaviour of IPMI with
> >> the bmc-sim implementation running the ppc powernv machine, and
> >> trying to clean up error messages and missing features.
> >
> > This all looks good to me. Thanks to Philippe for reviewing, too.
> >
> > I can give you an
> >
> > Acked-by: Corey Minyard <cminyard@mvista.com>
> >
> > if you are working on this for your own tree, or I can take it into
> > mine.
>
> Hey Corey,
>
> Thanks for all the review, and sorry I missed your question...
>
> I don't have anything further in my tree, I don't have an
> immediate need for it, it was just tidying up a few errors
> and warnings I noticed.
>
> I'd be happy for you to take it in your tree and send it up
> when it suits you.
Ok, it's in my tree. I had to fix up pci_ipmi_kcs.c, it defined
pci_ipmi_bt_get_fwinfo, not pci_ipmi_kcs_get_fwinfo.
-corey
>
> Thanks,
> Nick
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/5] ipmi: bmc-sim improvements
2025-04-11 18:20 ` Corey Minyard
@ 2025-04-12 5:19 ` Nicholas Piggin
0 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2025-04-12 5:19 UTC (permalink / raw)
To: corey; +Cc: Corey Minyard, qemu-devel
On Sat Apr 12, 2025 at 4:20 AM AEST, Corey Minyard wrote:
> On Fri, Apr 11, 2025 at 04:25:10PM +1000, Nicholas Piggin wrote:
>> On Wed Apr 2, 2025 at 5:17 AM AEST, Corey Minyard wrote:
>> > On Wed, Apr 02, 2025 at 12:01:47AM +1000, Nicholas Piggin wrote:
>> >> These little things came up when looking at behaviour of IPMI with
>> >> the bmc-sim implementation running the ppc powernv machine, and
>> >> trying to clean up error messages and missing features.
>> >
>> > This all looks good to me. Thanks to Philippe for reviewing, too.
>> >
>> > I can give you an
>> >
>> > Acked-by: Corey Minyard <cminyard@mvista.com>
>> >
>> > if you are working on this for your own tree, or I can take it into
>> > mine.
>>
>> Hey Corey,
>>
>> Thanks for all the review, and sorry I missed your question...
>>
>> I don't have anything further in my tree, I don't have an
>> immediate need for it, it was just tidying up a few errors
>> and warnings I noticed.
>>
>> I'd be happy for you to take it in your tree and send it up
>> when it suits you.
>
> Ok, it's in my tree. I had to fix up pci_ipmi_kcs.c, it defined
> pci_ipmi_bt_get_fwinfo, not pci_ipmi_kcs_get_fwinfo.
Thanks, I noiced that just now too I thought I'd had a config
that built it but clearly not :( Sorry.
Thanks,
Nick
^ permalink raw reply [flat|nested] 11+ messages in thread