tboot-devel — Developer support

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Łukasz H. <lu...@ha...>]

On Sat, 2022-03-12 at 09:34 +0200, Timo Lindfors wrote:
> On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> > I see that you have quite complex environment for testing tboot, if I
> > find my old GRUB patch and prepare patch for tboot that combined should
> > fix the issue, will you be able to run tests?
> 
> Yes, I'm happy to run tests :)

I am attaching two patches:

GRUB:
multiboot2__Implement_the_new_module_load_and_preferences_tag.patch

tboot:
Use_multiboot2_module_load_preference_tag.patch

On my test platform they fix the issue. Please check how they work on
your environment.

Thanks,
Lukasz

Re: [tboot-devel] suspend problem since kernel 5.15

[Derek D. <z2...@po...>]

I did a git bisect on the Linux kernel and found that the commit below
is the one that breaks tboot+suspend to ram. It is part of a series of
some cpu hotplug commits. I don't know how this would affect tboot
suspend but I paste the patch below hoping that maybe you knowledgeable
devs would get some ideas just by seeing it.

Just to be clear: if I build a kernel from the commit just before this
one I can suspend and resume, but if I build with this commit I can not
suspend, laptop gets stuck on blinking power LED. (Note I did this
bisect using tboot 1.10.5).

------------------------------------------------
commit 453e41085183980087f8a80dada523caf1131c3c (HEAD, refs/bisect/bad)
Author: Vincent Donnefort <vin...@ar...>
Date:   Tue Feb 16 10:35:06 2021 +0000

    cpu/hotplug: Add cpuhp_invoke_callback_range()

    Factorizing and unifying cpuhp callback range invocations,
especially for
    the hotunplug path, where two different ways of decrementing were
used. The
    first one, decrements before the callback is called:

     cpuhp_thread_fun()
         state = st->state;
         st->state--;
         cpuhp_invoke_callback(state);

    The second one, after:

     take_down_cpu()|cpuhp_down_callbacks()
         cpuhp_invoke_callback(st->state);
         st->state--;

    This is problematic for rolling back the steps in case of error, as
    depending on the decrement, the rollback will start from N or N-1.
It also
    makes tracing inconsistent, between steps run in the cpuhp thread and
    the others.

    Additionally, avoid useless cpuhp_thread_fun() loops by skipping empty
    steps.

    Signed-off-by: Vincent Donnefort <vin...@ar...>
    Signed-off-by: Peter Zijlstra (Intel) <pe...@in...>
    Signed-off-by: Ingo Molnar <mi...@ke...>
    Link:
https://lkml.kernel.org/r/202...@ar...
--------------------------------------------------------------

***
*** This is what the problem commit has done:
***

diff --git b/kernel/cpu.c a/kernel/cpu.c
index 680ed8f427c0..23505d6abd45 100644
--- b/kernel/cpu.c
+++ a/kernel/cpu.c
@@ -135,6 +135,11 @@ static struct cpuhp_step *cpuhp_get_step(enum
cpuhp_state state)
 	return cpuhp_hp_states + state;
 }

+static bool cpuhp_step_empty(bool bringup, struct cpuhp_step *step)
+{
+	return bringup ? !step->startup.single : !step->teardown.single;
+}
+
 /**
  * cpuhp_invoke_callback _ Invoke the callbacks for a given state
  * @cpu:	The cpu for which the callback should be invoked
@@ -157,26 +162,24 @@ static int cpuhp_invoke_callback(unsigned int cpu,
enum cpuhp_state state,

 	if (st->fail == state) {
 		st->fail = CPUHP_INVALID;
-
-		if (!(bringup ? step->startup.single : step->teardown.single))
-			return 0;
-
 		return -EAGAIN;
 	}

+	if (cpuhp_step_empty(bringup, step)) {
+		WARN_ON_ONCE(1);
+		return 0;
+	}
+
 	if (!step->multi_instance) {
 		WARN_ON_ONCE(lastp && *lastp);
 		cb = bringup ? step->startup.single : step->teardown.single;
-		if (!cb)
-			return 0;
+
 		trace_cpuhp_enter(cpu, st->target, state, cb);
 		ret = cb(cpu);
 		trace_cpuhp_exit(cpu, st->state, state, ret);
 		return ret;
 	}
 	cbm = bringup ? step->startup.multi : step->teardown.multi;
-	if (!cbm)
-		return 0;

 	/* Single invocation for instance add/remove */
 	if (node) {
@@ -475,6 +478,15 @@ cpuhp_set_state(struct cpuhp_cpu_state *st, enum
cpuhp_state target)
 static inline void
 cpuhp_reset_state(struct cpuhp_cpu_state *st, enum cpuhp_state prev_state)
 {
+	st->target = prev_state;
+
+	/*
+	 * Already rolling back. No need invert the bringup value or to change
+	 * the current state.
+	 */
+	if (st->rollback)
+		return;
+
 	st->rollback = true;

 	/*
@@ -488,7 +500,6 @@ cpuhp_reset_state(struct cpuhp_cpu_state *st, enum
cpuhp_state prev_state)
 			st->state++;
 	}

-	st->target = prev_state;
 	st->bringup = !st->bringup;
 }

@@ -591,10 +602,53 @@ static int finish_cpu(unsigned int cpu)
  * Hotplug state machine related functions
  */

-static void undo_cpu_up(unsigned int cpu, struct cpuhp_cpu_state *st)
+/*
+ * Get the next state to run. Empty ones will be skipped. Returns true if a
+ * state must be run.
+ *
+ * st->state will be modified ahead of time, to match state_to_run, as
if it
+ * has already ran.
+ */
+static bool cpuhp_next_state(bool bringup,
+			     enum cpuhp_state *state_to_run,
+			     struct cpuhp_cpu_state *st,
+			     enum cpuhp_state target)
 {
-	for (st->state--; st->state > st->target; st->state--)
-		cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
+	do {
+		if (bringup) {
+			if (st->state >= target)
+				return false;
+
+			*state_to_run = ++st->state;
+		} else {
+			if (st->state <= target)
+				return false;
+
+			*state_to_run = st->state--;
+		}
+
+		if (!cpuhp_step_empty(bringup, cpuhp_get_step(*state_to_run)))
+			break;
+	} while (true);
+
+	return true;
+}
+
+static int cpuhp_invoke_callback_range(bool bringup,
+				       unsigned int cpu,
+				       struct cpuhp_cpu_state *st,
+				       enum cpuhp_state target)
+{
+	enum cpuhp_state state;
+	int err = 0;
+
+	while (cpuhp_next_state(bringup, &state, st, target)) {
+		err = cpuhp_invoke_callback(cpu, state, bringup, NULL, NULL);
+		if (err)
+			break;
+	}
+
+	return err;
 }

 static inline bool can_rollback_cpu(struct cpuhp_cpu_state *st)
@@ -617,16 +671,12 @@ static int cpuhp_up_callbacks(unsigned int cpu,
struct cpuhp_cpu_state *st,
 	enum cpuhp_state prev_state = st->state;
 	int ret = 0;

-	while (st->state < target) {
-		st->state++;
-		ret = cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
-		if (ret) {
-			if (can_rollback_cpu(st)) {
-				st->target = prev_state;
-				undo_cpu_up(cpu, st);
-			}
-			break;
-		}
+	ret = cpuhp_invoke_callback_range(true, cpu, st, target);
+	if (ret) {
+		cpuhp_reset_state(st, prev_state);
+		if (can_rollback_cpu(st))
+			WARN_ON(cpuhp_invoke_callback_range(false, cpu, st,
+							    prev_state));
 	}
 	return ret;
 }
@@ -690,17 +740,9 @@ static void cpuhp_thread_fun(unsigned int cpu)
 		state = st->cb_state;
 		st->should_run = false;
 	} else {
-		if (bringup) {
-			st->state++;
-			state = st->state;
-			st->should_run = (st->state < st->target);
-			WARN_ON_ONCE(st->state > st->target);
-		} else {
-			state = st->state;
-			st->state--;
-			st->should_run = (st->state > st->target);
-			WARN_ON_ONCE(st->state < st->target);
-		}
+		st->should_run = cpuhp_next_state(bringup, &state, st, st->target);
+		if (!st->should_run)
+			goto end;
 	}

 	WARN_ON_ONCE(!cpuhp_is_ap_state(state));
@@ -728,6 +770,7 @@ static void cpuhp_thread_fun(unsigned int cpu)
 		st->should_run = false;
 	}

+end:
 	cpuhp_lock_release(bringup);
 	lockdep_release_cpus_lock();

@@ -881,19 +924,18 @@ static int take_cpu_down(void *_param)
 		return err;

 	/*
-	 * We get here while we are in CPUHP_TEARDOWN_CPU state and we must not
-	 * do this step again.
+	 * Must be called from CPUHP_TEARDOWN_CPU, which means, as we are going
+	 * down, that the current state is CPUHP_TEARDOWN_CPU - 1.
 	 */
-	WARN_ON(st->state != CPUHP_TEARDOWN_CPU);
-	st->state--;
+	WARN_ON(st->state != (CPUHP_TEARDOWN_CPU - 1));
+
 	/* Invoke the former CPU_DYING callbacks */
-	for (; st->state > target; st->state--) {
-		ret = cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
-		/*
-		 * DYING must not fail!
-		 */
-		WARN_ON_ONCE(ret);
-	}
+	ret = cpuhp_invoke_callback_range(false, cpu, st, target);
+
+	/*
+	 * DYING must not fail!
+	 */
+	WARN_ON_ONCE(ret);

 	/* Give up timekeeping duties */
 	tick_handover_do_timer();
@@ -975,27 +1017,22 @@ void cpuhp_report_idle_dead(void)
 				 cpuhp_complete_idle_dead, st, 0);
 }

-static void undo_cpu_down(unsigned int cpu, struct cpuhp_cpu_state *st)
-{
-	for (st->state++; st->state < st->target; st->state++)
-		cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
-}
-
 static int cpuhp_down_callbacks(unsigned int cpu, struct
cpuhp_cpu_state *st,
 				enum cpuhp_state target)
 {
 	enum cpuhp_state prev_state = st->state;
 	int ret = 0;

-	for (; st->state > target; st->state--) {
-		ret = cpuhp_invoke_callback(cpu, st->state, false, NULL, NULL);
-		if (ret) {
-			st->target = prev_state;
-			if (st->state < prev_state)
-				undo_cpu_down(cpu, st);
-			break;
-		}
+	ret = cpuhp_invoke_callback_range(false, cpu, st, target);
+	if (ret) {
+
+		cpuhp_reset_state(st, prev_state);
+
+		if (st->state < prev_state)
+			WARN_ON(cpuhp_invoke_callback_range(true, cpu, st,
+							    prev_state));
 	}
+
 	return ret;
 }

@@ -1168,14 +1205,12 @@ void notify_cpu_starting(unsigned int cpu)

 	rcu_cpu_starting(cpu);	/* Enables RCU usage on this CPU. */
 	cpumask_set_cpu(cpu, &cpus_booted_once_mask);
-	while (st->state < target) {
-		st->state++;
-		ret = cpuhp_invoke_callback(cpu, st->state, true, NULL, NULL);
-		/*
-		 * STARTING must not fail!
-		 */
-		WARN_ON_ONCE(ret);
-	}
+	ret = cpuhp_invoke_callback_range(true, cpu, st, target);
+
+	/*
+	 * STARTING must not fail!
+	 */
+	WARN_ON_ONCE(ret);
 }

 /*
@@ -1781,8 +1816,7 @@ static int cpuhp_issue_call(int cpu, enum
cpuhp_state state, bool bringup,
 	 * If there's nothing to do, we done.
 	 * Relies on the union for multi_instance.
 	 */
-	if ((bringup && !sp->startup.single) ||
-	    (!bringup && !sp->teardown.single))
+	if (cpuhp_step_empty(bringup, sp))
 		return 0;
 	/*
 	 * The non AP bound callbacks can fail on bringup. On teardown

[tboot-devel] [PATCH 1 of 3] Ignore modules that overlap with internal data structures

[Timo L. <tim...@ik...>]

# HG changeset patch
# User Timo Lindfors <tim...@ik...>
# Date 1647554321 -7200
#      Thu Mar 17 23:58:41 2022 +0200
# Node ID 94606b71c3dabaabee813971a223686257d65d52
# Parent  bcdf58c1d076bfe41c58ac93254914dc0f2fd449
Ignore modules that overlap with internal data structures
Without this patch the system can go to an infinite reboot loop as
corrupted module causes the system to reset.

Signed-off-by: Timo Lindfors <tim...@ik...>

diff -r bcdf58c1d076 -r 94606b71c3da include/config.h
--- a/include/config.h	Thu Mar 10 10:28:11 2022 +0200
+++ b/include/config.h	Thu Mar 17 23:58:41 2022 +0200
@@ -52,9 +52,10 @@
 
 /* these addrs must be in low memory so that they are mapped by the */
 /* kernel at startup */
+#define TBOOT_LOWMEM_START           0x60000
 
 /* address/size for memory-resident serial log (when enabled) */
-#define TBOOT_SERIAL_LOG_ADDR        0x60000
+#define TBOOT_SERIAL_LOG_ADDR        TBOOT_LOWMEM_START
 #define TBOOT_SERIAL_LOG_SIZE        0x08000
 
 /* address/size for modified e820 table */
@@ -72,6 +73,8 @@
                                       TBOOT_EFI_MEMMAP_COPY_SIZE)
 #define TBOOT_KERNEL_CMDLINE_SIZE    0x0400
 
+#define TBOOT_LOWMEM_END             (TBOOT_KERNEL_CMDLINE_ADDR + \
+                                      TBOOT_KERNEL_CMDLINE_SIZE)
 
 #ifndef NR_CPUS
 #define NR_CPUS     1024
diff -r bcdf58c1d076 -r 94606b71c3da tboot/common/loader.c
--- a/tboot/common/loader.c	Thu Mar 10 10:28:11 2022 +0200
+++ b/tboot/common/loader.c	Thu Mar 17 23:58:41 2022 +0200
@@ -1807,6 +1807,42 @@
 }
 
 /*
+ * Check if two memory regions overlap
+ */
+static bool
+regions_overlap(const void *base1, size_t size1, const void *base2, size_t size2) {
+    /*
+      11111
+        22222
+    */
+    if (base1 <= base2 && base2 < base1 + size1) {
+        return true;
+    }
+    /*
+        11111
+      22222
+    */
+    if (base2 <= base1 && base1 < base2 + size2) {
+        return true;
+    }
+    /*
+        1
+      22222
+    */
+    if (base2 <= base1 && base1 + size1 < base2 + size2) {
+        return true;
+    }
+    /*
+      11111
+        2
+    */
+    if (base1 <= base2 && base2 + size2 < base1 + size1) {
+        return true;
+    }
+    return false;
+}
+
+/*
  * will go through all modules to find an SINIT that matches the platform
  * (size can be NULL)
  */
@@ -1836,6 +1872,11 @@
 
         void *base2 = (void *)m->mod_start;
         uint32_t size2 = m->mod_end - (unsigned long)(base2);
+        if (regions_overlap(base2, size2,
+                            (void*)TBOOT_LOWMEM_START, TBOOT_LOWMEM_END - TBOOT_LOWMEM_START)) {
+            printk(TBOOT_DETA "Ignoring module as it overlaps with tboot's internal data structures\n");
+            continue;
+        }
         if ( is_sinit_acmod(base2, size2, false) &&
              does_acmod_match_platform((acm_hdr_t *)base2, NULL) ) {
             if ( base != NULL )

[tboot-devel] [PATCH 3 of 3] Allow selecting only SINIT modules that match platform

[Timo L. <tim...@ik...>]

# HG changeset patch
# User Timo Lindfors <tim...@ik...>
# Date 1647554330 -7200
#      Thu Mar 17 23:58:50 2022 +0200
# Node ID 538c14b1428d0625ebb3f9c3cae21656fd4c3b06
# Parent  e45ccbe6bf59ba534ad628f7be45e7c34629e19b
Allow selecting only SINIT modules that match platform
This introduces GRUB_TBOOT_SINIT_SELECT_MATCHING that defaults to
false.

Signed-off-by: Timo Lindfors <tim...@ik...>

diff -r e45ccbe6bf59 -r 538c14b1428d tboot/20_linux_tboot
--- a/tboot/20_linux_tboot	Thu Mar 17 23:58:45 2022 +0200
+++ b/tboot/20_linux_tboot	Thu Mar 17 23:58:50 2022 +0200
@@ -40,6 +40,7 @@
 [ -z "${GRUB_CMDLINE_LINUX_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
 [ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
+[ -z "${GRUB_TBOOT_SINIT_SELECT_MATCHING}" ] && unset GRUB_TBOOT_SINIT_SELECT_MATCHING
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Linux kernel parameters to append for tboot
@@ -48,6 +49,8 @@
 : ${GRUB_TBOOT_POLICY_DATA=''}
 # List of SINIT modules to use, glob patterns are supported
 : ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
+# Use only SINIT modules that match the current platform
+: ${GRUB_TBOOT_SINIT_SELECT_MATCHING='false'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -164,8 +167,16 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
+if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] && [ ! -e /dev/cpu/0/msr ]; then
+  modprobe msr
+fi
 sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
+        if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] \
+            && ! txt-acminfo "$i" | grep -qx "ACM matches platform"; then
+          # Skip SINIT that does not match
+          continue
+        fi
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`
 if [ -n "${GRUB_TBOOT_POLICY_DATA}" ]; then
diff -r e45ccbe6bf59 -r 538c14b1428d tboot/20_linux_xen_tboot
--- a/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:45 2022 +0200
+++ b/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:50 2022 +0200
@@ -41,6 +41,7 @@
 [ -z "${GRUB_CMDLINE_LINUX_XEN_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_XEN_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
 [ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
+[ -z "${GRUB_TBOOT_SINIT_SELECT_MATCHING}" ] && unset GRUB_TBOOT_SINIT_SELECT_MATCHING
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Xen parameters to append for tboot
@@ -51,6 +52,8 @@
 : ${GRUB_TBOOT_POLICY_DATA=''}
 # List of SINIT modules to use, glob patterns are supported
 : ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
+# Use only SINIT modules that match the current platform
+: ${GRUB_TBOOT_SINIT_SELECT_MATCHING='false'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -194,8 +197,16 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
+if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] && [ ! -e /dev/cpu/0/msr ]; then
+  modprobe msr
+fi
 sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
+        if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] \
+            && ! txt-acminfo "$i" | grep -qx "ACM matches platform"; then
+          # Skip SINIT that does not match
+          continue
+        fi
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`
 if [ -n "${GRUB_TBOOT_POLICY_DATA}" ]; then

[tboot-devel] [PATCH 2 of 3] Introduce GRUB_TBOOT_SINIT_LIST for selecting SINIT modules to use

[Timo L. <tim...@ik...>]

# HG changeset patch
# User Timo Lindfors <tim...@ik...>
# Date 1647554325 -7200
#      Thu Mar 17 23:58:45 2022 +0200
# Node ID e45ccbe6bf59ba534ad628f7be45e7c34629e19b
# Parent  94606b71c3dabaabee813971a223686257d65d52
Introduce GRUB_TBOOT_SINIT_LIST for selecting SINIT modules to use
Signed-off-by: Timo Lindfors <tim...@ik...>

diff -r 94606b71c3da -r e45ccbe6bf59 tboot/20_linux_tboot
--- a/tboot/20_linux_tboot	Thu Mar 17 23:58:41 2022 +0200
+++ b/tboot/20_linux_tboot	Thu Mar 17 23:58:45 2022 +0200
@@ -39,12 +39,15 @@
 [ -z "${GRUB_CMDLINE_TBOOT}" ] && unset GRUB_CMDLINE_TBOOT
 [ -z "${GRUB_CMDLINE_LINUX_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
+[ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Linux kernel parameters to append for tboot
 : ${GRUB_CMDLINE_LINUX_TBOOT='intel_iommu=on'}
 # Base name of LCP policy data file for list policy
 : ${GRUB_TBOOT_POLICY_DATA=''}
+# List of SINIT modules to use, glob patterns are supported
+: ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -161,7 +164,7 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
-sinit_list=`for i in /boot/*sinit* /boot/*SINIT*; do
+sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`
diff -r 94606b71c3da -r e45ccbe6bf59 tboot/20_linux_xen_tboot
--- a/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:41 2022 +0200
+++ b/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:45 2022 +0200
@@ -40,6 +40,7 @@
 [ -z "${GRUB_CMDLINE_XEN_TBOOT}" ] && unset GRUB_CMDLINE_XEN_TBOOT
 [ -z "${GRUB_CMDLINE_LINUX_XEN_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_XEN_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
+[ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Xen parameters to append for tboot
@@ -48,6 +49,8 @@
 : ${GRUB_CMDLINE_LINUX_XEN_TBOOT=''}
 # Base name of LCP policy data file for list policy
 : ${GRUB_TBOOT_POLICY_DATA=''}
+# List of SINIT modules to use, glob patterns are supported
+: ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -191,7 +194,7 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
-sinit_list=`for i in /boot/*sinit* /boot/*SINIT*; do
+sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Timo L. <tim...@ik...>]

On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> I see that you have quite complex environment for testing tboot, if I
> find my old GRUB patch and prepare patch for tboot that combined should
> fix the issue, will you be able to run tests?

Yes, I'm happy to run tests :)

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Łukasz H. <lu...@ha...>]

On Fri, 2022-03-11 at 11:23 +0200, Timo Lindfors wrote:
> Hi,
> 
> On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> > In few words - when multiple SINITs is loaded, there is a chance that
> > one (or more) of them will be overwritten by some TBOOT data structures
> > that have hardcoded addresses. In most cases it is memory log, but this
> > is not a rule.
> 
> This sounds annoying indeed. Would it help if we could somehow embed 
> or append the SINIT modules to tboot.gz itself? I'm trying to make the 
> technology as easy to use and robust as possible. Being able to use e.g. 
> the same Live CD on all pieces of hardware would be a huge win.
> 

That could help, but I can't give you a definite answer. There is also
a risk that bigger tboot.gz will cause some other memory corruption
errors. Keep in mind that besides tboot.gz and SINITs, GRUB loads also
kernel image and initrd. The best way to fix all possible problems is
to instruct GRUB not to load anything in memory regions occupied by
tboot's hardcoded structures.

I see that you have quite complex environment for testing tboot, if I
find my old GRUB patch and prepare patch for tboot that combined should
fix the issue, will you be able to run tests?

Thanks,
Lukasz

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Łukasz H. <lu...@ha...>]

Hi Timo

On Fri, 2022-03-11 at 09:09 +0200, Timo Lindfors wrote:
> Hi,
> 
> in https://sourceforge.net/p/tboot/mailman/message/37340469/ there was a 
> discussion about needing to get grub to accept a patch to reliably support 
> multiple SINIT modules. Any idea what's the status of this patch?
> 
> Using multiple SINIT modules is useful if you want to have a single image 
> that works on multiple devices. The intel-acm package in Debian non-free 
> provides these in /boot and it is very convenient that tboot can 
> choose the matching SINIT module at runtime.

As I left Intel and nobody has taken care about this patch, it has been
abandoned. As far as I remember, there were some minor changes
requested by GRUB maintainers, but overall idea has been accepted.

> 
> I was reminded of this issue since I hit it again on different hardware.
> 
> I've attached two serial console logs for tboot mercurial tip 
> (9c625ab2035b):
> 
> tboot_9c625ab2035b_2_SINIT_working.txt:
> - two SINIT ACMs are specified and the system boots correctly.
> 
> tboot_9c625ab2035b_26_SINIT_reboot.txt:
> - 26 SINIT ACMs are specified and the system enters an infinïte reboot 
> loop.
> 
> I do not see this problem on my BIOS system, only UEFI system, but it is 
> is difficult to say if this is actually related to the issue.
> 
> You can see more logs at https://lindi.iki.fi/lindi/tboot/smoketest/results.html
> The attached logs are all from test run 1646942019.
> 

In few words - when multiple SINITs is loaded, there is a chance that
one (or more) of them will be overwritten by some TBOOT data structures
that have hardcoded addresses. In most cases it is memory log, but this
is not a rule.

Everything depends on system memory map and where GRUB decided to put
SINITs. On some platforms you can load as many SINTIs as you want, on
other - only 2 or 3. So that's platform specific issue, fortunately, I
didn't come across a platform where this problem happens even with 1
SINIT loaded.

I will try to find some time to dig into grub-devel archive, find the
patch, fix it and resubmit it once again. However, it depends on OS
vendors when they will merge it to their distro.

> As a workaround, would you accept a patch that modifies 
> tboot/20_linux_tboot to use txt-acminfo to include only matching SINIT 
> modules in grub configuration? I could make this configurable in 
> /etc/default/grub-tboot. We could for example support the following three 
> options:
> 
> GRUB_TBOOT_SINIT=all
> - include all SINIT modules that are found, current behavior
> 
> GRUB_TBOOT_SINIT=detect
> - use txt-acminfo to find SINIT modules that match the current system.
> 
> GRUB_TBOOT_SINIT_LIST="module1 module2 module3"
> - use only the listed SINIT modules.
> 

That's sounds great to me. I am sure that Intel will accept this
change. It is much better to select proper SINIT during installation
that loads all possible ones every boot, only to always choose the same
right one.

Thanks,
Lukasz

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Timo L. <tim...@ik...>]

Hi,

On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> In few words - when multiple SINITs is loaded, there is a chance that
> one (or more) of them will be overwritten by some TBOOT data structures
> that have hardcoded addresses. In most cases it is memory log, but this
> is not a rule.

This sounds annoying indeed. Would it help if we could somehow embed 
or append the SINIT modules to tboot.gz itself? I'm trying to make the 
technology as easy to use and robust as possible. Being able to use e.g. 
the same Live CD on all pieces of hardware would be a huge win.

-Timo

[tboot-devel] status of the grub patch to support multiple SINIT modules?

[Timo L. <tim...@ik...>]

Hi,

in https://sourceforge.net/p/tboot/mailman/message/37340469/ there was a 
discussion about needing to get grub to accept a patch to reliably support 
multiple SINIT modules. Any idea what's the status of this patch?

Using multiple SINIT modules is useful if you want to have a single image 
that works on multiple devices. The intel-acm package in Debian non-free 
provides these in /boot and it is very convenient that tboot can 
choose the matching SINIT module at runtime.

I was reminded of this issue since I hit it again on different hardware.

I've attached two serial console logs for tboot mercurial tip 
(9c625ab2035b):

tboot_9c625ab2035b_2_SINIT_working.txt:
- two SINIT ACMs are specified and the system boots correctly.

tboot_9c625ab2035b_26_SINIT_reboot.txt:
- 26 SINIT ACMs are specified and the system enters an infinïte reboot 
loop.

I do not see this problem on my BIOS system, only UEFI system, but it is 
is difficult to say if this is actually related to the issue.

You can see more logs at https://lindi.iki.fi/lindi/tboot/smoketest/results.html
The attached logs are all from test run 1646942019.

As a workaround, would you accept a patch that modifies 
tboot/20_linux_tboot to use txt-acminfo to include only matching SINIT 
modules in grub configuration? I could make this configurable in 
/etc/default/grub-tboot. We could for example support the following three 
options:

GRUB_TBOOT_SINIT=all
- include all SINIT modules that are found, current behavior

GRUB_TBOOT_SINIT=detect
- use txt-acminfo to find SINIT modules that match the current system.

GRUB_TBOOT_SINIT_LIST="module1 module2 module3"
- use only the listed SINIT modules.

-Timo

Re: [tboot-devel] [PATCH] txt-acminfo: Map TXT heap using mmap

[Timo L. <tim...@ik...>]

On Wed, 9 Mar 2022, Timo Juhani Lindfors wrote:
> From: Timo Lindfors <tim...@ik...>
>
> Without this patch
>
> txt-acminfo 5th_gen_i5_i7_SINIT_79.BIN
>
> segfaults. This issue was introduced in

This is not the most beautiful patch I must say but I hope it at least 
helps somewhat and does not break anything. I tried adjusting the headers 
so that the duplicate typedef for txt_heap_t could be avoided. I also 
looked the possibility to have callers of does_acmod_match_platform call 
get_txt_heap. In both cases I hit all sorts of challenges with the way the 
include files are setup. So: feel free to rewrite this if you can come up 
with a cleaner way :)

-Timo

[tboot-devel] [PATCH] txt-acminfo: Map TXT heap using mmap

[Timo J. L. <tim...@ik...>]

From: Timo Lindfors <tim...@ik...>

Without this patch

txt-acminfo 5th_gen_i5_i7_SINIT_79.BIN

segfaults. This issue was introduced in

o  changeset:   627:d8a8e17f6d41
|  user:        Lukasz Hawrylko <lukas...@in...>
|  date:        Thu May 13 16:04:27 2021 +0200
|  summary:     Check for client/server match when selecting SINIT

Signed-off-by: Timo Lindfors <tim...@ik...>
---
 tboot/common/loader.c     |  4 ++--
 tboot/include/txt/acmod.h |  4 +++-
 tboot/txt/acmod.c         |  7 ++++---
 utils/txt-acminfo.c       | 26 +++++++++++++++++++++-----
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/tboot/common/loader.c b/tboot/common/loader.c
index cd254d7..0b9a814 100644
--- a/tboot/common/loader.c
+++ b/tboot/common/loader.c
@@ -1792,7 +1792,7 @@ find_platform_racm(loader_ctx *lctx, void **base, uint32_t *size)
         void *base2 = (void *)m->mod_start;
         uint32_t size2 = m->mod_end - (unsigned long)(base2);
         if ( is_racm_acmod(base2, size2, false) &&
-             does_acmod_match_platform((acm_hdr_t *)base2) ) {
+             does_acmod_match_platform((acm_hdr_t *)base2, NULL) ) {
             if ( base != NULL )
                 *base = base2;
             if ( size != NULL )
@@ -1837,7 +1837,7 @@ find_platform_sinit_module(loader_ctx *lctx, void **base, uint32_t *size)
         void *base2 = (void *)m->mod_start;
         uint32_t size2 = m->mod_end - (unsigned long)(base2);
         if ( is_sinit_acmod(base2, size2, false) &&
-             does_acmod_match_platform((acm_hdr_t *)base2) ) {
+             does_acmod_match_platform((acm_hdr_t *)base2, NULL) ) {
             if ( base != NULL )
                 *base = base2;
             if ( size != NULL )
diff --git a/tboot/include/txt/acmod.h b/tboot/include/txt/acmod.h
index 23336c2..fc60d9d 100644
--- a/tboot/include/txt/acmod.h
+++ b/tboot/include/txt/acmod.h
@@ -37,6 +37,8 @@
 #ifndef __TXT_ACMOD_H__
 #define __TXT_ACMOD_H__
 
+typedef void   txt_heap_t;
+
 /*
  * authenticated code (AC) module header (ver 0.0)
  */
@@ -179,7 +181,7 @@ extern bool is_racm_acmod(const void *acmod_base, uint32_t acmod_size, bool quie
 extern acm_hdr_t *copy_racm(const acm_hdr_t *racm);
 extern bool verify_racm(const acm_hdr_t *acm_hdr);
 extern bool is_sinit_acmod(const void *acmod_base, uint32_t acmod_size, bool quiet);
-extern bool does_acmod_match_platform(const acm_hdr_t* hdr);
+extern bool does_acmod_match_platform(const acm_hdr_t* hdr, const txt_heap_t* txt_heap);
 extern acm_hdr_t *copy_sinit(const acm_hdr_t *sinit);
 extern bool verify_acmod(const acm_hdr_t *acm_hdr);
 extern uint32_t get_supported_os_sinit_data_ver(const acm_hdr_t* hdr);
diff --git a/tboot/txt/acmod.c b/tboot/txt/acmod.c
index 1e92efa..798093f 100644
--- a/tboot/txt/acmod.c
+++ b/tboot/txt/acmod.c
@@ -576,7 +576,7 @@ bool is_sinit_acmod(const void *acmod_base, uint32_t acmod_size, bool quiet)
     return true;
 }
 
-bool does_acmod_match_platform(const acm_hdr_t* hdr)
+bool does_acmod_match_platform(const acm_hdr_t* hdr, const txt_heap_t *txt_heap)
 {
     /* used to ensure we don't print chipset/proc info for each module */
     static bool printed_host_info;
@@ -587,7 +587,8 @@ bool does_acmod_match_platform(const acm_hdr_t* hdr)
         return false;
 
     /* verify client/server platform match */
-    txt_heap_t *txt_heap = get_txt_heap();
+    if (txt_heap == NULL)
+        txt_heap = get_txt_heap();
     bios_data_t *bios_data = get_bios_data_start(txt_heap);
     if (info_table->version >= 5 && bios_data->version >= 6) {
         uint32_t bios_type = bios_data->flags.bits.mle.platform_type;
@@ -713,7 +714,7 @@ acm_hdr_t *get_bios_sinit(const void *sinit_region_base)
 
     /* is it a valid SINIT module? */
     if ( !is_sinit_acmod(sinit_region_base, bios_data->bios_sinit_size, false) ||
-         !does_acmod_match_platform((acm_hdr_t *)sinit_region_base) )
+         !does_acmod_match_platform((acm_hdr_t *)sinit_region_base, NULL) )
         return NULL;
 
     return (acm_hdr_t *)sinit_region_base;
diff --git a/utils/txt-acminfo.c b/utils/txt-acminfo.c
index 06a5ee8..5635e27 100644
--- a/utils/txt-acminfo.c
+++ b/utils/txt-acminfo.c
@@ -203,15 +203,31 @@ static bool match_platform(acm_hdr_t *hdr)
         close(fd_mem);
         return false;
     }
-    else {
-        if ( does_acmod_match_platform(hdr) )
-            printf("ACM matches platform\n");
-        else
-            printf("ACM does not match platform\n");
 
+    uint64_t txt_heap_size = *(volatile uint64_t *)(pub_config_base + TXTCR_HEAP_SIZE);
+    if (txt_heap_size == 0) {
+        printf("ERROR: No TXT heap is available\n");
         munmap(pub_config_base, TXT_CONFIG_REGS_SIZE);
+        close(fd_mem);
+        return false;
+    }
+
+    uint64_t txt_heap_base = *(volatile uint64_t *)(pub_config_base + TXTCR_HEAP_BASE);
+    txt_heap_t *txt_heap = mmap(NULL, txt_heap_size, PROT_READ, MAP_PRIVATE,
+                                fd_mem, txt_heap_base);
+    if ( txt_heap == MAP_FAILED ) {
+        printf("ERROR: cannot map TXT heap by mmap()\n");
+        munmap(pub_config_base, TXT_CONFIG_REGS_SIZE);
+        close(fd_mem);
+        return false;
     }
+    if ( does_acmod_match_platform(hdr, txt_heap) )
+        printf("ACM matches platform\n");
+    else
+        printf("ACM does not match platform\n");
 
+    munmap(txt_heap, txt_heap_size);
+    munmap(pub_config_base, TXT_CONFIG_REGS_SIZE);
     close(fd_mem);
     return true;
 }
-- 
2.30.2

Re: [tboot-devel] typo in lcp2_crtpollist man page

[Randzio, P. <paw...@in...>]

> just a quick note: lcp2_crtpollist man page says "algorightm" but should probably say "algorithm". Thought about sending a patch but it's probably easier for you to just fix this directly :)
Fixed ;)

-Paweł
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

[tboot-devel] typo in lcp2_crtpollist man page

[Timo L. <tim...@ik...>]

Hi,

just a quick note: lcp2_crtpollist man page says "algorightm" but should 
probably say "algorithm". Thought about sending a patch but it's probably 
easier for you to just fix this directly :)

-Timo

Re: [tboot-devel] new PGP for release tarballs?

[Randzio, P. <paw...@in...>]

> Good to know, can you release 1.10.5 so that I can upload it to Debian?
Done.

> Could you perhaps just put the public PGP key to version control if you cannot think of any other place? I don't see any way to ignore PGP validation in the workflow easily as only ".gpg" files are available and that's what the tooling tries to access.
Public key file uploaded to SF

-Paweł
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

Re: [tboot-devel] new PGP for release tarballs?

[Timo L. <tim...@ik...>]

On Wed, 9 Mar 2022, Randzio, Pawel wrote:
> First of all - package 1.10.4 is already outdated (I mistakenly made a release with bugs), I forgot to add packages for 1.10.5.
> Thanks for the reminder :)
>
> And as for the key - older packages were signed by Łukasz, and I think I haven't been instructed on how to or where to provide the key I've generated for myself.

Good to know, can you release 1.10.5 so that I can upload it to Debian?

Could you perhaps just put the public PGP key to version control if you 
cannot think of any other place? I don't see any way to ignore PGP 
validation in the workflow easily as only ".gpg" files are available and 
that's what the tooling tries to access.

-Timo

Re: [tboot-devel] new PGP for release tarballs?

[Randzio, P. <paw...@in...>]

Hi Timo,

First of all - package 1.10.4 is already outdated (I mistakenly made a release with bugs), I forgot to add packages for 1.10.5.
Thanks for the reminder :)

And as for the key - older packages were signed by Łukasz, and I think I haven't been instructed on how to or where to provide the key I've generated for myself.

-Paweł

-----Original Message-----
From: Timo Lindfors <tim...@ik...> 
Sent: Wednesday, March 9, 2022 10:34 AM
To: tbo...@li...
Subject: [tboot-devel] new PGP for release tarballs?

Hi,

I noticed that the tboot-1.10.4.tar.gz.gpg release is signed using key A16A6B495B7ED435EF129F21FF6B78E7EB6D8A8B. Is this key available from somewhere? Previously key 5CECC9E12872F424009D0E0B6F2F48CC4E0B23EF was used.

-Timo



_______________________________________________
tboot-devel mailing list
tbo...@li...
https://lists.sourceforge.net/lists/listinfo/tboot-devel
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

[tboot-devel] new PGP for release tarballs?

[Timo L. <tim...@ik...>]

Hi,

I noticed that the tboot-1.10.4.tar.gz.gpg release is signed using key 
A16A6B495B7ED435EF129F21FF6B78E7EB6D8A8B. Is this key available from 
somewhere? Previously key 5CECC9E12872F424009D0E0B6F2F48CC4E0B23EF was 
used.

-Timo

[tboot-devel] changeset 643:8635a40eab19 broke lcp2_mlehash

[Christopher B. <sal...@gm...>]

As of tboot 1.10.4, lcp2_mlehash has stopped working due to 
643:8635a40eab19 Touch ups for GCC12:


lcp2_mlehash --create --verbose /boot/tboot.gz
cmdline opt: mlefile:/boot/tboot.gz
[read_mle_file]
checking whether the file exists or not ... : existed!
trying to uncompress the file ... : succeeded!
creating a temporary file to uncompress ... : succeeded!
opening the decompressed file ... : succeeded!
testing decompression is ... : succeeded!
reading the decompressed file ... : succeeded!
[is_elf_image]
checking whether image is an elf image ... : failed! - Pointer is zero.

It looks like fix was designed to fix a use-after-free warning (which, 
due to -Werror is fatal), but the original code looks correct. I believe 
the gcc warning is bogus.

On a side note, could the -Werror flag be removed by default? Its useful 
for development but not in release versions: See 
https://flameeyes.blog/2009/02/25/future-proof-your-code-dont-use-werror/?utm_source=blogdoteu&utm_medium=referral&utm_campaign=permanent 

[tboot-devel] Patch: Add missing BE size_of_pcrs to hash buffer

[Christopher B. <sal...@gm...>]

First fix attempt was almost correct. System still doesn't boot with 
PCONF element. I forgot 1 important line though. Once added, the system 
now boots with the PCONF element. Tested with both 1 PCR and 2 PCRs.

# HG changeset patch
# User Christopher Byrne <sal...@gm...>
# Date 1645847383 21600
#      Fri Feb 25 21:49:43 2022 -0600
# Node ID ada51f373f12ee91feb2526220b6a94db92319ac
# Parent  6e831184f480ed34154e12eddc2d037d6a417a5f
lcptools-v2/pconf_legacy.c: Add missing BE size_of_pcrs to hash buffer

Signed-of-by: Christopher Byrne <sal...@gm...>

diff --git a/lcptools-v2/pconf_legacy.c b/lcptools-v2/pconf_legacy.c
--- a/lcptools-v2/pconf_legacy.c
+++ b/lcptools-v2/pconf_legacy.c
@@ -241,6 +241,7 @@
          ERROR("Error: failed to allocate buffer for composite digest.\n");
          return false;
      }
+    buff->size_of_pcrs = htonl(no_of_pcrs * SHA1_DIGEST_SIZE);
      memcpy_s(
          &buff->pcr_selection,
          sizeof buff->pcr_selection,

Re: [tboot-devel] suspend problem since kernel 5.15

[Derek D. <z2...@po...>]

On 2/1/22 5:25 AM, Łukasz Hawryłko wrote:
> Hi Derek
> 
> On Mon, 2022-01-31 at 17:26 +0000, Derek Dolney wrote:
>> I am using tboot 1.10.3 and all was working fine with Linux kernel
>> 5.10.88. However, I upgraded to kernel 5.15.16 and, while booting seems
>> to happen properly, suspend is broken. I am using a Lenovo T460p.
>> Usually when suspending the power button LED will blink 8 times and then
>> it goes into a sleep state. With the newer kernel I get power LED and
>> caps lock LED blinking, cpu fan runs fast, and can't get out of that
>> state. Need to hard powerdown.
>>
>> Attaching a txt-stat output. Any ideas what may be happening? Seems like
>> I maybe need to report to the kernel devs, but let me know if you have
>> some other suggestions. I could do a git bisect of the kernel source and
>> probably find the kernel code changes that broke suspend for me. Please
>> advise
> 
> Without serial console, I guess that as you have a laptop you don't
> have RS232 port, it will not be an easy task to debug the issue.
> 
> During S3 suspend Linux kernel is jumping to tboot's shutdown entry
> point to seal RAM content. You can try to comment out this behavior in
> Linux kernel to see if still you can see the issue. This experiment may
> tell us if hang is related to tboot's shutdown handler or not.
> 
> Lukasz
> 
Lukasz, thanks for the ideas. I tried commenting these 2 lines in
tboot_late_init():

// acpi_os_set_prepare_sleep(&tboot_sleep);
// acpi_os_set_prepare_extended_sleep(&tboot_extended_sleep);

Not entirely sure but my hope was that this would keep the kernel out of
tboot's shutdown handler. I don't know this code, please let me know if
you had something else in mind.

Anyway, it still hangs with these lines commented out. However, if I
boot this kernel (or the kernel with these lines not commented out)
without tboot, I can suspend and resume just fine.

Re: [tboot-devel] suspend problem since kernel 5.15

[Łukasz H. <lu...@ha...>]

Hi Derek

On Mon, 2022-01-31 at 17:26 +0000, Derek Dolney wrote:
> I am using tboot 1.10.3 and all was working fine with Linux kernel
> 5.10.88. However, I upgraded to kernel 5.15.16 and, while booting seems
> to happen properly, suspend is broken. I am using a Lenovo T460p.
> Usually when suspending the power button LED will blink 8 times and then
> it goes into a sleep state. With the newer kernel I get power LED and
> caps lock LED blinking, cpu fan runs fast, and can't get out of that
> state. Need to hard powerdown.
> 
> Attaching a txt-stat output. Any ideas what may be happening? Seems like
> I maybe need to report to the kernel devs, but let me know if you have
> some other suggestions. I could do a git bisect of the kernel source and
> probably find the kernel code changes that broke suspend for me. Please
> advise

Without serial console, I guess that as you have a laptop you don't
have RS232 port, it will not be an easy task to debug the issue.

During S3 suspend Linux kernel is jumping to tboot's shutdown entry
point to seal RAM content. You can try to comment out this behavior in
Linux kernel to see if still you can see the issue. This experiment may
tell us if hang is related to tboot's shutdown handler or not.

Lukasz

[tboot-devel] suspend problem since kernel 5.15

[Derek D. <z2...@po...>]

I am using tboot 1.10.3 and all was working fine with Linux kernel
5.10.88. However, I upgraded to kernel 5.15.16 and, while booting seems
to happen properly, suspend is broken. I am using a Lenovo T460p.
Usually when suspending the power button LED will blink 8 times and then
it goes into a sleep state. With the newer kernel I get power LED and
caps lock LED blinking, cpu fan runs fast, and can't get out of that
state. Need to hard powerdown.

Attaching a txt-stat output. Any ideas what may be happening? Seems like
I maybe need to report to the kernel devs, but let me know if you have
some other suggestions. I could do a git bisect of the kernel source and
probably find the kernel code changes that broke suspend for me. Please
advise

Thank you,
Derek

[tboot-devel] [PATCH] GCC update caused build error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow]

[Jun M. <jun...@in...>]

From: "Jun.Miao" <jun...@in...>

Add a CFLAGS "-Wo-stringop-overflow" to void build error in the fedora:36 by the
gcc -O2 optimization, which`s version: gcc (GCC) 12.0.1 20220118 (Red Hat 12.0.1-0).

build error log:
common/memlog.c: In function ‘memlog_init’:
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
   53 |        g_log->uuid = (uuid_t)TBOOT_LOG_UUID;
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:68:26: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
   68 |         g_log->zip_count = 0;
      |         ~~~~~~~~~~~~~~~~~^~~

Signed-off-by: Jun.Miao <jun...@in...>
---
 Config.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Config.mk b/Config.mk
index ed0aef0..b9342bc 100644
--- a/Config.mk
+++ b/Config.mk
@@ -20,6 +20,7 @@ include $(ROOTDIR)/Config.mk
 CFLAGS		:= $(shell echo $(CFLAGS) | sed -e s/-m64/-m32/)
 CFLAGS		+= -march=i686
 CFLAGS		+= -nostdinc
+CFLAGS         += -Wno-stringop-overflow
 CFLAGS		+= -fno-builtin -fno-common -fno-strict-aliasing
 CFLAGS		+= -fomit-frame-pointer
 CFLAGS		+= -pipe
-- 
2.35.0.rc2

[tboot-devel] [Pull Request] tboot/Config.mk: Add a CFLAGS "-Wo-stringop-overflow" to void build error

[Jun M. <jun...@in...>]

Summary: gcc update caused build error: 
	 writing 1 byte into a region of size 0 [-Werror=stringop-overflow]


Hi, Pawel

	Could you help me review this patch to fix this build error in fedora 36?
	Branch：tboot-1.10.3.tar.gpg

Thanks 
Jun.Miao