Refine HID report writing logic
Blocking write on the keyboard HID device causes screen freezing
during turning off the host power. To fix this issue, this commit
refines the logic using non-blocking write. As a side effect,
non-blocking write introduces event dropping when kernel HID driver
returns -EAGAIN when the driver is in busy state so this commit also
adds retry logic to cover the case.
Tested: Didn't see the screen freezing issue.
Change-Id: Ibd95f567c49f448cd053948c14c006de17c52420
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
diff --git a/ikvm_input.cpp b/ikvm_input.cpp
index c4cce50..480db3c 100644
--- a/ikvm_input.cpp
+++ b/ikvm_input.cpp
@@ -25,9 +25,8 @@
using namespace sdbusplus::xyz::openbmc_project::Common::File::Error;
Input::Input(const std::string& kbdPath, const std::string& ptrPath) :
- sendKeyboard(false), sendPointer(false), keyboardFd(-1), pointerFd(-1),
- keyboardReport{0}, pointerReport{0}, keyboardPath(kbdPath),
- pointerPath(ptrPath)
+ keyboardFd(-1), pointerFd(-1), keyboardReport{0}, pointerReport{0},
+ keyboardPath(kbdPath), pointerPath(ptrPath)
{
hidUdcStream.exceptions(std::ofstream::failbit | std::ofstream::badbit);
hidUdcStream.open(hidUdcPath, std::ios::out | std::ios::app);
@@ -79,7 +78,8 @@
if (!keyboardPath.empty())
{
- keyboardFd = open(keyboardPath.c_str(), O_RDWR | O_CLOEXEC);
+ keyboardFd = open(keyboardPath.c_str(),
+ O_RDWR | O_CLOEXEC | O_NONBLOCK);
if (keyboardFd < 0)
{
log<level::ERR>("Failed to open input device",
@@ -135,6 +135,12 @@
{
Server::ClientData* cd = (Server::ClientData*)cl->clientData;
Input* input = cd->input;
+ bool sendKeyboard = false;
+
+ if (input->keyboardFd < 0)
+ {
+ return;
+ }
if (down)
{
@@ -150,7 +156,7 @@
{
input->keyboardReport[i] = sc;
input->keysDown.insert(std::make_pair(key, i));
- input->sendKeyboard = true;
+ sendKeyboard = true;
break;
}
}
@@ -163,7 +169,7 @@
if (mod)
{
input->keyboardReport[0] |= mod;
- input->sendKeyboard = true;
+ sendKeyboard = true;
}
}
}
@@ -175,7 +181,7 @@
{
input->keyboardReport[it->second] = 0;
input->keysDown.erase(it);
- input->sendKeyboard = true;
+ sendKeyboard = true;
}
else
{
@@ -184,10 +190,15 @@
if (mod)
{
input->keyboardReport[0] &= ~mod;
- input->sendKeyboard = true;
+ sendKeyboard = true;
}
}
}
+
+ if (sendKeyboard)
+ {
+ input->writeKeyboard(input->keyboardReport);
+ }
}
void Input::pointerEvent(int buttonMask, int x, int y, rfbClientPtr cl)
@@ -197,6 +208,11 @@
Server* server = (Server*)cl->screen->screenData;
const Video& video = server->getVideo();
+ if (input->pointerFd < 0)
+ {
+ return;
+ }
+
input->pointerReport[0] = ((buttonMask & 0x4) >> 1) |
((buttonMask & 0x2) << 1) | (buttonMask & 0x1);
@@ -214,8 +230,8 @@
memcpy(&input->pointerReport[3], &yy, 2);
}
- input->sendPointer = true;
rfbDefaultPtrAddEvent(buttonMask, x, y, cl);
+ input->writePointer(input->pointerReport);
}
void Input::sendWakeupPacket()
@@ -249,23 +265,6 @@
}
}
-void Input::sendReport()
-{
- if (sendKeyboard && keyboardFd >= 0)
- {
- writeKeyboard(keyboardReport);
-
- sendKeyboard = false;
- }
-
- if (sendPointer && pointerFd >= 0)
- {
- writePointer(pointerReport);
-
- sendPointer = false;
- }
-}
-
uint8_t Input::keyToMod(rfbKeySym key)
{
uint8_t mod = 0;
@@ -489,14 +488,35 @@
bool Input::writeKeyboard(const uint8_t *report)
{
- if (write(keyboardFd, report, KEY_REPORT_LENGTH) != KEY_REPORT_LENGTH)
+ std::unique_lock<std::mutex> lk(keyMutex);
+ uint retryCount = HID_REPORT_RETRY_MAX;
+
+ while (retryCount > 0)
{
- if (errno != ESHUTDOWN && errno != EAGAIN)
+ if (write(keyboardFd, report, KEY_REPORT_LENGTH) == KEY_REPORT_LENGTH)
{
- log<level::ERR>("Failed to write keyboard report",
- entry("ERROR=%s", strerror(errno)));
+ break;
}
+ if (errno != EAGAIN)
+ {
+ if (errno != ESHUTDOWN)
+ {
+ log<level::ERR>("Failed to write keyboard report",
+ entry("ERROR=%s", strerror(errno)));
+ }
+
+ break;
+ }
+
+ lk.unlock();
+ std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ lk.lock();
+ retryCount--;
+ }
+
+ if (!retryCount || errno)
+ {
return false;
}
@@ -505,13 +525,31 @@
void Input::writePointer(const uint8_t *report)
{
- if (write(pointerFd, report, PTR_REPORT_LENGTH) != PTR_REPORT_LENGTH)
+ std::unique_lock<std::mutex> lk(ptrMutex);
+ uint retryCount = HID_REPORT_RETRY_MAX;
+
+ while (retryCount > 0)
{
- if (errno != ESHUTDOWN && errno != EAGAIN)
+ if (write(pointerFd, report, PTR_REPORT_LENGTH) == PTR_REPORT_LENGTH)
{
- log<level::ERR>("Failed to write pointer report",
- entry("ERROR=%s", strerror(errno)));
+ break;
}
+
+ if (errno != EAGAIN)
+ {
+ if (errno != ESHUTDOWN)
+ {
+ log<level::ERR>("Failed to write pointer report",
+ entry("ERROR=%s", strerror(errno)));
+ }
+
+ break;
+ }
+
+ lk.unlock();
+ std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ lk.lock();
+ retryCount--;
}
}
diff --git a/ikvm_input.hpp b/ikvm_input.hpp
index aae7cef..558251d 100644
--- a/ikvm_input.hpp
+++ b/ikvm_input.hpp
@@ -5,6 +5,7 @@
#include <filesystem>
#include <fstream>
#include <map>
+#include <mutex>
#include <string>
namespace ikvm
@@ -56,8 +57,6 @@
/* @brief Sends a wakeup data packet to the USB input device */
void sendWakeupPacket();
- /* @brief Sends an HID report to the USB input device */
- void sendReport();
private:
static constexpr int NUM_MODIFIER_BITS = 4;
@@ -84,6 +83,8 @@
/* @brief Path to the USB virtual hub */
static constexpr const char* usbVirtualHubPath =
"/sys/bus/platform/devices/1e6a0000.usb-vhub";
+ /* @brief Retry limit for writing an HID report */
+ static constexpr int HID_REPORT_RETRY_MAX = 5;
/*
* @brief Translates a RFB-specific key code to HID modifier bit
*
@@ -100,10 +101,6 @@
bool writeKeyboard(const uint8_t *report);
void writePointer(const uint8_t *report);
- /* @brief Indicates whether or not to send a keyboard report */
- bool sendKeyboard;
- /* @brief Indicates whether or not to send a pointer report */
- bool sendPointer;
/* @brief File descriptor for the USB keyboard device */
int keyboardFd;
/* @brief File descriptor for the USB mouse device */
@@ -123,6 +120,10 @@
std::map<int, int> keysDown;
/* @brief Handle of the HID gadget UDC */
std::ofstream hidUdcStream;
+ /* @brief Mutex for sending keyboard reports */
+ std::mutex keyMutex;
+ /* @brief Mutex for sending pointer reports */
+ std::mutex ptrMutex;
};
} // namespace ikvm
diff --git a/ikvm_server.cpp b/ikvm_server.cpp
index 0736d1f..7be99e4 100644
--- a/ikvm_server.cpp
+++ b/ikvm_server.cpp
@@ -79,8 +79,6 @@
if (server->clientHead)
{
- input.sendReport();
-
frameCounter++;
if (pendingResize && frameCounter > video.getFrameRate())
{