From f816c49c814632482c6bde9aafe4fd23187b9e8e Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Wed, 25 Sep 2024 09:49:45 +0000 Subject: [PATCH 1/3] idt: delay initialization of HV_DOORBELL_ADDR The doorbell page is not yet setup by the time idt_init is called, so HV_DOORBELL_ADDR was always initialized to 0. Instead setup HV_DOORBELL_ADDR immediatly after setting up the doorbell page. Signed-off-by: Tom Dohrmann --- kernel/src/cpu/percpu.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/kernel/src/cpu/percpu.rs b/kernel/src/cpu/percpu.rs index 712d902a9..6908c4b86 100644 --- a/kernel/src/cpu/percpu.rs +++ b/kernel/src/cpu/percpu.rs @@ -309,7 +309,7 @@ pub struct PerCpu { ghcb: OnceCell, /// `#HV` doorbell page for this CPU. - hv_doorbell: OnceCell<&'static HVDoorbell>, + hv_doorbell: Cell>, init_stack: Cell>, ist: IstStacks, @@ -341,7 +341,7 @@ impl PerCpu { shared: PerCpuShared::new(apic_id), ghcb: OnceCell::new(), - hv_doorbell: OnceCell::new(), + hv_doorbell: Cell::new(None), init_stack: Cell::new(None), ist: IstStacks::new(), current_stack: Cell::new(MemoryRegion::new(VirtAddr::null(), 0)), @@ -408,17 +408,14 @@ impl PerCpu { } pub fn hv_doorbell(&self) -> Option<&'static HVDoorbell> { - self.hv_doorbell.get().copied() + self.hv_doorbell.get() } /// Gets a pointer to the location of the HV doorbell pointer in the /// PerCpu structure. Pointers and references have the same layout, so /// the return type is equivalent to `*const *const HVDoorbell`. pub fn hv_doorbell_addr(&self) -> *const &'static HVDoorbell { - self.hv_doorbell - .get() - .map(ptr::from_ref) - .unwrap_or(ptr::null()) + self.hv_doorbell.as_ptr().cast() } pub fn get_top_of_stack(&self) -> VirtAddr { @@ -488,9 +485,11 @@ impl PerCpu { fn setup_hv_doorbell(&self) -> Result<(), SvsmError> { let doorbell = allocate_hv_doorbell_page(current_ghcb())?; - self.hv_doorbell - .set(doorbell) - .expect("Attempted to reinitialize the HV doorbell page"); + assert!( + self.hv_doorbell.get().is_none(), + "Attempted to reinitialize the HV doorbell page" + ); + self.hv_doorbell.set(Some(doorbell)); Ok(()) } From 7c472cd013cffe9960d4707ee15d75313e578fe1 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Wed, 25 Sep 2024 09:53:17 +0000 Subject: [PATCH 2/3] idt: restore fall-through The code in the handle_as_hv block expects to be able to fall through to default_return. The introduction of return_new_task interrupted that flow. Move return_new_task to another location. Fixes: 853439e4c ("kernel/task: Implement hook to run in new tasks") Signed-off-by: Tom Dohrmann --- kernel/src/cpu/idt/entry.S | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kernel/src/cpu/idt/entry.S b/kernel/src/cpu/idt/entry.S index dd2dcf213..c6079f41b 100644 --- a/kernel/src/cpu/idt/entry.S +++ b/kernel/src/cpu/idt/entry.S @@ -213,11 +213,6 @@ handle_as_hv: call process_hv_events // fall through to default_return -.globl return_new_task -return_new_task: - call setup_new_task - jmp default_return - .globl default_return default_return: // Ensure that interrupts are disabled before attempting any return. @@ -273,6 +268,11 @@ return_user: // Put user-mode specific return code here jmp return_all_paths +.globl return_new_task +return_new_task: + call setup_new_task + jmp default_return + // #DE Divide-by-Zero-Error Exception (Vector 0) default_entry_no_ist name=de handler=panic error_code=0 vector=0 From 8f368f8f4597ee17cd49e60e900f7208e6e05211 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Wed, 25 Sep 2024 09:56:01 +0000 Subject: [PATCH 3/3] idt: fix some typos Just some typos I noticed while reading the comments. Signed-off-by: Tom Dohrmann --- kernel/src/cpu/idt/entry.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/src/cpu/idt/entry.S b/kernel/src/cpu/idt/entry.S index c6079f41b..ad8ea49fe 100644 --- a/kernel/src/cpu/idt/entry.S +++ b/kernel/src/cpu/idt/entry.S @@ -117,7 +117,7 @@ hv_not_vmpl_switch: // point, there are two possibilities. If RIP is before the IRET // instruction itself, then the RSP at the time of #HV exception // points to the register context that was established for the previous - // exceptoin. In that case, the current RSP can be changed to point + // exception. In that case, the current RSP can be changed to point // to that exception context, and the #HV can be handled using that // register context, and when #HV processing completes, the subsequent // end-of-interrupt flow will restore the context at the time of the @@ -171,7 +171,7 @@ restart_hv: // register state at the time of the exception that was returning at // the time the #HV arrived. // At this point, RCX holds the stack pointer at the time of the - // IRET taht was aborted. The first QWORD below that pointer is + // IRET that was aborted. The first QWORD below that pointer is // reserved for the dummy error code, then the three QWORDS below that // will hold the RAX, RBX, and RCX values, which are presently stored // in the top three QWORDs of the current stack. @@ -185,7 +185,7 @@ restart_hv: continue_hv: // At this point, only the dummy error code and first three registers - // have been pushed onto the stack. Push the remainder o construct a + // have been pushed onto the stack. Push the remainder to construct a // full exception context. pushq %rdx pushq %rsi @@ -220,7 +220,7 @@ default_return: testb $3, 17*8(%rsp) // Check CS in exception frame jnz return_user return_all_paths: - // If interrupts were prerviously available, then check whether any #HV + // If interrupts were previously available, then check whether any #HV // events are pending. If so, proceed as if the original trap was // #HV. testl $0x200, 18*8(%rsp) // check EFLAGS.IF in exception frame