Ensure Get-Device-ID conforms to IPMI spec

* Refactor response from constant array to explicitly commented fields.
* Update comments for fields.
* Refer to FSP implementation where appropriate.

Note: Some of the fields present here should be filled in from MRW or
runtime settings in the future.

Resolves openbmc/openbmc#435.

Change-Id: I733274e237cf28d5048c44d527a8fd67a0392168
Signed-off-by: Nan Li <william.bjlinan@hotmail.com>

1 files changed, 59 insertions, 13 deletions

diff --git a/apphandler.cpp b/apphandler.cpp
index b8cc0aa..d5ebe2e 100644
--- a/apphandler.cpp
+++ b/apphandler.cpp
@@ -5,14 +5,24 @@
 #include <string.h>
 #include <stdint.h>
 #include <systemd/sd-bus.h>
+#include <array>
 
 extern sd_bus *bus;
 
 void register_netfn_app_functions() __attribute__((constructor));
 
-#define DEVICE_FW1 2
-#define DEVICE_FW2 3
-#define DEVICE_AUX 11
+// Offset in get device id command.
+typedef struct
+{
+   uint8_t id;
+   uint8_t revision;
+   uint8_t fw[2];
+   uint8_t ipmi_ver;
+   uint8_t addn_dev_support;
+   uint8_t manuf_id[3];
+   uint8_t prod_id[2];
+   uint8_t aux[4];
+}__attribute__((packed)) ipmi_device_id_t;
 
 //---------------------------------------------------------------------
 // Called by Host on seeing a SMS_ATN bit set. Return a hardcoded 
@@ -169,31 +179,67 @@ ipmi_ret_t ipmi_app_get_device_id(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
     char *ver = NULL;
     int r;
     rev_t rev = {0};
-
-    // TODO:
-    // This value is the IANA number assigned to "IBM Platform Firmware
-    // Division", which is also used by our service processor.  We may want
-    // a different number or at least a different version?
-    uint8_t dev_id[] = {0, 0, 0, 0, 2, 0xD, 0x41, 0xA7, 0x00, 0x43, 0x40, 0, 0, 0, 0};
+    ipmi_device_id_t dev_id{};
 
     // Data length
     *data_len = sizeof(dev_id);
 
+    // From IPMI spec, controller that have different application commands, or different
+    // definitions of OEM fields, are expected to have different Device ID values.
+    // Set to 0 now.
+
+    // Device Revision is set to 0 now.
+    // Bit7 identifies if device provide Device SDRs,  obmc don't have SDR,we use ipmi to
+    // simulate SDR, hence the value:
+    dev_id.revision = 0x80;
+
+    // Firmware revision is already implemented, so get it from appropriate position.
     r = sd_bus_get_property_string(bus,busname,objname,iface,"version", NULL, &ver);
     if ( r < 0 ) {
         fprintf(stderr, "Failed to obtain version property: %s\n", strerror(-r));
     } else {
         r = convert_version(ver, &rev);
         if( r >= 0 ) {
-            // bit7 identifies state of SDR repository, hence the mask
-            dev_id[DEVICE_FW1] |= 0x7F & rev.major;
+            // bit7 identifies if the device is available, 0=normal operation,
+            // 1=device firmware, SDR update or self-initialization in progress.
+            // our SDR is normal working condition, so mask:
+            dev_id.fw[0] = 0x7F & rev.major;
 
             rev.minor = (rev.minor > 99 ? 99 : rev.minor);
-            dev_id[DEVICE_FW2] = rev.minor % 10 + (rev.minor / 10) * 16;
-            memcpy(&dev_id[DEVICE_AUX], rev.d, 4);
+            dev_id.fw[1] = rev.minor % 10 + (rev.minor / 10) * 16;
+            memcpy(&dev_id.aux, rev.d, 4);
         }
     }
 
+    // IPMI Spec verison 2.0
+    dev_id.ipmi_ver = 2;
+
+    // Additional device Support.
+    // List the 'logical device' commands and functions that the controller supports
+    // that are in addition to the mandatory IPM and Application commands.
+    // [7] Chassis Device (device functions as chassis device per ICMB spec.)
+    // [6] Bridge (device responds to Bridge NetFn commands)
+    // [5] IPMB Event Generator
+    // [4] IPMB Event Receiver
+    // [3] FRU Inventory Device
+    // [2] SEL Device
+    // [1] SDR Repository Device
+    // [0] Sensor Device
+    // We support FRU/SEL/Sensor now:
+    dev_id.addn_dev_support = 0x8D;
+
+    // This value is the IANA number assigned to "IBM Platform Firmware
+    // Division", which is also used by our service processor.  We may want
+    // a different number or at least a different version?
+    dev_id.manuf_id[0] = 0x41;
+    dev_id.manuf_id[1] = 0xA7;
+    dev_id.manuf_id[2] = 0x00;
+
+    // Witherspoon's product ID is hardcoded to 4F42(ASCII 'OB').
+    // TODO: openbmc/openbmc#495
+    dev_id.prod_id[0] = 0x4F;
+    dev_id.prod_id[1] = 0x42;
+
     // Pack the actual response
     memcpy(response, &dev_id, *data_len);
     return rc;