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--;
}
}