From 3bc4424a5b24928f39f25ee3e4ff834892d0b2a6 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Mon, 17 Feb 2025 09:47:37 +0800 Subject: [PATCH] Add `unsafe` with explained comments --- ostd/src/arch/x86/irq.rs | 4 +++- ostd/src/arch/x86/kernel/apic/x2apic.rs | 22 +++++++++++++--------- ostd/src/arch/x86/tdx_guest.rs | 24 ++++++++++++++++++------ 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/ostd/src/arch/x86/irq.rs b/ostd/src/arch/x86/irq.rs index d94f28bb..072bb061 100644 --- a/ostd/src/arch/x86/irq.rs +++ b/ostd/src/arch/x86/irq.rs @@ -105,5 +105,7 @@ pub(crate) unsafe fn send_ipi(hw_cpu_id: HwCpuId, irq_num: u8, guard: &dyn PinCu ); let apic = apic::get_or_init(guard); - apic.send_ipi(icr); + // SAFETY: The ICR is valid to generate the request IPI. Generating the request IPI is safe + // as guaranteed by the caller. + unsafe { apic.send_ipi(icr) }; } diff --git a/ostd/src/arch/x86/kernel/apic/x2apic.rs b/ostd/src/arch/x86/kernel/apic/x2apic.rs index 7a74ae80..eb18fdcc 100644 --- a/ostd/src/arch/x86/kernel/apic/x2apic.rs +++ b/ostd/src/arch/x86/kernel/apic/x2apic.rs @@ -77,15 +77,19 @@ impl super::Apic for X2Apic { unsafe fn send_ipi(&self, icr: super::Icr) { let _guard = crate::trap::disable_local(); - wrmsr(IA32_X2APIC_ESR, 0); - wrmsr(IA32_X2APIC_ICR, icr.0); - loop { - let icr = rdmsr(IA32_X2APIC_ICR); - if ((icr >> 12) & 0x1) == 0 { - break; - } - if rdmsr(IA32_X2APIC_ESR) > 0 { - break; + // SAFETY: These `rdmsr` and `wrmsr` instructions write the interrupt command to APIC and + // wait for results. The caller guarantees it's safe to execute this interrupt command. + unsafe { + wrmsr(IA32_X2APIC_ESR, 0); + wrmsr(IA32_X2APIC_ICR, icr.0); + loop { + let icr = rdmsr(IA32_X2APIC_ICR); + if ((icr >> 12) & 0x1) == 0 { + break; + } + if rdmsr(IA32_X2APIC_ESR) > 0 { + break; + } } } } diff --git a/ostd/src/arch/x86/tdx_guest.rs b/ostd/src/arch/x86/tdx_guest.rs index e2b404dc..77b23dbc 100644 --- a/ostd/src/arch/x86/tdx_guest.rs +++ b/ostd/src/arch/x86/tdx_guest.rs @@ -57,13 +57,18 @@ pub unsafe fn unprotect_gpa_range(gpa: Paddr, page_num: usize) -> Result<(), Pag let _ = boot_pt::with_borrow(|boot_pt| { for i in 0..page_num { let vaddr = paddr_to_vaddr(gpa + i * PAGE_SIZE); - boot_pt.protect_base_page(vaddr, protect_op); + // SAFETY: The caller ensures that the address range exists in the linear mapping and + // can be mapped as shared pages. + unsafe { boot_pt.protect_base_page(vaddr, protect_op) }; } }); + // Protect the page in the kernel page table. let pt = KERNEL_PAGE_TABLE.get().unwrap(); let vaddr = paddr_to_vaddr(gpa); - pt.protect_flush_tlb(&(vaddr..vaddr + page_num * PAGE_SIZE), protect_op) + // SAFETY: The caller ensures that the address range exists in the linear mapping and can be + // mapped as shared pages. + unsafe { pt.protect_flush_tlb(&(vaddr..vaddr + page_num * PAGE_SIZE), protect_op) } .map_err(|_| PageConvertError::PageTable)?; map_gpa( @@ -106,21 +111,28 @@ pub unsafe fn protect_gpa_range(gpa: Paddr, page_num: usize) -> Result<(), PageC let _ = boot_pt::with_borrow(|boot_pt| { for i in 0..page_num { let vaddr = paddr_to_vaddr(gpa + i * PAGE_SIZE); - boot_pt.protect_base_page(vaddr, protect_op); + // SAFETY: The caller ensures that the address range exists in the linear mapping and + // can be mapped as non-shared pages. + unsafe { boot_pt.protect_base_page(vaddr, protect_op) }; } }); + // Protect the page in the kernel page table. let pt = KERNEL_PAGE_TABLE.get().unwrap(); let vaddr = paddr_to_vaddr(gpa); - pt.protect_flush_tlb(&(vaddr..vaddr + page_num * PAGE_SIZE), protect_op) + // SAFETY: The caller ensures that the address range exists in the linear mapping and can be + // mapped as non-shared pages. + unsafe { pt.protect_flush_tlb(&(vaddr..vaddr + page_num * PAGE_SIZE), protect_op) } .map_err(|_| PageConvertError::PageTable)?; map_gpa((gpa & PAGE_MASK) as u64, (page_num * PAGE_SIZE) as u64) .map_err(|_| PageConvertError::TdVmcall)?; for i in 0..page_num { + // SAFETY: The caller ensures that the address range represents physical memory so the + // memory can be accepted. unsafe { - accept_page(0, (gpa + i * PAGE_SIZE) as u64).map_err(|_| PageConvertError::TdCall)?; - } + accept_page(0, (gpa + i * PAGE_SIZE) as u64).map_err(|_| PageConvertError::TdCall)? + }; } Ok(()) }