Revert "Integrate with the vsoc driver futex and add tests"
This reverts commit 432a73712c8432760c9eeda136dee1638f2ee339.
This seems to cause the input_service to hang.
BUG=73814577
Test: Local build and boot
(cherry picked from commit eb9d882cc72aaf559cf459b35c46bb386818a323)
Change-Id: I08b5f172af1c295746abb5f814a24055824ea424
diff --git a/common/vsoc/lib/e2e_test_region_view.h b/common/vsoc/lib/e2e_test_region_view.h
index 70a3db6..1ea6aef 100644
--- a/common/vsoc/lib/e2e_test_region_view.h
+++ b/common/vsoc/lib/e2e_test_region_view.h
@@ -27,38 +27,6 @@
E2ERegionView<Layout>,
Layout> {
public:
-
- uint32_t read_guest_self_register() const {
- return this->data().guest_self_register;
- }
-
- void write_guest_self_register(uint32_t val) {
- this->data()->guest_self_register = val;
- }
-
- void signal_guest_self_register() {
- this->SendSignal(layout::Sides::OurSide,
- &this->data()->guest_self_register);
- }
-
- int wait_guest_self_register(uint32_t expected_value) {
- return this->WaitForSignal(
- &this->data()->guest_self_register, expected_value);
- }
-
- void signal_guest_to_host_register() {
- this->SendSignal(layout::Sides::OurSide,
- &this->data()->guest_to_host_signal);
- }
-
- uint32_t guest_to_host_signal_offset() const {
- return this->pointer_to_region_offset(&this->data().guest_to_host_signal);
- }
-
- uint32_t host_to_guest_signal_offset() const {
- return this->pointer_to_region_offset(&this->data().host_to_guest_signal);
- }
-
const char* guest_string(size_t index) const {
return const_cast<const char*>(this->data().data[index].guest_writable);
}
diff --git a/common/vsoc/lib/mock_region_view.h b/common/vsoc/lib/mock_region_view.h
index f9c3006..b6bcc45 100644
--- a/common/vsoc/lib/mock_region_view.h
+++ b/common/vsoc/lib/mock_region_view.h
@@ -59,13 +59,13 @@
nullptr, nullptr, 0);
}
- virtual int WaitForSignal(std::atomic<uint32_t>* uaddr,
+ virtual void WaitForSignal(std::atomic<uint32_t>* uaddr,
uint32_t expected_value) {
{
std::lock_guard<std::mutex> guard(mutex_);
if (tid_to_addr_.find(std::this_thread::get_id()) != tid_to_addr_.end()) {
// Same thread tries to wait
- return 0;
+ return;
}
tid_to_addr_.emplace(std::this_thread::get_id(), uaddr);
map_changed_.notify_one();
@@ -77,7 +77,6 @@
std::lock_guard<std::mutex> guard(mutex_);
tid_to_addr_.erase(std::this_thread::get_id());
}
- return 0;
}
// Mock methods from TypedRegionView
diff --git a/common/vsoc/lib/region_control.h b/common/vsoc/lib/region_control.h
index 7762603..5b30d30 100644
--- a/common/vsoc/lib/region_control.h
+++ b/common/vsoc/lib/region_control.h
@@ -82,27 +82,6 @@
// Wait for an interrupt from our peer
virtual void WaitForInterrupt() = 0;
- // Signals local waiters at the given region offset.
- // Defined only on the guest.
- // Return value is negative on error.
- virtual int SignalSelf(uint32_t offset) = 0;
-
- // Waits for a signal at the given region offset.
- // Defined only on the guest.
- // Return value is negative on error. The number of false wakes is returned
- // on success.
- virtual int WaitForSignal(uint32_t offset, uint32_t expected_value) = 0;
-
- template <typename T>
- T* region_offset_to_pointer(uint32_t offset) {
- if (offset > region_size()) {
- LOG(FATAL) << __FUNCTION__ << ": " << offset << " not in region @"
- << region_base_;
- }
- return reinterpret_cast<T*>(reinterpret_cast<uintptr_t>(region_base_) +
- offset);
- }
-
protected:
RegionControl() {}
void* region_base_{};
diff --git a/common/vsoc/lib/region_signaling_interface.h b/common/vsoc/lib/region_signaling_interface.h
index c1399e1..a853b7a 100644
--- a/common/vsoc/lib/region_signaling_interface.h
+++ b/common/vsoc/lib/region_signaling_interface.h
@@ -50,14 +50,7 @@
// signal_addr: the memory that will be signaled. Must be within the region.
//
// last_observed_value: the value that motivated the calling code to wait.
- //
- // The return values are:
- // -1 on failure
- // 0 indicates success with no tuning information
- // >0 indicates success. The number indicates how many times the thread
- // woke before it could return.
- // Large values indicate that the regions should be tuned.
- virtual int WaitForSignal(std::atomic<uint32_t>* signal_addr,
+ virtual void WaitForSignal(std::atomic<uint32_t>* signal_addr,
uint32_t last_observed_value) = 0;
};
diff --git a/common/vsoc/lib/region_view.cpp b/common/vsoc/lib/region_view.cpp
index b86defd..e15233f 100644
--- a/common/vsoc/lib/region_view.cpp
+++ b/common/vsoc/lib/region_view.cpp
@@ -1,6 +1,8 @@
#include "common/vsoc/lib/region_view.h"
+#include <linux/futex.h>
#include <sys/mman.h>
+#include <sys/syscall.h>
#include "common/libs/glog/logging.h"
@@ -11,11 +13,8 @@
using vsoc::layout::Sides;
-vsoc::RegionWorker::RegionWorker(RegionView* region,
- std::shared_ptr<RegionControl> control)
- : control_(control),
- region_(region),
- thread_(&vsoc::RegionWorker::Work, this) {}
+vsoc::RegionWorker::RegionWorker(RegionView* region)
+ : region_(region), thread_(&vsoc::RegionWorker::Work, this) {}
void vsoc::RegionWorker::Work() {
while (!stopping_) {
@@ -23,9 +22,9 @@
if (stopping_) {
return;
}
- region_->ProcessSignalsFromPeer([this](uint32_t offset) {
- control_->SignalSelf(offset);
- });
+ region_->ProcessSignalsFromPeer([](std::atomic<uint32_t>* uaddr) {
+ syscall(SYS_futex, uaddr, FUTEX_WAKE, -1, nullptr, nullptr, 0);
+ });
}
}
@@ -84,21 +83,22 @@
}
void vsoc::RegionView::ProcessSignalsFromPeer(
- std::function<void(uint32_t)> signal_handler) {
+ std::function<void(std::atomic<uint32_t>*)> signal_handler) {
const vsoc_signal_table_layout& table = incoming_signal_table();
const size_t num_offsets = (1 << table.num_nodes_lg2);
std::atomic<uint32_t>* offsets =
region_offset_to_pointer<std::atomic<uint32_t>>(
table.futex_uaddr_table_offset);
for (size_t i = 0; i < num_offsets; ++i) {
- uint32_t raw_offset = offsets[i].exchange(0);
- if (raw_offset) {
- bool round_trip = raw_offset & UADDR_OFFSET_ROUND_TRIP_FLAG;
- uint32_t offset = raw_offset & UADDR_OFFSET_MASK;
- signal_handler(offset);
+ uint32_t offset = offsets[i].exchange(0);
+ if (offset) {
+ bool round_trip = offset & UADDR_OFFSET_ROUND_TRIP_FLAG;
+ std::atomic<uint32_t>* uaddr =
+ region_offset_to_pointer<std::atomic<uint32_t>>(offset &
+ UADDR_OFFSET_MASK);
+ signal_handler(uaddr);
if (round_trip) {
- SendSignalToPeer(
- region_offset_to_pointer<std::atomic<uint32_t>>(offset), false);
+ SendSignalToPeer(uaddr, false);
}
}
}
@@ -116,7 +116,8 @@
return;
}
if (sides_to_signal & Sides::OurSide) {
- control_->SignalSelf(pointer_to_region_offset(uaddr));
+ syscall(SYS_futex, reinterpret_cast<int32_t*>(uaddr), FUTEX_WAKE, -1,
+ nullptr, nullptr, 0);
}
}
@@ -179,12 +180,10 @@
}
std::unique_ptr<vsoc::RegionWorker> vsoc::RegionView::StartWorker() {
- return std::unique_ptr<vsoc::RegionWorker>(new vsoc::RegionWorker(
- this, control_));
+ return std::unique_ptr<vsoc::RegionWorker>(new vsoc::RegionWorker(this));
}
-int vsoc::RegionView::WaitForSignal(std::atomic<uint32_t>* uaddr,
+void vsoc::RegionView::WaitForSignal(std::atomic<uint32_t>* uaddr,
uint32_t expected_value) {
- return control_->WaitForSignal(pointer_to_region_offset(uaddr),
- expected_value);
+ syscall(SYS_futex, uaddr, FUTEX_WAIT, expected_value, nullptr, nullptr, 0);
}
diff --git a/common/vsoc/lib/region_view.h b/common/vsoc/lib/region_view.h
index 2f07f71..d1517c2 100644
--- a/common/vsoc/lib/region_view.h
+++ b/common/vsoc/lib/region_view.h
@@ -37,7 +37,6 @@
namespace vsoc {
-class RegionControl;
class RegionView;
/**
@@ -48,12 +47,11 @@
*/
class RegionWorker {
public:
- RegionWorker(RegionView* region, std::shared_ptr<RegionControl> control);
+ explicit RegionWorker(RegionView* region);
~RegionWorker();
void Work();
protected:
- std::shared_ptr<RegionControl> control_;
RegionView* region_;
std::thread thread_;
volatile bool stopping_{};
@@ -104,7 +102,7 @@
// peer, usually a FUTEX_WAKE call, but can be customized for other
// purposes.
void ProcessSignalsFromPeer(
- std::function<void(uint32_t)> signal_handler);
+ std::function<void(std::atomic<uint32_t>*)> signal_handler);
// Post a signal to the guest, the host, or both.
// See futex(2) FUTEX_WAKE for details.
@@ -139,11 +137,8 @@
// signal_addr: the memory that will be signaled. Must be within the region.
//
// last_observed_value: the value that motivated the calling code to wait.
- //
- // The return value is -1 on error. On the guest positive values give the
- // number of false wakes.
- int WaitForSignal(std::atomic<uint32_t>* signal_addr,
- uint32_t last_observed_value) override;
+ void WaitForSignal(std::atomic<uint32_t>* signal_addr,
+ uint32_t last_observed_value);
// Starts the signal table scanner. This must be invoked by subclasses, which
// must store the returned unique_ptr as a class member.
@@ -175,7 +170,12 @@
protected:
template <typename T>
T* region_offset_to_pointer(uint32_t offset) {
- return control_->region_offset_to_pointer<T>(offset);
+ if (offset > control_->region_size()) {
+ LOG(FATAL) << __FUNCTION__ << ": " << offset << " not in region @"
+ << region_base_;
+ }
+ return reinterpret_cast<T*>(reinterpret_cast<uintptr_t>(region_base_) +
+ offset);
}
template <typename T>
@@ -193,7 +193,7 @@
// This is mostly for the RegionView's internal plumbing. Use TypedRegionView
// and RegionLayout to avoid this in most cases.
template <typename T>
- uint32_t pointer_to_region_offset(T* ptr) const {
+ uint32_t pointer_to_region_offset(T* ptr) {
uint32_t rval = reinterpret_cast<uintptr_t>(ptr) -
reinterpret_cast<uintptr_t>(region_base_);
if (rval > control_->region_size()) {
diff --git a/common/vsoc/shm/e2e_test_region_layout.h b/common/vsoc/shm/e2e_test_region_layout.h
index 3c5950e..7227726 100644
--- a/common/vsoc/shm/e2e_test_region_layout.h
+++ b/common/vsoc/shm/e2e_test_region_layout.h
@@ -120,7 +120,6 @@
// These fields are used to test the signaling mechanism.
std::atomic<uint32_t> host_to_guest_signal;
std::atomic<uint32_t> guest_to_host_signal;
- std::atomic<uint32_t> guest_self_register;
// There rest of the region will be filled by guest_host_strings.
// We actually use more than one of these, but we can't know how many
// until we examine the region.
diff --git a/common/vsoc/shm/version.h b/common/vsoc/shm/version.h
index e1c715b..12fefd8 100644
--- a/common/vsoc/shm/version.h
+++ b/common/vsoc/shm/version.h
@@ -160,15 +160,15 @@
// Changes to these structures will affect only the e2e_test_region
namespace e2e_test {
namespace {
-const uint32_t version = 2;
+const uint32_t version = 1;
}
static const std::size_t E2EManagerTestRegionLayout_size = 16;
static const std::size_t E2EMemoryFill_size = 64;
-static const std::size_t E2EPrimaryTestRegionLayout_size = 84;
-static const std::size_t E2ESecondaryTestRegionLayout_size = 84;
-static const std::size_t E2ETestRegionLayout_size = 84;
+static const std::size_t E2EPrimaryTestRegionLayout_size = 80;
+static const std::size_t E2ESecondaryTestRegionLayout_size = 80;
+static const std::size_t E2ETestRegionLayout_size = 80;
static const std::size_t E2ETestStageRegister_size = 4;
-static const std::size_t E2EUnfindableRegionLayout_size = 84;
+static const std::size_t E2EUnfindableRegionLayout_size = 80;
static const std::size_t E2EManagedTestRegionLayout_size = 4;
} // namespace e2e_test
diff --git a/guest/vsoc/lib/Android.mk b/guest/vsoc/lib/Android.mk
index ad27f69..2072415 100644
--- a/guest/vsoc/lib/Android.mk
+++ b/guest/vsoc/lib/Android.mk
@@ -15,30 +15,6 @@
LOCAL_PATH := $(call my-dir)
include $(CLEAR_VARS)
-LOCAL_MODULE := vsoc_driver_test
-LOCAL_MODULE_TAGS := optional
-LOCAL_SRC_FILES := vsoc_driver_test.cpp
-
-LOCAL_C_INCLUDES := \
- device/google/cuttlefish_common \
- device/google/cuttlefish_kernel
-
-LOCAL_CFLAGS += -DGTEST_OS_LINUX_ANDROID -DGTEST_HAS_STD_STRING -Werror -Wall
-
-LOCAL_STATIC_LIBRARIES := \
- libgtest
-
-LOCAL_SHARED_LIBRARIES := \
- vsoc_lib \
- cuttlefish_auto_resources \
- libcuttlefish_fs \
- libbase
-
-LOCAL_MULTILIB := first
-LOCAL_VENDOR_MODULE := true
-include $(BUILD_EXECUTABLE)
-
-include $(CLEAR_VARS)
LOCAL_MODULE := vsoc_guest_region_e2e_test
LOCAL_MODULE_TAGS := optional
LOCAL_SRC_FILES := guest_region_e2e_test.cpp
diff --git a/guest/vsoc/lib/guest_region_e2e_test.cpp b/guest/vsoc/lib/guest_region_e2e_test.cpp
index 8e3181e..5c027b7 100644
--- a/guest/vsoc/lib/guest_region_e2e_test.cpp
+++ b/guest/vsoc/lib/guest_region_e2e_test.cpp
@@ -118,9 +118,9 @@
primary->WaitForInterrupt();
int count = 0; // counts the number of signals received.
primary->ProcessSignalsFromPeer(
- [&primary, &count](uint32_t offset) {
+ [&primary, &count](std::atomic<uint32_t>* uaddr) {
++count;
- EXPECT_TRUE(offset == primary->host_to_guest_signal_offset());
+ EXPECT_TRUE(uaddr == &primary->data()->host_to_guest_signal);
});
EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on primary region";
@@ -130,9 +130,9 @@
secondary->WaitForInterrupt();
count = 0;
secondary->ProcessSignalsFromPeer(
- [&secondary, &count](uint32_t offset) {
+ [&secondary, &count](std::atomic<uint32_t>* uaddr) {
++count;
- EXPECT_TRUE(offset == secondary->host_to_guest_signal_offset());
+ EXPECT_TRUE(uaddr == &secondary->data()->host_to_guest_signal);
});
EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on secondary region";
diff --git a/guest/vsoc/lib/region_control.cpp b/guest/vsoc/lib/region_control.cpp
index 1fcbd13..d777912 100644
--- a/guest/vsoc/lib/region_control.cpp
+++ b/guest/vsoc/lib/region_control.cpp
@@ -43,8 +43,6 @@
virtual void InterruptSelf() override;
virtual void WaitForInterrupt() override;
virtual void* Map() override;
- virtual int SignalSelf(uint32_t offset) override;
- virtual int WaitForSignal(uint32_t offset, uint32_t expected_value) override;
protected:
int CreateFdScopedPermission(const char* managed_region_name,
@@ -74,32 +72,6 @@
region_fd_->Ioctl(VSOC_WAIT_FOR_INCOMING_INTERRUPT, 0);
}
-int GuestRegionControl::SignalSelf(uint32_t offset) {
- return region_fd_->Ioctl(VSOC_COND_WAKE, reinterpret_cast<void*>(offset));
-}
-
-int GuestRegionControl::WaitForSignal(uint32_t offset,
- uint32_t expected_value) {
- struct vsoc_cond_wait wait;
- wait.offset = offset;
- wait.value = expected_value;
- wait.wake_time_sec = 0;
- wait.wake_time_nsec = 0;
- wait.wait_type = VSOC_WAIT_IF_EQUAL;
- wait.wakes = 1000;
- wait.reserved_1 = 0;
- int rval = region_fd_->Ioctl(VSOC_COND_WAIT, &wait);
- if (rval == -1) {
- return rval;
- }
- // Clamp the number of wakes if it overflows an integer.
- rval = wait.wakes;
- if (rval >= 0) {
- return rval;
- }
- return INT_MAX;
-}
-
int GuestRegionControl::CreateFdScopedPermission(
const char* managed_region_name, uint32_t owner_offset,
uint32_t owned_value, uint32_t begin_offset,
diff --git a/guest/vsoc/lib/vsoc_driver_test.cpp b/guest/vsoc/lib/vsoc_driver_test.cpp
deleted file mode 100644
index ff20ecb..0000000
--- a/guest/vsoc/lib/vsoc_driver_test.cpp
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/*
- * Stand-alone tests for the ioctls in the vsoc driver.
- */
-
-#include "uapi/vsoc_shm.h"
-#include <atomic>
-#include <stdint.h>
-#include "common/vsoc/lib/e2e_test_region_view.h"
-#include "guest/vsoc/lib/manager_region_view.h"
-
-#include <android-base/logging.h>
-#include <gtest/gtest.h>
-
-void BasicWaitForSignal(vsoc::E2EPrimaryRegionView* region,
- uint32_t expected_start,
- uint32_t expected_finish) {
- ASSERT_EQ(expected_start, region->read_guest_self_register());
- int rval = region->wait_guest_self_register(expected_start);
- EXPECT_LE(0, rval);
- EXPECT_GT(5, rval);
- EXPECT_EQ(expected_finish, region->read_guest_self_register());
-}
-
-TEST(FutexTest, BasicFutexTests) {
- constexpr uint32_t INITIAL_SIGNAL = 0;
- constexpr uint32_t SILENT_UPDATE_SIGNAL = 1;
- constexpr uint32_t WAKE_SIGNAL = 2;
- auto region = vsoc::E2EPrimaryRegionView::GetInstance();
- ASSERT_TRUE(region != NULL);
- LOG(INFO) << "Regions are open";
- region->write_guest_self_register(INITIAL_SIGNAL);
- std::thread waiter(BasicWaitForSignal, region, INITIAL_SIGNAL, WAKE_SIGNAL);
- sleep(1);
- LOG(INFO) << "Still waiting. Trying to wake wrong address";
- region->signal_guest_to_host_register();
- sleep(1);
- LOG(INFO) << "Still waiting. Trying to wake without update";
- region->signal_guest_self_register();
- sleep(1);
- LOG(INFO) << "Still waiting. Trying to wake without signal";
- region->write_guest_self_register(SILENT_UPDATE_SIGNAL);
- sleep(1);
- LOG(INFO) << "Still waiting. Trying to wake with signal";
- region->write_guest_self_register(WAKE_SIGNAL);
- region->signal_guest_self_register();
- waiter.join();
- LOG(INFO) << "Wake worked";
- LOG(INFO) << "PASS: BasicPeerTests";
-}
-
-int main(int argc, char* argv[]) {
- android::base::InitLogging(argv);
- testing::InitGoogleTest(&argc, argv);
- return RUN_ALL_TESTS();
-}
diff --git a/host/vsoc/lib/host_region_e2e_test.cpp b/host/vsoc/lib/host_region_e2e_test.cpp
index cab1bc7..0be719f 100644
--- a/host/vsoc/lib/host_region_e2e_test.cpp
+++ b/host/vsoc/lib/host_region_e2e_test.cpp
@@ -89,11 +89,11 @@
primary->WaitForInterrupt();
int count = 0; // counts the number of signals received.
primary->ProcessSignalsFromPeer(
- [&primary, &count](uint32_t offset) {
+ [&primary, &count](std::atomic<uint32_t>* uaddr) {
++count;
- EXPECT_EQ(primary->guest_to_host_signal_offset(), offset);
+ EXPECT_TRUE(uaddr == &primary->data()->guest_to_host_signal);
});
- EXPECT_EQ(1, count);
+ EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on primary region";
secondary->SendSignal(vsoc::layout::Sides::Peer,
&secondary->data()->host_to_guest_signal);
@@ -101,11 +101,11 @@
secondary->WaitForInterrupt();
count = 0;
secondary->ProcessSignalsFromPeer(
- [secondary, &count](uint32_t offset) {
+ [secondary, &count](std::atomic<uint32_t>* uaddr) {
++count;
- EXPECT_EQ(secondary->guest_to_host_signal_offset(), offset);
+ EXPECT_TRUE(uaddr == &secondary->data()->guest_to_host_signal);
});
- EXPECT_EQ(1, count);
+ EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on secondary region";
EXPECT_FALSE(primary->HasIncomingInterrupt());
diff --git a/host/vsoc/lib/region_control.cpp b/host/vsoc/lib/region_control.cpp
index 491f600..3797424 100644
--- a/host/vsoc/lib/region_control.cpp
+++ b/host/vsoc/lib/region_control.cpp
@@ -19,7 +19,6 @@
#include <stdio.h>
#include <string.h>
-#include <linux/futex.h>
#include <sys/mman.h>
#include <sys/socket.h>
#include <sys/syscall.h>
@@ -120,17 +119,6 @@
return region_base_;
}
-
- virtual int SignalSelf(uint32_t offset) override {
- return syscall(SYS_futex, region_offset_to_pointer<int32_t*>(offset),
- FUTEX_WAKE, -1, nullptr, nullptr, 0);
- }
-
- virtual int WaitForSignal(uint32_t offset, uint32_t expected_value) override {
- return syscall(SYS_futex, region_offset_to_pointer<int32_t*>(offset),
- FUTEX_WAIT, expected_value, nullptr, nullptr, 0);
- }
-
protected:
const char* region_name_{};
cvd::SharedFD incoming_interrupt_fd_;
