Implement a log buffer to store log messages. #52

vsntk18 · 2023-06-19T16:52:14Z

Implemented a global buffer to store log messages and passed this buffer from stage2 to svsm kernel. Replaced console logging with buffer logging.

Percpu line buffer will probably be necessary, and is not yet implemented. As a rust newbee, I appreciate your feedback on if I am using the right data structures in the right way. Thanks!

00xc · 2023-06-20T12:19:44Z

Probably a good idea to rebase from the main branch to fix the cargo fmt checks. Also, make sure your author email matches your Signed-off-by email!

EDIT: nevermind, no need for a rebase, but the fmt checks should be fixed.

vsntk18 · 2023-06-20T12:34:40Z

Probably a good idea to rebase from the main branch to fix the cargo fmt checks. Also, make sure your author email matches your Signed-off-by email!

EDIT: nevermind, no need for a rebase, but the fmt checks should be fixed.

sorry, will fix the fmt checks. I was under the impression that commit hook reports the deviations.

src/log_buffer.rs

00xc · 2023-06-20T12:51:32Z

sorry, will fix the fmt checks. I was under the impression that commit hook reports the deviations.

The git hook should warn you, but won't fix the issues automatically.

src/svsm.rs

vsntk18 · 2023-06-23T19:05:28Z

Reworked the findings so far.
Added percpu line buffer implementation.

vsntk18 · 2023-06-29T19:52:55Z

Status so far on this PR:

Implemented a global log buffer
Added initialization and migration code for the log buffer.
Implemented a percpu line buffer
Added a feature called enable-console-log to print log messages to console.

joergroedel

Provided some comments in the reviews. Please also move the logbuffer specific code into a separate sub-directory/module.

src/line_buffer.rs

src/log_buffer.rs

src/line_buffer.rs

vsntk18 · 2023-07-11T17:02:00Z

Provided some comments in the reviews. Please also move the logbuffer specific code into a separate sub-directory/module.

vsntk18 · 2023-07-11T17:09:41Z

In this PR:

Moved log buffer implementation to separate directory.
Moved line buffer implementation to src/cpu directory.
Reworked line buffer to use len member instead of head and tail. However kept the implementation as u8 array not as a FixedString, since it seemed simple. May be this is still arguable.
skipped all the intermediate utf8 checking as suggested by Carlos.

src/cpu/line_buffer.rs

src/string.rs

vsntk18 · 2024-06-06T17:22:15Z

Rebased the PR to the latest HEAD.

vsntk18 · 2024-06-10T08:46:34Z

Needs rebase due to percpu related changes in 7e5a9a7

00xc · 2024-06-17T13:59:59Z

kernel/src/log_buffer/mod.rs

+    }
+
+    pub fn write_log(&mut self, s: &FixedString<LINE_BUFFER_SIZE>) {
+        self.buf.write(&s.to_string());


This creates an intermediate allocation. Perhaps write() should take a generic char iterator which it can consume.

Changed write() interface to take char iterator as a parameter.

00xc · 2024-06-17T14:00:35Z

kernel/src/log_buffer/mod.rs

+    // A method used for testing
+    pub fn read_log(&mut self) -> Vec<u8> {


If it's only used for testing gate it behind #[cfg(test)] to avoid unused warnings.

00xc · 2024-06-17T14:02:47Z

kernel/src/log_buffer/mod.rs

+    pub unsafe fn migrate(&mut self, lb: *const SpinLock<LogBuffer>) {
+        unsafe {
+            self.buf = (*lb).lock().buf;
+        }
+    }


Why not pass a regular reference to the spinlock?

Yes, some compilation problems made me use that. But I have changed to use a regular reference now.

00xc · 2024-06-17T14:03:05Z

kernel/src/log_buffer/mod.rs

+    }
+}
+
+static mut LB: SpinLock<LogBuffer> = SpinLock::new(LogBuffer::new());


Why does it need to be mut?

Not necessary. Removed it. Thanks!

00xc · 2024-06-17T14:04:10Z

kernel/src/log_buffer/mod.rs

+#[cfg(test)]
+const BUF_SIZE: usize = 64;
+
+#[cfg(test)]
+use crate::types::LINE_BUFFER_SIZE;
+
+#[test]
+fn test_read_write_normal() {


All of these should be in a test module, e.g.

#[cfg(test)] mod test { use crate::types::LINE_BUFFER_SIZE; const BUF_SIZE: usize = 64; #[test] fn test_read_write_normal() { // ... } }

00xc · 2024-06-17T14:05:43Z

kernel/src/utils/string_ring_buffer.rs

+//
+// Copyright (c) 2022-2024 SUSE LLC
+//
+// Author: Roy Hopkins <roy.hopkins@suse.com>


Probably forgot to update author name :)

No, actually Roy created this file in one of the branches. I cherry picked it to my branch. So I will let it stay. :)

00xc · 2024-06-17T14:06:12Z

kernel/src/utils/string_ring_buffer.rs

+#[test]
+fn test_ring_one_string() {


This test and the ones below should also be in a test module.

00xc · 2024-06-17T14:07:45Z

kernel/src/cpu/line_buffer.rs

+
+    fn log(&self, record: &log::Record<'_>) {
+        let comp: &'static str = self.component;
+        let line_buf: &mut LineBuffer = &mut this_cpu().get_line_buffer();


There's no need to add the type here, RefMut implements Deref and DerefMut, so access to the LineBuffer methods is transparent.

00xc · 2024-06-17T14:08:57Z

kernel/src/cpu/line_buffer.rs

+pub fn install_buffer_logger(component: &'static str) {
+    BUFFER_LOGGER
+        .init(&BufferLogger::new(component))
+        .expect("already initialized the logger");


I'd say return the error here and panic at the caller, just like we had before.

The firmware field of GpaMap was initialized once but never read. This raises a warning in the last clippy release. Signed-off-by: Thomas Leroy <thomas.leroy@suse.com>

The log buffer is implemented using a ring buffer that contains the characters within log entries. This patch introduces a generic StringRingBuffer that can be used for safely managing the contents of strings within a ring buffer that is suitable for use with the log buffer. This separates the ring buffer implementation from the log and allows implementation of ring buffer specific unit testing, which is included within this patch. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de> Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>

memory. This log buffer is based on the string ring buffer in utils/StringRingBuffer. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

log messages. Each individual log message gets written into the global log buffer from the percpu line buffer. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

kernel. Introduce a new struct MigrateInfo which contains the data to be migrated from stage2 to svsm kernel. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

This feature can be enable to print log messages to console in addition to storing them in the log buffer. Enable this feature by default for debug builds. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

Test the log_buffer by calling methods like write_log() and read_log(). Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

00xc reviewed Jun 20, 2023

View reviewed changes

src/log_buffer.rs Outdated Show resolved Hide resolved

00xc reviewed Jun 20, 2023

View reviewed changes

src/log_buffer.rs Outdated Show resolved Hide resolved

00xc reviewed Jun 20, 2023

View reviewed changes

src/svsm.rs Outdated Show resolved Hide resolved

vsntk18 force-pushed the svsm_log_buffer branch 5 times, most recently from 26e9a9f to 5648746 Compare June 23, 2023 19:03

vsntk18 force-pushed the svsm_log_buffer branch 3 times, most recently from 87b4c26 to fc7ff02 Compare June 29, 2023 19:47

vsntk18 force-pushed the svsm_log_buffer branch 3 times, most recently from c51a833 to 97c017b Compare July 4, 2023 11:19

joergroedel requested changes Jul 4, 2023

View reviewed changes

src/line_buffer.rs Outdated Show resolved Hide resolved

src/line_buffer.rs Outdated Show resolved Hide resolved

src/log_buffer.rs Outdated Show resolved Hide resolved

00xc reviewed Jul 5, 2023

View reviewed changes

src/line_buffer.rs Outdated Show resolved Hide resolved

vsntk18 force-pushed the svsm_log_buffer branch from 97c017b to 5af0ce1 Compare July 11, 2023 17:00

vsntk18 closed this Jul 11, 2023

vsntk18 reopened this Jul 11, 2023

vsntk18 force-pushed the svsm_log_buffer branch from 5af0ce1 to 8a5d881 Compare August 9, 2023 18:16

00xc reviewed Aug 10, 2023

View reviewed changes

src/cpu/line_buffer.rs Outdated Show resolved Hide resolved

00xc reviewed Aug 10, 2023

View reviewed changes

src/string.rs Outdated Show resolved Hide resolved

00xc reviewed Aug 10, 2023

View reviewed changes

src/string.rs Outdated Show resolved Hide resolved

joergroedel added the wait-for-review PR needs for approval by reviewers label Feb 13, 2024

joergroedel self-assigned this Feb 16, 2024

joergroedel added needs-rebase The PR needs to be rebased to the latest upstream branch wait-for-review PR needs for approval by reviewers and removed wait-for-review PR needs for approval by reviewers labels Feb 16, 2024

vsntk18 force-pushed the svsm_log_buffer branch 2 times, most recently from ecd3216 to 8fce96e Compare February 19, 2024 11:02

joergroedel removed the needs-rebase The PR needs to be rebased to the latest upstream branch label Feb 26, 2024

00xc added the needs-rebase The PR needs to be rebased to the latest upstream branch label May 30, 2024

vsntk18 force-pushed the svsm_log_buffer branch 4 times, most recently from 7476cee to 3d2c06d Compare June 6, 2024 17:17

joergroedel removed the needs-rebase The PR needs to be rebased to the latest upstream branch label Jun 7, 2024

vsntk18 force-pushed the svsm_log_buffer branch 4 times, most recently from 37851c2 to 4e8cd5e Compare June 17, 2024 07:59

00xc reviewed Jun 17, 2024

View reviewed changes

p4zuu and others added 7 commits June 18, 2024 20:06

igvmbuilder: remove firmware field in GpaMap

fae503d

The firmware field of GpaMap was initialized once but never read. This raises a warning in the last clippy release. Signed-off-by: Thomas Leroy <thomas.leroy@suse.com>

Implement a log buffer which stores all log messages in

eaa9b0d

memory. This log buffer is based on the string ring buffer in utils/StringRingBuffer. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

Implement a percpu line buffer to store the individual

f13d49b

log messages. Each individual log message gets written into the global log buffer from the percpu line buffer. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

Initialize log buffer and migrate it from stage2 to svsm

453f8c9

kernel. Introduce a new struct MigrateInfo which contains the data to be migrated from stage2 to svsm kernel. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

Add a feature named enable-console-log

dddab64

This feature can be enable to print log messages to console in addition to storing them in the log buffer. Enable this feature by default for debug builds. Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

log_buffer: add unit tests to test the functionality

0a410dd

Test the log_buffer by calling methods like write_log() and read_log(). Signed-off-by: Vasant Karasulli <vkarasulli@suse.de>

vsntk18 force-pushed the svsm_log_buffer branch from 4e8cd5e to 0a410dd Compare June 19, 2024 09:02

00xc added the needs-rebase The PR needs to be rebased to the latest upstream branch label Jul 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a log buffer to store log messages. #52

Implement a log buffer to store log messages. #52

vsntk18 commented Jun 19, 2023 •

edited

Loading

00xc commented Jun 20, 2023 •

edited

Loading

vsntk18 commented Jun 20, 2023 •

edited

Loading

00xc commented Jun 20, 2023

vsntk18 commented Jun 23, 2023

vsntk18 commented Jun 29, 2023

joergroedel left a comment

vsntk18 commented Jul 11, 2023

vsntk18 commented Jul 11, 2023

vsntk18 commented Jun 6, 2024

vsntk18 commented Jun 10, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

00xc Jun 17, 2024

vsntk18 Jun 19, 2024

		// A method used for testing
		pub fn read_log(&mut self) -> Vec<u8> {

Implement a log buffer to store log messages. #52

Are you sure you want to change the base?

Implement a log buffer to store log messages. #52

Conversation

vsntk18 commented Jun 19, 2023 • edited Loading

00xc commented Jun 20, 2023 • edited Loading

vsntk18 commented Jun 20, 2023 • edited Loading

00xc commented Jun 20, 2023

vsntk18 commented Jun 23, 2023

vsntk18 commented Jun 29, 2023

joergroedel left a comment

Choose a reason for hiding this comment

vsntk18 commented Jul 11, 2023

vsntk18 commented Jul 11, 2023

vsntk18 commented Jun 6, 2024

vsntk18 commented Jun 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsntk18 commented Jun 19, 2023 •

edited

Loading

00xc commented Jun 20, 2023 •

edited

Loading

vsntk18 commented Jun 20, 2023 •

edited

Loading