qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] ipmi: Bug fixes, add new interfaces
@ 2019-09-19 21:39 minyard
  2019-09-19 21:39 ` [PATCH 01/15] ipmi: Fix watchdog NMI handling minyard
                   ` (16 more replies)
  0 siblings, 17 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

I haven't gotten a lot of commentary on this, but I assume that means
that everything is ok.  It's been posted a few times and the last time
I received no issues, just a couple of reviews.  I would like more
review.  But I'm not quite sure what to do about that, I've been
hanging on to these changes far too long.

The following changes since commit a77d20bafcd4cb7684168a9b4c6dc2a321aaeb50:

  Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190919-pull-request' into staging (2019-09-19 17:16:07 +0100)

are available in the Git repository at:

  https://github.com/cminyard/qemu.git tags/ipmi-for-release-2019-09-19

for you to fetch changes up to d9b74295c6528fd68cebdea116b283e46543b2a2:

  pc: Add an SMB0 ACPI device to q35 (2019-09-19 14:41:58 -0500)

----------------------------------------------------------------
ipmi: Some bug fixes and new interfaces

Some bug fixes for the watchdog and hopeful the BT tests.

Change the IPMI UUID handling to give the user the ability to set it or
not have it.

Add a PCI interface.

Add an SMBus interfaces.

-corey

----------------------------------------------------------------
Corey Minyard (15):
      ipmi: Fix watchdog NMI handling
      ipmi: Fix the get watchdog command
      ipmi: Generate an interrupt on watchdog pretimeout expiry
      tests:ipmi: Fix IPMI BT tests
      qdev: Add a no default uuid property
      ipmi: Add a UUID device property
      ipmi: Split out KCS-specific code from ISA KCS code
      ipmi: Split out BT-specific code from ISA BT code
      ipmi: Allow a size value to be passed for I/O space
      smbios:ipmi: Ignore IPMI devices with no fwinfo function
      ipmi: Add PCI IPMI interfaces
      ipmi: Add an SMBus IPMI interface
      acpi: Add i2c serial bus CRS handling
      ipmi: Fix SSIF ACPI handling to use the right CRS
      pc: Add an SMB0 ACPI device to q35

 default-configs/i386-softmmu.mak |   3 +
 hw/acpi/aml-build.c              |  40 ++++
 hw/acpi/ipmi-stub.c              |   2 +-
 hw/acpi/ipmi.c                   |  13 +-
 hw/i386/Kconfig                  |   3 +
 hw/i386/acpi-build.c             |  17 +-
 hw/i386/pc_piix.c                |  12 +-
 hw/i386/pc_q35.c                 |   9 +-
 hw/ipmi/Kconfig                  |  15 ++
 hw/ipmi/Makefile.objs            |   5 +-
 hw/ipmi/ipmi.c                   |   6 +-
 hw/ipmi/ipmi_bmc_sim.c           |  30 ++-
 hw/ipmi/ipmi_bt.c                | 437 ++++++++++++++++++++++++++++++++++++++
 hw/ipmi/ipmi_kcs.c               | 423 +++++++++++++++++++++++++++++++++++++
 hw/ipmi/isa_ipmi_bt.c            | 443 ++-------------------------------------
 hw/ipmi/isa_ipmi_kcs.c           | 419 ++----------------------------------
 hw/ipmi/pci_ipmi_bt.c            | 146 +++++++++++++
 hw/ipmi/pci_ipmi_kcs.c           | 146 +++++++++++++
 hw/ipmi/smbus_ipmi.c             | 384 +++++++++++++++++++++++++++++++++
 hw/smbios/smbios_type_38.c       |   3 +
 include/hw/acpi/aml-build.h      |  18 ++
 include/hw/acpi/ipmi.h           |   2 +-
 include/hw/i386/pc.h             |   2 +
 include/hw/ipmi/ipmi.h           |   7 +-
 include/hw/ipmi/ipmi_bt.h        |  73 +++++++
 include/hw/ipmi/ipmi_kcs.h       |  76 +++++++
 include/hw/pci/pci.h             |   1 +
 include/hw/qdev-properties.h     |   7 +
 qemu-options.hx                  |  10 +-
 tests/Makefile.include           |   3 +-
 tests/data/acpi/q35/DSDT         | Bin 7841 -> 7879 bytes
 tests/data/acpi/q35/DSDT.bridge  | Bin 7858 -> 7896 bytes
 tests/data/acpi/q35/DSDT.cphp    | Bin 8304 -> 8342 bytes
 tests/data/acpi/q35/DSDT.dimmpxm | Bin 9494 -> 9532 bytes
 tests/data/acpi/q35/DSDT.ipmibt  | Bin 7916 -> 7954 bytes
 tests/data/acpi/q35/DSDT.memhp   | Bin 9200 -> 9238 bytes
 tests/data/acpi/q35/DSDT.mmio64  | Bin 8971 -> 9009 bytes
 tests/data/acpi/q35/DSDT.numamem | Bin 7847 -> 7885 bytes
 tests/ipmi-bt-test.c             |   6 +-
 39 files changed, 1902 insertions(+), 859 deletions(-)
 create mode 100644 hw/ipmi/ipmi_bt.c
 create mode 100644 hw/ipmi/ipmi_kcs.c
 create mode 100644 hw/ipmi/pci_ipmi_bt.c
 create mode 100644 hw/ipmi/pci_ipmi_kcs.c
 create mode 100644 hw/ipmi/smbus_ipmi.c
 create mode 100644 include/hw/ipmi/ipmi_bt.h
 create mode 100644 include/hw/ipmi/ipmi_kcs.h





^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 01/15] ipmi: Fix watchdog NMI handling
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-20 15:45   ` Cédric Le Goater
  2019-09-19 21:39 ` [PATCH 02/15] ipmi: Fix the get watchdog command minyard
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

The wrong logic was used for detection (so it wouldn't work at all)
and the wrong interface was used to inject the NMI if the detection
logic was correct.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi.c         | 6 +++---
 hw/ipmi/ipmi_bmc_sim.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 136c86b7a7..cbe158f815 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -28,9 +28,8 @@
 #include "qom/object_interfaces.h"
 #include "sysemu/runstate.h"
 #include "qapi/error.h"
-#include "qapi/qapi-commands-misc.h"
-#include "qapi/visitor.h"
 #include "qemu/module.h"
+#include "hw/nmi.h"
 
 static uint32_t ipmi_current_uuid = 1;
 
@@ -60,7 +59,8 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
         if (checkonly) {
             return 0;
         }
-        qmp_inject_nmi(NULL);
+        /* We don't care what CPU we use. */
+        nmi_monitor_handle(0, NULL);
         return 0;
 
     case IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP:
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 246a6d390c..8f63bb7181 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -1194,7 +1194,7 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
         break;
 
     case IPMI_BMC_WATCHDOG_PRE_NMI:
-        if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
+        if (k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
             /* NMI not supported. */
             rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
             return;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 02/15] ipmi: Fix the get watchdog command
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
  2019-09-19 21:39 ` [PATCH 01/15] ipmi: Fix watchdog NMI handling minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-20 15:50   ` Cédric Le Goater
  2019-09-19 21:39 ` [PATCH 03/15] ipmi: Generate an interrupt on watchdog pretimeout expiry minyard
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

It wasn't returning the set timeout like it should have been.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 8f63bb7181..afb99e33d7 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -1228,6 +1228,8 @@ static void get_watchdog_timer(IPMIBmcSim *ibs,
     rsp_buffer_push(rsp, ibs->watchdog_action);
     rsp_buffer_push(rsp, ibs->watchdog_pretimeout);
     rsp_buffer_push(rsp, ibs->watchdog_expired);
+    rsp_buffer_push(rsp, ibs->watchdog_timeout & 0xff);
+    rsp_buffer_push(rsp, (ibs->watchdog_timeout >> 8) & 0xff);
     if (ibs->watchdog_running) {
         long timeout;
         timeout = ((ibs->watchdog_expiry - ipmi_getmonotime() + 50000000)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 03/15] ipmi: Generate an interrupt on watchdog pretimeout expiry
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
  2019-09-19 21:39 ` [PATCH 01/15] ipmi: Fix watchdog NMI handling minyard
  2019-09-19 21:39 ` [PATCH 02/15] ipmi: Fix the get watchdog command minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-23  5:41   ` Cédric Le Goater
  2019-09-19 21:39 ` [PATCH 04/15] tests:ipmi: Fix IPMI BT tests minyard
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Add the watchdog pretimeout to the bits that cause an interrupt on attn.
Otherwise the user won't know.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index afb99e33d7..6e6cd1b47d 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -477,7 +477,9 @@ static int attn_set(IPMIBmcSim *ibs)
 
 static int attn_irq_enabled(IPMIBmcSim *ibs)
 {
-    return (IPMI_BMC_MSG_INTS_ON(ibs) && IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE_SET(ibs))
+    return (IPMI_BMC_MSG_INTS_ON(ibs) &&
+            (IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE_SET(ibs) ||
+             IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK_SET(ibs)))
         || (IPMI_BMC_EVBUF_FULL_INT_ENABLED(ibs) &&
             IPMI_BMC_MSG_FLAG_EVT_BUF_FULL_SET(ibs));
 }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 04/15] tests:ipmi: Fix IPMI BT tests
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (2 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 03/15] ipmi: Generate an interrupt on watchdog pretimeout expiry minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 05/15] qdev: Add a no default uuid property minyard
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

The IPMI BT tests had a race condition, if it receive an IPMI command
to enable interrupt, it would write the message to enable interrupts
after it wrote the command response.  So the test code could
receive the command response and issue the next command before the
device handled the interrupt enable command, and thus no interrupt.

So send the message to enable interrupt before the command response.

Also add some sleeps to give qemu time to handle responses, there was
no delay before, and it could result in an invalid timeout.

And re-enable the tests, as hopefully they are fixed now.

Note that I was unable to reproduce this even with the instructions
Peter gave me, but hopefully this fixes the issue.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 tests/Makefile.include | 3 +--
 tests/ipmi-bt-test.c   | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 793632ca72..479664f899 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -170,8 +170,7 @@ check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF)
 check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF)
-# Disabled temporarily as it fails intermittently especially under NetBSD VM
-# check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF)
+check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF)
 check-qtest-i386-y += tests/i440fx-test$(EXESUF)
 check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
 check-qtest-i386-y += tests/device-plug-test$(EXESUF)
diff --git a/tests/ipmi-bt-test.c b/tests/ipmi-bt-test.c
index fc4c83b5db..a42207d416 100644
--- a/tests/ipmi-bt-test.c
+++ b/tests/ipmi-bt-test.c
@@ -30,7 +30,7 @@
 #include <netinet/tcp.h>
 
 
-#include "libqtest.h"
+#include "libqtest-single.h"
 #include "qemu-common.h"
 
 #define IPMI_IRQ        5
@@ -99,6 +99,7 @@ static void bt_wait_b_busy(void)
     unsigned int count = 1000;
     while (IPMI_BT_CTLREG_GET_B_BUSY() != 0) {
         g_assert(--count != 0);
+        usleep(100);
     }
 }
 
@@ -107,6 +108,7 @@ static void bt_wait_b2h_atn(void)
     unsigned int count = 1000;
     while (IPMI_BT_CTLREG_GET_B2H_ATN() == 0) {
         g_assert(--count != 0);
+        usleep(100);
     }
 }
 
@@ -240,13 +242,13 @@ static void emu_msg_handler(void)
         write_emu_msg(msg, msg_len);
     } else if ((msg[1] == set_bmc_globals_cmd[0]) &&
                (msg[2] == set_bmc_globals_cmd[1])) {
+        write_emu_msg(enable_irq_cmd, sizeof(enable_irq_cmd));
         memcpy(msg + 1, set_bmc_globals_rsp, sizeof(set_bmc_globals_rsp));
         msg_len = sizeof(set_bmc_globals_rsp) + 1;
         msg[msg_len] = -ipmb_checksum(msg, msg_len, 0);
         msg_len++;
         msg[msg_len++] = 0xa0;
         write_emu_msg(msg, msg_len);
-        write_emu_msg(enable_irq_cmd, sizeof(enable_irq_cmd));
     } else {
         g_assert(0);
     }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 05/15] qdev: Add a no default uuid property
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (3 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 04/15] tests:ipmi: Fix IPMI BT tests minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-23  5:47   ` Cédric Le Goater
  2019-09-19 21:39 ` [PATCH 06/15] ipmi: Add a UUID device property minyard
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

This is for IPMI, which will behave differently if the UUID is
not set.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: Fam Zheng <famz@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/hw/qdev-properties.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 2e98dd60db..c6a8cb5516 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -238,6 +238,13 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
 #define DEFINE_PROP_AUDIODEV(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_audiodev, QEMUSoundCard)
 
+#define DEFINE_PROP_UUID_NODEFAULT(_name, _state, _field) {        \
+        .name      = (_name),                                      \
+        .info      = &qdev_prop_uuid,                              \
+        .offset    = offsetof(_state, _field)                      \
+            + type_check(QemuUUID, typeof_field(_state, _field)),  \
+        }
+
 #define DEFINE_PROP_END_OF_LIST()               \
     {}
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/15] ipmi: Add a UUID device property
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (4 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 05/15] qdev: Add a no default uuid property minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-23  5:51   ` Cédric Le Goater
  2019-09-19 21:39 ` [PATCH 07/15] ipmi: Split out KCS-specific code from ISA KCS code minyard
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Using the UUID that qemu generates probably isn't the best thing
to do, allow it to be passed in via properties, and use QemuUUID
for the type.

If the UUID is not set, return an unsupported command error.  This
way we are not providing an all-zero (or randomly generated) GUID
to the IPMI user.  This lets the host fall back to the other
method of using the get device id command to determind the BMC
being accessed.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Cc: Cédric Le Goater <clg@kaod.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 22 ++++++++++++++--------
 qemu-options.hx        | 10 +++++++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 6e6cd1b47d..71e56f3b13 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -223,7 +223,7 @@ struct IPMIBmcSim {
     uint8_t restart_cause;
 
     uint8_t acpi_power_state[2];
-    uint8_t uuid[16];
+    QemuUUID uuid;
 
     IPMISel sel;
     IPMISdr sdr;
@@ -941,8 +941,19 @@ static void get_device_guid(IPMIBmcSim *ibs,
 {
     unsigned int i;
 
+    /* An uninitialized uuid is all zeros, use that to know if it is set. */
     for (i = 0; i < 16; i++) {
-        rsp_buffer_push(rsp, ibs->uuid[i]);
+        if (ibs->uuid.data[i]) {
+            goto uuid_set;
+        }
+    }
+    /* No uuid is set, return an error. */
+    rsp_buffer_set_error(rsp, IPMI_CC_INVALID_CMD);
+    return;
+
+ uuid_set:
+    for (i = 0; i < 16; i++) {
+        rsp_buffer_push(rsp, ibs->uuid.data[i]);
     }
 }
 
@@ -1986,12 +1997,6 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp)
     ibs->acpi_power_state[0] = 0;
     ibs->acpi_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);
 
@@ -2011,6 +2016,7 @@ static Property ipmi_sim_properties[] = {
     DEFINE_PROP_UINT8("fwrev2", IPMIBmcSim, fwrev2, 0),
     DEFINE_PROP_UINT32("mfg_id", IPMIBmcSim, mfg_id, 0),
     DEFINE_PROP_UINT16("product_id", IPMIBmcSim, product_id, 0),
+    DEFINE_PROP_UUID_NODEFAULT("guid", IPMIBmcSim, uuid),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index bbfd936d29..ed9292f65e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -701,7 +701,7 @@ possible drivers and properties, use @code{-device help} and
 @code{-device @var{driver},help}.
 
 Some drivers are:
-@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}]
+@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}][,guid=@var{uuid}]
 
 Add an IPMI BMC.  This is a simulation of a hardware management
 interface processor that normally sits on a system.  It provides
@@ -714,8 +714,8 @@ controllers.  If you don't know what this means, it is safe to ignore
 it.
 
 @table @option
-@item bmc=@var{id}
-The BMC to connect to, one of ipmi-bmc-sim or ipmi-bmc-extern above.
+@item id=@var{id}
+The BMC id for interfaces to use this device.
 @item slave_addr=@var{val}
 Define slave address to use for the BMC.  The default is 0x20.
 @item sdrfile=@var{file}
@@ -724,6 +724,10 @@ file containing raw Sensor Data Records (SDR) data. The default is none.
 size of a Field Replaceable Unit (FRU) area.  The default is 1024.
 @item frudatafile=@var{file}
 file containing raw Field Replaceable Unit (FRU) inventory data. The default is none.
+@item guid=@var{uuid}
+value for the GUID for the BMC, in standard UUID format.  If this is set,
+get "Get GUID" command to the BMC will return it.  Otherwise "Get GUID"
+will return an error.
 @end table
 
 @item -device ipmi-bmc-extern,id=@var{id},chardev=@var{id}[,slave_addr=@var{val}]
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/15] ipmi: Split out KCS-specific code from ISA KCS code
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (5 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 06/15] ipmi: Add a UUID device property minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 08/15] ipmi: Split out BT-specific code from ISA BT code minyard
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Get ready for PCI and other KCS interfaces.

No functional changes, just split the code into the generic KCS code
and the ISA-specific code.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ipmi/Makefile.objs      |   2 +-
 hw/ipmi/ipmi_kcs.c         | 408 ++++++++++++++++++++++++++++++++++++
 hw/ipmi/isa_ipmi_kcs.c     | 417 ++-----------------------------------
 include/hw/ipmi/ipmi_kcs.h |  75 +++++++
 4 files changed, 505 insertions(+), 397 deletions(-)
 create mode 100644 hw/ipmi/ipmi_kcs.c
 create mode 100644 include/hw/ipmi/ipmi_kcs.h

diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
index 1b422bbee0..6835d2f64a 100644
--- a/hw/ipmi/Makefile.objs
+++ b/hw/ipmi/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-$(CONFIG_IPMI) += ipmi.o
+common-obj-$(CONFIG_IPMI) += ipmi.o ipmi_kcs.o
 common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_bmc_sim.o
 common-obj-$(CONFIG_IPMI_EXTERN) += ipmi_bmc_extern.o
 common-obj-$(CONFIG_ISA_IPMI_KCS) += isa_ipmi_kcs.o
diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
new file mode 100644
index 0000000000..dab1af8bc8
--- /dev/null
+++ b/hw/ipmi/ipmi_kcs.c
@@ -0,0 +1,408 @@
+/*
+ * QEMU IPMI KCS emulation
+ *
+ * Copyright (c) 2015,2017 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 "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/ipmi/ipmi_kcs.h"
+
+#define IPMI_KCS_OBF_BIT        0
+#define IPMI_KCS_IBF_BIT        1
+#define IPMI_KCS_SMS_ATN_BIT    2
+#define IPMI_KCS_CD_BIT         3
+
+#define IPMI_KCS_OBF_MASK          (1 << IPMI_KCS_OBF_BIT)
+#define IPMI_KCS_GET_OBF(d)        (((d) >> IPMI_KCS_OBF_BIT) & 0x1)
+#define IPMI_KCS_SET_OBF(d, v)     (d) = (((d) & ~IPMI_KCS_OBF_MASK) | \
+                                       (((v) & 1) << IPMI_KCS_OBF_BIT))
+#define IPMI_KCS_IBF_MASK          (1 << IPMI_KCS_IBF_BIT)
+#define IPMI_KCS_GET_IBF(d)        (((d) >> IPMI_KCS_IBF_BIT) & 0x1)
+#define IPMI_KCS_SET_IBF(d, v)     (d) = (((d) & ~IPMI_KCS_IBF_MASK) | \
+                                       (((v) & 1) << IPMI_KCS_IBF_BIT))
+#define IPMI_KCS_SMS_ATN_MASK      (1 << IPMI_KCS_SMS_ATN_BIT)
+#define IPMI_KCS_GET_SMS_ATN(d)    (((d) >> IPMI_KCS_SMS_ATN_BIT) & 0x1)
+#define IPMI_KCS_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_KCS_SMS_ATN_MASK) | \
+                                       (((v) & 1) << IPMI_KCS_SMS_ATN_BIT))
+#define IPMI_KCS_CD_MASK           (1 << IPMI_KCS_CD_BIT)
+#define IPMI_KCS_GET_CD(d)         (((d) >> IPMI_KCS_CD_BIT) & 0x1)
+#define IPMI_KCS_SET_CD(d, v)      (d) = (((d) & ~IPMI_KCS_CD_MASK) | \
+                                       (((v) & 1) << IPMI_KCS_CD_BIT))
+
+#define IPMI_KCS_IDLE_STATE        0
+#define IPMI_KCS_READ_STATE        1
+#define IPMI_KCS_WRITE_STATE       2
+#define IPMI_KCS_ERROR_STATE       3
+
+#define IPMI_KCS_GET_STATE(d)    (((d) >> 6) & 0x3)
+#define IPMI_KCS_SET_STATE(d, v) ((d) = ((d) & ~0xc0) | (((v) & 0x3) << 6))
+
+#define IPMI_KCS_ABORT_STATUS_CMD       0x60
+#define IPMI_KCS_WRITE_START_CMD        0x61
+#define IPMI_KCS_WRITE_END_CMD          0x62
+#define IPMI_KCS_READ_CMD               0x68
+
+#define IPMI_KCS_STATUS_NO_ERR          0x00
+#define IPMI_KCS_STATUS_ABORTED_ERR     0x01
+#define IPMI_KCS_STATUS_BAD_CC_ERR      0x02
+#define IPMI_KCS_STATUS_LENGTH_ERR      0x06
+
+static void ipmi_kcs_raise_irq(IPMIKCS *ik)
+{
+    if (ik->use_irq && ik->irqs_enabled && ik->raise_irq) {
+        ik->raise_irq(ik);
+    }
+}
+
+static void ipmi_kcs_lower_irq(IPMIKCS *ik)
+{
+    if (ik->lower_irq) {
+        ik->lower_irq(ik);
+    }
+}
+
+#define SET_OBF() \
+    do {                                                                      \
+        IPMI_KCS_SET_OBF(ik->status_reg, 1);                                  \
+        if (!ik->obf_irq_set) {                                               \
+            ik->obf_irq_set = 1;                                              \
+            if (!ik->atn_irq_set) {                                           \
+                ipmi_kcs_raise_irq(ik);                                  \
+            }                                                                 \
+        }                                                                     \
+    } while (0)
+
+static void ipmi_kcs_signal(IPMIKCS *ik, IPMIInterface *ii)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+
+    ik->do_wake = 1;
+    while (ik->do_wake) {
+        ik->do_wake = 0;
+        iic->handle_if_event(ii);
+    }
+}
+
+static void ipmi_kcs_handle_event(IPMIInterface *ii)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+
+    if (ik->cmd_reg == IPMI_KCS_ABORT_STATUS_CMD) {
+        if (IPMI_KCS_GET_STATE(ik->status_reg) != IPMI_KCS_ERROR_STATE) {
+            ik->waiting_rsp++; /* Invalidate the message */
+            ik->outmsg[0] = IPMI_KCS_STATUS_ABORTED_ERR;
+            ik->outlen = 1;
+            ik->outpos = 0;
+            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_ERROR_STATE);
+            SET_OBF();
+        }
+        goto out;
+    }
+
+    switch (IPMI_KCS_GET_STATE(ik->status_reg)) {
+    case IPMI_KCS_IDLE_STATE:
+        if (ik->cmd_reg == IPMI_KCS_WRITE_START_CMD) {
+            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_WRITE_STATE);
+            ik->cmd_reg = -1;
+            ik->write_end = 0;
+            ik->inlen = 0;
+            SET_OBF();
+        }
+        break;
+
+    case IPMI_KCS_READ_STATE:
+    handle_read:
+        if (ik->outpos >= ik->outlen) {
+            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_IDLE_STATE);
+            SET_OBF();
+        } else if (ik->data_in_reg == IPMI_KCS_READ_CMD) {
+            ik->data_out_reg = ik->outmsg[ik->outpos];
+            ik->outpos++;
+            SET_OBF();
+        } else {
+            ik->outmsg[0] = IPMI_KCS_STATUS_BAD_CC_ERR;
+            ik->outlen = 1;
+            ik->outpos = 0;
+            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_ERROR_STATE);
+            SET_OBF();
+            goto out;
+        }
+        break;
+
+    case IPMI_KCS_WRITE_STATE:
+        if (ik->data_in_reg != -1) {
+            /*
+             * Don't worry about input overrun here, that will be
+             * handled in the BMC.
+             */
+            if (ik->inlen < sizeof(ik->inmsg)) {
+                ik->inmsg[ik->inlen] = ik->data_in_reg;
+            }
+            ik->inlen++;
+        }
+        if (ik->write_end) {
+            IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(ik->bmc);
+            ik->outlen = 0;
+            ik->write_end = 0;
+            ik->outpos = 0;
+            bk->handle_command(ik->bmc, ik->inmsg, ik->inlen, sizeof(ik->inmsg),
+                               ik->waiting_rsp);
+            goto out_noibf;
+        } else if (ik->cmd_reg == IPMI_KCS_WRITE_END_CMD) {
+            ik->cmd_reg = -1;
+            ik->write_end = 1;
+        }
+        SET_OBF();
+        break;
+
+    case IPMI_KCS_ERROR_STATE:
+        if (ik->data_in_reg != -1) {
+            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_READ_STATE);
+            ik->data_in_reg = IPMI_KCS_READ_CMD;
+            goto handle_read;
+        }
+        break;
+    }
+
+    if (ik->cmd_reg != -1) {
+        /* Got an invalid command */
+        ik->outmsg[0] = IPMI_KCS_STATUS_BAD_CC_ERR;
+        ik->outlen = 1;
+        ik->outpos = 0;
+        IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_ERROR_STATE);
+    }
+
+ out:
+    ik->cmd_reg = -1;
+    ik->data_in_reg = -1;
+    IPMI_KCS_SET_IBF(ik->status_reg, 0);
+ out_noibf:
+    return;
+}
+
+static void ipmi_kcs_handle_rsp(IPMIInterface *ii, uint8_t msg_id,
+                                unsigned char *rsp, unsigned int rsp_len)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+
+    if (ik->waiting_rsp == msg_id) {
+        ik->waiting_rsp++;
+        if (rsp_len > sizeof(ik->outmsg)) {
+            ik->outmsg[0] = rsp[0];
+            ik->outmsg[1] = rsp[1];
+            ik->outmsg[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
+            ik->outlen = 3;
+        } else {
+            memcpy(ik->outmsg, rsp, rsp_len);
+            ik->outlen = rsp_len;
+        }
+        IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_READ_STATE);
+        ik->data_in_reg = IPMI_KCS_READ_CMD;
+        ipmi_kcs_signal(ik, ii);
+    }
+}
+
+
+static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
+{
+    IPMIInterface *ii = opaque;
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+    uint32_t ret;
+
+    switch (addr & 1) {
+    case 0:
+        ret = ik->data_out_reg;
+        IPMI_KCS_SET_OBF(ik->status_reg, 0);
+        if (ik->obf_irq_set) {
+            ik->obf_irq_set = 0;
+            if (!ik->atn_irq_set) {
+                ipmi_kcs_lower_irq(ik);
+            }
+        }
+        break;
+    case 1:
+        ret = ik->status_reg;
+        if (ik->atn_irq_set) {
+            ik->atn_irq_set = 0;
+            if (!ik->obf_irq_set) {
+                ipmi_kcs_lower_irq(ik);
+            }
+        }
+        break;
+    }
+    return ret;
+}
+
+static void ipmi_kcs_ioport_write(void *opaque, hwaddr addr, uint64_t val,
+                                  unsigned size)
+{
+    IPMIInterface *ii = opaque;
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+
+    if (IPMI_KCS_GET_IBF(ik->status_reg)) {
+        return;
+    }
+
+    switch (addr & 1) {
+    case 0:
+        ik->data_in_reg = val;
+        break;
+
+    case 1:
+        ik->cmd_reg = val;
+        break;
+    }
+    IPMI_KCS_SET_IBF(ik->status_reg, 1);
+    ipmi_kcs_signal(ik, ii);
+}
+
+const MemoryRegionOps ipmi_kcs_io_ops = {
+    .read = ipmi_kcs_ioport_read,
+    .write = ipmi_kcs_ioport_write,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void ipmi_kcs_set_atn(IPMIInterface *ii, int val, int irq)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+
+    IPMI_KCS_SET_SMS_ATN(ik->status_reg, val);
+    if (val) {
+        if (irq && !ik->atn_irq_set) {
+            ik->atn_irq_set = 1;
+            if (!ik->obf_irq_set) {
+                ipmi_kcs_raise_irq(ik);
+            }
+        }
+    } else {
+        if (ik->atn_irq_set) {
+            ik->atn_irq_set = 0;
+            if (!ik->obf_irq_set) {
+                ipmi_kcs_lower_irq(ik);
+            }
+        }
+    }
+}
+
+static void ipmi_kcs_set_irq_enable(IPMIInterface *ii, int val)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+
+    ik->irqs_enabled = val;
+}
+
+static void ipmi_kcs_init(IPMIInterface *ii, Error **errp)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIKCS *ik = iic->get_backend_data(ii);
+
+    ik->io_length = 2;
+    memory_region_init_io(&ik->io, NULL, &ipmi_kcs_io_ops, ii, "ipmi-kcs", 2);
+}
+
+int ipmi_kcs_vmstate_post_load(void *opaque, int version)
+{
+    IPMIKCS *ik = opaque;
+
+    /* Make sure all the values are sane. */
+    if (ik->outpos >= MAX_IPMI_MSG_SIZE || ik->outlen >= MAX_IPMI_MSG_SIZE ||
+        ik->outpos >= ik->outlen) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "ipmi:kcs: vmstate transfer received bad out values: %d %d\n",
+                      ik->outpos, ik->outlen);
+        ik->outpos = 0;
+        ik->outlen = 0;
+    }
+
+    if (ik->inlen >= MAX_IPMI_MSG_SIZE) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "ipmi:kcs: vmstate transfer received bad in value: %d\n",
+                      ik->inlen);
+        ik->inlen = 0;
+    }
+
+    return 0;
+}
+
+static bool vmstate_kcs_before_version2(void *opaque, int version)
+{
+    return version <= 1;
+}
+
+const VMStateDescription vmstate_IPMIKCS = {
+    .name = TYPE_IPMI_INTERFACE_PREFIX "kcs",
+    .version_id = 2,
+    .minimum_version_id = 1,
+    .post_load = ipmi_kcs_vmstate_post_load,
+    .fields      = (VMStateField[]) {
+        VMSTATE_BOOL(obf_irq_set, IPMIKCS),
+        VMSTATE_BOOL(atn_irq_set, IPMIKCS),
+        VMSTATE_UNUSED_TEST(vmstate_kcs_before_version2, 1), /* Was use_irq */
+        VMSTATE_BOOL(irqs_enabled, IPMIKCS),
+        VMSTATE_UINT32(outpos, IPMIKCS),
+        VMSTATE_UINT32_V(outlen, IPMIKCS, 2),
+        VMSTATE_UINT8_ARRAY(outmsg, IPMIKCS, MAX_IPMI_MSG_SIZE),
+        VMSTATE_UINT32_V(inlen, IPMIKCS, 2),
+        VMSTATE_UINT8_ARRAY(inmsg, IPMIKCS, MAX_IPMI_MSG_SIZE),
+        VMSTATE_BOOL(write_end, IPMIKCS),
+        VMSTATE_UINT8(status_reg, IPMIKCS),
+        VMSTATE_UINT8(data_out_reg, IPMIKCS),
+        VMSTATE_INT16(data_in_reg, IPMIKCS),
+        VMSTATE_INT16(cmd_reg, IPMIKCS),
+        VMSTATE_UINT8(waiting_rsp, IPMIKCS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+void ipmi_kcs_get_fwinfo(IPMIKCS *ik, IPMIFwInfo *info)
+{
+    info->interface_name = "kcs";
+    info->interface_type = IPMI_SMBIOS_KCS;
+    info->ipmi_spec_major_revision = 2;
+    info->ipmi_spec_minor_revision = 0;
+    info->base_address = ik->io_base;
+    info->i2c_slave_address = ik->bmc->slave_addr;
+    info->register_length = ik->io_length;
+    info->register_spacing = 1;
+    info->memspace = IPMI_MEMSPACE_IO;
+    info->irq_type = IPMI_LEVEL_IRQ;
+}
+
+void ipmi_kcs_class_init(IPMIInterfaceClass *iic)
+{
+    iic->init = ipmi_kcs_init;
+    iic->set_atn = ipmi_kcs_set_atn;
+    iic->handle_rsp = ipmi_kcs_handle_rsp;
+    iic->handle_if_event = ipmi_kcs_handle_event;
+    iic->set_irq_enable = ipmi_kcs_set_irq_enable;
+}
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 374b2a0709..8e32774f85 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -1,7 +1,7 @@
 /*
  * QEMU ISA IPMI KCS emulation
  *
- * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
+ * Copyright (c) 2015,2017 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
@@ -26,338 +26,12 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
-#include "hw/ipmi/ipmi.h"
 #include "hw/irq.h"
+#include "hw/ipmi/ipmi_kcs.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 
-#define IPMI_KCS_OBF_BIT        0
-#define IPMI_KCS_IBF_BIT        1
-#define IPMI_KCS_SMS_ATN_BIT    2
-#define IPMI_KCS_CD_BIT         3
-
-#define IPMI_KCS_OBF_MASK          (1 << IPMI_KCS_OBF_BIT)
-#define IPMI_KCS_GET_OBF(d)        (((d) >> IPMI_KCS_OBF_BIT) & 0x1)
-#define IPMI_KCS_SET_OBF(d, v)     (d) = (((d) & ~IPMI_KCS_OBF_MASK) | \
-                                       (((v) & 1) << IPMI_KCS_OBF_BIT))
-#define IPMI_KCS_IBF_MASK          (1 << IPMI_KCS_IBF_BIT)
-#define IPMI_KCS_GET_IBF(d)        (((d) >> IPMI_KCS_IBF_BIT) & 0x1)
-#define IPMI_KCS_SET_IBF(d, v)     (d) = (((d) & ~IPMI_KCS_IBF_MASK) | \
-                                       (((v) & 1) << IPMI_KCS_IBF_BIT))
-#define IPMI_KCS_SMS_ATN_MASK      (1 << IPMI_KCS_SMS_ATN_BIT)
-#define IPMI_KCS_GET_SMS_ATN(d)    (((d) >> IPMI_KCS_SMS_ATN_BIT) & 0x1)
-#define IPMI_KCS_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_KCS_SMS_ATN_MASK) | \
-                                       (((v) & 1) << IPMI_KCS_SMS_ATN_BIT))
-#define IPMI_KCS_CD_MASK           (1 << IPMI_KCS_CD_BIT)
-#define IPMI_KCS_GET_CD(d)         (((d) >> IPMI_KCS_CD_BIT) & 0x1)
-#define IPMI_KCS_SET_CD(d, v)      (d) = (((d) & ~IPMI_KCS_CD_MASK) | \
-                                       (((v) & 1) << IPMI_KCS_CD_BIT))
-
-#define IPMI_KCS_IDLE_STATE        0
-#define IPMI_KCS_READ_STATE        1
-#define IPMI_KCS_WRITE_STATE       2
-#define IPMI_KCS_ERROR_STATE       3
-
-#define IPMI_KCS_GET_STATE(d)    (((d) >> 6) & 0x3)
-#define IPMI_KCS_SET_STATE(d, v) ((d) = ((d) & ~0xc0) | (((v) & 0x3) << 6))
-
-#define IPMI_KCS_ABORT_STATUS_CMD       0x60
-#define IPMI_KCS_WRITE_START_CMD        0x61
-#define IPMI_KCS_WRITE_END_CMD          0x62
-#define IPMI_KCS_READ_CMD               0x68
-
-#define IPMI_KCS_STATUS_NO_ERR          0x00
-#define IPMI_KCS_STATUS_ABORTED_ERR     0x01
-#define IPMI_KCS_STATUS_BAD_CC_ERR      0x02
-#define IPMI_KCS_STATUS_LENGTH_ERR      0x06
-
-typedef struct IPMIKCS {
-    IPMIBmc *bmc;
-
-    bool do_wake;
-
-    qemu_irq irq;
-
-    uint32_t io_base;
-    unsigned long io_length;
-    MemoryRegion io;
-
-    bool obf_irq_set;
-    bool atn_irq_set;
-    bool use_irq;
-    bool irqs_enabled;
-
-    uint8_t outmsg[MAX_IPMI_MSG_SIZE];
-    uint32_t outpos;
-    uint32_t outlen;
-
-    uint8_t inmsg[MAX_IPMI_MSG_SIZE];
-    uint32_t inlen;
-    bool write_end;
-
-    uint8_t status_reg;
-    uint8_t data_out_reg;
-
-    int16_t data_in_reg; /* -1 means not written */
-    int16_t cmd_reg;
-
-    /*
-     * This is a response number that we send with the command to make
-     * sure that the response matches the command.
-     */
-    uint8_t waiting_rsp;
-} IPMIKCS;
-
-#define SET_OBF() \
-    do {                                                                      \
-        IPMI_KCS_SET_OBF(ik->status_reg, 1);                                  \
-        if (ik->use_irq && ik->irqs_enabled && !ik->obf_irq_set) {            \
-            ik->obf_irq_set = 1;                                              \
-            if (!ik->atn_irq_set) {                                           \
-                qemu_irq_raise(ik->irq);                                      \
-            }                                                                 \
-        }                                                                     \
-    } while (0)
-
-static void ipmi_kcs_signal(IPMIKCS *ik, IPMIInterface *ii)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-
-    ik->do_wake = 1;
-    while (ik->do_wake) {
-        ik->do_wake = 0;
-        iic->handle_if_event(ii);
-    }
-}
-
-static void ipmi_kcs_handle_event(IPMIInterface *ii)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-
-    if (ik->cmd_reg == IPMI_KCS_ABORT_STATUS_CMD) {
-        if (IPMI_KCS_GET_STATE(ik->status_reg) != IPMI_KCS_ERROR_STATE) {
-            ik->waiting_rsp++; /* Invalidate the message */
-            ik->outmsg[0] = IPMI_KCS_STATUS_ABORTED_ERR;
-            ik->outlen = 1;
-            ik->outpos = 0;
-            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_ERROR_STATE);
-            SET_OBF();
-        }
-        goto out;
-    }
-
-    switch (IPMI_KCS_GET_STATE(ik->status_reg)) {
-    case IPMI_KCS_IDLE_STATE:
-        if (ik->cmd_reg == IPMI_KCS_WRITE_START_CMD) {
-            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_WRITE_STATE);
-            ik->cmd_reg = -1;
-            ik->write_end = 0;
-            ik->inlen = 0;
-            SET_OBF();
-        }
-        break;
-
-    case IPMI_KCS_READ_STATE:
-    handle_read:
-        if (ik->outpos >= ik->outlen) {
-            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_IDLE_STATE);
-            SET_OBF();
-        } else if (ik->data_in_reg == IPMI_KCS_READ_CMD) {
-            ik->data_out_reg = ik->outmsg[ik->outpos];
-            ik->outpos++;
-            SET_OBF();
-        } else {
-            ik->outmsg[0] = IPMI_KCS_STATUS_BAD_CC_ERR;
-            ik->outlen = 1;
-            ik->outpos = 0;
-            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_ERROR_STATE);
-            SET_OBF();
-            goto out;
-        }
-        break;
-
-    case IPMI_KCS_WRITE_STATE:
-        if (ik->data_in_reg != -1) {
-            /*
-             * Don't worry about input overrun here, that will be
-             * handled in the BMC.
-             */
-            if (ik->inlen < sizeof(ik->inmsg)) {
-                ik->inmsg[ik->inlen] = ik->data_in_reg;
-            }
-            ik->inlen++;
-        }
-        if (ik->write_end) {
-            IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(ik->bmc);
-            ik->outlen = 0;
-            ik->write_end = 0;
-            ik->outpos = 0;
-            bk->handle_command(ik->bmc, ik->inmsg, ik->inlen, sizeof(ik->inmsg),
-                               ik->waiting_rsp);
-            goto out_noibf;
-        } else if (ik->cmd_reg == IPMI_KCS_WRITE_END_CMD) {
-            ik->cmd_reg = -1;
-            ik->write_end = 1;
-        }
-        SET_OBF();
-        break;
-
-    case IPMI_KCS_ERROR_STATE:
-        if (ik->data_in_reg != -1) {
-            IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_READ_STATE);
-            ik->data_in_reg = IPMI_KCS_READ_CMD;
-            goto handle_read;
-        }
-        break;
-    }
-
-    if (ik->cmd_reg != -1) {
-        /* Got an invalid command */
-        ik->outmsg[0] = IPMI_KCS_STATUS_BAD_CC_ERR;
-        ik->outlen = 1;
-        ik->outpos = 0;
-        IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_ERROR_STATE);
-    }
-
- out:
-    ik->cmd_reg = -1;
-    ik->data_in_reg = -1;
-    IPMI_KCS_SET_IBF(ik->status_reg, 0);
- out_noibf:
-    return;
-}
-
-static void ipmi_kcs_handle_rsp(IPMIInterface *ii, uint8_t msg_id,
-                                unsigned char *rsp, unsigned int rsp_len)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-
-    if (ik->waiting_rsp == msg_id) {
-        ik->waiting_rsp++;
-        if (rsp_len > sizeof(ik->outmsg)) {
-            ik->outmsg[0] = rsp[0];
-            ik->outmsg[1] = rsp[1];
-            ik->outmsg[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
-            ik->outlen = 3;
-        } else {
-            memcpy(ik->outmsg, rsp, rsp_len);
-            ik->outlen = rsp_len;
-        }
-        IPMI_KCS_SET_STATE(ik->status_reg, IPMI_KCS_READ_STATE);
-        ik->data_in_reg = IPMI_KCS_READ_CMD;
-        ipmi_kcs_signal(ik, ii);
-    }
-}
-
-
-static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
-{
-    IPMIInterface *ii = opaque;
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-    uint32_t ret;
-
-    switch (addr & 1) {
-    case 0:
-        ret = ik->data_out_reg;
-        IPMI_KCS_SET_OBF(ik->status_reg, 0);
-        if (ik->obf_irq_set) {
-            ik->obf_irq_set = 0;
-            if (!ik->atn_irq_set) {
-                qemu_irq_lower(ik->irq);
-            }
-        }
-        break;
-    case 1:
-        ret = ik->status_reg;
-        if (ik->atn_irq_set) {
-            ik->atn_irq_set = 0;
-            if (!ik->obf_irq_set) {
-                qemu_irq_lower(ik->irq);
-            }
-        }
-        break;
-    }
-    return ret;
-}
-
-static void ipmi_kcs_ioport_write(void *opaque, hwaddr addr, uint64_t val,
-                                  unsigned size)
-{
-    IPMIInterface *ii = opaque;
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-
-    if (IPMI_KCS_GET_IBF(ik->status_reg)) {
-        return;
-    }
-
-    switch (addr & 1) {
-    case 0:
-        ik->data_in_reg = val;
-        break;
-
-    case 1:
-        ik->cmd_reg = val;
-        break;
-    }
-    IPMI_KCS_SET_IBF(ik->status_reg, 1);
-    ipmi_kcs_signal(ik, ii);
-}
-
-const MemoryRegionOps ipmi_kcs_io_ops = {
-    .read = ipmi_kcs_ioport_read,
-    .write = ipmi_kcs_ioport_write,
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static void ipmi_kcs_set_atn(IPMIInterface *ii, int val, int irq)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-
-    IPMI_KCS_SET_SMS_ATN(ik->status_reg, val);
-    if (val) {
-        if (irq && !ik->atn_irq_set && ik->use_irq && ik->irqs_enabled) {
-            ik->atn_irq_set = 1;
-            if (!ik->obf_irq_set) {
-                qemu_irq_raise(ik->irq);
-            }
-        }
-    } else {
-        if (ik->atn_irq_set) {
-            ik->atn_irq_set = 0;
-            if (!ik->obf_irq_set) {
-                qemu_irq_lower(ik->irq);
-            }
-        }
-    }
-}
-
-static void ipmi_kcs_set_irq_enable(IPMIInterface *ii, int val)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-
-    ik->irqs_enabled = val;
-}
-
-static void ipmi_kcs_init(IPMIInterface *ii, Error **errp)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIKCS *ik = iic->get_backend_data(ii);
-
-    ik->io_length = 2;
-    memory_region_init_io(&ik->io, NULL, &ipmi_kcs_io_ops, ii, "ipmi-kcs", 2);
-}
-
 #define TYPE_ISA_IPMI_KCS "isa-ipmi-kcs"
 #define ISA_IPMI_KCS(obj) OBJECT_CHECK(ISAIPMIKCSDevice, (obj), \
                                        TYPE_ISA_IPMI_KCS)
@@ -365,36 +39,32 @@ static void ipmi_kcs_init(IPMIInterface *ii, Error **errp)
 typedef struct ISAIPMIKCSDevice {
     ISADevice dev;
     int32_t isairq;
+    qemu_irq irq;
     IPMIKCS kcs;
     uint32_t uuid;
 } ISAIPMIKCSDevice;
 
-static void ipmi_kcs_get_fwinfo(IPMIInterface *ii, IPMIFwInfo *info)
+static void isa_ipmi_kcs_get_fwinfo(IPMIInterface *ii, IPMIFwInfo *info)
 {
     ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(ii);
 
-    info->interface_name = "kcs";
-    info->interface_type = IPMI_SMBIOS_KCS;
-    info->ipmi_spec_major_revision = 2;
-    info->ipmi_spec_minor_revision = 0;
-    info->base_address = iik->kcs.io_base;
-    info->i2c_slave_address = iik->kcs.bmc->slave_addr;
-    info->register_length = iik->kcs.io_length;
-    info->register_spacing = 1;
-    info->memspace = IPMI_MEMSPACE_IO;
-    info->irq_type = IPMI_LEVEL_IRQ;
+    ipmi_kcs_get_fwinfo(&iik->kcs, info);
     info->interrupt_number = iik->isairq;
     info->uuid = iik->uuid;
 }
 
-static void ipmi_kcs_class_init(IPMIInterfaceClass *iic)
+static void isa_ipmi_kcs_raise_irq(IPMIKCS *ik)
 {
-    iic->init = ipmi_kcs_init;
-    iic->set_atn = ipmi_kcs_set_atn;
-    iic->handle_rsp = ipmi_kcs_handle_rsp;
-    iic->handle_if_event = ipmi_kcs_handle_event;
-    iic->set_irq_enable = ipmi_kcs_set_irq_enable;
-    iic->get_fwinfo = ipmi_kcs_get_fwinfo;
+    ISAIPMIKCSDevice *iik = ik->opaque;
+
+    qemu_irq_raise(iik->irq);
+}
+
+static void isa_ipmi_kcs_lower_irq(IPMIKCS *ik)
+{
+    ISAIPMIKCSDevice *iik = ik->opaque;
+
+    qemu_irq_lower(iik->irq);
 }
 
 static void ipmi_isa_realize(DeviceState *dev, Error **errp)
@@ -412,14 +82,17 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
     iik->uuid = ipmi_next_uuid();
 
     iik->kcs.bmc->intf = ii;
+    iik->kcs.opaque = iik;
 
     iic->init(ii, errp);
     if (*errp)
         return;
 
     if (iik->isairq > 0) {
-        isa_init_irq(isadev, &iik->kcs.irq, iik->isairq);
+        isa_init_irq(isadev, &iik->irq, iik->isairq);
         iik->kcs.use_irq = 1;
+        iik->kcs.raise_irq = isa_ipmi_kcs_raise_irq;
+        iik->kcs.lower_irq = isa_ipmi_kcs_lower_irq;
     }
 
     qdev_set_legacy_instance_id(dev, iik->kcs.io_base, iik->kcs.io_length);
@@ -427,60 +100,11 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
     isa_register_ioport(isadev, &iik->kcs.io, iik->kcs.io_base);
 }
 
-static int ipmi_kcs_vmstate_post_load(void *opaque, int version)
-{
-    IPMIKCS *ik = opaque;
-
-    /* Make sure all the values are sane. */
-    if (ik->outpos >= MAX_IPMI_MSG_SIZE || ik->outlen >= MAX_IPMI_MSG_SIZE ||
-        ik->outpos >= ik->outlen) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "ipmi:kcs: vmstate transfer received bad out values: %d %d\n",
-                      ik->outpos, ik->outlen);
-        ik->outpos = 0;
-        ik->outlen = 0;
-    }
-
-    if (ik->inlen >= MAX_IPMI_MSG_SIZE) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "ipmi:kcs: vmstate transfer received bad in value: %d\n",
-                      ik->inlen);
-        ik->inlen = 0;
-    }
-
-    return 0;
-}
-
 static bool vmstate_kcs_before_version2(void *opaque, int version)
 {
     return version <= 1;
 }
 
-static const VMStateDescription vmstate_IPMIKCS = {
-    .name = TYPE_IPMI_INTERFACE_PREFIX "kcs",
-    .version_id = 2,
-    .minimum_version_id = 1,
-    .post_load = ipmi_kcs_vmstate_post_load,
-    .fields      = (VMStateField[]) {
-        VMSTATE_BOOL(obf_irq_set, IPMIKCS),
-        VMSTATE_BOOL(atn_irq_set, IPMIKCS),
-        VMSTATE_UNUSED_TEST(vmstate_kcs_before_version2, 1), /* Was use_irq */
-        VMSTATE_BOOL(irqs_enabled, IPMIKCS),
-        VMSTATE_UINT32(outpos, IPMIKCS),
-        VMSTATE_UINT32_V(outlen, IPMIKCS, 2),
-        VMSTATE_UINT8_ARRAY(outmsg, IPMIKCS, MAX_IPMI_MSG_SIZE),
-        VMSTATE_UINT32_V(inlen, IPMIKCS, 2),
-        VMSTATE_UINT8_ARRAY(inmsg, IPMIKCS, MAX_IPMI_MSG_SIZE),
-        VMSTATE_BOOL(write_end, IPMIKCS),
-        VMSTATE_UINT8(status_reg, IPMIKCS),
-        VMSTATE_UINT8(data_out_reg, IPMIKCS),
-        VMSTATE_INT16(data_in_reg, IPMIKCS),
-        VMSTATE_INT16(cmd_reg, IPMIKCS),
-        VMSTATE_UINT8(waiting_rsp, IPMIKCS),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static const VMStateDescription vmstate_ISAIPMIKCSDevice = {
     .name = TYPE_IPMI_INTERFACE,
     .version_id = 2,
@@ -531,6 +155,7 @@ static void isa_ipmi_kcs_class_init(ObjectClass *oc, void *data)
 
     iic->get_backend_data = isa_ipmi_kcs_get_backend_data;
     ipmi_kcs_class_init(iic);
+    iic->get_fwinfo = isa_ipmi_kcs_get_fwinfo;
 }
 
 static const TypeInfo isa_ipmi_kcs_info = {
diff --git a/include/hw/ipmi/ipmi_kcs.h b/include/hw/ipmi/ipmi_kcs.h
new file mode 100644
index 0000000000..91d76d08f4
--- /dev/null
+++ b/include/hw/ipmi/ipmi_kcs.h
@@ -0,0 +1,75 @@
+/*
+ * QEMU IPMI KCS emulation
+ *
+ * Copyright (c) 2015,2017 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.
+ */
+
+#ifndef HW_IPMI_KCS_H
+#define HW_IPMI_KCS_H
+
+#include "hw/ipmi/ipmi.h"
+
+typedef struct IPMIKCS {
+    IPMIBmc *bmc;
+
+    bool do_wake;
+
+    bool obf_irq_set;
+    bool atn_irq_set;
+    bool irqs_enabled;
+
+    uint8_t outmsg[MAX_IPMI_MSG_SIZE];
+    uint32_t outpos;
+    uint32_t outlen;
+
+    uint8_t inmsg[MAX_IPMI_MSG_SIZE];
+    uint32_t inlen;
+    bool write_end;
+
+    uint8_t status_reg;
+    uint8_t data_out_reg;
+
+    int16_t data_in_reg; /* -1 means not written */
+    int16_t cmd_reg;
+
+    /*
+     * This is a response number that we send with the command to make
+     * sure that the response matches the command.
+     */
+    uint8_t waiting_rsp;
+
+    uint32_t io_base;
+    unsigned long io_length;
+    MemoryRegion io;
+
+    void (*raise_irq)(struct IPMIKCS *ik);
+    void (*lower_irq)(struct IPMIKCS *ik);
+    void *opaque;
+
+    bool use_irq;
+} IPMIKCS;
+
+void ipmi_kcs_get_fwinfo(IPMIKCS *ik, IPMIFwInfo *info);
+void ipmi_kcs_class_init(IPMIInterfaceClass *iic);
+extern const VMStateDescription vmstate_IPMIKCS;
+int ipmi_kcs_vmstate_post_load(void *opaque, int version);
+
+#endif /* HW_IPMI_KCS_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 08/15] ipmi: Split out BT-specific code from ISA BT code
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (6 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 07/15] ipmi: Split out KCS-specific code from ISA KCS code minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 09/15] ipmi: Allow a size value to be passed for I/O space minyard
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Get ready for PCI and other BT interfaces.

No functional changes, just split the code into generic BT code
and ISA-specific BT code.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ipmi/Makefile.objs     |   2 +-
 hw/ipmi/ipmi_bt.c         | 426 ++++++++++++++++++++++++++++++++++++
 hw/ipmi/isa_ipmi_bt.c     | 441 ++------------------------------------
 include/hw/ipmi/ipmi_bt.h |  72 +++++++
 4 files changed, 520 insertions(+), 421 deletions(-)
 create mode 100644 hw/ipmi/ipmi_bt.c
 create mode 100644 include/hw/ipmi/ipmi_bt.h

diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
index 6835d2f64a..4ffa45a66c 100644
--- a/hw/ipmi/Makefile.objs
+++ b/hw/ipmi/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-$(CONFIG_IPMI) += ipmi.o ipmi_kcs.o
+common-obj-$(CONFIG_IPMI) += ipmi.o ipmi_kcs.o ipmi_bt.o
 common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_bmc_sim.o
 common-obj-$(CONFIG_IPMI_EXTERN) += ipmi_bmc_extern.o
 common-obj-$(CONFIG_ISA_IPMI_KCS) += isa_ipmi_kcs.o
diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
new file mode 100644
index 0000000000..e6765ca4f8
--- /dev/null
+++ b/hw/ipmi/ipmi_bt.c
@@ -0,0 +1,426 @@
+/*
+ * QEMU IPMI BT 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 "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/ipmi/ipmi_bt.h"
+
+/* Control register */
+#define IPMI_BT_CLR_WR_BIT         0
+#define IPMI_BT_CLR_RD_BIT         1
+#define IPMI_BT_H2B_ATN_BIT        2
+#define IPMI_BT_B2H_ATN_BIT        3
+#define IPMI_BT_SMS_ATN_BIT        4
+#define IPMI_BT_HBUSY_BIT          6
+#define IPMI_BT_BBUSY_BIT          7
+
+#define IPMI_BT_GET_CLR_WR(d)      (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1)
+
+#define IPMI_BT_GET_CLR_RD(d)      (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1)
+
+#define IPMI_BT_GET_H2B_ATN(d)     (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1)
+
+#define IPMI_BT_B2H_ATN_MASK       (1 << IPMI_BT_B2H_ATN_BIT)
+#define IPMI_BT_GET_B2H_ATN(d)     (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1)
+#define IPMI_BT_SET_B2H_ATN(d, v)  ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \
+                                        (!!(v) << IPMI_BT_B2H_ATN_BIT)))
+
+#define IPMI_BT_SMS_ATN_MASK       (1 << IPMI_BT_SMS_ATN_BIT)
+#define IPMI_BT_GET_SMS_ATN(d)     (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1)
+#define IPMI_BT_SET_SMS_ATN(d, v)  ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \
+                                        (!!(v) << IPMI_BT_SMS_ATN_BIT)))
+
+#define IPMI_BT_HBUSY_MASK         (1 << IPMI_BT_HBUSY_BIT)
+#define IPMI_BT_GET_HBUSY(d)       (((d) >> IPMI_BT_HBUSY_BIT) & 0x1)
+#define IPMI_BT_SET_HBUSY(d, v)    ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \
+                                       (!!(v) << IPMI_BT_HBUSY_BIT)))
+
+#define IPMI_BT_BBUSY_MASK         (1 << IPMI_BT_BBUSY_BIT)
+#define IPMI_BT_SET_BBUSY(d, v)    ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \
+                                       (!!(v) << IPMI_BT_BBUSY_BIT)))
+
+
+/* Mask register */
+#define IPMI_BT_B2H_IRQ_EN_BIT     0
+#define IPMI_BT_B2H_IRQ_BIT        1
+
+#define IPMI_BT_B2H_IRQ_EN_MASK      (1 << IPMI_BT_B2H_IRQ_EN_BIT)
+#define IPMI_BT_GET_B2H_IRQ_EN(d)    (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1)
+#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\
+                                        (!!(v) << IPMI_BT_B2H_IRQ_EN_BIT)))
+
+#define IPMI_BT_B2H_IRQ_MASK         (1 << IPMI_BT_B2H_IRQ_BIT)
+#define IPMI_BT_GET_B2H_IRQ(d)       (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1)
+#define IPMI_BT_SET_B2H_IRQ(d, v)    ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \
+                                        (!!(v) << IPMI_BT_B2H_IRQ_BIT)))
+
+#define IPMI_CMD_GET_BT_INTF_CAP        0x36
+
+static void ipmi_bt_raise_irq(IPMIBT *ib)
+{
+    if (ib->use_irq && ib->irqs_enabled && ib->raise_irq) {
+        ib->raise_irq(ib);
+    }
+}
+
+static void ipmi_bt_lower_irq(IPMIBT *ib)
+{
+    if (ib->lower_irq) {
+        ib->lower_irq(ib);
+    }
+}
+
+static void ipmi_bt_handle_event(IPMIInterface *ii)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    if (ib->inlen < 4) {
+        goto out;
+    }
+    /* Note that overruns are handled by handle_command */
+    if (ib->inmsg[0] != (ib->inlen - 1)) {
+        /* Length mismatch, just ignore. */
+        IPMI_BT_SET_BBUSY(ib->control_reg, 1);
+        ib->inlen = 0;
+        goto out;
+    }
+    if ((ib->inmsg[1] == (IPMI_NETFN_APP << 2)) &&
+                        (ib->inmsg[3] == IPMI_CMD_GET_BT_INTF_CAP)) {
+        /* We handle this one ourselves. */
+        ib->outmsg[0] = 9;
+        ib->outmsg[1] = ib->inmsg[1] | 0x04;
+        ib->outmsg[2] = ib->inmsg[2];
+        ib->outmsg[3] = ib->inmsg[3];
+        ib->outmsg[4] = 0;
+        ib->outmsg[5] = 1; /* Only support 1 outstanding request. */
+        if (sizeof(ib->inmsg) > 0xff) { /* Input buffer size */
+            ib->outmsg[6] = 0xff;
+        } else {
+            ib->outmsg[6] = (unsigned char) sizeof(ib->inmsg);
+        }
+        if (sizeof(ib->outmsg) > 0xff) { /* Output buffer size */
+            ib->outmsg[7] = 0xff;
+        } else {
+            ib->outmsg[7] = (unsigned char) sizeof(ib->outmsg);
+        }
+        ib->outmsg[8] = 10; /* Max request to response time */
+        ib->outmsg[9] = 0; /* Don't recommend retries */
+        ib->outlen = 10;
+        IPMI_BT_SET_BBUSY(ib->control_reg, 0);
+        IPMI_BT_SET_B2H_ATN(ib->control_reg, 1);
+        if (!IPMI_BT_GET_B2H_IRQ(ib->mask_reg) &&
+                IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
+            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
+            ipmi_bt_raise_irq(ib);
+        }
+        goto out;
+    }
+    ib->waiting_seq = ib->inmsg[2];
+    ib->inmsg[2] = ib->inmsg[1];
+    {
+        IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(ib->bmc);
+        bk->handle_command(ib->bmc, ib->inmsg + 2, ib->inlen - 2,
+                           sizeof(ib->inmsg), ib->waiting_rsp);
+    }
+ out:
+    return;
+}
+
+static void ipmi_bt_handle_rsp(IPMIInterface *ii, uint8_t msg_id,
+                                unsigned char *rsp, unsigned int rsp_len)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    if (ib->waiting_rsp == msg_id) {
+        ib->waiting_rsp++;
+        if (rsp_len > (sizeof(ib->outmsg) - 2)) {
+            ib->outmsg[0] = 4;
+            ib->outmsg[1] = rsp[0];
+            ib->outmsg[2] = ib->waiting_seq;
+            ib->outmsg[3] = rsp[1];
+            ib->outmsg[4] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
+            ib->outlen = 5;
+        } else {
+            ib->outmsg[0] = rsp_len + 1;
+            ib->outmsg[1] = rsp[0];
+            ib->outmsg[2] = ib->waiting_seq;
+            memcpy(ib->outmsg + 3, rsp + 1, rsp_len - 1);
+            ib->outlen = rsp_len + 2;
+        }
+        IPMI_BT_SET_BBUSY(ib->control_reg, 0);
+        IPMI_BT_SET_B2H_ATN(ib->control_reg, 1);
+        if (!IPMI_BT_GET_B2H_IRQ(ib->mask_reg) &&
+                IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
+            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
+            ipmi_bt_raise_irq(ib);
+        }
+    }
+}
+
+
+static uint64_t ipmi_bt_ioport_read(void *opaque, hwaddr addr, unsigned size)
+{
+    IPMIInterface *ii = opaque;
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+    uint32_t ret = 0xff;
+
+    switch (addr & 3) {
+    case 0:
+        ret = ib->control_reg;
+        break;
+    case 1:
+        if (ib->outpos < ib->outlen) {
+            ret = ib->outmsg[ib->outpos];
+            ib->outpos++;
+            if (ib->outpos == ib->outlen) {
+                ib->outpos = 0;
+                ib->outlen = 0;
+            }
+        } else {
+            ret = 0xff;
+        }
+        break;
+    case 2:
+        ret = ib->mask_reg;
+        break;
+    }
+    return ret;
+}
+
+static void ipmi_bt_signal(IPMIBT *ib, IPMIInterface *ii)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+
+    ib->do_wake = 1;
+    while (ib->do_wake) {
+        ib->do_wake = 0;
+        iic->handle_if_event(ii);
+    }
+}
+
+static void ipmi_bt_ioport_write(void *opaque, hwaddr addr, uint64_t val,
+                                 unsigned size)
+{
+    IPMIInterface *ii = opaque;
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    switch (addr & 3) {
+    case 0:
+        if (IPMI_BT_GET_CLR_WR(val)) {
+            ib->inlen = 0;
+        }
+        if (IPMI_BT_GET_CLR_RD(val)) {
+            ib->outpos = 0;
+        }
+        if (IPMI_BT_GET_B2H_ATN(val)) {
+            IPMI_BT_SET_B2H_ATN(ib->control_reg, 0);
+        }
+        if (IPMI_BT_GET_SMS_ATN(val)) {
+            IPMI_BT_SET_SMS_ATN(ib->control_reg, 0);
+        }
+        if (IPMI_BT_GET_HBUSY(val)) {
+            /* Toggle */
+            IPMI_BT_SET_HBUSY(ib->control_reg,
+                              !IPMI_BT_GET_HBUSY(ib->control_reg));
+        }
+        if (IPMI_BT_GET_H2B_ATN(val)) {
+            IPMI_BT_SET_BBUSY(ib->control_reg, 1);
+            ipmi_bt_signal(ib, ii);
+        }
+        break;
+
+    case 1:
+        if (ib->inlen < sizeof(ib->inmsg)) {
+            ib->inmsg[ib->inlen] = val;
+        }
+        ib->inlen++;
+        break;
+
+    case 2:
+        if (IPMI_BT_GET_B2H_IRQ_EN(val) !=
+                        IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
+            if (IPMI_BT_GET_B2H_IRQ_EN(val)) {
+                if (IPMI_BT_GET_B2H_ATN(ib->control_reg) ||
+                        IPMI_BT_GET_SMS_ATN(ib->control_reg)) {
+                    IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
+                    ipmi_bt_raise_irq(ib);
+                }
+                IPMI_BT_SET_B2H_IRQ_EN(ib->mask_reg, 1);
+            } else {
+                if (IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
+                    IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
+                    ipmi_bt_lower_irq(ib);
+                }
+                IPMI_BT_SET_B2H_IRQ_EN(ib->mask_reg, 0);
+            }
+        }
+        if (IPMI_BT_GET_B2H_IRQ(val) && IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
+            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
+            ipmi_bt_lower_irq(ib);
+        }
+        break;
+    }
+}
+
+static const MemoryRegionOps ipmi_bt_io_ops = {
+    .read = ipmi_bt_ioport_read,
+    .write = ipmi_bt_ioport_write,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void ipmi_bt_set_atn(IPMIInterface *ii, int val, int irq)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    if (!!val == IPMI_BT_GET_SMS_ATN(ib->control_reg)) {
+        return;
+    }
+
+    IPMI_BT_SET_SMS_ATN(ib->control_reg, val);
+    if (val) {
+        if (irq && !IPMI_BT_GET_B2H_ATN(ib->control_reg) &&
+                IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
+            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
+            ipmi_bt_raise_irq(ib);
+        }
+    } else {
+        if (!IPMI_BT_GET_B2H_ATN(ib->control_reg) &&
+                IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
+            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
+            ipmi_bt_lower_irq(ib);
+        }
+    }
+}
+
+static void ipmi_bt_handle_reset(IPMIInterface *ii, bool is_cold)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    if (is_cold) {
+        /* Disable the BT interrupt on reset */
+        if (IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
+            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
+            ipmi_bt_lower_irq(ib);
+        }
+        IPMI_BT_SET_B2H_IRQ_EN(ib->mask_reg, 0);
+    }
+}
+
+static void ipmi_bt_set_irq_enable(IPMIInterface *ii, int val)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    ib->irqs_enabled = val;
+}
+
+static void ipmi_bt_init(IPMIInterface *ii, Error **errp)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMIBT *ib = iic->get_backend_data(ii);
+
+    ib->io_length = 3;
+
+    memory_region_init_io(&ib->io, NULL, &ipmi_bt_io_ops, ii, "ipmi-bt", 3);
+}
+
+int ipmi_bt_vmstate_post_load(void *opaque, int version)
+{
+    IPMIBT *ib = opaque;
+
+    /* Make sure all the values are sane. */
+    if (ib->outpos >= MAX_IPMI_MSG_SIZE || ib->outlen >= MAX_IPMI_MSG_SIZE ||
+        ib->outpos >= ib->outlen) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "ipmi:bt: vmstate transfer received bad out values: %d %d\n",
+                      ib->outpos, ib->outlen);
+        ib->outpos = 0;
+        ib->outlen = 0;
+    }
+
+    if (ib->inlen >= MAX_IPMI_MSG_SIZE) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "ipmi:bt: vmstate transfer received bad in value: %d\n",
+                      ib->inlen);
+        ib->inlen = 0;
+    }
+
+    return 0;
+}
+
+const VMStateDescription vmstate_IPMIBT = {
+    .name = TYPE_IPMI_INTERFACE_PREFIX "bt",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = ipmi_bt_vmstate_post_load,
+    .fields      = (VMStateField[]) {
+        VMSTATE_BOOL(obf_irq_set, IPMIBT),
+        VMSTATE_BOOL(atn_irq_set, IPMIBT),
+        VMSTATE_BOOL(irqs_enabled, IPMIBT),
+        VMSTATE_UINT32(outpos, IPMIBT),
+        VMSTATE_UINT32(outlen, IPMIBT),
+        VMSTATE_UINT8_ARRAY(outmsg, IPMIBT, MAX_IPMI_MSG_SIZE),
+        VMSTATE_UINT32(inlen, IPMIBT),
+        VMSTATE_UINT8_ARRAY(inmsg, IPMIBT, MAX_IPMI_MSG_SIZE),
+        VMSTATE_UINT8(control_reg, IPMIBT),
+        VMSTATE_UINT8(mask_reg, IPMIBT),
+        VMSTATE_UINT8(waiting_rsp, IPMIBT),
+        VMSTATE_UINT8(waiting_seq, IPMIBT),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+void ipmi_bt_get_fwinfo(struct IPMIBT *ib, IPMIFwInfo *info)
+{
+    info->interface_name = "bt";
+    info->interface_type = IPMI_SMBIOS_BT;
+    info->ipmi_spec_major_revision = 2;
+    info->ipmi_spec_minor_revision = 0;
+    info->base_address = ib->io_base;
+    info->register_length = ib->io_length;
+    info->register_spacing = 1;
+    info->memspace = IPMI_MEMSPACE_IO;
+    info->irq_type = IPMI_LEVEL_IRQ;
+}
+
+void ipmi_bt_class_init(IPMIInterfaceClass *iic)
+{
+    iic->init = ipmi_bt_init;
+    iic->set_atn = ipmi_bt_set_atn;
+    iic->handle_rsp = ipmi_bt_handle_rsp;
+    iic->handle_if_event = ipmi_bt_handle_event;
+    iic->set_irq_enable = ipmi_bt_set_irq_enable;
+    iic->reset = ipmi_bt_handle_reset;
+}
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index a696096cbb..c102778712 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -26,403 +26,46 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
-#include "hw/ipmi/ipmi.h"
 #include "hw/irq.h"
+#include "hw/ipmi/ipmi_bt.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 
-/* Control register */
-#define IPMI_BT_CLR_WR_BIT         0
-#define IPMI_BT_CLR_RD_BIT         1
-#define IPMI_BT_H2B_ATN_BIT        2
-#define IPMI_BT_B2H_ATN_BIT        3
-#define IPMI_BT_SMS_ATN_BIT        4
-#define IPMI_BT_HBUSY_BIT          6
-#define IPMI_BT_BBUSY_BIT          7
-
-#define IPMI_BT_GET_CLR_WR(d)      (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1)
-
-#define IPMI_BT_GET_CLR_RD(d)      (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1)
-
-#define IPMI_BT_GET_H2B_ATN(d)     (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1)
-
-#define IPMI_BT_B2H_ATN_MASK       (1 << IPMI_BT_B2H_ATN_BIT)
-#define IPMI_BT_GET_B2H_ATN(d)     (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1)
-#define IPMI_BT_SET_B2H_ATN(d, v)  ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \
-                                        (!!(v) << IPMI_BT_B2H_ATN_BIT)))
-
-#define IPMI_BT_SMS_ATN_MASK       (1 << IPMI_BT_SMS_ATN_BIT)
-#define IPMI_BT_GET_SMS_ATN(d)     (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1)
-#define IPMI_BT_SET_SMS_ATN(d, v)  ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \
-                                        (!!(v) << IPMI_BT_SMS_ATN_BIT)))
-
-#define IPMI_BT_HBUSY_MASK         (1 << IPMI_BT_HBUSY_BIT)
-#define IPMI_BT_GET_HBUSY(d)       (((d) >> IPMI_BT_HBUSY_BIT) & 0x1)
-#define IPMI_BT_SET_HBUSY(d, v)    ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \
-                                       (!!(v) << IPMI_BT_HBUSY_BIT)))
-
-#define IPMI_BT_BBUSY_MASK         (1 << IPMI_BT_BBUSY_BIT)
-#define IPMI_BT_SET_BBUSY(d, v)    ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \
-                                       (!!(v) << IPMI_BT_BBUSY_BIT)))
-
-
-/* Mask register */
-#define IPMI_BT_B2H_IRQ_EN_BIT     0
-#define IPMI_BT_B2H_IRQ_BIT        1
-
-#define IPMI_BT_B2H_IRQ_EN_MASK      (1 << IPMI_BT_B2H_IRQ_EN_BIT)
-#define IPMI_BT_GET_B2H_IRQ_EN(d)    (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1)
-#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\
-                                        (!!(v) << IPMI_BT_B2H_IRQ_EN_BIT)))
-
-#define IPMI_BT_B2H_IRQ_MASK         (1 << IPMI_BT_B2H_IRQ_BIT)
-#define IPMI_BT_GET_B2H_IRQ(d)       (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1)
-#define IPMI_BT_SET_B2H_IRQ(d, v)    ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \
-                                        (!!(v) << IPMI_BT_B2H_IRQ_BIT)))
-
-typedef struct IPMIBT {
-    IPMIBmc *bmc;
-
-    bool do_wake;
-
-    qemu_irq irq;
-
-    uint32_t io_base;
-    unsigned long io_length;
-    MemoryRegion io;
-
-    bool obf_irq_set;
-    bool atn_irq_set;
-    bool use_irq;
-    bool irqs_enabled;
-
-    uint8_t outmsg[MAX_IPMI_MSG_SIZE];
-    uint32_t outpos;
-    uint32_t outlen;
-
-    uint8_t inmsg[MAX_IPMI_MSG_SIZE];
-    uint32_t inlen;
-
-    uint8_t control_reg;
-    uint8_t mask_reg;
-
-    /*
-     * This is a response number that we send with the command to make
-     * sure that the response matches the command.
-     */
-    uint8_t waiting_rsp;
-    uint8_t waiting_seq;
-} IPMIBT;
-
-#define IPMI_CMD_GET_BT_INTF_CAP        0x36
-
-static void ipmi_bt_handle_event(IPMIInterface *ii)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    if (ib->inlen < 4) {
-        goto out;
-    }
-    /* Note that overruns are handled by handle_command */
-    if (ib->inmsg[0] != (ib->inlen - 1)) {
-        /* Length mismatch, just ignore. */
-        IPMI_BT_SET_BBUSY(ib->control_reg, 1);
-        ib->inlen = 0;
-        goto out;
-    }
-    if ((ib->inmsg[1] == (IPMI_NETFN_APP << 2)) &&
-                        (ib->inmsg[3] == IPMI_CMD_GET_BT_INTF_CAP)) {
-        /* We handle this one ourselves. */
-        ib->outmsg[0] = 9;
-        ib->outmsg[1] = ib->inmsg[1] | 0x04;
-        ib->outmsg[2] = ib->inmsg[2];
-        ib->outmsg[3] = ib->inmsg[3];
-        ib->outmsg[4] = 0;
-        ib->outmsg[5] = 1; /* Only support 1 outstanding request. */
-        if (sizeof(ib->inmsg) > 0xff) { /* Input buffer size */
-            ib->outmsg[6] = 0xff;
-        } else {
-            ib->outmsg[6] = (unsigned char) sizeof(ib->inmsg);
-        }
-        if (sizeof(ib->outmsg) > 0xff) { /* Output buffer size */
-            ib->outmsg[7] = 0xff;
-        } else {
-            ib->outmsg[7] = (unsigned char) sizeof(ib->outmsg);
-        }
-        ib->outmsg[8] = 10; /* Max request to response time */
-        ib->outmsg[9] = 0; /* Don't recommend retries */
-        ib->outlen = 10;
-        IPMI_BT_SET_BBUSY(ib->control_reg, 0);
-        IPMI_BT_SET_B2H_ATN(ib->control_reg, 1);
-        if (ib->use_irq && ib->irqs_enabled &&
-                !IPMI_BT_GET_B2H_IRQ(ib->mask_reg) &&
-                IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
-            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
-            qemu_irq_raise(ib->irq);
-        }
-        goto out;
-    }
-    ib->waiting_seq = ib->inmsg[2];
-    ib->inmsg[2] = ib->inmsg[1];
-    {
-        IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(ib->bmc);
-        bk->handle_command(ib->bmc, ib->inmsg + 2, ib->inlen - 2,
-                           sizeof(ib->inmsg), ib->waiting_rsp);
-    }
- out:
-    return;
-}
-
-static void ipmi_bt_handle_rsp(IPMIInterface *ii, uint8_t msg_id,
-                                unsigned char *rsp, unsigned int rsp_len)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    if (ib->waiting_rsp == msg_id) {
-        ib->waiting_rsp++;
-        if (rsp_len > (sizeof(ib->outmsg) - 2)) {
-            ib->outmsg[0] = 4;
-            ib->outmsg[1] = rsp[0];
-            ib->outmsg[2] = ib->waiting_seq;
-            ib->outmsg[3] = rsp[1];
-            ib->outmsg[4] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
-            ib->outlen = 5;
-        } else {
-            ib->outmsg[0] = rsp_len + 1;
-            ib->outmsg[1] = rsp[0];
-            ib->outmsg[2] = ib->waiting_seq;
-            memcpy(ib->outmsg + 3, rsp + 1, rsp_len - 1);
-            ib->outlen = rsp_len + 2;
-        }
-        IPMI_BT_SET_BBUSY(ib->control_reg, 0);
-        IPMI_BT_SET_B2H_ATN(ib->control_reg, 1);
-        if (ib->use_irq && ib->irqs_enabled &&
-                !IPMI_BT_GET_B2H_IRQ(ib->mask_reg) &&
-                IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
-            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
-            qemu_irq_raise(ib->irq);
-        }
-    }
-}
-
-
-static uint64_t ipmi_bt_ioport_read(void *opaque, hwaddr addr, unsigned size)
-{
-    IPMIInterface *ii = opaque;
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-    uint32_t ret = 0xff;
-
-    switch (addr & 3) {
-    case 0:
-        ret = ib->control_reg;
-        break;
-    case 1:
-        if (ib->outpos < ib->outlen) {
-            ret = ib->outmsg[ib->outpos];
-            ib->outpos++;
-            if (ib->outpos == ib->outlen) {
-                ib->outpos = 0;
-                ib->outlen = 0;
-            }
-        } else {
-            ret = 0xff;
-        }
-        break;
-    case 2:
-        ret = ib->mask_reg;
-        break;
-    }
-    return ret;
-}
-
-static void ipmi_bt_signal(IPMIBT *ib, IPMIInterface *ii)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-
-    ib->do_wake = 1;
-    while (ib->do_wake) {
-        ib->do_wake = 0;
-        iic->handle_if_event(ii);
-    }
-}
-
-static void ipmi_bt_ioport_write(void *opaque, hwaddr addr, uint64_t val,
-                                 unsigned size)
-{
-    IPMIInterface *ii = opaque;
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    switch (addr & 3) {
-    case 0:
-        if (IPMI_BT_GET_CLR_WR(val)) {
-            ib->inlen = 0;
-        }
-        if (IPMI_BT_GET_CLR_RD(val)) {
-            ib->outpos = 0;
-        }
-        if (IPMI_BT_GET_B2H_ATN(val)) {
-            IPMI_BT_SET_B2H_ATN(ib->control_reg, 0);
-        }
-        if (IPMI_BT_GET_SMS_ATN(val)) {
-            IPMI_BT_SET_SMS_ATN(ib->control_reg, 0);
-        }
-        if (IPMI_BT_GET_HBUSY(val)) {
-            /* Toggle */
-            IPMI_BT_SET_HBUSY(ib->control_reg,
-                              !IPMI_BT_GET_HBUSY(ib->control_reg));
-        }
-        if (IPMI_BT_GET_H2B_ATN(val)) {
-            IPMI_BT_SET_BBUSY(ib->control_reg, 1);
-            ipmi_bt_signal(ib, ii);
-        }
-        break;
-
-    case 1:
-        if (ib->inlen < sizeof(ib->inmsg)) {
-            ib->inmsg[ib->inlen] = val;
-        }
-        ib->inlen++;
-        break;
-
-    case 2:
-        if (IPMI_BT_GET_B2H_IRQ_EN(val) !=
-                        IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
-            if (IPMI_BT_GET_B2H_IRQ_EN(val)) {
-                if (IPMI_BT_GET_B2H_ATN(ib->control_reg) ||
-                        IPMI_BT_GET_SMS_ATN(ib->control_reg)) {
-                    IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
-                    qemu_irq_raise(ib->irq);
-                }
-                IPMI_BT_SET_B2H_IRQ_EN(ib->mask_reg, 1);
-            } else {
-                if (IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
-                    IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
-                    qemu_irq_lower(ib->irq);
-                }
-                IPMI_BT_SET_B2H_IRQ_EN(ib->mask_reg, 0);
-            }
-        }
-        if (IPMI_BT_GET_B2H_IRQ(val) && IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
-            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
-            qemu_irq_lower(ib->irq);
-        }
-        break;
-    }
-}
-
-static const MemoryRegionOps ipmi_bt_io_ops = {
-    .read = ipmi_bt_ioport_read,
-    .write = ipmi_bt_ioport_write,
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 1,
-    },
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static void ipmi_bt_set_atn(IPMIInterface *ii, int val, int irq)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    if (!!val == IPMI_BT_GET_SMS_ATN(ib->control_reg)) {
-        return;
-    }
-
-    IPMI_BT_SET_SMS_ATN(ib->control_reg, val);
-    if (val) {
-        if (irq && ib->use_irq && ib->irqs_enabled &&
-                !IPMI_BT_GET_B2H_ATN(ib->control_reg) &&
-                IPMI_BT_GET_B2H_IRQ_EN(ib->mask_reg)) {
-            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 1);
-            qemu_irq_raise(ib->irq);
-        }
-    } else {
-        if (!IPMI_BT_GET_B2H_ATN(ib->control_reg) &&
-                IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
-            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
-            qemu_irq_lower(ib->irq);
-        }
-    }
-}
-
-static void ipmi_bt_handle_reset(IPMIInterface *ii, bool is_cold)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    if (is_cold) {
-        /* Disable the BT interrupt on reset */
-        if (IPMI_BT_GET_B2H_IRQ(ib->mask_reg)) {
-            IPMI_BT_SET_B2H_IRQ(ib->mask_reg, 0);
-            qemu_irq_lower(ib->irq);
-        }
-        IPMI_BT_SET_B2H_IRQ_EN(ib->mask_reg, 0);
-    }
-}
-
-static void ipmi_bt_set_irq_enable(IPMIInterface *ii, int val)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    ib->irqs_enabled = val;
-}
-
-static void ipmi_bt_init(IPMIInterface *ii, Error **errp)
-{
-    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-    IPMIBT *ib = iic->get_backend_data(ii);
-
-    ib->io_length = 3;
-
-    memory_region_init_io(&ib->io, NULL, &ipmi_bt_io_ops, ii, "ipmi-bt", 3);
-}
-
-
 #define TYPE_ISA_IPMI_BT "isa-ipmi-bt"
 #define ISA_IPMI_BT(obj) OBJECT_CHECK(ISAIPMIBTDevice, (obj), \
-                                       TYPE_ISA_IPMI_BT)
+                                      TYPE_ISA_IPMI_BT)
 
 typedef struct ISAIPMIBTDevice {
     ISADevice dev;
     int32_t isairq;
+    qemu_irq irq;
     IPMIBT bt;
     uint32_t uuid;
 } ISAIPMIBTDevice;
 
-static void ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
+static void isa_ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
 {
     ISAIPMIBTDevice *iib = ISA_IPMI_BT(ii);
 
-    info->interface_name = "bt";
-    info->interface_type = IPMI_SMBIOS_BT;
-    info->ipmi_spec_major_revision = 2;
-    info->ipmi_spec_minor_revision = 0;
-    info->base_address = iib->bt.io_base;
-    info->register_length = iib->bt.io_length;
-    info->register_spacing = 1;
-    info->memspace = IPMI_MEMSPACE_IO;
-    info->irq_type = IPMI_LEVEL_IRQ;
+    ipmi_bt_get_fwinfo(&iib->bt, info);
     info->interrupt_number = iib->isairq;
     info->i2c_slave_address = iib->bt.bmc->slave_addr;
     info->uuid = iib->uuid;
 }
 
-static void ipmi_bt_class_init(IPMIInterfaceClass *iic)
+static void isa_ipmi_bt_raise_irq(IPMIBT *ib)
 {
-    iic->init = ipmi_bt_init;
-    iic->set_atn = ipmi_bt_set_atn;
-    iic->handle_rsp = ipmi_bt_handle_rsp;
-    iic->handle_if_event = ipmi_bt_handle_event;
-    iic->set_irq_enable = ipmi_bt_set_irq_enable;
-    iic->reset = ipmi_bt_handle_reset;
-    iic->get_fwinfo = ipmi_bt_get_fwinfo;
+    ISAIPMIBTDevice *iib = ib->opaque;
+
+    qemu_irq_raise(iib->irq);
+}
+
+static void isa_ipmi_bt_lower_irq(IPMIBT *ib)
+{
+    ISAIPMIBTDevice *iib = ib->opaque;
+
+    qemu_irq_lower(iib->irq);
 }
 
 static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
@@ -440,14 +83,17 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
     iib->uuid = ipmi_next_uuid();
 
     iib->bt.bmc->intf = ii;
+    iib->bt.opaque = iib;
 
     iic->init(ii, errp);
     if (*errp)
         return;
 
     if (iib->isairq > 0) {
-        isa_init_irq(isadev, &iib->bt.irq, iib->isairq);
+        isa_init_irq(isadev, &iib->irq, iib->isairq);
         iib->bt.use_irq = 1;
+        iib->bt.raise_irq = isa_ipmi_bt_raise_irq;
+        iib->bt.lower_irq = isa_ipmi_bt_lower_irq;
     }
 
     qdev_set_legacy_instance_id(dev, iib->bt.io_base, iib->bt.io_length);
@@ -455,52 +101,6 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
     isa_register_ioport(isadev, &iib->bt.io, iib->bt.io_base);
 }
 
-static int ipmi_bt_vmstate_post_load(void *opaque, int version)
-{
-    IPMIBT *ib = opaque;
-
-    /* Make sure all the values are sane. */
-    if (ib->outpos >= MAX_IPMI_MSG_SIZE || ib->outlen >= MAX_IPMI_MSG_SIZE ||
-        ib->outpos >= ib->outlen) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "ipmi:bt: vmstate transfer received bad out values: %d %d\n",
-                      ib->outpos, ib->outlen);
-        ib->outpos = 0;
-        ib->outlen = 0;
-    }
-
-    if (ib->inlen >= MAX_IPMI_MSG_SIZE) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "ipmi:bt: vmstate transfer received bad in value: %d\n",
-                      ib->inlen);
-        ib->inlen = 0;
-    }
-
-    return 0;
-}
-
-const VMStateDescription vmstate_IPMIBT = {
-    .name = TYPE_IPMI_INTERFACE_PREFIX "bt",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .post_load = ipmi_bt_vmstate_post_load,
-    .fields      = (VMStateField[]) {
-        VMSTATE_BOOL(obf_irq_set, IPMIBT),
-        VMSTATE_BOOL(atn_irq_set, IPMIBT),
-        VMSTATE_BOOL(irqs_enabled, IPMIBT),
-        VMSTATE_UINT32(outpos, IPMIBT),
-        VMSTATE_UINT32(outlen, IPMIBT),
-        VMSTATE_UINT8_ARRAY(outmsg, IPMIBT, MAX_IPMI_MSG_SIZE),
-        VMSTATE_UINT32(inlen, IPMIBT),
-        VMSTATE_UINT8_ARRAY(inmsg, IPMIBT, MAX_IPMI_MSG_SIZE),
-        VMSTATE_UINT8(control_reg, IPMIBT),
-        VMSTATE_UINT8(mask_reg, IPMIBT),
-        VMSTATE_UINT8(waiting_rsp, IPMIBT),
-        VMSTATE_UINT8(waiting_seq, IPMIBT),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static const VMStateDescription vmstate_ISAIPMIBTDevice = {
     .name = TYPE_IPMI_INTERFACE_PREFIX "isa-bt",
     .version_id = 2,
@@ -548,6 +148,7 @@ static void isa_ipmi_bt_class_init(ObjectClass *oc, void *data)
 
     iic->get_backend_data = isa_ipmi_bt_get_backend_data;
     ipmi_bt_class_init(iic);
+    iic->get_fwinfo = isa_ipmi_bt_get_fwinfo;
 }
 
 static const TypeInfo isa_ipmi_bt_info = {
diff --git a/include/hw/ipmi/ipmi_bt.h b/include/hw/ipmi/ipmi_bt.h
new file mode 100644
index 0000000000..9667aaa88a
--- /dev/null
+++ b/include/hw/ipmi/ipmi_bt.h
@@ -0,0 +1,72 @@
+/*
+ * QEMU IPMI BT 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.
+ */
+
+#ifndef HW_IPMI_BT_H
+#define HW_IPMI_BT_H
+
+#include "hw/ipmi/ipmi.h"
+
+typedef struct IPMIBT {
+    IPMIBmc *bmc;
+
+    bool do_wake;
+
+    bool obf_irq_set;
+    bool atn_irq_set;
+    bool irqs_enabled;
+
+    uint8_t outmsg[MAX_IPMI_MSG_SIZE];
+    uint32_t outpos;
+    uint32_t outlen;
+
+    uint8_t inmsg[MAX_IPMI_MSG_SIZE];
+    uint32_t inlen;
+
+    uint8_t control_reg;
+    uint8_t mask_reg;
+
+    /*
+     * This is a response number that we send with the command to make
+     * sure that the response matches the command.
+     */
+    uint8_t waiting_rsp;
+    uint8_t waiting_seq;
+
+    uint32_t io_base;
+    unsigned long io_length;
+    MemoryRegion io;
+
+    void (*raise_irq)(struct IPMIBT *ib);
+    void (*lower_irq)(struct IPMIBT *ib);
+    void *opaque;
+
+    bool use_irq;
+} IPMIBT;
+
+void ipmi_bt_get_fwinfo(IPMIBT *ik, IPMIFwInfo *info);
+void ipmi_bt_class_init(IPMIInterfaceClass *iic);
+extern const VMStateDescription vmstate_IPMIBT;
+int ipmi_bt_vmstate_post_load(void *opaque, int version);
+
+#endif /* HW_IPMI_BT_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/15] ipmi: Allow a size value to be passed for I/O space
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (7 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 08/15] ipmi: Split out BT-specific code from ISA BT code minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 10/15] smbios:ipmi: Ignore IPMI devices with no fwinfo function minyard
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

PCI device I/O must be >= 8 bytes in length or they don't work.
Allow the size to be passed in, the default size of 2 or 3
won't work.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bt.c          | 19 +++++++++++++++----
 hw/ipmi/ipmi_kcs.c         | 23 +++++++++++++++++++----
 hw/ipmi/isa_ipmi_bt.c      |  2 +-
 hw/ipmi/isa_ipmi_kcs.c     |  2 +-
 include/hw/ipmi/ipmi.h     |  7 ++++++-
 include/hw/ipmi/ipmi_bt.h  |  1 +
 include/hw/ipmi/ipmi_kcs.h |  1 +
 7 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
index e6765ca4f8..22f94fb98d 100644
--- a/hw/ipmi/ipmi_bt.c
+++ b/hw/ipmi/ipmi_bt.c
@@ -189,7 +189,7 @@ static uint64_t ipmi_bt_ioport_read(void *opaque, hwaddr addr, unsigned size)
     IPMIBT *ib = iic->get_backend_data(ii);
     uint32_t ret = 0xff;
 
-    switch (addr & 3) {
+    switch (addr & ib->size_mask) {
     case 0:
         ret = ib->control_reg;
         break;
@@ -208,6 +208,9 @@ static uint64_t ipmi_bt_ioport_read(void *opaque, hwaddr addr, unsigned size)
     case 2:
         ret = ib->mask_reg;
         break;
+    default:
+        ret = 0xff;
+        break;
     }
     return ret;
 }
@@ -230,7 +233,7 @@ static void ipmi_bt_ioport_write(void *opaque, hwaddr addr, uint64_t val,
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
     IPMIBT *ib = iic->get_backend_data(ii);
 
-    switch (addr & 3) {
+    switch (addr & ib->size_mask) {
     case 0:
         if (IPMI_BT_GET_CLR_WR(val)) {
             ib->inlen = 0;
@@ -285,6 +288,9 @@ static void ipmi_bt_ioport_write(void *opaque, hwaddr addr, uint64_t val,
             ipmi_bt_lower_irq(ib);
         }
         break;
+    default:
+        /* Ignore. */
+        break;
     }
 }
 
@@ -346,14 +352,19 @@ static void ipmi_bt_set_irq_enable(IPMIInterface *ii, int val)
     ib->irqs_enabled = val;
 }
 
-static void ipmi_bt_init(IPMIInterface *ii, Error **errp)
+static void ipmi_bt_init(IPMIInterface *ii, unsigned int min_size, Error **errp)
 {
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
     IPMIBT *ib = iic->get_backend_data(ii);
 
+    if (min_size == 0) {
+        min_size = 4;
+    }
+    ib->size_mask = min_size - 1;
     ib->io_length = 3;
 
-    memory_region_init_io(&ib->io, NULL, &ipmi_bt_io_ops, ii, "ipmi-bt", 3);
+    memory_region_init_io(&ib->io, NULL, &ipmi_bt_io_ops, ii, "ipmi-bt",
+                          min_size);
 }
 
 int ipmi_bt_vmstate_post_load(void *opaque, int version)
diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
index dab1af8bc8..a77612946a 100644
--- a/hw/ipmi/ipmi_kcs.c
+++ b/hw/ipmi/ipmi_kcs.c
@@ -232,7 +232,7 @@ static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
     IPMIKCS *ik = iic->get_backend_data(ii);
     uint32_t ret;
 
-    switch (addr & 1) {
+    switch (addr & ik->size_mask) {
     case 0:
         ret = ik->data_out_reg;
         IPMI_KCS_SET_OBF(ik->status_reg, 0);
@@ -243,6 +243,7 @@ static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
             }
         }
         break;
+
     case 1:
         ret = ik->status_reg;
         if (ik->atn_irq_set) {
@@ -252,6 +253,9 @@ static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
             }
         }
         break;
+
+    default:
+        ret = 0xff;
     }
     return ret;
 }
@@ -267,7 +271,7 @@ static void ipmi_kcs_ioport_write(void *opaque, hwaddr addr, uint64_t val,
         return;
     }
 
-    switch (addr & 1) {
+    switch (addr & ik->size_mask) {
     case 0:
         ik->data_in_reg = val;
         break;
@@ -275,6 +279,10 @@ static void ipmi_kcs_ioport_write(void *opaque, hwaddr addr, uint64_t val,
     case 1:
         ik->cmd_reg = val;
         break;
+
+    default:
+        /* Ignore. */
+        break;
     }
     IPMI_KCS_SET_IBF(ik->status_reg, 1);
     ipmi_kcs_signal(ik, ii);
@@ -321,13 +329,20 @@ static void ipmi_kcs_set_irq_enable(IPMIInterface *ii, int val)
     ik->irqs_enabled = val;
 }
 
-static void ipmi_kcs_init(IPMIInterface *ii, Error **errp)
+/* min_size must be a power of 2. */
+static void ipmi_kcs_init(IPMIInterface *ii, unsigned int min_size,
+                          Error **errp)
 {
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
     IPMIKCS *ik = iic->get_backend_data(ii);
 
+    if (min_size == 0) {
+        min_size = 2;
+    }
+    ik->size_mask = min_size - 1;
     ik->io_length = 2;
-    memory_region_init_io(&ik->io, NULL, &ipmi_kcs_io_ops, ii, "ipmi-kcs", 2);
+    memory_region_init_io(&ik->io, NULL, &ipmi_kcs_io_ops, ii, "ipmi-kcs",
+                          min_size);
 }
 
 int ipmi_kcs_vmstate_post_load(void *opaque, int version)
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index c102778712..9a87ffd3f0 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -85,7 +85,7 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
     iib->bt.bmc->intf = ii;
     iib->bt.opaque = iib;
 
-    iic->init(ii, errp);
+    iic->init(ii, 0, errp);
     if (*errp)
         return;
 
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 8e32774f85..ca3ea36a3f 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -84,7 +84,7 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
     iik->kcs.bmc->intf = ii;
     iik->kcs.opaque = iik;
 
-    iic->init(ii, errp);
+    iic->init(ii, 0, errp);
     if (*errp)
         return;
 
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 70871da0a7..6f2413b39b 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -118,7 +118,12 @@ typedef struct IPMIInterface IPMIInterface;
 typedef struct IPMIInterfaceClass {
     InterfaceClass parent;
 
-    void (*init)(struct IPMIInterface *s, Error **errp);
+    /*
+     * min_size is the requested I/O size and must be a power of 2.
+     * This is so PCI (or other busses) can request a bigger range.
+     * Use 0 for the default.
+     */
+    void (*init)(struct IPMIInterface *s, unsigned int min_size, Error **errp);
 
     /*
      * Perform various operations on the hardware.  If checkonly is
diff --git a/include/hw/ipmi/ipmi_bt.h b/include/hw/ipmi/ipmi_bt.h
index 9667aaa88a..8a4316ea7c 100644
--- a/include/hw/ipmi/ipmi_bt.h
+++ b/include/hw/ipmi/ipmi_bt.h
@@ -56,6 +56,7 @@ typedef struct IPMIBT {
     uint32_t io_base;
     unsigned long io_length;
     MemoryRegion io;
+    unsigned long size_mask;
 
     void (*raise_irq)(struct IPMIBT *ib);
     void (*lower_irq)(struct IPMIBT *ib);
diff --git a/include/hw/ipmi/ipmi_kcs.h b/include/hw/ipmi/ipmi_kcs.h
index 91d76d08f4..6e6ef4c539 100644
--- a/include/hw/ipmi/ipmi_kcs.h
+++ b/include/hw/ipmi/ipmi_kcs.h
@@ -59,6 +59,7 @@ typedef struct IPMIKCS {
     uint32_t io_base;
     unsigned long io_length;
     MemoryRegion io;
+    unsigned long size_mask;
 
     void (*raise_irq)(struct IPMIKCS *ik);
     void (*lower_irq)(struct IPMIKCS *ik);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/15] smbios:ipmi: Ignore IPMI devices with no fwinfo function
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (8 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 09/15] ipmi: Allow a size value to be passed for I/O space minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 11/15] ipmi: Add PCI IPMI interfaces minyard
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Not all devices have fwinfo (like the coming PCI one), so ignore
them if the their fwinfo function is NULL.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/smbios/smbios_type_38.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/smbios/smbios_type_38.c b/hw/smbios/smbios_type_38.c
index 0c08f282de..168b886647 100644
--- a/hw/smbios/smbios_type_38.c
+++ b/hw/smbios/smbios_type_38.c
@@ -94,6 +94,9 @@ static void smbios_add_ipmi_devices(BusState *bus)
             ii = IPMI_INTERFACE(obj);
             iic = IPMI_INTERFACE_GET_CLASS(obj);
             memset(&info, 0, sizeof(info));
+            if (!iic->get_fwinfo) {
+                continue;
+            }
             iic->get_fwinfo(ii, &info);
             smbios_build_one_type_38(&info);
             continue;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 11/15] ipmi: Add PCI IPMI interfaces
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (9 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 10/15] smbios:ipmi: Ignore IPMI devices with no fwinfo function minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 12/15] ipmi: Add an SMBus IPMI interface minyard
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Pretty straightforward, just hook the current KCS and BT code into
the PCI system with the proper configuration.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 default-configs/i386-softmmu.mak |   2 +
 hw/i386/Kconfig                  |   2 +
 hw/ipmi/Kconfig                  |  10 +++
 hw/ipmi/Makefile.objs            |   2 +
 hw/ipmi/pci_ipmi_bt.c            | 146 +++++++++++++++++++++++++++++++
 hw/ipmi/pci_ipmi_kcs.c           | 146 +++++++++++++++++++++++++++++++
 include/hw/pci/pci.h             |   1 +
 7 files changed, 309 insertions(+)
 create mode 100644 hw/ipmi/pci_ipmi_bt.c
 create mode 100644 hw/ipmi/pci_ipmi_kcs.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index ba3fb3ff50..2294c0be5a 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -10,6 +10,8 @@
 #CONFIG_ISA_DEBUG=n
 #CONFIG_ISA_IPMI_BT=n
 #CONFIG_ISA_IPMI_KCS=n
+#CONFIG_PCI_IPMI_KCS=n
+#CONFIG_PCI_IPMI_BT=n
 #CONFIG_PCI_DEVICES=n
 #CONFIG_PVPANIC=n
 #CONFIG_QXL=n
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index c7a9d6315c..d10f4e3e8b 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -8,6 +8,8 @@ config PC
     imply HYPERV
     imply ISA_IPMI_KCS
     imply ISA_IPMI_BT
+    imply PCI_IPMI_KCS
+    imply PCI_IPMI_BT
     imply ISA_DEBUG
     imply PARALLEL
     imply PCI_DEVICES
diff --git a/hw/ipmi/Kconfig b/hw/ipmi/Kconfig
index b944fae100..12db4e81ad 100644
--- a/hw/ipmi/Kconfig
+++ b/hw/ipmi/Kconfig
@@ -20,3 +20,13 @@ config ISA_IPMI_BT
     bool
     depends on ISA_BUS
     select IPMI
+
+config PCI_IPMI_KCS
+    bool
+    depends on PCI
+    select IPMI
+
+config PCI_IPMI_BT
+    bool
+    depends on PCI
+    select IPMI
diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
index 4ffa45a66c..2d7f080a86 100644
--- a/hw/ipmi/Makefile.objs
+++ b/hw/ipmi/Makefile.objs
@@ -2,4 +2,6 @@ common-obj-$(CONFIG_IPMI) += ipmi.o ipmi_kcs.o ipmi_bt.o
 common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_bmc_sim.o
 common-obj-$(CONFIG_IPMI_EXTERN) += ipmi_bmc_extern.o
 common-obj-$(CONFIG_ISA_IPMI_KCS) += isa_ipmi_kcs.o
+common-obj-$(CONFIG_PCI_IPMI_KCS) += pci_ipmi_kcs.o
 common-obj-$(CONFIG_ISA_IPMI_BT) += isa_ipmi_bt.o
+common-obj-$(CONFIG_PCI_IPMI_BT) += pci_ipmi_bt.o
diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
new file mode 100644
index 0000000000..6ed925a665
--- /dev/null
+++ b/hw/ipmi/pci_ipmi_bt.c
@@ -0,0 +1,146 @@
+/*
+ * QEMU PCI IPMI BT emulation
+ *
+ * Copyright (c) 2017 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 "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "hw/ipmi/ipmi_bt.h"
+#include "hw/pci/pci.h"
+
+#define TYPE_PCI_IPMI_BT "pci-ipmi-bt"
+#define PCI_IPMI_BT(obj) OBJECT_CHECK(PCIIPMIBTDevice, (obj), \
+                                       TYPE_PCI_IPMI_BT)
+
+typedef struct PCIIPMIBTDevice {
+    PCIDevice dev;
+    IPMIBT bt;
+    bool irq_enabled;
+    uint32_t uuid;
+} PCIIPMIBTDevice;
+
+static void pci_ipmi_raise_irq(IPMIBT *ik)
+{
+    PCIIPMIBTDevice *pik = ik->opaque;
+
+    pci_set_irq(&pik->dev, true);
+}
+
+static void pci_ipmi_lower_irq(IPMIBT *ik)
+{
+    PCIIPMIBTDevice *pik = ik->opaque;
+
+    pci_set_irq(&pik->dev, false);
+}
+
+static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
+{
+    PCIIPMIBTDevice *pik = PCI_IPMI_BT(pd);
+    IPMIInterface *ii = IPMI_INTERFACE(pd);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+
+    if (!pik->bt.bmc) {
+        error_setg(errp, "IPMI device requires a bmc attribute to be set");
+        return;
+    }
+
+    pik->uuid = ipmi_next_uuid();
+
+    pik->bt.bmc->intf = ii;
+    pik->bt.opaque = pik;
+
+    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;
+
+    iic->init(ii, 8, errp);
+    if (*errp) {
+        return;
+    }
+    pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->bt.io);
+}
+
+const VMStateDescription vmstate_PCIIPMIBTDevice = {
+    .name = TYPE_IPMI_INTERFACE_PREFIX "pci-bt",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCIIPMIBTDevice),
+        VMSTATE_STRUCT(bt, PCIIPMIBTDevice, 1, vmstate_IPMIBT, IPMIBT),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pci_ipmi_bt_instance_init(Object *obj)
+{
+    PCIIPMIBTDevice *pik = PCI_IPMI_BT(obj);
+
+    ipmi_bmc_find_and_link(obj, (Object **) &pik->bt.bmc);
+}
+
+static void *pci_ipmi_bt_get_backend_data(IPMIInterface *ii)
+{
+    PCIIPMIBTDevice *pik = PCI_IPMI_BT(ii);
+
+    return &pik->bt;
+}
+
+static void pci_ipmi_bt_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    PCIDeviceClass *pdc = PCI_DEVICE_CLASS(oc);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_CLASS(oc);
+
+    pdc->vendor_id = PCI_VENDOR_ID_QEMU;
+    pdc->device_id = PCI_DEVICE_ID_QEMU_IPMI;
+    pdc->revision = 1;
+    pdc->class_id = PCI_CLASS_SERIAL_IPMI;
+
+    dc->vmsd = &vmstate_PCIIPMIBTDevice;
+    dc->desc = "PCI IPMI BT";
+    pdc->realize = pci_ipmi_bt_realize;
+
+    iic->get_backend_data = pci_ipmi_bt_get_backend_data;
+    ipmi_bt_class_init(iic);
+}
+
+static const TypeInfo pci_ipmi_bt_info = {
+    .name          = TYPE_PCI_IPMI_BT,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIIPMIBTDevice),
+    .instance_init = pci_ipmi_bt_instance_init,
+    .class_init    = pci_ipmi_bt_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_IPMI_INTERFACE },
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { }
+    }
+};
+
+static void pci_ipmi_bt_register_types(void)
+{
+    type_register_static(&pci_ipmi_bt_info);
+}
+
+type_init(pci_ipmi_bt_register_types)
diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
new file mode 100644
index 0000000000..eeba63baa4
--- /dev/null
+++ b/hw/ipmi/pci_ipmi_kcs.c
@@ -0,0 +1,146 @@
+/*
+ * QEMU PCI IPMI KCS emulation
+ *
+ * Copyright (c) 2017 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 "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "hw/ipmi/ipmi_kcs.h"
+#include "hw/pci/pci.h"
+
+#define TYPE_PCI_IPMI_KCS "pci-ipmi-kcs"
+#define PCI_IPMI_KCS(obj) OBJECT_CHECK(PCIIPMIKCSDevice, (obj), \
+                                       TYPE_PCI_IPMI_KCS)
+
+typedef struct PCIIPMIKCSDevice {
+    PCIDevice dev;
+    IPMIKCS kcs;
+    bool irq_enabled;
+    uint32_t uuid;
+} PCIIPMIKCSDevice;
+
+static void pci_ipmi_raise_irq(IPMIKCS *ik)
+{
+    PCIIPMIKCSDevice *pik = ik->opaque;
+
+    pci_set_irq(&pik->dev, true);
+}
+
+static void pci_ipmi_lower_irq(IPMIKCS *ik)
+{
+    PCIIPMIKCSDevice *pik = ik->opaque;
+
+    pci_set_irq(&pik->dev, false);
+}
+
+static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
+{
+    PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(pd);
+    IPMIInterface *ii = IPMI_INTERFACE(pd);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+
+    if (!pik->kcs.bmc) {
+        error_setg(errp, "IPMI device requires a bmc attribute to be set");
+        return;
+    }
+
+    pik->uuid = ipmi_next_uuid();
+
+    pik->kcs.bmc->intf = ii;
+    pik->kcs.opaque = pik;
+
+    pci_config_set_prog_interface(pd->config, 0x01); /* KCS */
+    pci_config_set_interrupt_pin(pd->config, 0x01);
+    pik->kcs.use_irq = 1;
+    pik->kcs.raise_irq = pci_ipmi_raise_irq;
+    pik->kcs.lower_irq = pci_ipmi_lower_irq;
+
+    iic->init(ii, 8, errp);
+    if (*errp) {
+        return;
+    }
+    pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->kcs.io);
+}
+
+const VMStateDescription vmstate_PCIIPMIKCSDevice = {
+    .name = TYPE_IPMI_INTERFACE_PREFIX "pci-kcs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCIIPMIKCSDevice),
+        VMSTATE_STRUCT(kcs, PCIIPMIKCSDevice, 1, vmstate_IPMIKCS, IPMIKCS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pci_ipmi_kcs_instance_init(Object *obj)
+{
+    PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(obj);
+
+    ipmi_bmc_find_and_link(obj, (Object **) &pik->kcs.bmc);
+}
+
+static void *pci_ipmi_kcs_get_backend_data(IPMIInterface *ii)
+{
+    PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(ii);
+
+    return &pik->kcs;
+}
+
+static void pci_ipmi_kcs_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    PCIDeviceClass *pdc = PCI_DEVICE_CLASS(oc);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_CLASS(oc);
+
+    pdc->vendor_id = PCI_VENDOR_ID_QEMU;
+    pdc->device_id = PCI_DEVICE_ID_QEMU_IPMI;
+    pdc->revision = 1;
+    pdc->class_id = PCI_CLASS_SERIAL_IPMI;
+
+    dc->vmsd = &vmstate_PCIIPMIKCSDevice;
+    dc->desc = "PCI IPMI KCS";
+    pdc->realize = pci_ipmi_kcs_realize;
+
+    iic->get_backend_data = pci_ipmi_kcs_get_backend_data;
+    ipmi_kcs_class_init(iic);
+}
+
+static const TypeInfo pci_ipmi_kcs_info = {
+    .name          = TYPE_PCI_IPMI_KCS,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIIPMIKCSDevice),
+    .instance_init = pci_ipmi_kcs_instance_init,
+    .class_init    = pci_ipmi_kcs_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_IPMI_INTERFACE },
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { }
+    }
+};
+
+static void pci_ipmi_kcs_register_types(void)
+{
+    type_register_static(&pci_ipmi_kcs_info);
+}
+
+type_init(pci_ipmi_kcs_register_types)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 1b840e61a2..f3f0ffd5fb 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -55,6 +55,7 @@ extern bool pci_available;
 /* QEMU/Bochs VGA (0x1234) */
 #define PCI_VENDOR_ID_QEMU               0x1234
 #define PCI_DEVICE_ID_QEMU_VGA           0x1111
+#define PCI_DEVICE_ID_QEMU_IPMI          0x1112
 
 /* VMWare (0x15ad) */
 #define PCI_VENDOR_ID_VMWARE             0x15ad
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 12/15] ipmi: Add an SMBus IPMI interface
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (10 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 11/15] ipmi: Add PCI IPMI interfaces minyard
@ 2019-09-19 21:39 ` minyard
  2022-06-28 16:21   ` Peter Maydell
  2019-09-19 21:39 ` [PATCH 13/15] acpi: Add i2c serial bus CRS handling minyard
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 default-configs/i386-softmmu.mak |   1 +
 hw/i386/Kconfig                  |   1 +
 hw/ipmi/Kconfig                  |   5 +
 hw/ipmi/Makefile.objs            |   1 +
 hw/ipmi/smbus_ipmi.c             | 384 +++++++++++++++++++++++++++++++
 5 files changed, 392 insertions(+)
 create mode 100644 hw/ipmi/smbus_ipmi.c

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 2294c0be5a..4229900f57 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -12,6 +12,7 @@
 #CONFIG_ISA_IPMI_KCS=n
 #CONFIG_PCI_IPMI_KCS=n
 #CONFIG_PCI_IPMI_BT=n
+#CONFIG_IPMI_SSIF=n
 #CONFIG_PCI_DEVICES=n
 #CONFIG_PVPANIC=n
 #CONFIG_QXL=n
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index d10f4e3e8b..c5c9d4900e 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -10,6 +10,7 @@ config PC
     imply ISA_IPMI_BT
     imply PCI_IPMI_KCS
     imply PCI_IPMI_BT
+    imply IPMI_SSIF
     imply ISA_DEBUG
     imply PARALLEL
     imply PCI_DEVICES
diff --git a/hw/ipmi/Kconfig b/hw/ipmi/Kconfig
index 12db4e81ad..9befd4f422 100644
--- a/hw/ipmi/Kconfig
+++ b/hw/ipmi/Kconfig
@@ -30,3 +30,8 @@ config PCI_IPMI_BT
     bool
     depends on PCI
     select IPMI
+
+config IPMI_SSIF
+    bool
+    depends on I2C
+    select IPMI
diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
index 2d7f080a86..3cca10bc50 100644
--- a/hw/ipmi/Makefile.objs
+++ b/hw/ipmi/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-$(CONFIG_ISA_IPMI_KCS) += isa_ipmi_kcs.o
 common-obj-$(CONFIG_PCI_IPMI_KCS) += pci_ipmi_kcs.o
 common-obj-$(CONFIG_ISA_IPMI_BT) += isa_ipmi_bt.o
 common-obj-$(CONFIG_PCI_IPMI_BT) += pci_ipmi_bt.o
+common-obj-$(CONFIG_IPMI_SSIF) += smbus_ipmi.o
diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
new file mode 100644
index 0000000000..2a9470d9df
--- /dev/null
+++ b/hw/ipmi/smbus_ipmi.c
@@ -0,0 +1,384 @@
+/*
+ * QEMU IPMI SMBus (SSIF) emulation
+ *
+ * Copyright (c) 2015,2016 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 "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "hw/i2c/smbus_slave.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "hw/ipmi/ipmi.h"
+
+#define TYPE_SMBUS_IPMI "smbus-ipmi"
+#define SMBUS_IPMI(obj) OBJECT_CHECK(SMBusIPMIDevice, (obj), TYPE_SMBUS_IPMI)
+
+#define SSIF_IPMI_REQUEST                       2
+#define SSIF_IPMI_MULTI_PART_REQUEST_START      6
+#define SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE     7
+#define SSIF_IPMI_MULTI_PART_REQUEST_END        8
+#define SSIF_IPMI_RESPONSE                      3
+#define SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE    9
+#define SSIF_IPMI_MULTI_PART_RETRY              0xa
+
+#define MAX_SSIF_IPMI_MSG_SIZE 255
+#define MAX_SSIF_IPMI_MSG_CHUNK 32
+
+#define IPMI_GET_SYS_INTF_CAP_CMD 0x57
+
+typedef struct SMBusIPMIDevice {
+    SMBusDevice parent;
+
+    IPMIBmc *bmc;
+
+    uint8_t outmsg[MAX_SSIF_IPMI_MSG_SIZE];
+    uint32_t outlen;
+    uint32_t currblk;
+
+    /* Holds the SMBUS message currently being sent to the host. */
+    uint8_t outbuf[MAX_SSIF_IPMI_MSG_CHUNK + 1]; /* len + message. */
+    uint32_t outpos;
+
+    uint8_t inmsg[MAX_SSIF_IPMI_MSG_SIZE];
+    uint32_t inlen;
+
+    /*
+     * This is a response number that we send with the command to make
+     * sure that the response matches the command.
+     */
+    uint8_t waiting_rsp;
+
+    uint32_t uuid;
+} SMBusIPMIDevice;
+
+static void smbus_ipmi_handle_event(IPMIInterface *ii)
+{
+    /* No interrupts, so nothing to do here. */
+}
+
+static void smbus_ipmi_handle_rsp(IPMIInterface *ii, uint8_t msg_id,
+                                  unsigned char *rsp, unsigned int rsp_len)
+{
+    SMBusIPMIDevice *sid = SMBUS_IPMI(ii);
+
+    if (sid->waiting_rsp == msg_id) {
+        sid->waiting_rsp++;
+
+        if (rsp_len > MAX_SSIF_IPMI_MSG_SIZE) {
+            rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+            rsp_len = MAX_SSIF_IPMI_MSG_SIZE;
+        }
+        memcpy(sid->outmsg, rsp, rsp_len);
+        sid->outlen = rsp_len;
+        sid->outpos = 0;
+        sid->currblk = 0;
+    }
+}
+
+static void smbus_ipmi_set_atn(IPMIInterface *ii, int val, int irq)
+{
+    /* This is where PEC would go. */
+}
+
+static void smbus_ipmi_set_irq_enable(IPMIInterface *ii, int val)
+{
+}
+
+static void smbus_ipmi_send_msg(SMBusIPMIDevice *sid)
+{
+    uint8_t *msg = sid->inmsg;
+    uint32_t len = sid->inlen;
+    IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(sid->bmc);
+
+    sid->outlen = 0;
+    sid->outpos = 0;
+    sid->currblk = 0;
+
+    if (msg[0] == (IPMI_NETFN_APP << 2) && msg[1] == IPMI_GET_SYS_INTF_CAP_CMD)
+    {
+        /* We handle this ourself. */
+        sid->outmsg[0] = (IPMI_NETFN_APP + 1) << 2;
+        sid->outmsg[1] = msg[1];
+        if (len < 3) {
+            sid->outmsg[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
+            sid->outlen = 3;
+        } else if ((msg[2] & 0x0f) != 0) {
+            sid->outmsg[2] = IPMI_CC_INVALID_DATA_FIELD;
+            sid->outlen = 3;
+        } else {
+            sid->outmsg[2] = 0;
+            sid->outmsg[3] = 0;
+            sid->outmsg[4] = (2 << 6); /* Multi-part supported. */
+            sid->outmsg[5] = MAX_SSIF_IPMI_MSG_SIZE;
+            sid->outmsg[6] = MAX_SSIF_IPMI_MSG_SIZE;
+            sid->outlen = 7;
+        }
+        return;
+    }
+
+    bk->handle_command(sid->bmc, sid->inmsg, sid->inlen, sizeof(sid->inmsg),
+                       sid->waiting_rsp);
+}
+
+static uint8_t ipmi_receive_byte(SMBusDevice *dev)
+{
+    SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
+
+    if (sid->outpos >= sizeof(sid->outbuf)) {
+        return 0xff;
+    }
+
+    return sid->outbuf[sid->outpos++];
+}
+
+static int ipmi_load_readbuf(SMBusIPMIDevice *sid)
+{
+    unsigned int block = sid->currblk, pos, len;
+
+    if (sid->outlen == 0) {
+        return -1;
+    }
+
+    if (sid->outlen <= 32) {
+        if (block != 0) {
+            return -1;
+        }
+        sid->outbuf[0] = sid->outlen;
+        memcpy(sid->outbuf + 1, sid->outmsg, sid->outlen);
+        sid->outpos = 0;
+        return 0;
+    }
+
+    if (block == 0) {
+        sid->outbuf[0] = 32;
+        sid->outbuf[1] = 0;
+        sid->outbuf[2] = 1;
+        memcpy(sid->outbuf + 3, sid->outmsg, 30);
+        sid->outpos = 0;
+        return 0;
+    }
+
+    /*
+     * Calculate the position in outmsg.  30 for the first block, 31
+     * for the rest of the blocks.
+     */
+    pos = 30 + (block - 1) * 31;
+
+    if (pos >= sid->outlen) {
+        return -1;
+    }
+
+    len = sid->outlen - pos;
+    if (len > 31) {
+        /* More chunks after this. */
+        len = 31;
+        /* Blocks start at 0 for the first middle transaction. */
+        sid->outbuf[1] = block - 1;
+    } else {
+        sid->outbuf[1] = 0xff; /* End of message marker. */
+    }
+
+    sid->outbuf[0] = len + 1;
+    memcpy(sid->outbuf + 2, sid->outmsg + pos, len);
+    sid->outpos = 0;
+    return 0;
+}
+
+static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
+{
+    SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
+    bool send = false;
+    uint8_t cmd;
+    int ret = 0;
+
+    /* length is guaranteed to be >= 1. */
+    cmd = *buf++;
+    len--;
+
+    /* Handle read request, which don't have any data in the write part. */
+    switch (cmd) {
+    case SSIF_IPMI_RESPONSE:
+        sid->currblk = 0;
+        ret = ipmi_load_readbuf(sid);
+        break;
+
+    case SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE:
+        sid->currblk++;
+        ret = ipmi_load_readbuf(sid);
+        break;
+
+    case SSIF_IPMI_MULTI_PART_RETRY:
+        if (len >= 1) {
+            sid->currblk = buf[0];
+            ret = ipmi_load_readbuf(sid);
+        } else {
+            ret = -1;
+        }
+        break;
+
+    default:
+        break;
+    }
+
+    /* This should be a message write, make the length is there and correct. */
+    if (len >= 1) {
+        if (*buf != len - 1 || *buf > MAX_SSIF_IPMI_MSG_CHUNK) {
+            return -1; /* Bogus message */
+        }
+        buf++;
+        len--;
+    }
+
+    switch (cmd) {
+    case SSIF_IPMI_REQUEST:
+        send = true;
+        /* FALLTHRU */
+    case SSIF_IPMI_MULTI_PART_REQUEST_START:
+        if (len < 2) {
+            return -1; /* Bogus. */
+        }
+        memcpy(sid->inmsg, buf, len);
+        sid->inlen = len;
+        break;
+
+    case SSIF_IPMI_MULTI_PART_REQUEST_END:
+        send = true;
+        /* FALLTHRU */
+    case SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE:
+        if (!sid->inlen) {
+            return -1; /* Bogus. */
+        }
+        if (sid->inlen + len > MAX_SSIF_IPMI_MSG_SIZE) {
+            sid->inlen = 0; /* Discard the message. */
+            return -1; /* Bogus. */
+        }
+        if (len < 32) {
+            /*
+             * Special hack, a multi-part middle that is less than 32 bytes
+             * marks the end of a message.  The specification is fairly
+             * confusing, so some systems to this, even sending a zero
+             * length end message to mark the end.
+             */
+            send = true;
+        }
+        memcpy(sid->inmsg + sid->inlen, buf, len);
+        sid->inlen += len;
+        break;
+    }
+
+    if (send && sid->inlen) {
+        smbus_ipmi_send_msg(sid);
+    }
+
+    return ret;
+}
+
+static const VMStateDescription vmstate_smbus_ipmi = {
+    .name = TYPE_SMBUS_IPMI,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_SMBUS_DEVICE(parent, SMBusIPMIDevice),
+        VMSTATE_UINT8(waiting_rsp, SMBusIPMIDevice),
+        VMSTATE_UINT32(outlen, SMBusIPMIDevice),
+        VMSTATE_UINT32(currblk, SMBusIPMIDevice),
+        VMSTATE_UINT8_ARRAY(outmsg, SMBusIPMIDevice, MAX_SSIF_IPMI_MSG_SIZE),
+        VMSTATE_UINT32(outpos, SMBusIPMIDevice),
+        VMSTATE_UINT8_ARRAY(outbuf, SMBusIPMIDevice,
+                            MAX_SSIF_IPMI_MSG_CHUNK + 1),
+        VMSTATE_UINT32(inlen, SMBusIPMIDevice),
+        VMSTATE_UINT8_ARRAY(inmsg, SMBusIPMIDevice, MAX_SSIF_IPMI_MSG_SIZE),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void smbus_ipmi_realize(DeviceState *dev, Error **errp)
+{
+    SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
+    IPMIInterface *ii = IPMI_INTERFACE(dev);
+
+    if (!sid->bmc) {
+        error_setg(errp, "IPMI device requires a bmc attribute to be set");
+        return;
+    }
+
+    sid->uuid = ipmi_next_uuid();
+
+    sid->bmc->intf = ii;
+}
+
+static void smbus_ipmi_init(Object *obj)
+{
+    SMBusIPMIDevice *sid = SMBUS_IPMI(obj);
+
+    ipmi_bmc_find_and_link(OBJECT(obj), (Object **) &sid->bmc);
+}
+
+static void smbus_ipmi_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
+{
+    SMBusIPMIDevice *sid = SMBUS_IPMI(ii);
+
+    info->interface_name = "smbus";
+    info->interface_type = IPMI_SMBIOS_SSIF;
+    info->ipmi_spec_major_revision = 2;
+    info->ipmi_spec_minor_revision = 0;
+    info->i2c_slave_address = sid->bmc->slave_addr;
+    info->base_address = sid->parent.i2c.address;
+    info->memspace = IPMI_MEMSPACE_SMBUS;
+    info->register_spacing = 1;
+    info->uuid = sid->uuid;
+}
+
+static void smbus_ipmi_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_CLASS(oc);
+    SMBusDeviceClass *sc = SMBUS_DEVICE_CLASS(oc);
+
+    sc->receive_byte = ipmi_receive_byte;
+    sc->write_data = ipmi_write_data;
+    dc->vmsd = &vmstate_smbus_ipmi;
+    dc->realize = smbus_ipmi_realize;
+    iic->set_atn = smbus_ipmi_set_atn;
+    iic->handle_rsp = smbus_ipmi_handle_rsp;
+    iic->handle_if_event = smbus_ipmi_handle_event;
+    iic->set_irq_enable = smbus_ipmi_set_irq_enable;
+    iic->get_fwinfo = smbus_ipmi_get_fwinfo;
+}
+
+static const TypeInfo smbus_ipmi_info = {
+    .name          = TYPE_SMBUS_IPMI,
+    .parent        = TYPE_SMBUS_DEVICE,
+    .instance_size = sizeof(SMBusIPMIDevice),
+    .instance_init = smbus_ipmi_init,
+    .class_init    = smbus_ipmi_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_IPMI_INTERFACE },
+        { }
+    }
+};
+
+static void smbus_ipmi_register_types(void)
+{
+    type_register_static(&smbus_ipmi_info);
+}
+
+type_init(smbus_ipmi_register_types)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 13/15] acpi: Add i2c serial bus CRS handling
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (11 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 12/15] ipmi: Add an SMBus IPMI interface minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 14/15] ipmi: Fix SSIF ACPI handling to use the right CRS minyard
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

This will be required for getting IPMI SSIF (SMBus interface) into
the ACPI tables.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/acpi/aml-build.c         | 40 +++++++++++++++++++++++++++++++++++++
 include/hw/acpi/aml-build.h | 18 +++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 78aee1a2f9..2c3702b882 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1874,3 +1874,43 @@ build_hdr:
     build_header(linker, tbl, (void *)(tbl->data + fadt_start),
                  "FACP", tbl->len - fadt_start, f->rev, oem_id, oem_table_id);
 }
+
+/* ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors */
+static Aml *aml_serial_bus_device(uint8_t serial_bus_type, uint8_t flags,
+                                  uint16_t type_flags,
+                                  uint8_t revid, uint16_t data_length,
+                                  uint16_t resource_source_len)
+{
+    Aml *var = aml_alloc();
+    uint16_t length = data_length + resource_source_len + 9;
+
+    build_append_byte(var->buf, 0x8e); /* Serial Bus Connection Descriptor */
+    build_append_int_noprefix(var->buf, length, sizeof(length));
+    build_append_byte(var->buf, 1);    /* Revision ID */
+    build_append_byte(var->buf, 0);    /* Resource Source Index */
+    build_append_byte(var->buf, serial_bus_type); /* Serial Bus Type */
+    build_append_byte(var->buf, flags); /* General Flags */
+    build_append_int_noprefix(var->buf, type_flags, /* Type Specific Flags */
+                              sizeof(type_flags));
+    build_append_byte(var->buf, revid); /* Type Specification Revision ID */
+    build_append_int_noprefix(var->buf, data_length, sizeof(data_length));
+
+    return var;
+}
+
+/* ACPI 5.0: 6.4.3.8.2.1 I2C Serial Bus Connection Resource Descriptor */
+Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source)
+{
+    uint16_t resource_source_len = strlen(resource_source) + 1;
+    Aml *var = aml_serial_bus_device(AML_SERIAL_BUS_TYPE_I2C, 0, 0, 1,
+                                     6, resource_source_len);
+
+    /* Connection Speed.  Just set to 100K for now, it doesn't really matter. */
+    build_append_int_noprefix(var->buf, 100000, 4);
+    build_append_int_noprefix(var->buf, address, sizeof(address));
+
+    /* This is a string, not a name, so just copy it directly in. */
+    g_array_append_vals(var->buf, resource_source, resource_source_len);
+
+    return var;
+}
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 991cf05134..de4a406568 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -223,6 +223,23 @@ struct AcpiBuildTables {
     BIOSLinker *linker;
 } AcpiBuildTables;
 
+/*
+ * ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors
+ * Serial Bus Type
+ */
+#define AML_SERIAL_BUS_TYPE_I2C  1
+#define AML_SERIAL_BUS_TYPE_SPI  2
+#define AML_SERIAL_BUS_TYPE_UART 3
+
+/*
+ * ACPI 5.0: 6.4.3.8.2 Serial Bus Connection Descriptors
+ * General Flags
+ */
+/* Slave Mode */
+#define AML_SERIAL_BUS_FLAG_MASTER_DEVICE       (1 << 0)
+/* Consumer/Producer */
+#define AML_SERIAL_BUS_FLAG_CONSUME_ONLY        (1 << 1)
+
 /**
  * init_aml_allocator:
  *
@@ -347,6 +364,7 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed min_fixed,
 Aml *aml_dma(AmlDmaType typ, AmlDmaBusMaster bm, AmlTransferSize sz,
              uint8_t channel);
 Aml *aml_sleep(uint64_t msec);
+Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source);
 
 /* Block AML object primitives */
 Aml *aml_scope(const char *name_format, ...) GCC_FMT_ATTR(1, 2);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 14/15] ipmi: Fix SSIF ACPI handling to use the right CRS
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (12 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 13/15] acpi: Add i2c serial bus CRS handling minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-19 21:39 ` [PATCH 15/15] pc: Add an SMB0 ACPI device to q35 minyard
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

Pass in the CRS so that it can be set to the SMBus for IPMI later.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/acpi/ipmi-stub.c    |  2 +-
 hw/acpi/ipmi.c         | 13 +++++++------
 hw/i386/acpi-build.c   |  2 +-
 include/hw/acpi/ipmi.h |  2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/acpi/ipmi-stub.c b/hw/acpi/ipmi-stub.c
index f525f71c2d..8634fb325c 100644
--- a/hw/acpi/ipmi-stub.c
+++ b/hw/acpi/ipmi-stub.c
@@ -10,6 +10,6 @@
 #include "qemu/osdep.h"
 #include "hw/acpi/ipmi.h"
 
-void build_acpi_ipmi_devices(Aml *table, BusState *bus)
+void build_acpi_ipmi_devices(Aml *table, BusState *bus, const char *resource)
 {
 }
diff --git a/hw/acpi/ipmi.c b/hw/acpi/ipmi.c
index 651e2e94ea..96e48eba15 100644
--- a/hw/acpi/ipmi.c
+++ b/hw/acpi/ipmi.c
@@ -13,7 +13,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ipmi.h"
 
-static Aml *aml_ipmi_crs(IPMIFwInfo *info)
+static Aml *aml_ipmi_crs(IPMIFwInfo *info, const char *resource)
 {
     Aml *crs = aml_resource_template();
 
@@ -48,7 +48,8 @@ static Aml *aml_ipmi_crs(IPMIFwInfo *info)
                             info->register_spacing, info->register_length));
         break;
     case IPMI_MEMSPACE_SMBUS:
-        aml_append(crs, aml_return(aml_int(info->base_address)));
+        aml_append(crs, aml_i2c_serial_bus_device(info->base_address,
+                                                  resource));
         break;
     default:
         abort();
@@ -61,7 +62,7 @@ static Aml *aml_ipmi_crs(IPMIFwInfo *info)
     return crs;
 }
 
-static Aml *aml_ipmi_device(IPMIFwInfo *info)
+static Aml *aml_ipmi_device(IPMIFwInfo *info, const char *resource)
 {
     Aml *dev;
     uint16_t version = ((info->ipmi_spec_major_revision << 8)
@@ -74,14 +75,14 @@ static Aml *aml_ipmi_device(IPMIFwInfo *info)
     aml_append(dev, aml_name_decl("_STR", aml_string("ipmi_%s",
                                                      info->interface_name)));
     aml_append(dev, aml_name_decl("_UID", aml_int(info->uuid)));
-    aml_append(dev, aml_name_decl("_CRS", aml_ipmi_crs(info)));
+    aml_append(dev, aml_name_decl("_CRS", aml_ipmi_crs(info, resource)));
     aml_append(dev, aml_name_decl("_IFT", aml_int(info->interface_type)));
     aml_append(dev, aml_name_decl("_SRV", aml_int(version)));
 
     return dev;
 }
 
-void build_acpi_ipmi_devices(Aml *scope, BusState *bus)
+void build_acpi_ipmi_devices(Aml *scope, BusState *bus, const char *resource)
 {
 
     BusChild *kid;
@@ -101,6 +102,6 @@ void build_acpi_ipmi_devices(Aml *scope, BusState *bus)
         iic = IPMI_INTERFACE_GET_CLASS(obj);
         memset(&info, 0, sizeof(info));
         iic->get_fwinfo(ii, &info);
-        aml_append(scope, aml_ipmi_device(&info));
+        aml_append(scope, aml_ipmi_device(&info, resource));
     }
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e54e571a75..8acf12df9a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1290,7 +1290,7 @@ static void build_isa_devices_aml(Aml *table)
     } else if (!obj) {
         error_report("No ISA bus, unable to define IPMI ACPI data");
     } else {
-        build_acpi_ipmi_devices(scope, BUS(obj));
+        build_acpi_ipmi_devices(scope, BUS(obj), "\\_SB.PCI0.ISA");
     }
 
     aml_append(table, scope);
diff --git a/include/hw/acpi/ipmi.h b/include/hw/acpi/ipmi.h
index c38483565c..c14ad682ac 100644
--- a/include/hw/acpi/ipmi.h
+++ b/include/hw/acpi/ipmi.h
@@ -16,6 +16,6 @@
  * bus matches the given bus.  The resource is the ACPI resource that
  * contains the IPMI device, this is required for the I2C CRS.
  */
-void build_acpi_ipmi_devices(Aml *table, BusState *bus);
+void build_acpi_ipmi_devices(Aml *table, BusState *bus, const char *resource);
 
 #endif /* HW_ACPI_IPMI_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 15/15] pc: Add an SMB0 ACPI device to q35
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (13 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 14/15] ipmi: Fix SSIF ACPI handling to use the right CRS minyard
@ 2019-09-19 21:39 ` minyard
  2019-09-20 11:43 ` [PATCH 00/15] ipmi: Bug fixes, add new interfaces Paolo Bonzini
  2019-09-20 12:57 ` Peter Maydell
  16 siblings, 0 replies; 30+ messages in thread
From: minyard @ 2019-09-19 21:39 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

From: Corey Minyard <cminyard@mvista.com>

This is so I2C devices can be found in the ACPI namespace.  Currently
that's only IPMI, but devices can be easily added now.

Adding the devices required some PCI information, and the bus itself
to be added to the PCMachineState structure.

Note that this only works on Q35, the ACPI for PIIX4 is not capable
of handling an SMBus device.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/i386/acpi-build.c             |  15 +++++++++++++++
 hw/i386/pc_piix.c                |  12 ++++++------
 hw/i386/pc_q35.c                 |   9 +++++----
 include/hw/i386/pc.h             |   2 ++
 tests/data/acpi/q35/DSDT         | Bin 7841 -> 7879 bytes
 tests/data/acpi/q35/DSDT.bridge  | Bin 7858 -> 7896 bytes
 tests/data/acpi/q35/DSDT.cphp    | Bin 8304 -> 8342 bytes
 tests/data/acpi/q35/DSDT.dimmpxm | Bin 9494 -> 9532 bytes
 tests/data/acpi/q35/DSDT.ipmibt  | Bin 7916 -> 7954 bytes
 tests/data/acpi/q35/DSDT.memhp   | Bin 9200 -> 9238 bytes
 tests/data/acpi/q35/DSDT.mmio64  | Bin 8971 -> 9009 bytes
 tests/data/acpi/q35/DSDT.numamem | Bin 7847 -> 7885 bytes
 12 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 8acf12df9a..4e0f9f425a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1809,6 +1809,18 @@ static Aml *build_q35_osc_method(void)
     return method;
 }
 
+static void build_smb0(Aml *table, I2CBus *smbus, int devnr, int func)
+{
+    Aml *scope = aml_scope("_SB.PCI0");
+    Aml *dev = aml_device("SMB0");
+
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("APP0005")));
+    aml_append(dev, aml_name_decl("_ADR", aml_int(devnr << 16 | func)));
+    build_acpi_ipmi_devices(dev, BUS(smbus), "\\_SB.PCI0.SMB0");
+    aml_append(scope, dev);
+    aml_append(table, scope);
+}
+
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker,
            AcpiPmInfo *pm, AcpiMiscInfo *misc,
@@ -1862,6 +1874,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         build_q35_isa_bridge(dsdt);
         build_isa_devices_aml(dsdt);
         build_q35_pci0_int(dsdt);
+        if (pcms->smbus && !pcmc->do_not_add_smb_acpi) {
+            build_smb0(dsdt, pcms->smbus, ICH9_SMB_DEV, ICH9_SMB_FUNC);
+        }
     }
 
     if (pcmc->legacy_cpu_hotplug) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2362675149..6824b72124 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -283,15 +283,14 @@ else {
 
     if (pcmc->pci_enabled && acpi_enabled) {
         DeviceState *piix4_pm;
-        I2CBus *smbus;
 
         smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
         /* TODO: Populate SPD eeprom data.  */
-        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
-                              pcms->gsi[9], smi_irq,
-                              pc_machine_is_smm_enabled(pcms),
-                              &piix4_pm);
-        smbus_eeprom_init(smbus, 8, NULL, 0);
+        pcms->smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
+                                    pcms->gsi[9], smi_irq,
+                                    pc_machine_is_smm_enabled(pcms),
+                                    &piix4_pm);
+        smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
 
         object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
                                  TYPE_HOTPLUG_HANDLER,
@@ -476,6 +475,7 @@ static void pc_i440fx_3_1_machine_options(MachineClass *m)
 
     pc_i440fx_4_0_machine_options(m);
     m->is_default = 0;
+    pcmc->do_not_add_smb_acpi = true;
     m->smbus_no_migration_support = true;
     m->alias = NULL;
     pcmc->pvh_enabled = false;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d4e8a1cb9f..8fad20f314 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -316,10 +316,10 @@ static void pc_q35_init(MachineState *machine)
 
     if (pcms->smbus_enabled) {
         /* TODO: Populate SPD eeprom data.  */
-        smbus_eeprom_init(ich9_smb_init(host_bus,
-                                        PCI_DEVFN(ICH9_SMB_DEV, ICH9_SMB_FUNC),
-                                        0xb100),
-                          8, NULL, 0);
+        pcms->smbus = ich9_smb_init(host_bus,
+                                    PCI_DEVFN(ICH9_SMB_DEV, ICH9_SMB_FUNC),
+                                    0xb100);
+        smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
     }
 
     pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
@@ -421,6 +421,7 @@ static void pc_q35_3_1_machine_options(MachineClass *m)
 
     pc_q35_4_0_machine_options(m);
     m->default_kernel_irqchip_split = false;
+    pcmc->do_not_add_smb_acpi = true;
     m->smbus_no_migration_support = true;
     m->alias = NULL;
     pcmc->pvh_enabled = false;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 062feeb69e..6df4f4b6fb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -38,6 +38,7 @@ struct PCMachineState {
     HotplugHandler *acpi_dev;
     ISADevice *rtc;
     PCIBus *bus;
+    I2CBus *smbus;
     FWCfgState *fw_cfg;
     qemu_irq *gsi;
     PFlashCFI01 *flash[2];
@@ -117,6 +118,7 @@ typedef struct PCMachineClass {
     bool rsdp_in_ram;
     int legacy_acpi_table_size;
     unsigned acpi_data_size;
+    bool do_not_add_smb_acpi;
 
     /* SMBIOS compat: */
     bool smbios_defaults;
diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index f9f36d1645c9b57aea38350d67dfaa143845697d..77ea60ffed421c566138fe6341421f579129a582 100644
GIT binary patch
delta 62
zcmZ2zd)$`GCD<k8xEuomWBo=hV@Xw2z4&0K_yA{5gXkv7U|%N#j(87G7aleN23C%E
RN0%TTW(IkN%{G#$tN;mh4yXVC

delta 24
fcmX?ZyU>=)CD<iop&SDPquoX>W690Qk}0eJT(JhZ

diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index 29176832ca9842c6654273ae1246321aa38b2821..fbc2d40000428b402586ea9302b5ccf36ef8de1e 100644
GIT binary patch
delta 62
zcmdmFd&8E?CD<k8h8zO}qx?oLV@Xw2z4&0K_yA{5gXkv7U|%N#j(87G7aleN23C%E
RN0%TTW(IkN%{G!{tN;Tx4vYW*

delta 24
fcmca%yUCWzCD<iolN<vB<Gqbs#*&+pB}-WWXCMci

diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index 19bdb5d21050f24aaacbafb1f84d6e1d541876c6..6a896cb2142feadbcabc6276b59c138a7e93f540 100644
GIT binary patch
delta 62
zcmez1FwK$6CD<iongRm@<ByG8#*(V4dhx+d@d3`B2GLED!M;ug9Pu8WE<9`k46GdS
RjxIqw%nb4jn{6ab*a06=4(I>?

delta 24
fcmbQ{_`!k8CD<jTK!JgQar;ItW690QlE&-+VHF1X

diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index 727fe489b4c8cdd39476ff61e7d7664c816f5291..23fdf5e60a5069f60d6c680ac9c68c4a8a81318e 100644
GIT binary patch
delta 62
zcmbQ{wa1IgCD<jzMwNkq@!&=-V@Xw2z4&0K_yA{5gXkv7U|%N#j(87G7aleN23C%E
RN0%TTW(IkN%{G#^xB>h%4&?v<

delta 24
fcmdnvHO-65CD<iIOqGFwv0)>ZvE=4t$(!5&ShNQA

diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index 9634930e6125de4375d87a56a353f636985599d4..c3fca0a71efa7b55c958a49f305389426fbe7922 100644
GIT binary patch
delta 62
zcmaE3JIRjACD<iINS=X#aq&j3I!RSkz4&0K_yA{5gXkv7U|%N#j(87G7aleN23C%E
RN0%TTW(IkN&Fzw@tN{Nt4#fZf

delta 24
fcmbPa_r{jXCD<k8jT{35WAa9>I?2uJBvV)cXHo~&

diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index dad5dc8db2f13bdb0de001da42c13b18286c3061..2abd0e36cd1344cbca3fa4ab59c5db2ea326d125 100644
GIT binary patch
delta 62
zcmez1KFx#6CD<iIOof4gv0x*Yv81Z1UVN}qe1Nm3L3ER3u&<K=N4$rp3lEzB11m?o
Rqe~DEGlM+CW*f;ZTmbN04s`$k

delta 24
fcmbQ{@xh(TCD<k8gE9jHqrpZlW690QlAE~zWv~Z^

diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 20f627ed08a0cae4e144f3e4dd7dd5f1d8d0318c..b32034a11c1f8a0a156df3765df44b14a88dbb4d 100644
GIT binary patch
delta 62
zcmeBn+vvvS66_LUsLa5?sI!sFSW;D0FFx2QKET=2Ai7C1*w@K`Bi_T)g@;Xmft4fP
R(Itq7nL(amvyJ3=P5{<m4j2Fc

delta 24
fcmdn!*6qgS66_Mft<1o{_<AFkvE=4t$#a|lSziY!

diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 7b96a972804e95e191d9d3bf9a965e90f6f7e555..d8b2b47f8b47067d375021a30086ca97d8aca43f 100644
GIT binary patch
delta 62
zcmZ2(d)AiACD<k8tQ-Raqvl2~V@Xw2z4&0K_yA{5gXkv7U|%N#j(87G7aleN23C%E
RN0%TTW(IkN%{G$RtN{IN4r%}Z

delta 24
fcmX?WyWEz`CD<ioxf}xn<BN@4#*&+pC9_xoWX}hC

-- 
2.17.1



^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/15] ipmi: Bug fixes, add new interfaces
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (14 preceding siblings ...)
  2019-09-19 21:39 ` [PATCH 15/15] pc: Add an SMB0 ACPI device to q35 minyard
@ 2019-09-20 11:43 ` Paolo Bonzini
  2019-09-20 12:19   ` Corey Minyard
  2019-09-20 12:57 ` Peter Maydell
  16 siblings, 1 reply; 30+ messages in thread
From: Paolo Bonzini @ 2019-09-20 11:43 UTC (permalink / raw)
  To: minyard, Peter Maydell
  Cc: Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Cédric Le Goater, Marc-André Lureau, Igor Mammedov,
	David Gibson

On 19/09/19 23:39, minyard@acm.org wrote:
> I haven't gotten a lot of commentary on this, but I assume that means
> that everything is ok.  It's been posted a few times and the last time
> I received no issues, just a couple of reviews.  I would like more
> review.  But I'm not quite sure what to do about that, I've been
> hanging on to these changes far too long.

It's just that not many people here are IPMI-savvy.  I took a quick look
at patches 5 and 15, and they look fine.

Paolo

> The following changes since commit a77d20bafcd4cb7684168a9b4c6dc2a321aaeb50:
> 
>   Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190919-pull-request' into staging (2019-09-19 17:16:07 +0100)
> 
> are available in the Git repository at:
> 
>   https://github.com/cminyard/qemu.git tags/ipmi-for-release-2019-09-19
> 
> for you to fetch changes up to d9b74295c6528fd68cebdea116b283e46543b2a2:
> 
>   pc: Add an SMB0 ACPI device to q35 (2019-09-19 14:41:58 -0500)
> 
> ----------------------------------------------------------------
> ipmi: Some bug fixes and new interfaces
> 
> Some bug fixes for the watchdog and hopeful the BT tests.
> 
> Change the IPMI UUID handling to give the user the ability to set it or
> not have it.
> 
> Add a PCI interface.
> 
> Add an SMBus interfaces.
> 
> -corey
> 
> ----------------------------------------------------------------
> Corey Minyard (15):
>       ipmi: Fix watchdog NMI handling
>       ipmi: Fix the get watchdog command
>       ipmi: Generate an interrupt on watchdog pretimeout expiry
>       tests:ipmi: Fix IPMI BT tests
>       qdev: Add a no default uuid property
>       ipmi: Add a UUID device property
>       ipmi: Split out KCS-specific code from ISA KCS code
>       ipmi: Split out BT-specific code from ISA BT code
>       ipmi: Allow a size value to be passed for I/O space
>       smbios:ipmi: Ignore IPMI devices with no fwinfo function
>       ipmi: Add PCI IPMI interfaces
>       ipmi: Add an SMBus IPMI interface
>       acpi: Add i2c serial bus CRS handling
>       ipmi: Fix SSIF ACPI handling to use the right CRS
>       pc: Add an SMB0 ACPI device to q35
> 
>  default-configs/i386-softmmu.mak |   3 +
>  hw/acpi/aml-build.c              |  40 ++++
>  hw/acpi/ipmi-stub.c              |   2 +-
>  hw/acpi/ipmi.c                   |  13 +-
>  hw/i386/Kconfig                  |   3 +
>  hw/i386/acpi-build.c             |  17 +-
>  hw/i386/pc_piix.c                |  12 +-
>  hw/i386/pc_q35.c                 |   9 +-
>  hw/ipmi/Kconfig                  |  15 ++
>  hw/ipmi/Makefile.objs            |   5 +-
>  hw/ipmi/ipmi.c                   |   6 +-
>  hw/ipmi/ipmi_bmc_sim.c           |  30 ++-
>  hw/ipmi/ipmi_bt.c                | 437 ++++++++++++++++++++++++++++++++++++++
>  hw/ipmi/ipmi_kcs.c               | 423 +++++++++++++++++++++++++++++++++++++
>  hw/ipmi/isa_ipmi_bt.c            | 443 ++-------------------------------------
>  hw/ipmi/isa_ipmi_kcs.c           | 419 ++----------------------------------
>  hw/ipmi/pci_ipmi_bt.c            | 146 +++++++++++++
>  hw/ipmi/pci_ipmi_kcs.c           | 146 +++++++++++++
>  hw/ipmi/smbus_ipmi.c             | 384 +++++++++++++++++++++++++++++++++
>  hw/smbios/smbios_type_38.c       |   3 +
>  include/hw/acpi/aml-build.h      |  18 ++
>  include/hw/acpi/ipmi.h           |   2 +-
>  include/hw/i386/pc.h             |   2 +
>  include/hw/ipmi/ipmi.h           |   7 +-
>  include/hw/ipmi/ipmi_bt.h        |  73 +++++++
>  include/hw/ipmi/ipmi_kcs.h       |  76 +++++++
>  include/hw/pci/pci.h             |   1 +
>  include/hw/qdev-properties.h     |   7 +
>  qemu-options.hx                  |  10 +-
>  tests/Makefile.include           |   3 +-
>  tests/data/acpi/q35/DSDT         | Bin 7841 -> 7879 bytes
>  tests/data/acpi/q35/DSDT.bridge  | Bin 7858 -> 7896 bytes
>  tests/data/acpi/q35/DSDT.cphp    | Bin 8304 -> 8342 bytes
>  tests/data/acpi/q35/DSDT.dimmpxm | Bin 9494 -> 9532 bytes
>  tests/data/acpi/q35/DSDT.ipmibt  | Bin 7916 -> 7954 bytes
>  tests/data/acpi/q35/DSDT.memhp   | Bin 9200 -> 9238 bytes
>  tests/data/acpi/q35/DSDT.mmio64  | Bin 8971 -> 9009 bytes
>  tests/data/acpi/q35/DSDT.numamem | Bin 7847 -> 7885 bytes
>  tests/ipmi-bt-test.c             |   6 +-
>  39 files changed, 1902 insertions(+), 859 deletions(-)
>  create mode 100644 hw/ipmi/ipmi_bt.c
>  create mode 100644 hw/ipmi/ipmi_kcs.c
>  create mode 100644 hw/ipmi/pci_ipmi_bt.c
>  create mode 100644 hw/ipmi/pci_ipmi_kcs.c
>  create mode 100644 hw/ipmi/smbus_ipmi.c
>  create mode 100644 include/hw/ipmi/ipmi_bt.h
>  create mode 100644 include/hw/ipmi/ipmi_kcs.h
> 
> 
> 



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/15] ipmi: Bug fixes, add new interfaces
  2019-09-20 11:43 ` [PATCH 00/15] ipmi: Bug fixes, add new interfaces Paolo Bonzini
@ 2019-09-20 12:19   ` Corey Minyard
  0 siblings, 0 replies; 30+ messages in thread
From: Corey Minyard @ 2019-09-20 12:19 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Peter Maydell, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Cédric Le Goater, Marc-André Lureau, Igor Mammedov,
	David Gibson

On Fri, Sep 20, 2019 at 01:43:32PM +0200, Paolo Bonzini wrote:
> On 19/09/19 23:39, minyard@acm.org wrote:
> > I haven't gotten a lot of commentary on this, but I assume that means
> > that everything is ok.  It's been posted a few times and the last time
> > I received no issues, just a couple of reviews.  I would like more
> > review.  But I'm not quite sure what to do about that, I've been
> > hanging on to these changes far too long.
> 
> It's just that not many people here are IPMI-savvy.  I took a quick look
> at patches 5 and 15, and they look fine.

Thanks a bunch for looking at those.  Patch 15 was my biggest worry.

-corey

> 
> Paolo
> 
> > The following changes since commit a77d20bafcd4cb7684168a9b4c6dc2a321aaeb50:
> > 
> >   Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190919-pull-request' into staging (2019-09-19 17:16:07 +0100)
> > 
> > are available in the Git repository at:
> > 
> >   https://github.com/cminyard/qemu.git tags/ipmi-for-release-2019-09-19
> > 
> > for you to fetch changes up to d9b74295c6528fd68cebdea116b283e46543b2a2:
> > 
> >   pc: Add an SMB0 ACPI device to q35 (2019-09-19 14:41:58 -0500)
> > 
> > ----------------------------------------------------------------
> > ipmi: Some bug fixes and new interfaces
> > 
> > Some bug fixes for the watchdog and hopeful the BT tests.
> > 
> > Change the IPMI UUID handling to give the user the ability to set it or
> > not have it.
> > 
> > Add a PCI interface.
> > 
> > Add an SMBus interfaces.
> > 
> > -corey
> > 
> > ----------------------------------------------------------------
> > Corey Minyard (15):
> >       ipmi: Fix watchdog NMI handling
> >       ipmi: Fix the get watchdog command
> >       ipmi: Generate an interrupt on watchdog pretimeout expiry
> >       tests:ipmi: Fix IPMI BT tests
> >       qdev: Add a no default uuid property
> >       ipmi: Add a UUID device property
> >       ipmi: Split out KCS-specific code from ISA KCS code
> >       ipmi: Split out BT-specific code from ISA BT code
> >       ipmi: Allow a size value to be passed for I/O space
> >       smbios:ipmi: Ignore IPMI devices with no fwinfo function
> >       ipmi: Add PCI IPMI interfaces
> >       ipmi: Add an SMBus IPMI interface
> >       acpi: Add i2c serial bus CRS handling
> >       ipmi: Fix SSIF ACPI handling to use the right CRS
> >       pc: Add an SMB0 ACPI device to q35
> > 
> >  default-configs/i386-softmmu.mak |   3 +
> >  hw/acpi/aml-build.c              |  40 ++++
> >  hw/acpi/ipmi-stub.c              |   2 +-
> >  hw/acpi/ipmi.c                   |  13 +-
> >  hw/i386/Kconfig                  |   3 +
> >  hw/i386/acpi-build.c             |  17 +-
> >  hw/i386/pc_piix.c                |  12 +-
> >  hw/i386/pc_q35.c                 |   9 +-
> >  hw/ipmi/Kconfig                  |  15 ++
> >  hw/ipmi/Makefile.objs            |   5 +-
> >  hw/ipmi/ipmi.c                   |   6 +-
> >  hw/ipmi/ipmi_bmc_sim.c           |  30 ++-
> >  hw/ipmi/ipmi_bt.c                | 437 ++++++++++++++++++++++++++++++++++++++
> >  hw/ipmi/ipmi_kcs.c               | 423 +++++++++++++++++++++++++++++++++++++
> >  hw/ipmi/isa_ipmi_bt.c            | 443 ++-------------------------------------
> >  hw/ipmi/isa_ipmi_kcs.c           | 419 ++----------------------------------
> >  hw/ipmi/pci_ipmi_bt.c            | 146 +++++++++++++
> >  hw/ipmi/pci_ipmi_kcs.c           | 146 +++++++++++++
> >  hw/ipmi/smbus_ipmi.c             | 384 +++++++++++++++++++++++++++++++++
> >  hw/smbios/smbios_type_38.c       |   3 +
> >  include/hw/acpi/aml-build.h      |  18 ++
> >  include/hw/acpi/ipmi.h           |   2 +-
> >  include/hw/i386/pc.h             |   2 +
> >  include/hw/ipmi/ipmi.h           |   7 +-
> >  include/hw/ipmi/ipmi_bt.h        |  73 +++++++
> >  include/hw/ipmi/ipmi_kcs.h       |  76 +++++++
> >  include/hw/pci/pci.h             |   1 +
> >  include/hw/qdev-properties.h     |   7 +
> >  qemu-options.hx                  |  10 +-
> >  tests/Makefile.include           |   3 +-
> >  tests/data/acpi/q35/DSDT         | Bin 7841 -> 7879 bytes
> >  tests/data/acpi/q35/DSDT.bridge  | Bin 7858 -> 7896 bytes
> >  tests/data/acpi/q35/DSDT.cphp    | Bin 8304 -> 8342 bytes
> >  tests/data/acpi/q35/DSDT.dimmpxm | Bin 9494 -> 9532 bytes
> >  tests/data/acpi/q35/DSDT.ipmibt  | Bin 7916 -> 7954 bytes
> >  tests/data/acpi/q35/DSDT.memhp   | Bin 9200 -> 9238 bytes
> >  tests/data/acpi/q35/DSDT.mmio64  | Bin 8971 -> 9009 bytes
> >  tests/data/acpi/q35/DSDT.numamem | Bin 7847 -> 7885 bytes
> >  tests/ipmi-bt-test.c             |   6 +-
> >  39 files changed, 1902 insertions(+), 859 deletions(-)
> >  create mode 100644 hw/ipmi/ipmi_bt.c
> >  create mode 100644 hw/ipmi/ipmi_kcs.c
> >  create mode 100644 hw/ipmi/pci_ipmi_bt.c
> >  create mode 100644 hw/ipmi/pci_ipmi_kcs.c
> >  create mode 100644 hw/ipmi/smbus_ipmi.c
> >  create mode 100644 include/hw/ipmi/ipmi_bt.h
> >  create mode 100644 include/hw/ipmi/ipmi_kcs.h
> > 
> > 
> > 
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/15] ipmi: Bug fixes, add new interfaces
  2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
                   ` (15 preceding siblings ...)
  2019-09-20 11:43 ` [PATCH 00/15] ipmi: Bug fixes, add new interfaces Paolo Bonzini
@ 2019-09-20 12:57 ` Peter Maydell
  2019-09-20 17:36   ` Corey Minyard
  16 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2019-09-20 12:57 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

On Thu, 19 Sep 2019 at 22:39, <minyard@acm.org> wrote:
>
> I haven't gotten a lot of commentary on this, but I assume that means
> that everything is ok.  It's been posted a few times and the last time
> I received no issues, just a couple of reviews.  I would like more
> review.  But I'm not quite sure what to do about that, I've been
> hanging on to these changes far too long.
>
> The following changes since commit a77d20bafcd4cb7684168a9b4c6dc2a321aaeb50:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190919-pull-request' into staging (2019-09-19 17:16:07 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cminyard/qemu.git tags/ipmi-for-release-2019-09-19
>
> for you to fetch changes up to d9b74295c6528fd68cebdea116b283e46543b2a2:
>
>   pc: Add an SMB0 ACPI device to q35 (2019-09-19 14:41:58 -0500)
>
> ----------------------------------------------------------------
> ipmi: Some bug fixes and new interfaces
>
> Some bug fixes for the watchdog and hopeful the BT tests.
>
> Change the IPMI UUID handling to give the user the ability to set it or
> not have it.
>
> Add a PCI interface.
>
> Add an SMBus interfaces.

Hi -- is this intended to be a pull request to be applied to
master? It's in the form of a pullreq but the subject header
says "PATCH" and you seem to be asking for more review, so I'm
not sure...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 01/15] ipmi: Fix watchdog NMI handling
  2019-09-19 21:39 ` [PATCH 01/15] ipmi: Fix watchdog NMI handling minyard
@ 2019-09-20 15:45   ` Cédric Le Goater
  0 siblings, 0 replies; 30+ messages in thread
From: Cédric Le Goater @ 2019-09-20 15:45 UTC (permalink / raw)
  To: minyard, Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
	David Gibson

On 19/09/2019 23:39, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> The wrong logic was used for detection (so it wouldn't work at all)
> and the wrong interface was used to inject the NMI if the detection
> logic was correct.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.

> ---
>  hw/ipmi/ipmi.c         | 6 +++---
>  hw/ipmi/ipmi_bmc_sim.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index 136c86b7a7..cbe158f815 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -28,9 +28,8 @@
>  #include "qom/object_interfaces.h"
>  #include "sysemu/runstate.h"
>  #include "qapi/error.h"
> -#include "qapi/qapi-commands-misc.h"
> -#include "qapi/visitor.h"
>  #include "qemu/module.h"
> +#include "hw/nmi.h"
>  
>  static uint32_t ipmi_current_uuid = 1;
>  
> @@ -60,7 +59,8 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>          if (checkonly) {
>              return 0;
>          }
> -        qmp_inject_nmi(NULL);
> +        /* We don't care what CPU we use. */
> +        nmi_monitor_handle(0, NULL);
>          return 0;
>  
>      case IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP:
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 246a6d390c..8f63bb7181 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -1194,7 +1194,7 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>          break;
>  
>      case IPMI_BMC_WATCHDOG_PRE_NMI:
> -        if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
> +        if (k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
>              /* NMI not supported. */
>              rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>              return;
> 



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 02/15] ipmi: Fix the get watchdog command
  2019-09-19 21:39 ` [PATCH 02/15] ipmi: Fix the get watchdog command minyard
@ 2019-09-20 15:50   ` Cédric Le Goater
  0 siblings, 0 replies; 30+ messages in thread
From: Cédric Le Goater @ 2019-09-20 15:50 UTC (permalink / raw)
  To: minyard, Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
	David Gibson

On 19/09/2019 23:39, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> It wasn't returning the set timeout like it should have been.

Looking at 

  27.7 Get Watchdog Timer Command

This looks correct.


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  hw/ipmi/ipmi_bmc_sim.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 8f63bb7181..afb99e33d7 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -1228,6 +1228,8 @@ static void get_watchdog_timer(IPMIBmcSim *ibs,
>      rsp_buffer_push(rsp, ibs->watchdog_action);
>      rsp_buffer_push(rsp, ibs->watchdog_pretimeout);
>      rsp_buffer_push(rsp, ibs->watchdog_expired);
> +    rsp_buffer_push(rsp, ibs->watchdog_timeout & 0xff);
> +    rsp_buffer_push(rsp, (ibs->watchdog_timeout >> 8) & 0xff);
>      if (ibs->watchdog_running) {
>          long timeout;
>          timeout = ((ibs->watchdog_expiry - ipmi_getmonotime() + 50000000)
> 



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/15] ipmi: Bug fixes, add new interfaces
  2019-09-20 12:57 ` Peter Maydell
@ 2019-09-20 17:36   ` Corey Minyard
  2019-09-20 17:51     ` Peter Maydell
  0 siblings, 1 reply; 30+ messages in thread
From: Corey Minyard @ 2019-09-20 17:36 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

On Fri, Sep 20, 2019 at 01:57:48PM +0100, Peter Maydell wrote:
> On Thu, 19 Sep 2019 at 22:39, <minyard@acm.org> wrote:
> >
> > I haven't gotten a lot of commentary on this, but I assume that means
> > that everything is ok.  It's been posted a few times and the last time
> > I received no issues, just a couple of reviews.  I would like more
> > review.  But I'm not quite sure what to do about that, I've been
> > hanging on to these changes far too long.
> >
> > The following changes since commit a77d20bafcd4cb7684168a9b4c6dc2a321aaeb50:
> >
> >   Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190919-pull-request' into staging (2019-09-19 17:16:07 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/cminyard/qemu.git tags/ipmi-for-release-2019-09-19
> >
> > for you to fetch changes up to d9b74295c6528fd68cebdea116b283e46543b2a2:
> >
> >   pc: Add an SMB0 ACPI device to q35 (2019-09-19 14:41:58 -0500)
> >
> > ----------------------------------------------------------------
> > ipmi: Some bug fixes and new interfaces
> >
> > Some bug fixes for the watchdog and hopeful the BT tests.
> >
> > Change the IPMI UUID handling to give the user the ability to set it or
> > not have it.
> >
> > Add a PCI interface.
> >
> > Add an SMBus interfaces.
> 
> Hi -- is this intended to be a pull request to be applied to
> master? It's in the form of a pullreq but the subject header
> says "PATCH" and you seem to be asking for more review, so I'm
> not sure...

Dang, it was meant to be a pull request for master.  I would like
more review, and I didn't think I would get any, but it turns out
I have gotten it for the most important parts, so pulling it
should be good, I think.

I can add the reviews and request a pull properly, if you like.

Thanks,

-corey

> 
> thanks
> -- PMM


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/15] ipmi: Bug fixes, add new interfaces
  2019-09-20 17:36   ` Corey Minyard
@ 2019-09-20 17:51     ` Peter Maydell
  0 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2019-09-20 17:51 UTC (permalink / raw)
  To: Corey Minyard
  Cc: Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Cédric Le Goater, Paolo Bonzini,
	Igor Mammedov, David Gibson

On Fri, 20 Sep 2019 at 18:36, Corey Minyard <minyard@acm.org> wrote:
>
> On Fri, Sep 20, 2019 at 01:57:48PM +0100, Peter Maydell wrote:
> > Hi -- is this intended to be a pull request to be applied to
> > master? It's in the form of a pullreq but the subject header
> > says "PATCH" and you seem to be asking for more review, so I'm
> > not sure...
>
> Dang, it was meant to be a pull request for master.  I would like
> more review, and I didn't think I would get any, but it turns out
> I have gotten it for the most important parts, so pulling it
> should be good, I think.
>
> I can add the reviews and request a pull properly, if you like.

If you got some extra reviewed-by tags it would be nice to respin
with them added, I guess. You can just resend the cover letter,
no need to send all the patches again.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 03/15] ipmi: Generate an interrupt on watchdog pretimeout expiry
  2019-09-19 21:39 ` [PATCH 03/15] ipmi: Generate an interrupt on watchdog pretimeout expiry minyard
@ 2019-09-23  5:41   ` Cédric Le Goater
  0 siblings, 0 replies; 30+ messages in thread
From: Cédric Le Goater @ 2019-09-23  5:41 UTC (permalink / raw)
  To: minyard, Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
	David Gibson

On 19/09/2019 23:39, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Add the watchdog pretimeout to the bits that cause an interrupt on attn.
> Otherwise the user won't know.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.

> ---
>  hw/ipmi/ipmi_bmc_sim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index afb99e33d7..6e6cd1b47d 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -477,7 +477,9 @@ static int attn_set(IPMIBmcSim *ibs)
>  
>  static int attn_irq_enabled(IPMIBmcSim *ibs)
>  {
> -    return (IPMI_BMC_MSG_INTS_ON(ibs) && IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE_SET(ibs))
> +    return (IPMI_BMC_MSG_INTS_ON(ibs) &&
> +            (IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE_SET(ibs) ||
> +             IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK_SET(ibs)))
>          || (IPMI_BMC_EVBUF_FULL_INT_ENABLED(ibs) &&
>              IPMI_BMC_MSG_FLAG_EVT_BUF_FULL_SET(ibs));
>  }
> 



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 05/15] qdev: Add a no default uuid property
  2019-09-19 21:39 ` [PATCH 05/15] qdev: Add a no default uuid property minyard
@ 2019-09-23  5:47   ` Cédric Le Goater
  0 siblings, 0 replies; 30+ messages in thread
From: Cédric Le Goater @ 2019-09-23  5:47 UTC (permalink / raw)
  To: minyard, Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
	David Gibson

On 19/09/2019 23:39, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> This is for IPMI, which will behave differently if the UUID is
> not set.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> Cc: Fam Zheng <famz@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.


> ---
>  include/hw/qdev-properties.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 2e98dd60db..c6a8cb5516 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -238,6 +238,13 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
>  #define DEFINE_PROP_AUDIODEV(_n, _s, _f) \
>      DEFINE_PROP(_n, _s, _f, qdev_prop_audiodev, QEMUSoundCard)
>  
> +#define DEFINE_PROP_UUID_NODEFAULT(_name, _state, _field) {        \
> +        .name      = (_name),                                      \
> +        .info      = &qdev_prop_uuid,                              \
> +        .offset    = offsetof(_state, _field)                      \
> +            + type_check(QemuUUID, typeof_field(_state, _field)),  \
> +        }
> +
>  #define DEFINE_PROP_END_OF_LIST()               \
>      {}
>  
> 



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 06/15] ipmi: Add a UUID device property
  2019-09-19 21:39 ` [PATCH 06/15] ipmi: Add a UUID device property minyard
@ 2019-09-23  5:51   ` Cédric Le Goater
  0 siblings, 0 replies; 30+ messages in thread
From: Cédric Le Goater @ 2019-09-23  5:51 UTC (permalink / raw)
  To: minyard, Peter Maydell
  Cc: Corey Minyard, Fam Zheng, Michael S . Tsirkin, QEMU Developers,
	Marc-André Lureau, Paolo Bonzini, Igor Mammedov,
	David Gibson

On 19/09/2019 23:39, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Using the UUID that qemu generates probably isn't the best thing
> to do, allow it to be passed in via properties, and use QemuUUID
> for the type.
> 
> If the UUID is not set, return an unsupported command error.  This
> way we are not providing an all-zero (or randomly generated) GUID
> to the IPMI user.  This lets the host fall back to the other
> method of using the get device id command to determind the BMC
> being accessed.


Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.


> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> Cc: Cédric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/ipmi/ipmi_bmc_sim.c | 22 ++++++++++++++--------
>  qemu-options.hx        | 10 +++++++---
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 6e6cd1b47d..71e56f3b13 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -223,7 +223,7 @@ struct IPMIBmcSim {
>      uint8_t restart_cause;
>  
>      uint8_t acpi_power_state[2];
> -    uint8_t uuid[16];
> +    QemuUUID uuid;
>  
>      IPMISel sel;
>      IPMISdr sdr;
> @@ -941,8 +941,19 @@ static void get_device_guid(IPMIBmcSim *ibs,
>  {
>      unsigned int i;
>  
> +    /* An uninitialized uuid is all zeros, use that to know if it is set. */
>      for (i = 0; i < 16; i++) {
> -        rsp_buffer_push(rsp, ibs->uuid[i]);
> +        if (ibs->uuid.data[i]) {
> +            goto uuid_set;
> +        }
> +    }
> +    /* No uuid is set, return an error. */
> +    rsp_buffer_set_error(rsp, IPMI_CC_INVALID_CMD);
> +    return;
> +
> + uuid_set:
> +    for (i = 0; i < 16; i++) {
> +        rsp_buffer_push(rsp, ibs->uuid.data[i]);
>      }
>  }
>  
> @@ -1986,12 +1997,6 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp)
>      ibs->acpi_power_state[0] = 0;
>      ibs->acpi_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);
>  
> @@ -2011,6 +2016,7 @@ static Property ipmi_sim_properties[] = {
>      DEFINE_PROP_UINT8("fwrev2", IPMIBmcSim, fwrev2, 0),
>      DEFINE_PROP_UINT32("mfg_id", IPMIBmcSim, mfg_id, 0),
>      DEFINE_PROP_UINT16("product_id", IPMIBmcSim, product_id, 0),
> +    DEFINE_PROP_UUID_NODEFAULT("guid", IPMIBmcSim, uuid),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index bbfd936d29..ed9292f65e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -701,7 +701,7 @@ possible drivers and properties, use @code{-device help} and
>  @code{-device @var{driver},help}.
>  
>  Some drivers are:
> -@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}]
> +@item -device ipmi-bmc-sim,id=@var{id}[,slave_addr=@var{val}][,sdrfile=@var{file}][,furareasize=@var{val}][,furdatafile=@var{file}][,guid=@var{uuid}]
>  
>  Add an IPMI BMC.  This is a simulation of a hardware management
>  interface processor that normally sits on a system.  It provides
> @@ -714,8 +714,8 @@ controllers.  If you don't know what this means, it is safe to ignore
>  it.
>  
>  @table @option
> -@item bmc=@var{id}
> -The BMC to connect to, one of ipmi-bmc-sim or ipmi-bmc-extern above.
> +@item id=@var{id}
> +The BMC id for interfaces to use this device.
>  @item slave_addr=@var{val}
>  Define slave address to use for the BMC.  The default is 0x20.
>  @item sdrfile=@var{file}
> @@ -724,6 +724,10 @@ file containing raw Sensor Data Records (SDR) data. The default is none.
>  size of a Field Replaceable Unit (FRU) area.  The default is 1024.
>  @item frudatafile=@var{file}
>  file containing raw Field Replaceable Unit (FRU) inventory data. The default is none.
> +@item guid=@var{uuid}
> +value for the GUID for the BMC, in standard UUID format.  If this is set,
> +get "Get GUID" command to the BMC will return it.  Otherwise "Get GUID"
> +will return an error.
>  @end table
>  
>  @item -device ipmi-bmc-extern,id=@var{id},chardev=@var{id}[,slave_addr=@var{val}]
> 



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 12/15] ipmi: Add an SMBus IPMI interface
  2019-09-19 21:39 ` [PATCH 12/15] ipmi: Add an SMBus IPMI interface minyard
@ 2022-06-28 16:21   ` Peter Maydell
  2022-07-29 15:34     ` Peter Maydell
  2022-07-29 15:56     ` Corey Minyard
  0 siblings, 2 replies; 30+ messages in thread
From: Peter Maydell @ 2022-06-28 16:21 UTC (permalink / raw)
  To: minyard
  Cc: QEMU Developers, Michael S . Tsirkin, Igor Mammedov,
	M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
	Marc-André Lureau

On Thu, 19 Sept 2019 at 22:39, <minyard@acm.org> wrote:
>
> From: Corey Minyard <cminyard@mvista.com>
>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---

Very old patch, but Coverity has decided it doesn't like something
in this function that's still basically the same in the current codebase
(CID 1487146):

> +static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
> +{
> +    SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
> +    bool send = false;
> +    uint8_t cmd;
> +    int ret = 0;
> +
> +    /* length is guaranteed to be >= 1. */
> +    cmd = *buf++;
> +    len--;
> +
> +    /* Handle read request, which don't have any data in the write part. */
> +    switch (cmd) {
> +    case SSIF_IPMI_RESPONSE:
> +        sid->currblk = 0;
> +        ret = ipmi_load_readbuf(sid);
> +        break;
> +
> +    case SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE:
> +        sid->currblk++;
> +        ret = ipmi_load_readbuf(sid);
> +        break;
> +
> +    case SSIF_IPMI_MULTI_PART_RETRY:
> +        if (len >= 1) {
> +            sid->currblk = buf[0];
> +            ret = ipmi_load_readbuf(sid);
> +        } else {
> +            ret = -1;
> +        }
> +        break;
> +
> +    default:
> +        break;
> +    }
> +
> +    /* This should be a message write, make the length is there and correct. */
> +    if (len >= 1) {
> +        if (*buf != len - 1 || *buf > MAX_SSIF_IPMI_MSG_CHUNK) {
> +            return -1; /* Bogus message */
> +        }
> +        buf++;
> +        len--;
> +    }

After all of this preamble, len can be zero...

> +
> +    switch (cmd) {
> +    case SSIF_IPMI_REQUEST:
> +        send = true;
> +        /* FALLTHRU */
> +    case SSIF_IPMI_MULTI_PART_REQUEST_START:
> +        if (len < 2) {
> +            return -1; /* Bogus. */
> +        }
> +        memcpy(sid->inmsg, buf, len);
> +        sid->inlen = len;
> +        break;
> +
> +    case SSIF_IPMI_MULTI_PART_REQUEST_END:
> +        send = true;
> +        /* FALLTHRU */
> +    case SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE:
> +        if (!sid->inlen) {
> +            return -1; /* Bogus. */
> +        }
> +        if (sid->inlen + len > MAX_SSIF_IPMI_MSG_SIZE) {
> +            sid->inlen = 0; /* Discard the message. */
> +            return -1; /* Bogus. */
> +        }

...this error checking on the values of the 'middle' request
means that after one 'middle' request we can end up with
sid->inlen == MAX_SSIF_IPMI_MSG_SIZE (ie we filled the
entire sid->inmsg[] array).

But then if we get another 'middle' request with len == 0,
that will pass this error checking because (sid->inlen + len == MSG_SIZE)
and fall through into...

> +        if (len < 32) {
> +            /*
> +             * Special hack, a multi-part middle that is less than 32 bytes
> +             * marks the end of a message.  The specification is fairly
> +             * confusing, so some systems to this, even sending a zero
> +             * length end message to mark the end.
> +             */
> +            send = true;
> +        }
> +        memcpy(sid->inmsg + sid->inlen, buf, len);

...calling memcpy() with argument 1 being a pointer that points
one past the end of the array. Even though len will be 0 and
we won't memcpy() anything, this is (depending on how you choose
to intepret things the C standard doesn't come right out and state
explicitly) undefined behaviour, because memcpy() wants to be passed
valid pointers, even if you ask it to do no work with a zero len.

This isn't going to be a visible bug in practical terms, but it would
make Coverity happy if we either (a) rejected a request with an empty
length or else (b) skipped the memcpy(). I don't know enough about
IPMI to know which is better.

> +        sid->inlen += len;
> +        break;
> +    }
> +
> +    if (send && sid->inlen) {
> +        smbus_ipmi_send_msg(sid);
> +    }
> +
> +    return ret;
> +}

thanks
-- PMM


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 12/15] ipmi: Add an SMBus IPMI interface
  2022-06-28 16:21   ` Peter Maydell
@ 2022-07-29 15:34     ` Peter Maydell
  2022-07-29 15:56     ` Corey Minyard
  1 sibling, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2022-07-29 15:34 UTC (permalink / raw)
  To: minyard
  Cc: QEMU Developers, Michael S . Tsirkin, Igor Mammedov,
	M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
	Marc-André Lureau

On Tue, 28 Jun 2022 at 17:21, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 19 Sept 2019 at 22:39, <minyard@acm.org> wrote:
> >
> > From: Corey Minyard <cminyard@mvista.com>
> >
> > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> > ---
>
> Very old patch, but Coverity has decided it doesn't like something
> in this function that's still basically the same in the current codebase
> (CID 1487146):

Ping ?

> > +static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
> > +{
> > +    SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
> > +    bool send = false;
> > +    uint8_t cmd;
> > +    int ret = 0;
> > +
> > +    /* length is guaranteed to be >= 1. */
> > +    cmd = *buf++;
> > +    len--;
> > +
> > +    /* Handle read request, which don't have any data in the write part. */
> > +    switch (cmd) {
> > +    case SSIF_IPMI_RESPONSE:
> > +        sid->currblk = 0;
> > +        ret = ipmi_load_readbuf(sid);
> > +        break;
> > +
> > +    case SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE:
> > +        sid->currblk++;
> > +        ret = ipmi_load_readbuf(sid);
> > +        break;
> > +
> > +    case SSIF_IPMI_MULTI_PART_RETRY:
> > +        if (len >= 1) {
> > +            sid->currblk = buf[0];
> > +            ret = ipmi_load_readbuf(sid);
> > +        } else {
> > +            ret = -1;
> > +        }
> > +        break;
> > +
> > +    default:
> > +        break;
> > +    }
> > +
> > +    /* This should be a message write, make the length is there and correct. */
> > +    if (len >= 1) {
> > +        if (*buf != len - 1 || *buf > MAX_SSIF_IPMI_MSG_CHUNK) {
> > +            return -1; /* Bogus message */
> > +        }
> > +        buf++;
> > +        len--;
> > +    }
>
> After all of this preamble, len can be zero...
>
> > +
> > +    switch (cmd) {
> > +    case SSIF_IPMI_REQUEST:
> > +        send = true;
> > +        /* FALLTHRU */
> > +    case SSIF_IPMI_MULTI_PART_REQUEST_START:
> > +        if (len < 2) {
> > +            return -1; /* Bogus. */
> > +        }
> > +        memcpy(sid->inmsg, buf, len);
> > +        sid->inlen = len;
> > +        break;
> > +
> > +    case SSIF_IPMI_MULTI_PART_REQUEST_END:
> > +        send = true;
> > +        /* FALLTHRU */
> > +    case SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE:
> > +        if (!sid->inlen) {
> > +            return -1; /* Bogus. */
> > +        }
> > +        if (sid->inlen + len > MAX_SSIF_IPMI_MSG_SIZE) {
> > +            sid->inlen = 0; /* Discard the message. */
> > +            return -1; /* Bogus. */
> > +        }
>
> ...this error checking on the values of the 'middle' request
> means that after one 'middle' request we can end up with
> sid->inlen == MAX_SSIF_IPMI_MSG_SIZE (ie we filled the
> entire sid->inmsg[] array).
>
> But then if we get another 'middle' request with len == 0,
> that will pass this error checking because (sid->inlen + len == MSG_SIZE)
> and fall through into...
>
> > +        if (len < 32) {
> > +            /*
> > +             * Special hack, a multi-part middle that is less than 32 bytes
> > +             * marks the end of a message.  The specification is fairly
> > +             * confusing, so some systems to this, even sending a zero
> > +             * length end message to mark the end.
> > +             */
> > +            send = true;
> > +        }
> > +        memcpy(sid->inmsg + sid->inlen, buf, len);
>
> ...calling memcpy() with argument 1 being a pointer that points
> one past the end of the array. Even though len will be 0 and
> we won't memcpy() anything, this is (depending on how you choose
> to intepret things the C standard doesn't come right out and state
> explicitly) undefined behaviour, because memcpy() wants to be passed
> valid pointers, even if you ask it to do no work with a zero len.
>
> This isn't going to be a visible bug in practical terms, but it would
> make Coverity happy if we either (a) rejected a request with an empty
> length or else (b) skipped the memcpy(). I don't know enough about
> IPMI to know which is better.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 12/15] ipmi: Add an SMBus IPMI interface
  2022-06-28 16:21   ` Peter Maydell
  2022-07-29 15:34     ` Peter Maydell
@ 2022-07-29 15:56     ` Corey Minyard
  2022-07-29 16:01       ` Peter Maydell
  1 sibling, 1 reply; 30+ messages in thread
From: Corey Minyard @ 2022-07-29 15:56 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Michael S . Tsirkin, Igor Mammedov,
	M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
	Marc-André Lureau

On Tue, Jun 28, 2022 at 05:21:44PM +0100, Peter Maydell wrote:
> On Thu, 19 Sept 2019 at 22:39, <minyard@acm.org> wrote:
> >
> > From: Corey Minyard <cminyard@mvista.com>
> >
> > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> > ---
> 

Thank you for the ping.  Comments inline...

> Very old patch, but Coverity has decided it doesn't like something
> in this function that's still basically the same in the current codebase
> (CID 1487146):
> 
> > +static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
> > +{
> > +    SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
> > +    bool send = false;
> > +    uint8_t cmd;
> > +    int ret = 0;
> > +
> > +    /* length is guaranteed to be >= 1. */
> > +    cmd = *buf++;
> > +    len--;
> > +
> > +    /* Handle read request, which don't have any data in the write part. */
> > +    switch (cmd) {
> > +    case SSIF_IPMI_RESPONSE:
> > +        sid->currblk = 0;
> > +        ret = ipmi_load_readbuf(sid);
> > +        break;
> > +
> > +    case SSIF_IPMI_MULTI_PART_RESPONSE_MIDDLE:
> > +        sid->currblk++;
> > +        ret = ipmi_load_readbuf(sid);
> > +        break;
> > +
> > +    case SSIF_IPMI_MULTI_PART_RETRY:
> > +        if (len >= 1) {
> > +            sid->currblk = buf[0];
> > +            ret = ipmi_load_readbuf(sid);
> > +        } else {
> > +            ret = -1;
> > +        }
> > +        break;
> > +
> > +    default:
> > +        break;
> > +    }
> > +
> > +    /* This should be a message write, make the length is there and correct. */
> > +    if (len >= 1) {
> > +        if (*buf != len - 1 || *buf > MAX_SSIF_IPMI_MSG_CHUNK) {
> > +            return -1; /* Bogus message */
> > +        }
> > +        buf++;
> > +        len--;
> > +    }
> 
> After all of this preamble, len can be zero...
> 
> > +
> > +    switch (cmd) {
> > +    case SSIF_IPMI_REQUEST:
> > +        send = true;
> > +        /* FALLTHRU */
> > +    case SSIF_IPMI_MULTI_PART_REQUEST_START:
> > +        if (len < 2) {
> > +            return -1; /* Bogus. */
> > +        }
> > +        memcpy(sid->inmsg, buf, len);
> > +        sid->inlen = len;
> > +        break;
> > +
> > +    case SSIF_IPMI_MULTI_PART_REQUEST_END:
> > +        send = true;
> > +        /* FALLTHRU */
> > +    case SSIF_IPMI_MULTI_PART_REQUEST_MIDDLE:
> > +        if (!sid->inlen) {
> > +            return -1; /* Bogus. */
> > +        }
> > +        if (sid->inlen + len > MAX_SSIF_IPMI_MSG_SIZE) {
> > +            sid->inlen = 0; /* Discard the message. */
> > +            return -1; /* Bogus. */
> > +        }
> 
> ...this error checking on the values of the 'middle' request
> means that after one 'middle' request we can end up with
> sid->inlen == MAX_SSIF_IPMI_MSG_SIZE (ie we filled the
> entire sid->inmsg[] array).
> 
> But then if we get another 'middle' request with len == 0,
> that will pass this error checking because (sid->inlen + len == MSG_SIZE)
> and fall through into...
> 
> > +        if (len < 32) {
> > +            /*
> > +             * Special hack, a multi-part middle that is less than 32 bytes
> > +             * marks the end of a message.  The specification is fairly
> > +             * confusing, so some systems to this, even sending a zero
> > +             * length end message to mark the end.
> > +             */
> > +            send = true;
> > +        }
> > +        memcpy(sid->inmsg + sid->inlen, buf, len);
> 
> ...calling memcpy() with argument 1 being a pointer that points
> one past the end of the array. Even though len will be 0 and
> we won't memcpy() anything, this is (depending on how you choose
> to intepret things the C standard doesn't come right out and state
> explicitly) undefined behaviour, because memcpy() wants to be passed
> valid pointers, even if you ask it to do no work with a zero len.
> 
> This isn't going to be a visible bug in practical terms, but it would
> make Coverity happy if we either (a) rejected a request with an empty
> length or else (b) skipped the memcpy(). I don't know enough about
> IPMI to know which is better.

Hmm.  In some cases you have to accept a zero-length packet (as
described in the comments), but if you said:

  if (len > 0)
      memcpy(sid->inmsg + sid->inlen, buf, len);

would that make Coverity happy?  I was under the impression that if you
passed zero into len, you could pass anything into the data on a memcpy.
But apparently not; I can make this change.

-corey

> 
> > +        sid->inlen += len;
> > +        break;
> > +    }
> > +
> > +    if (send && sid->inlen) {
> > +        smbus_ipmi_send_msg(sid);
> > +    }
> > +
> > +    return ret;
> > +}
> 
> thanks
> -- PMM
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 12/15] ipmi: Add an SMBus IPMI interface
  2022-07-29 15:56     ` Corey Minyard
@ 2022-07-29 16:01       ` Peter Maydell
  0 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2022-07-29 16:01 UTC (permalink / raw)
  To: minyard
  Cc: QEMU Developers, Michael S . Tsirkin, Igor Mammedov,
	M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
	Marc-André Lureau

On Fri, 29 Jul 2022 at 16:56, Corey Minyard <minyard@acm.org> wrote:
>
> On Tue, Jun 28, 2022 at 05:21:44PM +0100, Peter Maydell wrote:
> > On Thu, 19 Sept 2019 at 22:39, <minyard@acm.org> wrote:
> > >
> > > From: Corey Minyard <cminyard@mvista.com>
> > >
> > > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> > > ---
> >
>
> Thank you for the ping.  Comments inline...

> > ...calling memcpy() with argument 1 being a pointer that points
> > one past the end of the array. Even though len will be 0 and
> > we won't memcpy() anything, this is (depending on how you choose
> > to intepret things the C standard doesn't come right out and state
> > explicitly) undefined behaviour, because memcpy() wants to be passed
> > valid pointers, even if you ask it to do no work with a zero len.
> >
> > This isn't going to be a visible bug in practical terms, but it would
> > make Coverity happy if we either (a) rejected a request with an empty
> > length or else (b) skipped the memcpy(). I don't know enough about
> > IPMI to know which is better.
>
> Hmm.  In some cases you have to accept a zero-length packet (as
> described in the comments), but if you said:
>
>   if (len > 0)
>       memcpy(sid->inmsg + sid->inlen, buf, len);
>
> would that make Coverity happy?  I was under the impression that if you
> passed zero into len, you could pass anything into the data on a memcpy.
> But apparently not; I can make this change.

Yes, putting an if() around the memcpy() will be enough to avoid
the undefined behaviour. (NB that you want braces {} on it ;-))

thanks
-- PMM


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2022-07-29 16:14 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 21:39 [PATCH 00/15] ipmi: Bug fixes, add new interfaces minyard
2019-09-19 21:39 ` [PATCH 01/15] ipmi: Fix watchdog NMI handling minyard
2019-09-20 15:45   ` Cédric Le Goater
2019-09-19 21:39 ` [PATCH 02/15] ipmi: Fix the get watchdog command minyard
2019-09-20 15:50   ` Cédric Le Goater
2019-09-19 21:39 ` [PATCH 03/15] ipmi: Generate an interrupt on watchdog pretimeout expiry minyard
2019-09-23  5:41   ` Cédric Le Goater
2019-09-19 21:39 ` [PATCH 04/15] tests:ipmi: Fix IPMI BT tests minyard
2019-09-19 21:39 ` [PATCH 05/15] qdev: Add a no default uuid property minyard
2019-09-23  5:47   ` Cédric Le Goater
2019-09-19 21:39 ` [PATCH 06/15] ipmi: Add a UUID device property minyard
2019-09-23  5:51   ` Cédric Le Goater
2019-09-19 21:39 ` [PATCH 07/15] ipmi: Split out KCS-specific code from ISA KCS code minyard
2019-09-19 21:39 ` [PATCH 08/15] ipmi: Split out BT-specific code from ISA BT code minyard
2019-09-19 21:39 ` [PATCH 09/15] ipmi: Allow a size value to be passed for I/O space minyard
2019-09-19 21:39 ` [PATCH 10/15] smbios:ipmi: Ignore IPMI devices with no fwinfo function minyard
2019-09-19 21:39 ` [PATCH 11/15] ipmi: Add PCI IPMI interfaces minyard
2019-09-19 21:39 ` [PATCH 12/15] ipmi: Add an SMBus IPMI interface minyard
2022-06-28 16:21   ` Peter Maydell
2022-07-29 15:34     ` Peter Maydell
2022-07-29 15:56     ` Corey Minyard
2022-07-29 16:01       ` Peter Maydell
2019-09-19 21:39 ` [PATCH 13/15] acpi: Add i2c serial bus CRS handling minyard
2019-09-19 21:39 ` [PATCH 14/15] ipmi: Fix SSIF ACPI handling to use the right CRS minyard
2019-09-19 21:39 ` [PATCH 15/15] pc: Add an SMB0 ACPI device to q35 minyard
2019-09-20 11:43 ` [PATCH 00/15] ipmi: Bug fixes, add new interfaces Paolo Bonzini
2019-09-20 12:19   ` Corey Minyard
2019-09-20 12:57 ` Peter Maydell
2019-09-20 17:36   ` Corey Minyard
2019-09-20 17:51     ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).