Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/tlb: do not broadcast per-cpu TLB flushes #417

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

msft-jlange
Copy link
Contributor

VM ranges that are specific to a single CPU do not require TLB invalidations to be broadcast to multiple processors. This is especially important during the early boot phase when no other processors are online and when the infrastructure required to broadcast TLB invalidations may not yet be fully initialized. The same is true for temporary mappings established in a per-CPU address range.

kernel/src/mm/ptguards.rs Outdated Show resolved Hide resolved
kernel/src/cpu/tlb.rs Outdated Show resolved Hide resolved
@msft-jlange msft-jlange force-pushed the tlb_flush branch 2 times, most recently from f963244 to 168fe7a Compare July 24, 2024 17:37
Copy link
Member

@00xc 00xc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few minor suggestions, other than that this looks good to me. I'll merge once comments are marked as resolved.

kernel/src/cpu/control_regs.rs Outdated Show resolved Hide resolved
kernel/src/cpu/efer.rs Outdated Show resolved Hide resolved
kernel/src/cpu/tlb.rs Outdated Show resolved Hide resolved
All platforms capable of virtualization support PGE and NX, so there is
no reason to make these features optional in the SVSM code base.

Signed-off-by: Jon Lange <[email protected]>
VM ranges that are specific to a single CPU do not require TLB
invalidations to be broadcast to multiple processors.  This is
especially important during the early boot phase when no other
processors are online and when the infrastructure required to broadcast
TLB invalidations may not yet be fully initialized.  The same is true
for temporary mappings established in a per-CPU address range.

Signed-off-by: Jon Lange <[email protected]>
@00xc
Copy link
Member

00xc commented Jul 24, 2024

In-SVSM tests fail after the last commit in this PR (e1c168b):

[SVSM] test greq::msg::tests::encrypt_decrypt_payload ...
[SVSM] ERROR: Panic: CPU[0] KVM: entry failed, hardware error 0xffffffff
EAX=00000020 EBX=00000000 ECX=00000000 EDX=00000000
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=00000000 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 00000000 00000000
CS =0000 00000000 00000000 00000000
SS =0000 00000000 00000000 00000000
DS =0000 00000000 00000000 00000000
FS =0000 00000000 00000000 00000000
GS =0000 00000000 00000000 00000000
LDT=0000 00000000 00000000 00000000
TR =0000 00000000 00000000 00000000
GDT=     0000000000000000 00000000
IDT=     0000000000000000 00000000
CR0=80010031 CR2=0000000000000000 CR3=0000000000000000 CR4=000000f0
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 
DR6=0000000000000000 DR7=0000000000000000
EFER=0000000000000d00
Code=<??> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??

@00xc
Copy link
Member

00xc commented Jul 24, 2024

In-SVSM tests fail after the last commit in this PR (e1c168b):

Happens only on debug builds it seems. It also can happen even earlier, so the test that failed above is not to blame:

[SVSM] COCONUT Secure Virtual Machine Service Module
[SVSM] Order-00: total pages:    47 free pages:     0
[SVSM] Order-01: total pages:     1 free pages:     1
[SVSM] Order-02: total pages:     0 free pages:     0
[SVSM] Order-03: total pages:     0 free pages:     0
[SVSM] Order-04: total pages:     0 free pages:     0
[SVSM] Order-05: total pages:   117 free pages:   117
[SVSM] Total memory: 15172KiB free memory: 14984KiB
[SVSM] Boot stack starts        @ 0xffffff800011f000
[SVSM] BSP Runtime stack starts @ 0xffffff000020c000
[SVSM] Guest Memory Regions:
[SVSM]   000000000000000000-000000000080000000
[SVSM]   000000000100000000-000000000280000000
[SVSM] Invalidating boot region 000000000000000000-0000000000000a0000
KVM: entry failed, hardware error 0xffffffff
EAX=0000000a EBX=00000000 ECX=00000000 EDX=00000000
ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
EIP=00000000 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0000 00000000 00000000 00000000
CS =0000 00000000 00000000 00000000
SS =0000 00000000 00000000 00000000
DS =0000 00000000 00000000 00000000
FS =0000 00000000 00000000 00000000
GS =0000 00000000 00000000 00000000
LDT=0000 00000000 00000000 00000000
TR =0000 00000000 00000000 00000000
GDT=     0000000000000000 00000000
IDT=     0000000000000000 00000000
CR0=80010031 CR2=0000000000000000 CR3=0000000000000000 CR4=000000f0
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 
DR6=0000000000000000 DR7=0000000000000000
EFER=0000000000000d00
Code=<??> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??

@00xc
Copy link
Member

00xc commented Jul 24, 2024

Aaaand it went away with the latest nightly toolchain version, so not sure why it was failing. It also only failed when applying the VMR changes - applying only the PerCPUPageMappingGuard changes made the tests pass.

I'll try again tomorrow and see if everything works properly.

@00xc 00xc merged commit fcb76e5 into coconut-svsm:main Jul 25, 2024
3 checks passed
@msft-jlange msft-jlange deleted the tlb_flush branch July 25, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants