search

coconut-svsm / svsm

COCONUT-SVSM
MIT License
122 stars 42 forks source link

stage2: relocate stage2 and boot data to 8 MB #432

Closed msft-jlange closed 3 months ago

msft-jlange commented 4 months ago

Stage2 has grown so that it cannot fit below 640 KB with enough space to provide a heap for stage2 execution. This change relocates stage2 (and stage1 and the kernel images) up to 8 MB so that all of the memory below 640 KB can be used for the stage2 heap, and so that stage2 can continue to grow without concern for hitting any other size limits.

p4zuu commented 3 months ago

SVSM kernel doesn't boot on my testing machine (Milan) unless I fix the leal 16(%ecx), %ecx in boot_stage2.rs, I guess it's a typo :)

AdamCDunlap commented 3 months ago

I tried testing this PR on Google's internal non-igvm boot path, but it looks like even svsm's main branch broke this (we haven't merged with github in a while). Internally we agreed that we'll commit to getting support for igvm before merging with github, so we have no objections to this PR.

I also tried our prototype igvm boot with this PR and it did as well as the other branch I've been testing on, so no objections there either.

roy-hopkins commented 3 months ago

I tried testing this PR on Google's internal non-igvm boot path, but it looks like even svsm's main branch broke this (we haven't merged with github in a while). Internally we agreed that we'll commit to getting support for igvm before merging with github, so we have no objections to this PR.

I've just raised a PR that should fix non-IGVM boot for the main branch. https://github.com/coconut-svsm/svsm/pull/439

msft-jlange commented 3 months ago

I tried testing this PR on Google's internal non-igvm boot path, but it looks like even svsm's main branch broke this

Thanks for testing! Roy has a PR open to fix the existing stage1 issue, so hopefully once that merges, the latest version of this PR will function correctly.

AdamCDunlap commented 3 months ago

I tried with both this PR and Roy's stage1 PR (which fixed the issue on its own) and I'm getting an error while trying to read the sev metadata GUID tables from svsm.bin: Section [unknown SNP metadata section type: 0x0] has length that's not a positive multiple of a 4K page size: 0x0

Again, we don't want to block this PR, but it does seem like it messes up the GUID tables.

roy-hopkins commented 3 months ago

I tried with both this PR and Roy's stage1 PR (which fixed the issue on its own) and I'm getting an error while trying to read the sev metadata GUID tables from svsm.bin: Section [unknown SNP metadata section type: 0x0] has length that's not a positive multiple of a 4K page size: 0x0

Did you test with Jon's previous fix to gen_meta.c? The previous version was leaving svsm_meta->descs[0] uninitialised.

AdamCDunlap commented 3 months ago

I tried with both this PR and Roy's stage1 PR (which fixed the issue on its own) and I'm getting an error while trying to read the sev metadata GUID tables from svsm.bin: Section [unknown SNP metadata section type: 0x0] has length that's not a positive multiple of a 4K page size: 0x0

Did you test with Jon's previous fix to gen_meta.c? The previous version was leaving svsm_meta->descs[0] uninitialised.

I tried with the latest version and the SVSM kernel booted. Thanks!