Get mutable references to the memory information

[YtvwlD]

This is (hopefully) the last part I split off of #133.

This PR adds the functions basic_memory_info_tag_mut, memory_map_tag_mut adn efi_memory_map_tag_mut. The memory map tags each get their own memory_areas_mut.

This may sound wrong at first and yes, this is really hacky, but I'm afraid that I need this.

The correct way to pass memory information from the bootloader to the kernel would be

1. gather memory information
2. build the appropriate tags
3. pass them into the builder
4. build the information struct

But steps 2 and 4 allocate memory which in turn might invalidate the gathered memory map.
One could either risk this and just pass possibly outdated information – or do it the other way round:

1. approximate how much memory areas there are
2. build the whole information struct
3. get the correct memory information
4. get the appropriate tags, mutably
5. put the information in there

(The downside to this approach is that there may be empty areas at the end (which seems to be fine) or that there are not enough areas.)

[phip1611]

fine with me. Users of this library could do this anyway with unsafe magic.. so just give them the necessary tools directly.

Please address the two open points. Otherwise, LGTM

[YtvwlD]

Oh, yes, I missed that while rebasing.

[phip1611]

But steps 2 and 4 allocate memory which in turn might invalidate the gathered memory map.

Wait. On legacy boot systems, the memory map describes which regions of the physical memory are valid RAM, which is for ACPI, and which is reserved. It does not contain the information that the area where your bootloader lives is "taken". A bootloader has to find this information by itself.. by it's load address and it's length.

Hence, also the hope that an allocation should be reflected in the memory map is wrong/not possible.

In a UEFI boot flow, the memory map reflects which memory is taken by your image/kernel. If you use a heap that is statically backed inside the binary, there is no need to update the memory map.

[YtvwlD]

Hm, I'm talking about UEFI and the default allocator: When I create boxes for the tags, they'll get put somewhere on the heap. This might be inside an existing memory region or it might allocate a new LOADER_DATA region.

The legacy map is only really accurate after calling ExitBootServices, I fear (after which the allocator doesn't work anymore).

Doing static allocations might work, but I'm not quite fond of that.

[phip1611]

// SAFETY: BootInformation contains a const ptr to memory that is never mutated.
// Sending this pointer to other threads is sound.
unsafe impl Send for BootInformation {}

If we merge this, this needs to be removed, probably. Another breaking change. I think this could be okay, as one can wrap the MBI in a (custom) Mutex..

[YtvwlD]

Huh, I don't know? The struct does need to be mutable for these methods to be called, but I'm not sure that's enough.

[phip1611]

I'm going to postpone this a bit and merge a few other things first. The rebase needed here will have a few conflicts - I'm sorry for that!

[YtvwlD]

Sure, just ping me when I should rebase.

[phip1611]

ping :)

[YtvwlD]

So, this works, but it really moves the mut through the whole codebase and I'm not happy about it.

Edit: It's somehow usable, but the lifetimes of the _mut methods are wrong. I'm not able to set all memory information.

[YtvwlD]

I don't understand the miri failure, but this has better lifetimes and the integration test should work. I'm still not happy about having to touch so much code.

[YtvwlD]

Using AsMut might help with not-carrying-mut-through-the-whole-code, but I'm unsure whether this would look any better.

[YtvwlD]

I think this looks better, though I'm not sure combining EFIMemoryAreaIter and EFIMemoryAreaIterMut is the way to go. (Combining TagIter and TagIterMut didn't work because there's pointers in them.)

[YtvwlD]

That unnecessary pub(self) wasn't introduced by this pull request.

[phip1611]

After thinking a lot about this, I came to the conclusion that this is indeed helpful. However, I'm unsure if the proposed design is too invasive. I think, there is a simpler approach. Let me try something and I'll get back to you! :) Thanks for your patience!

TL;DR: My idea is to use UnsafeCell under the hood

[YtvwlD]

Oh, great!

(Don't mind me, this is just a rebase.)

[YtvwlD]

(This is just a rebase.)

[phip1611]

Hey. I somehow forgot about this, sorry. I'll look into it soon, hopefully. But I'm already concerned about API complexity and cognitive load

[YtvwlD]

Yeah sure, take your time. I think this is the simplest change I can come up with right now, but if you find a simpler one, that'd be great.

[phip1611]

Sorry. I'm closing this due to massive internal refactorings, necessary for memory safety and removing any UB, making this unmergable.

However, I think that the new design (based in multiboot2-common) can act as base to allow mutable
references to multiboot2 structures

[YtvwlD]

Hm, I've been trying to port this over to the new design, but I'm currently stuck with quite a lot of duplications (BytesRefMut, TagIterMut, and about half of the functions in DynSizedStructure), so I'm not sure that this is the way to go.

[phip1611]

I think having additional ByresRefMut and TagIterMut is fine and necessary. The duplication is fine.

I'm not sure how structDynSizedStructure and trait MaybeDynSized need to be altered. Perhaps we also need a new trait MaybeDynSizedMut trait additionally?

PS: Did you find the provided write-up and diagrams helpful to understand the new design? I've put much work into it - would love to hear feedbacn

[YtvwlD]

Thanks for the input and pointing me to the diagrams! They make the two stages of parsing clearer, but I didn't fully grasp the last one, yet.

In any case, this seems to be a bit harder than I thought; mostly because TagIterMut doesn't seem to be straightforward, so I might take some time to get it right.

(In the mean time, what follows is a partial rebase; please ignore it. :)

[phip1611]

I'm sorry for the inconveniences. But all the refactorings were necessary to get rid of all UB.

but I didn't fully grasp the last one, yet.

What is "the last one", for you? Please reach out to me, so I can improve the docs or diagrams