rivosinc / salus

Risc-V hypervisor for TEE development
92 stars 25 forks source link

Add huge pages support #289

Closed sboeuf closed 1 year ago

sboeuf commented 1 year ago

Adding the huge pages support so that a guest can rely on pages greater than 4k in size.

Refers #92

sboeuf commented 1 year ago

@abrestic-rivos Here is the first draft for supporting huge pages. It updates the existing code to handle huge pages and prevent errors from being returned. It also implements page promotion/demotion logic based on the new TEE-Host API. Please let me know if there's an aspect of the huge pages support that I might have missed and that needs to be addressed. In the meantime, I'll need to come up with some testing in tellus to validate this can work as expected.

sboeuf commented 1 year ago

@abrestic-rivos

btw, another thing you'll need to do for this to actually work that i realized we hadn't done yet is to map the host such that it maps physical memory in 2M chunks. in other words, for each 2M-aligned chunk of host VM GPA space, the base of that chunk should map to a 2M-aligned SPA, and all the 4k pages within that 2M chunk should be contiguous in SPA space.

Just to clarify, when you say 2M, that's because you assume 2M huge pages, but I assume for 1G huge page, the host must map in 1G chunks, is that correct?

currently we only do 16kB chunks so that it can create the base of the G-stage page-tables.

Can you point me to the place where this is performed in the code?

abrestic-rivos commented 1 year ago

Just to clarify, when you say 2M, that's because you assume 2M huge pages, but I assume for 1G huge page, the host must map in 1G chunks, is that correct?

Right. But mapping the host in 1G-aligned chunks would waste a ton of memory, especially since we're only booting with ~4GB in our current QEMU config. Using 2M chunks still gives us the ability to exercise hugepages without being too much of a burden.

Can you point me to the place where this is performed in the code?

See PageTracker::from(), and then where those pages are used in HostVm::add_measured_pages() and HostVm::add_zero_pages().

sboeuf commented 1 year ago

Just to clarify, when you say 2M, that's because you assume 2M huge pages, but I assume for 1G huge page, the host must map in 1G chunks, is that correct?

Right. But mapping the host in 1G-aligned chunks would waste a ton of memory, especially since we're only booting with ~4GB in our current QEMU config. Using 2M chunks still gives us the ability to exercise hugepages without being too much of a burden.

Got it!

Can you point me to the place where this is performed in the code?

See PageTracker::from(), and then where those pages are used in HostVm::add_measured_pages() and HostVm::add_zero_pages().

Thanks!

sboeuf commented 1 year ago

Weird. I've updated tellus/guestvm through the commit test-workloads: Test huge pages support, but the change isn't part of the test build. I thought the docker image rivosinc/rivos-run-salus-qemu@v1 was created on the fly and therefore any change to test-workloads would be propagated. Did I miss something?

glg-rv commented 1 year ago

These changes look good to me. Admit I missed most of the discussions happening on this PR and haven't checked every single part of the page size adds to parameters, but it is much simpler than what I was fearing.

sboeuf commented 1 year ago

These changes look good to me.

Cool!

Admit I missed most of the discussions happening on this PR and haven't checked every single part of the page size adds to parameters, but it is much simpler than what I was fearing.

Well maybe it's simpler than you thought because we're not supporting huge pages everywhere. It's only the ability to map huge pages for TVMs and not to use them for internal states or other purposes.

glg-rv commented 1 year ago

Admit I missed most of the discussions happening on this PR and haven't checked every single part of the page size adds to parameters, but it is much simpler than what I was fearing.

Well maybe it's simpler than you thought because we're not supporting huge pages everywhere. It's only the ability to map huge pages for TVMs and not to use them for internal states or other purposes.

Heh. Well, I saw that. I think it's a good first step.

Regarding the internal mappings, we might need to sync over the plans because I have a few upcoming changes on the hypervisor mappings.

sboeuf commented 1 year ago

@abrestic-rivos I've addressed your comments and I've added a few unit tests.

sboeuf commented 1 year ago

PR has been rebased on latest main branch.