Re: [PATCH v8 0/7] Basic recovery for machine checks inside SGX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tony,

On 10/1/2021 9:47 AM, Tony Luck wrote:
Now version 8

Note that linux-kernel@xxxxxxxxxxxxxxx and x86@xxxxxxxxxx are still
dropped from the distribution. Time to get some internal agreement on these
changes before bothering the x86 maintainers with yet another version.

So I'm still looking for Acked-by: or Reviewed-by: on any bits of this
series that are worthy, and comments on the problems I need to fix
in the not-worthy parts.

Tested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>

Details of testing:
I was curious how the signaling worked after reading some snippets that a vDSO does not generate a signal. To help my understanding I created the test below in the SGX selftests that implements the test sequence you document in patch 6/7 and with it I can see how the SIGBUS is delivered:

[BEGIN TEST OUTPUT]
#  RUN           enclave.mce ...
MCE test from pid 2746 on addr 0x7fc879644000
Set up error injection on virt 0x7fc879644000 and press any key to continue test ...

# mce: Test terminated unexpectedly by signal 7
[END TEST OUTPUT]

Below is the test I ran. It only implements steps 3 to 7 from the test sequence you document in patch 6/7. It does still require manual intervention to determine the physical address and trigger the error injection on the physical address. It also currently treats the SIGBUS as a test failure, which I did to help clearly see the signal, but it could be changed to TEST_F_SIGNAL to have a SIGBUS mean success. The test is thus not appropriate for the SGX selftests in its current form but is provided as informational to describe the testing I did. It applies on top of the recent SGX selftest changes from: https://lore.kernel.org/lkml/cover.1631731214.git.reinette.chatre@xxxxxxxxx/

---8<---
 tools/testing/selftests/sgx/defines.h   |  7 +++++
 tools/testing/selftests/sgx/main.c      | 35 +++++++++++++++++++++
 tools/testing/selftests/sgx/test_encl.c | 41 +++++++++++++++++++++++++
 3 files changed, 83 insertions(+)

diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h
index 02d775789ea7..2b471ba68e91 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -24,6 +24,7 @@ enum encl_op_type {
 	ENCL_OP_PUT_TO_ADDRESS,
 	ENCL_OP_GET_FROM_ADDRESS,
 	ENCL_OP_NOP,
+	ENCL_OP_MCE,
 	ENCL_OP_MAX,
 };

@@ -53,4 +54,10 @@ struct encl_op_get_from_addr {
 	uint64_t addr;
 };

+struct encl_op_mce {
+	struct encl_op_header header;
+	uint64_t addr;
+	uint64_t value;
+	uint64_t delay_cycles;
+};
 #endif /* DEFINES_H */
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 79669c245f94..2979306a687a 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -594,4 +594,39 @@ TEST_F(enclave, pte_permissions)
 	EXPECT_EQ(self->run.exception_addr, 0);
 }

+TEST_F_TIMEOUT(enclave, mce, 600)
+{
+	struct encl_op_mce mce_op;
+	unsigned long data_start;
+
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
+
+	memset(&self->run, 0, sizeof(self->run));
+	self->run.tcs = self->encl.encl_base;
+
+	data_start = self->encl.encl_base +
+		     encl_get_data_offset(&self->encl) +
+		     PAGE_SIZE;
+
+	printf("MCE test from pid %d on addr 0x%lx\n", getpid(), data_start);
+	/*
+	 * Sanity check to ensure it is possible to write to page that will
+	 * have its permissions manipulated.
+	 */
+
+ printf("Set up error injection on virt 0x%lx and press any key to continue test ...\n", data_start);
+	getchar();
+	mce_op.value = MAGIC;
+	mce_op.addr = data_start;
+	mce_op.delay_cycles = 600000000;
+	mce_op.header.type = ENCL_OP_MCE;
+
+	EXPECT_EQ(ENCL_CALL(&mce_op, &self->run, true), 0);
+
+	EXPECT_EEXIT(&self->run);
+	EXPECT_EQ(self->run.exception_vector, 0);
+	EXPECT_EQ(self->run.exception_error_code, 0);
+	EXPECT_EQ(self->run.exception_addr, 0);
+}
+
 TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
index 4fca01cfd898..223a80529ba6 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -11,6 +11,11 @@
  */
 static uint8_t encl_buffer[8192] = { 1 };

+static inline void clflush(volatile void *__p)
+{
+	asm volatile("clflush %0" : "+m" (*(volatile char __force *)__p));
+}
+
 static void *memcpy(void *dest, const void *src, size_t n)
 {
 	size_t i;
@@ -35,6 +40,30 @@ static void do_encl_op_get_from_buf(void *op)
 	memcpy(&op2->value, &encl_buffer[0], 8);
 }

+static __always_inline unsigned long long read_tsc(void)
+{
+	unsigned long low, high;
+	asm volatile("rdtsc" : "=a" (low), "=d" (high) :: "ecx");
+	return (low | high << 32);
+}
+
+static __always_inline void rep_nop(void)
+{
+	asm volatile("rep;nop": : :"memory");
+}
+
+void delay_mce(unsigned cycles)
+{
+	unsigned long long start, now;
+	start = read_tsc();
+	for (;;) {
+		now = read_tsc();
+		if (now - start >= cycles)
+			break;
+		rep_nop();
+	}
+}
+
 static void do_encl_op_put_to_addr(void *_op)
 {
 	struct encl_op_put_to_addr *op = _op;
@@ -49,6 +78,17 @@ static void do_encl_op_get_from_addr(void *_op)
 	memcpy(&op->value, (void *)op->addr, 8);
 }

+static void do_encl_op_mce(void *_op)
+{
+	struct encl_op_mce *op = _op;
+
+	memcpy((void *)op->addr, &op->value, 8);
+	clflush((void *)op->addr);
+	delay_mce(op->delay_cycles);
+	memcpy(&op->value, (void *)op->addr, 8);
+}
+
+
 static void do_encl_op_nop(void *_op)
 {

@@ -62,6 +102,7 @@ void encl_body(void *rdi,  void *rsi)
 		do_encl_op_put_to_addr,
 		do_encl_op_get_from_addr,
 		do_encl_op_nop,
+		do_encl_op_mce,
 	};

 	struct encl_op_header *op = (struct encl_op_header *)rdi;



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux