Page MenuHomeFreeBSD

arm64/vmm: Use the VNCR_EL2 memory page to store guest registers
Needs ReviewPublic

Authored by kajetan.puchalski_arm.com on Tue, Apr 21, 11:08 AM.

Details

Reviewers
andrew
manu
Group Reviewers
arm64
Summary

Wherever possible, move the storage space for guest register values from
the hypctx struct into a preallocated memory page matching the layout of
the page pointed to by VNCR_EL2.
This will streamline implementing support for nested virtualization, but
the implementation itself is not reliant on the presence of nested
virtualization architecture features.

Signed-off-by: Kajetan Puchalski <kajetan.puchalski@arm.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 72410
Build 69293: arc lint + arc unit

Event Timeline

This is really quite an obnoxious change to deal with for Morello, where some of these registers are 128-bit capabilities not 64-bit integers...

sys/arm64/vmm/arm64.h
258

Isn't this using a byte offset, but the enum values are consecutive?

This is really quite an obnoxious change to deal with for Morello, where some of these registers are 128-bit capabilities not 64-bit integers...

Which ones specifically?
Do you just mean it's harder to deal with because without this you'd just expand the sizes of registers in hypctx, whereas the VNCR layout doesn't have the same flexibility?

sys/arm64/vmm/arm64.h
258

Enum values for registers defined with VNCR(...) are just the value of VNCR_START + <offset within the VNCR page>. The offsets are not necessarily evenly spaced out, e.g.:

VNCR_VSTTBR_EL2 0x30
VNCR_VTCR_EL2   0x40
VNCR_VSTCR_EL2  0x48

The idea here is that for any VNCR register you can just get its enum value, subtract VNCR_START and it'll give you the offset that you need to add to the address stored in VNCR to get the memory location of said register.

This is really quite an obnoxious change to deal with for Morello, where some of these registers are 128-bit capabilities not 64-bit integers...

Which ones specifically?
Do you just mean it's harder to deal with because without this you'd just expand the sizes of registers in hypctx, whereas the VNCR layout doesn't have the same flexibility?

Yes, exactly. This loses all type system flexibility. At least if you just made a struct that matched the VNCR layout we could change the types, and ok it wouldn't then match, but that wouldn't matter for our use case (and if a CHERI + Arm + nested virt system ever exists then the layout can be made to match that). Whereas now we'd have to go and meticulously tweak all the magic VNCR constants to give us a layout that works, and tweak the one-function-to-rule-them-all to be two functions, one for integers and one for capabilities, since we have multiple types.

sys/arm64/vmm/arm64.h
258

Oh, I see, VNCR_REG hides all that. That's really quite weird, having the non-VNCR ones just count up from 0, whilst the VNCR ones count up in increments of 8. I don't think two different things should be in the same enum.

Yes, exactly. This loses all type system flexibility. At least if you just made a struct that matched the VNCR layout we could change the types, and ok it wouldn't then match, but that wouldn't matter for our use case (and if a CHERI + Arm + nested virt system ever exists then the layout can be made to match that). Whereas now we'd have to go and meticulously tweak all the magic VNCR constants to give us a layout that works, and tweak the one-function-to-rule-them-all to be two functions, one for integers and one for capabilities, since we have multiple types.

I know very little about CHERI & Morello, so I'm probably missing some context here. But if I understand correctly, you'd still need to add lots of CHERI-specific handling all over the hypervisor to make it work, the assumption that system registers are all uint64_t is baked into just about every piece of the logic.
However, it seems to me that you could adapt these changes fairly easily? For instance:

  1. Add CHERI-specific values to the big enum just before VNCR_START, e.g. CHERI_<name>
  2. Allocate an array of whatever type you need to store them
  3. In hypctx_sys_reg, add if (reg > CHERI_START && reg < VNCR_START) { call some macro to access said CHERI-specific registers }

It'll compile down to the same thing.

sys/arm64/vmm/arm64.h
258

It depends how you see it, the idea behind this is that the enum values are there to tell you where to find the storage space for a given register. In the case of non-VNCR registers that's the array index, in the case of VNCR registers it's the offset. But that's just an implementation detail and the user doesn't need to directly worry about it. They're in the same enum because they're all system registers.

The primary benefit of doing this type of enum sharing is that all of this gets completely compiled away. When you write hypctx_read_sys_reg(hypctx, REG) the compiler can statically determine whether you're asking for a VNCR or non-VNCR register, this entire function call gets compiled down to just a single mrs. If I separated the enums and the values would overlap then there'd be a need for either two separate functions or some runtime checks.

We will need to handle accessing different sized registers to support FEAT_D128, e.g. VTTBR_EL2 will be 128 bit there.

sys/arm64/vmm/arm64.h
47

I guess we could do something like this so all the values are 64-bit offsets

212

This and host_ctx_addr should be a pointer type

sys/arm64/vmm/vmm_arm64.c
561–562

It's probably best to merge these into the hypctx allocation, e.g. by adding them to the size el2_hypctx_size returns. For non-VHE we need to map them into the EL2 address space below with the call to el2_map_enter