loader: update the page table builder to be no_std

[babayet2]

Discussed w/ @chris-oo, this PR updates the page table builder in the loader to be no_std, and to accept a generic buffer for its working memory

The goal is to prepare the library to be used within the openhcl_boot shim, which is a no_std environment. The generic buffer change allows the loader code to continue using a vectors for its working memory, whereas the boot shim will be providing its working buffer as ArrayVec stashed in the global section

[chris-oo]

this seems like a lot - any way we can make it even simpler?

[babayet2]

yeah I don't love the boilerplate either, I've chipped it down this far and would ideally take it further, but there is a tradeoff here: we have to choose whether we prefer boilerplate or memory inefficiency at igvm build time.

it's not clear to me how to reduce the trait size while keeping the buffers extendable - but if we don't care about extendable buffers, we could just pass large mutable "slices" back and forth for working memory. we would do that entirely without generics, the caller would just always pass a reference to an ArrayVec

is there another approach you're seeing that I'm missing?

[chris-oo]

See below but i wonder if we can just replace this with a simpler trait or an enum.

[chris-oo]

IE i'm wondering here if instead we can just require the caller to allocate all this space upfront. Or if we can just simplify saying "the caller provides a builder method where given a page_table_count, we return something that gives access to a mutable count of PageTable entries"?

I really don't want a trait with a bunch of methods. Alternatively, maybe can we just have the page table builder accept an enum for storage something like:

enum Storage {
Vec(Vec<..>),
ArrayVec(Vec..),
}

Then something like

impl Storage {
fn extend(&mut self, page_table_count)  { ... } 
}

etc. Thoughts?

[babayet2]

IE i'm wondering here if instead we can just require the caller to allocate all this space upfront.
Allocating all this space upfront seems to me identical to just using an ArrayVec, is that what you're suggesting? If so I think that is the cleanest approach, IMO much better to avoid boilerplate and risk using an extra few pages during IGVM build time. We'd maybe care about this extra memory more at runtime, but the only case where we're leveraging this code at runtime (the boot shim), we have to allocate upfront anyway .

Or if we can just simplify saying "the caller provides a builder method where given a page_table_count, we return something that gives access to a mutable count of PageTable entries"?
We would need to define the interface that is implemented on what is returned from this builder, no? Don't we come back to the same problem?

If there is just a simple syntactical trick I'm missing here, and this is possible, I'm all for it.

I really don't want a trait with a bunch of methods.
Agreed yeah, I'm on board with getting rid of it

Alternatively, maybe can we just have the page table builder accept an enum for storage something [...]
This would require Vec being importable in page_table, we can't do that if it's no_std

[chris-oo]

Right, i'm wondering if we need to support dynamic growth at all. Just let the builder internally manage slices of data it can grow with, and that's fine if we have to size it correctly in other cases. Note that ArrayVec is a bit annoying since you would probably want to not put such a large one on the stack normally. What do you think this would look like?

also you can use vec it in no_std, it just requires extern crate alloc, or specifying it via alloc.

[babayet2]

Note that ArrayVec is a bit annoying since you would probably want to not put such a large one on the stack normally
Ahhhh right, we don't want straight up ArrayVecs then

What do you think this would look like?

1. PageTableBuilder::build will consume references to two slices, &mut [u8], and &mut [PageTable]
2. The callers will initialize slices at their max potential size, on either the heap (igvmfilegen) or static section (boot shim). For igvmfilegen, something like vec![0; PAGE_TABLE_MAX_SIZE].as_mut_slice(), vec![PageTable::from_zeroes(); PAGE_TABLE_MAX_COUNT].as_mut_slice()
3. PageTableBuilder::build will internally track the number of PageTables
4. The flattener will consume the references to both slices and the number of PageTables. It will return a truncated slice of u8s.

also you can use vec it in no_std, it just requires extern crate alloc, or specifying it via alloc.
Oh very cool, I did a bit of reading on alloc now. Is there some reason we shouldn't write an allocator for the boot shim then, and leave the PageTableBuilder unmodified?

[chris-oo]

I'd prefer we didn't rely on a global allocator because then we need to dynamically size the region (we don't know how large the pagetable is without doing more math) or size the region to the largest mem we need to support. All doable, but I'd prefer we keep the global allocator minimally scoped to just the protobuf decode case we have today.

If we see these code changes become too ugly, maybe we can reconsider. But it would also mean that the allocator we have needs to then become more featured (IE support persisting pages post-bootshim, and page alignment) and I'd prefer we didn't have to do that.

[babayet2]

@chris-oo pushed a new version without the big trait and generics, where we just allocate space up front

LMK if it's looking good in principle, let me know, it will need to be fixed up for ARM