ps2: Unify device data buffering

All PS/2 device currently implement various ad-hoc mechanisms to
handle multi-byte commands. This is error-prone and makes it hard to
implement new devices. Create a buffering mechanism in the base class
to avoid this.

Change-Id: If5638b0ab68decea8de7631ecead0a9ebad1547b
Signed-off-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/9765
Reviewed-by: Gabe Black <gabeblack@google.com>

8 files changed, 112 insertions, 127 deletions

diff --git a/src/dev/ps2/device.cc b/src/dev/ps2/device.cc
index deedb49bf..c4d33d505 100644
--- a/src/dev/ps2/device.cc
+++ b/src/dev/ps2/device.cc
@@ -44,12 +44,14 @@
 #include "dev/ps2/device.hh"
 
 #include "base/logging.hh"
+#include "debug/PS2.hh"
 #include "dev/ps2.hh"
 #include "params/PS2Device.hh"
 
 PS2Device::PS2Device(const PS2DeviceParams *p)
     : SimObject(p)
 {
+    inBuffer.reserve(16);
 }
 
 void
@@ -58,6 +60,8 @@ PS2Device::serialize(CheckpointOut &cp) const
     std::vector<uint8_t> buffer(outBuffer.size());
     std::copy(outBuffer.begin(), outBuffer.end(), buffer.begin());
     arrayParamOut(cp, "outBuffer", buffer);
+
+    SERIALIZE_CONTAINER(inBuffer);
 }
 
 void
@@ -67,6 +71,8 @@ PS2Device::unserialize(CheckpointIn &cp)
     arrayParamIn(cp, "outBuffer", buffer);
     for (auto c : buffer)
         outBuffer.push_back(c);
+
+    UNSERIALIZE_CONTAINER(inBuffer);
 }
 
 void
@@ -90,7 +96,10 @@ PS2Device::hostRead()
 void
 PS2Device::hostWrite(uint8_t c)
 {
-    recv(c);
+    DPRINTF(PS2, "PS2: Host -> device: %#x\n", c);
+    inBuffer.push_back(c);
+    if (recv(inBuffer))
+        inBuffer.clear();
 }
 
 void
@@ -98,6 +107,7 @@ PS2Device::send(const uint8_t *data, size_t size)
 {
     assert(data || size == 0);
     while (size) {
+        DPRINTF(PS2, "PS2: Device -> host: %#x\n", *data);
         outBuffer.push_back(*(data++));
         size--;
     }
diff --git a/src/dev/ps2/device.hh b/src/dev/ps2/device.hh
index 5252ac8d8..0ff31d4dc 100644
--- a/src/dev/ps2/device.hh
+++ b/src/dev/ps2/device.hh
@@ -45,6 +45,7 @@
 #define __DEV_PS2_DEVICE_HH__
 
 #include <deque>
+#include <vector>
 
 #include "sim/sim_object.hh"
 
@@ -92,8 +93,18 @@ class PS2Device : public SimObject
   protected: /* Device interface */
     /**
      * Data received from host.
+     *
+     * Data sent to the device is buffered one byte at a time. Each
+     * time a byte is added, this function is called and passed the
+     * current buffer. It should return true if it has consumed the
+     * data and the buffer can be cleared, or false if more data is
+     * needed to process the current command.
+     *
+     * @param data Pending input data (at least one byte)
+     * @return false if more data is needed to process the current
+     * command, true otherwise.
      */
-    virtual void recv(uint8_t data) = 0;
+    virtual bool recv(const std::vector<uint8_t> &data) = 0;
 
     /**
      * Send data from a PS/2 device to a host
@@ -128,6 +139,9 @@ class PS2Device : public SimObject
     /** Device -> host FIFO */
     std::deque<uint8_t> outBuffer;
 
+    /** Host -> device buffer */
+    std::vector<uint8_t> inBuffer;
+
     std::function<void()> dataAvailableCallback;
 };
 
diff --git a/src/dev/ps2/keyboard.cc b/src/dev/ps2/keyboard.cc
index 46b89facc..1f8b54411 100644
--- a/src/dev/ps2/keyboard.cc
+++ b/src/dev/ps2/keyboard.cc
@@ -52,7 +52,6 @@ const uint8_t PS2Keyboard::ID[] = {0xab, 0x83};
 
 PS2Keyboard::PS2Keyboard(const PS2KeyboardParams *p)
     : PS2Device(p),
-      lastCommand(NoCommand),
       shiftDown(false),
       enabled(false)
 {
@@ -64,7 +63,6 @@ void
 PS2Keyboard::serialize(CheckpointOut &cp) const
 {
     PS2Device::serialize(cp);
-    SERIALIZE_SCALAR(lastCommand);
     SERIALIZE_SCALAR(shiftDown);
     SERIALIZE_SCALAR(enabled);
 }
@@ -73,40 +71,28 @@ void
 PS2Keyboard::unserialize(CheckpointIn &cp)
 {
     PS2Device::unserialize(cp);
-    UNSERIALIZE_SCALAR(lastCommand);
     UNSERIALIZE_SCALAR(shiftDown);
     UNSERIALIZE_SCALAR(enabled);
 }
 
-void
-PS2Keyboard::recv(uint8_t data)
+bool
+PS2Keyboard::recv(const std::vector<uint8_t> &data)
 {
-    if (lastCommand != NoCommand) {
-        switch (lastCommand) {
-          case LEDWrite:
+    switch (data[0]) {
+      case LEDWrite:
+        if (data.size() == 1) {
+            DPRINTF(PS2, "Got LED write command.\n");
+            sendAck();
+            return false;
+        } else {
             DPRINTF(PS2, "Setting LEDs: "
                     "caps lock %s, num lock %s, scroll lock %s\n",
-                    bits(data, 2) ? "on" : "off",
-                    bits(data, 1) ? "on" : "off",
-                    bits(data, 0) ? "on" : "off");
-            sendAck();
-            lastCommand = NoCommand;
-            break;
-          case TypematicInfo:
-            DPRINTF(PS2, "Setting typematic info to %#02x.\n", data);
+                    bits(data[1], 2) ? "on" : "off",
+                    bits(data[1], 1) ? "on" : "off",
+                    bits(data[1], 0) ? "on" : "off");
             sendAck();
-            lastCommand = NoCommand;
-            break;
+            return true;
         }
-        return;
-    }
-
-    switch (data) {
-      case LEDWrite:
-        DPRINTF(PS2, "Got LED write command.\n");
-        sendAck();
-        lastCommand = LEDWrite;
-        break;
       case DiagnosticEcho:
         panic("Keyboard diagnostic echo unimplemented.\n");
       case AlternateScanCodes:
@@ -115,27 +101,32 @@ PS2Keyboard::recv(uint8_t data)
         DPRINTF(PS2, "Got keyboard read ID command.\n");
         sendAck();
         send((uint8_t *)&ID, sizeof(ID));
-        break;
+        return true;
       case TypematicInfo:
-        DPRINTF(PS2, "Setting typematic info.\n");
-        sendAck();
-        lastCommand = TypematicInfo;
-        break;
+        if (data.size() == 1) {
+            DPRINTF(PS2, "Setting typematic info.\n");
+            sendAck();
+            return false;
+        } else {
+            DPRINTF(PS2, "Setting typematic info to %#02x.\n", data[1]);
+            sendAck();
+            return true;
+        }
       case Enable:
         DPRINTF(PS2, "Enabling the keyboard.\n");
         enabled = true;
         sendAck();
-        break;
+        return true;
       case Disable:
         DPRINTF(PS2, "Disabling the keyboard.\n");
         enabled = false;
         sendAck();
-        break;
+        return true;
       case DefaultsAndDisable:
         DPRINTF(PS2, "Disabling and resetting the keyboard.\n");
         enabled = false;
         sendAck();
-        break;
+        return true;
       case AllKeysToTypematic:
         panic("Setting all keys to typemantic unimplemented.\n");
       case AllKeysToMakeRelease:
@@ -156,7 +147,7 @@ PS2Keyboard::recv(uint8_t data)
       case Reset:
         panic("Keyboard reset unimplemented.\n");
       default:
-        panic("Unknown keyboard command %#02x.\n", data);
+        panic("Unknown keyboard command %#02x.\n", data[0]);
     }
 }
 
diff --git a/src/dev/ps2/keyboard.hh b/src/dev/ps2/keyboard.hh
index f5d83049f..33db1cc76 100644
--- a/src/dev/ps2/keyboard.hh
+++ b/src/dev/ps2/keyboard.hh
@@ -74,11 +74,8 @@ class PS2Keyboard : public PS2Device, VncKeyboard
         Resend = 0xFE,
         Reset = 0xFF
     };
-    static const uint16_t NoCommand = (uint16_t)(-1);
 
 
-    uint16_t lastCommand;
-
     /** is the shift key currently down */
     bool shiftDown;
 
@@ -92,7 +89,7 @@ class PS2Keyboard : public PS2Device, VncKeyboard
     void unserialize(CheckpointIn &cp) override;
 
   protected: // PS2Device
-    void recv(uint8_t data) override;
+    bool recv(const std::vector<uint8_t> &data) override;
 
   public: // VncKeyboard
     void keyPress(uint32_t key, bool down) override;
diff --git a/src/dev/ps2/mouse.cc b/src/dev/ps2/mouse.cc
index 1bdf39c59..5b6765bcc 100644
--- a/src/dev/ps2/mouse.cc
+++ b/src/dev/ps2/mouse.cc
@@ -52,57 +52,42 @@ const uint8_t BatSuccessful = 0xaa;
 
 PS2Mouse::PS2Mouse(const PS2MouseParams *p)
     : PS2Device(p),
-      lastCommand(NoCommand),
       status(0), resolution(4), sampleRate(100)
 {
 }
 
-void
-PS2Mouse::recv(uint8_t data)
+bool
+PS2Mouse::recv(const std::vector<uint8_t> &data)
 {
-    if (lastCommand != NoCommand) {
-        switch(lastCommand) {
-          case SetResolution:
-            DPRINTF(PS2, "Mouse resolution set to %d.\n", data);
-            resolution = data;
-            sendAck();
-            lastCommand = NoCommand;
-            break;
-          case SampleRate:
-            DPRINTF(PS2, "Mouse sample rate %d samples "
-                    "per second.\n", data);
-            sampleRate = data;
-            sendAck();
-            lastCommand = NoCommand;
-            break;
-          default:
-            panic("Not expecting data for a mouse command.\n");
-        }
-        return;
-    }
-    switch (data) {
+    switch (data[0]) {
       case Scale1to1:
         DPRINTF(PS2, "Setting mouse scale to 1:1.\n");
         status.twoToOne = 0;
         sendAck();
-        break;
+        return true;
       case Scale2to1:
         DPRINTF(PS2, "Setting mouse scale to 2:1.\n");
         status.twoToOne = 1;
         sendAck();
-        break;
+        return true;
       case SetResolution:
-        DPRINTF(PS2, "Setting mouse resolution.\n");
-        lastCommand = SetResolution;
-        sendAck();
-        break;
+        if (data.size() == 1) {
+            DPRINTF(PS2, "Setting mouse resolution.\n");
+            sendAck();
+            return false;
+        } else {
+            DPRINTF(PS2, "Mouse resolution set to %d.\n", data[1]);
+            resolution = data[1];
+            sendAck();
+            return true;
+        }
       case GetStatus:
         DPRINTF(PS2, "Getting mouse status.\n");
         sendAck();
         send((uint8_t *)&(status), 1);
         send(&resolution, sizeof(resolution));
         send(&sampleRate, sizeof(sampleRate));
-        break;
+        return true;
       case ReadData:
         panic("Reading mouse data unimplemented.\n");
       case ResetWrapMode:
@@ -115,22 +100,29 @@ PS2Mouse::recv(uint8_t data)
         DPRINTF(PS2, "Mouse ID requested.\n");
         sendAck();
         send(ID, sizeof(ID));
-        break;
+        return true;
       case SampleRate:
-        DPRINTF(PS2, "Setting mouse sample rate.\n");
-        lastCommand = SampleRate;
-        sendAck();
-        break;
+        if (data.size() == 1) {
+            DPRINTF(PS2, "Setting mouse sample rate.\n");
+            sendAck();
+            return false;
+        } else {
+            DPRINTF(PS2, "Mouse sample rate %d samples "
+                    "per second.\n", data[1]);
+            sampleRate = data[1];
+            sendAck();
+            return true;
+        }
       case DisableReporting:
         DPRINTF(PS2, "Disabling data reporting.\n");
         status.enabled = 0;
         sendAck();
-        break;
+        return true;
       case EnableReporting:
         DPRINTF(PS2, "Enabling data reporting.\n");
         status.enabled = 1;
         sendAck();
-        break;
+        return true;
       case DefaultsAndDisable:
         DPRINTF(PS2, "Disabling and resetting mouse.\n");
         sampleRate = 100;
@@ -138,7 +130,7 @@ PS2Mouse::recv(uint8_t data)
         status.twoToOne = 0;
         status.enabled = 0;
         sendAck();
-        break;
+        return true;
       case Resend:
         panic("Mouse resend unimplemented.\n");
       case Reset:
@@ -150,11 +142,11 @@ PS2Mouse::recv(uint8_t data)
         sendAck();
         send(&BatSuccessful, sizeof(BatSuccessful));
         send(ID, sizeof(ID));
-        break;
+        return true;
       default:
-        warn("Unknown mouse command %#02x.\n", data);
+        warn("Unknown mouse command %#02x.\n", data[0]);
         send(Resend);
-        break;
+        return true;
     }
 }
 
@@ -163,8 +155,6 @@ PS2Mouse::serialize(CheckpointOut &cp) const
 {
     PS2Device::serialize(cp);
 
-    SERIALIZE_SCALAR(lastCommand);
-
     SERIALIZE_SCALAR(status);
     SERIALIZE_SCALAR(resolution);
     SERIALIZE_SCALAR(sampleRate);
@@ -175,8 +165,6 @@ PS2Mouse::unserialize(CheckpointIn &cp)
 {
     PS2Device::unserialize(cp);
 
-    UNSERIALIZE_SCALAR(lastCommand);
-
     UNSERIALIZE_SCALAR(status);
     UNSERIALIZE_SCALAR(resolution);
     UNSERIALIZE_SCALAR(sampleRate);
diff --git a/src/dev/ps2/mouse.hh b/src/dev/ps2/mouse.hh
index e0b4c5384..9150f3f8f 100644
--- a/src/dev/ps2/mouse.hh
+++ b/src/dev/ps2/mouse.hh
@@ -71,7 +71,6 @@ class PS2Mouse : public PS2Device
         Resend = 0xFE,
         Reset = 0xFF
     };
-    static const uint16_t NoCommand = (uint16_t)(-1);
 
     BitUnion8(Status)
         Bitfield<6> remote;
@@ -81,8 +80,6 @@ class PS2Mouse : public PS2Device
         Bitfield<0> rightButton;
     EndBitUnion(Status)
 
-    uint16_t lastCommand;
-
     Status status;
     uint8_t resolution;
     uint8_t sampleRate;
@@ -94,7 +91,7 @@ class PS2Mouse : public PS2Device
     void unserialize(CheckpointIn &cp) override;
 
   protected: // PS2Device
-    void recv(uint8_t data) override;
+    bool recv(const std::vector<uint8_t> &data) override;
 };
 
 #endif // __DEV_PS2_MOUSE_hH__
diff --git a/src/dev/ps2/touchkit.cc b/src/dev/ps2/touchkit.cc
index 96dd2e9c7..fd81c5758 100644
--- a/src/dev/ps2/touchkit.cc
+++ b/src/dev/ps2/touchkit.cc
@@ -54,7 +54,6 @@ const uint8_t PS2TouchKit::ID[] = {0x00};
 PS2TouchKit::PS2TouchKit(const PS2TouchKitParams *p)
     : PS2Device(p),
       vnc(p->vnc),
-      ackNext(false),
       driverInitialized(false)
 {
     if (vnc)
@@ -66,7 +65,6 @@ PS2TouchKit::serialize(CheckpointOut &cp) const
 {
     PS2Device::serialize(cp);
 
-    SERIALIZE_SCALAR(ackNext);
     SERIALIZE_SCALAR(driverInitialized);
 }
 
@@ -75,36 +73,28 @@ PS2TouchKit::unserialize(CheckpointIn &cp)
 {
     PS2Device::unserialize(cp);
 
-    UNSERIALIZE_SCALAR(ackNext);
     UNSERIALIZE_SCALAR(driverInitialized);
 }
 
-void
-PS2TouchKit::recv(uint8_t data)
+bool
+PS2TouchKit::recv(const std::vector<uint8_t> &data)
 {
-    if (ackNext) {
-        ackNext--;
-        sendAck();
-        return;
-    }
-
-    switch (data) {
+    switch (data[0]) {
       case Ps2::Ps2Reset:
         sendAck();
         send(Ps2::SelfTestPass);
-        break;
+        return true;
 
       case Ps2::SetResolution:
       case Ps2::SetRate:
       case Ps2::SetStatusLed:
         sendAck();
-        ackNext = 1;
-        break;
+        return data.size() == 2;
 
       case Ps2::ReadId:
         sendAck();
         send((const uint8_t *)&ID, sizeof(ID));
-        break;
+        return true;
 
       case Ps2::TpReadId:
         // We're not a trackpoint device, this should make the probe
@@ -113,7 +103,7 @@ PS2TouchKit::recv(uint8_t data)
         send(0);
         send(0);
         sendAck();
-        break;
+        return true;
 
       case Ps2::SetScaling1_1:
       case Ps2::SetScaling1_2:
@@ -121,27 +111,32 @@ PS2TouchKit::recv(uint8_t data)
       case Ps2::Enable:
       case Ps2::SetDefaults:
         sendAck();
-        break;
+        return true;
 
       case Ps2::StatusRequest:
         sendAck();
         send(0);
         send(2); // default resolution
         send(100); // default sample rate
-        break;
+        return true;
 
       case Ps2::TouchKitId:
-        ackNext = 2;
         sendAck();
-        send(Ps2::TouchKitId);
-        send(1);
-        send('A');
-
-        driverInitialized = true;
-        break;
+        if (data.size() == 1) {
+            send(Ps2::TouchKitId);
+            send(1);
+            send('A');
+
+            return false;
+        } else if (data.size() == 3) {
+            driverInitialized = true;
+            return true;
+        } else {
+            return false;
+        }
 
       default:
-        panic("Unknown byte received: %d\n", data);
+        panic("Unknown byte received: %d\n", data[0]);
     }
 }
 
diff --git a/src/dev/ps2/touchkit.hh b/src/dev/ps2/touchkit.hh
index 4541f3f29..fa1bc52a4 100644
--- a/src/dev/ps2/touchkit.hh
+++ b/src/dev/ps2/touchkit.hh
@@ -57,7 +57,7 @@ class PS2TouchKit : public PS2Device, public VncMouse
     void unserialize(CheckpointIn &cp) override;
 
   protected: // PS2Device
-    void recv(uint8_t data) override;
+    bool recv(const std::vector<uint8_t> &data) override;
 
   public: // VncMouse
     void mouseAt(uint16_t x, uint16_t y, uint8_t buttons) override;
@@ -67,13 +67,6 @@ class PS2TouchKit : public PS2Device, public VncMouse
     VncInput *const vnc;
 
     /**
-     * If the controller should ignore the next N data bytes and
-     * acknowledge them. This occurs when the driver is attempting to
-     * setup some feature we don't care about.
-     */
-    int ackNext;
-
-    /**
      * Has the driver been initialized in TouchKit mode? The model
      * suppresses touch event generation until this is true.
      */