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

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony,

On 10/1/2021 9:47 AM, Tony Luck wrote:
> Now version 8
> 
> Note that linux-kernel@vger.kernel.org and x86@kernel.org 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@intel.com>

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@intel.com/

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