Import the kernel parts of bhyve/arm64

[andrew]

Summary

To support virtual machines on arm64 add the vmm code. This is based on
earlier work by Mihai Carabas and Alexandru Elisei at University
Politehnica of Bucharest, with further work by myself and Mark Johnston.

All AArch64 CPUs should work, however only the GICv3 interrupt
controller is supported. There is initial support to allow the GICv2
to be supported in the future. Only pure Armv8.0 virtualisation is
supported, the Virtualization Host Extensions are not currently used.

With a separate userspace patch and U-Boot port FreeBSD guests are able
to boot to multiuser mode, and the hypervisor can be tested with the
kvm unit tests. Linux partially boots, but hangs before entering
userspace. Other operating systems are untested.

Sponsored by: Arm Ltd
Sponsored by: Innovate UK
Sponsored by: The FreeBSD Foundation
Sponsored by: University Politehnica of Bucharest

[markj]

sys/arm64/vmm/io/vgic_v3.c

2153

Same here.

sys/arm64/vmm/vmm_arm64.c

1204

Should these be hypctx->elr_el1 and spsr_el1, respectively?

sys/arm64/vmm/vmm_hyp.c

506

Aren't we using a stale value for tf_esr here? vmm_enter_guest() doesn't update it, nor does the EL1 synchronous exception handler. It'll be updated by the vmm_hyp_reg_store() call below, or I'm missing something.

[editor_callfortesting.org]

In D37428#936851, @editor_callfortesting.org wrote:

What CPU/motherboard/system configurations is this known to work on and people should be testing with?

Response from elsewhere: Ampere® Altra® Processors

[andrew]

sys/arm64/arm64/identcpu.c

107

My plan is to not move this, but will need to decide on a new KPI for vmm.ko to use.

sys/arm64/vmm/vmm_arm64.c

1204

I don't think we need VM_REG_GUEST_ELR and should rename VM_REG_ELR_EL2 to VM_REG_GUEST_PC.

I guess VM_REG_GUEST_SPSR should be VM_REG_GUEST_CPSR, but will need to check how it should be used in the gdb stub.

[markj]

sys/arm64/vmm/vmm_arm64.c

1204

ELR_EL2 -> GUEST_PC makes sense to me.

The guest ELR could still be useful for gdb, no?

[andrew]

sys/arm64/vmm/vmm_arm64.c

1204

For that we would need to update the GDB stub to support a custom target description [1][2]. This would let us define all registers the stub exports.

[1] https://www.sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#qXfer-target-description-read
[2] https://www.sourceware.org/gdb/onlinedocs/gdb/Target-Descriptions.htm

[andrew]

Large update from myself an @markj:

• Update vm_gla2gpa_nofault to work on different granule sizes (only 4k is tested)
• Initialize the EL2 memory space allocator properly
• Fix a lock leak in vmmdev_ioctl()
• Various style fixes
• Remove VM_SUSPEND_TRIPLEFAULT
• Clean up VM_REG_* names and add more needed registers
• Update struct vm_guest_paging
• Remove structs task_switch_reason and vm_task_switch
• Add support for tcr2_el1 for when it's not res0
• Add missing breaks to switch statements
• Add vgic_v3_detach
• Remove unneeded zeroing
• Use ERET
• Fix the EXCP_TYPE_EL1_SYNC EL2 code

Still todo:

• Move to DPCPU for the current vcpu pointer
• Add a license to vgic_v3_reg.h

[andrew]

sys/arm64/vmm/io/vgic_v3_reg.h

2

Needs a license

[andrew]

sys/arm64/vmm/io/vgic_v3_reg.h

2

It looks like this is (c) @alexandru.elisei_arm.com who appears to be on Sabbatical for 4 weeks.

[alexandru.elisei_gmail.com]

sys/arm64/vmm/io/vgic_v3_reg.h

2

It seems I forgot to add a license when adding this file. When I wrote it, I wasn't working for Arm, so the copyright goes to my personal gmail address:

/*
 * Copyright (C) 2018 Alexandru Elisei <[e-mail]>
 * All rights reserved.
 *
 * This software was developed by Alexandru Elisei under sponsorship
 * from the FreeBSD Foundation.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY AUTHOR AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

[imp]

Notes about All Rights Reserved. Not a huge deal, but it would be great if we could drop as many of these as we could. I have to ask if we can drop it in these cases, but if we can't, it's not blocking. Things seem far enough along that it's not a bad time to raise it: substantial progress has been made, and it won't derail the last few issues I see here.

sys/arm64/include/vmm.h

5

Ideally, we'd see is Mihai would allow us to drop this...

sys/arm64/vmm/hyp.h

5

It would be nice of Alexandru could also drop this too, in addition to the one he proposed adding.

sys/arm64/vmm/io/vgic_v3_reg.h

2

Thanks for the copyright notice to use. FreeBSD has moved away from All Rights Reserved. after the copyright notice because is archaic legal boilerplate that's not really been relevant in decades. Would you consider dropping it? It's up to you, but that's a change in our template since 2018.

sys/arm64/vmm/io/vtimer.c

5

Drop. Foundation doesn't want this anymore.

[andrew]

sys/arm64/vmm/io/vgic_v3_reg.h

2

If you were sponsored by The FreeBSD Foundation at the time it would probably be (C) 2018 The FreeBSD Foundation

[emaste]

sys/arm64/vmm/io/vgic_v3_reg.h

2

Yes the standard copyright for Foundation-sponsored work is

/*
 * Copyright (C) <YEAR> The FreeBSD Foundation
 *
 * This software was developed by <person> under sponsorship
 * of The FreeBSD Foundation.
 *
...

[alexandru.elisei_gmail.com]

sys/arm64/vmm/io/vgic_v3_reg.h

2

Wasn't aware of that. You can change all the files where I've mistakenly added me as the sole copyright holder to say the above.

[markj]

sys/arm64/vmm/vmm_arm64.c

136

This assumes that the exception vectors are at the beginning of vmm_hyp_blob, but that's not strongly enforced. For instance, on CheriBSD I had a problem where the linker inserted a CHERI-specific note in front of the vectors. Is there a better way to do this? It's rather fragile.

[andrew]

sys/arm64/vmm/vmm_arm64.c

136

Can you move the note with the linker script? The vectors are placed at the start of the EL2 code because they are in the .vmm_vectors section that is first in the linker script.

[markj]

sys/arm64/vmm/vmm_arm64.c

136

That's what I did, but it's really just a workaround. I think this is likely to keep coming up.

[andrew]

Updates from myself and @markj:

• Style clean up
• Don't panic if the vgicv3 driver isn't attached to the VM
• Use DPCPU for the current vcpu
• Add a license to vgic_v3_reg.h
• Add SPDX-License-Identifier to all files
• Remove "All rights reserved" from FreeBSD Foundation and Alexandru Elisei copyright

[andrew]

I asked @alexandru.elisei_arm.com before removing All rights reserved.

[markj]

sys/arm64/vmm/vmm_hyp.c

714

I don't understand this loop - shouldn't we be incrementing by PAGE_SIZE >> 12 here?

It looks like the TODO is referencing the TLBI macros defined in vm_s2_tlbi_range()?

[andrew]

• Cleanups from @markj
• Use the TLBI macros in vmm_el2_tlbi

[andrew]

Add VM_MUNMAP_MEMSEG from @markj

[markj]

I think this is ok to commit. Once you go ahead, I'll commit userspace bits in the coming weeks. The u-boot port is already available.

Some tasks for the near future:

• Merging common bits of sys/amd64/vmm and sys/arm64/vmm. Most of the ioctls and VM lifecycle code is shared, so we should aim to deduplicate; shared code could go in sys/vmm or sys/dev/vmm.
• A port of bhyve's gdb stub. (I'm working on this and hope to finish by the end of next week.)
• ACPI and EDK2 support.

[bz]

In D37428#972792, @markj wrote:

Some tasks for the near future:

• ACPI and EDK2 support.

That would be highly appreciated! :)

And thanks to everyone who has worked on this. I cannot wait to try (again).

[jrtc27]

One big issue and a couple of old comments I seemingly had never submitted (also had a bunch of others that appear to be obsolete)

sys/arm64/vmm/io/vtimer.c

60

This is never read

74

Is this being addressed?

sys/modules/vmm/Makefile

48

CC doesn't include crucial things like -target and, in the case of CheriBSD downstream, -march and -mabi. The lack of -target means that, if you're using an LLVM that has its default target triple as any of i386, powerpc, mips{,64}{,el} or riscv64 (or a whole host of other non-FreeBSD triples), you'll get an explicit emulation passed to LD that will be for a different architecture than AArch64 (note that amd64 does not pass an explicit emulation, so cross-compiling from FreeBSD/amd64 happens to not show this issue).

Also -fPIE doesn't make sense for *linking*, it's a code generation option. I'm guessing you really want -pie here?..

[jrtc27]

sys/modules/vmm/Makefile

48

(This also applies to files.arm64 AFAICT)

[andrew]

• Remove an old comment
• Remove an unneeded variable
• Disable sanitizers as they are unlikely to work
• Disable branch protection as it's untested, and lacks exception handlers
• Use ld directly to link vmm_hyp_blob.elf.full

[markj]

sys/arm64/vmm/vmm_arm64.c

961

We need to check for failure from ptp_hold().

[andrew]

• Remove VPIPT support, it's reserved from the 2023-09 system registers XML
• Check the result of ptp_hold
• Support non-PAGE_SIZE guests in vmmops_gla2gpa

[jrtc27]

Various points that all combine to break Linux during early boot. Hacking up the redistributor code to make it work for the vcpus=1 case lets Linux boot somewhat (it no longer panics in early boot and instead its virtio block driver fails to probe so it has no rootfs and falls back on an initramfs shell), but proper reworking is needed to support vcpus>1 sensibly.

It's a miracle FreeBSD is somehow able to cope with this messed up redistributor situation.

sys/arm64/vmm/io/vgic_v3.c

477

maxcpus is the system limit not the actual number present, so unless you configure the maximum all at once you'll end up with last_vcpu always being false and so the list is never terminated. But there are bigger issues here.

1315

1350

1779

Unlike the distributor, which is a CPU-local view, the redistributor is global and so each vCPU needs its own 2*64K block, with the values seen a function of the address used, not the vCPU accessing it.

sys/arm64/vmm/vmm.c

234

Be consistent about this or VM_MAXCPU. A sysctl that doesn't consistently set everything is useless. vm_create uses the macro, not this.

246

This is way too big for the GIC redistributor

[jrtc27]

sys/arm64/vmm/io/vgic_v3.c

1779

(Uh, right, distributor is a global thing too, but its state is global, versus the redistributor where there's one per PE)

[andrew]

• Limit the number of CPUs based on the vgic limit
• Fix GICR_ICFGR1
• Fix the redist register space to no alias
• Use vm_maxcpu to set the vcpu limit
• Limit which bits to read from a register

Includes cleanups based on a similar change from @jrtc27. Earlier
versions of this patch have been tested to allow Linux to boot
(along with userspace fixes)

[jrtc27]

sys/arm64/vmm/io/vgic_v3.c

1852

This is unhelpful, it means the memory region cannot just be a constant in things like u-boot. Why can't we just remove this check? We can handle CPUs past vm_get_maxcpus in the read and write functions, and vgic_v3_max_cpu_count caps at vm_get_maxcpus, so last_vcpu should still be computed correctly, just with a bunch of RES0 in memory at the end past the last real tables.

[jrtc27]

sys/arm64/vmm/io/vgic_v3.c

1640

This can be null

1694

This can be null

[jrtc27]

https://github.com/CTSRD-CHERI/cheribsd/pull/1958 has the further changes I have made to this to allow Linux to boot under CheriBSD, and should all apply to FreeBSD.

[jrtc27]

There's a bunch of code duplicated with amd64 here, e.g. in vmm_dev.c, and as such lacks a load of changes made to the amd64 code since the creation of the arm64 port. In particular, 7a0c23da4eaa63f00e53aa18f3ab1f2bb32f593a is missing, which means you need to sleep (and hope it's long enough) before booting a VM with the same name. Do you intend to pull in all the various changes that the amd64 version has seen, and ideally unify the two? It's only ioctls that are somewhat MD, I believe (some MI and some MD).

sys/arm64/vmm/vmm_dev.c

841

These two calls should be synchronous; see 7a0c23da4eaa63f00e53aa18f3ab1f2bb32f593a

[bz]

Can we at some point simply get this in (if it is in a state to attach to the build) and make further enhancements in git and have history for these changes rather than trying to polish the rather large review to perfection? I know there is a tradeoff at some point of how much stuff is outstanding...

[jrtc27]

In D37428#980518, @bz wrote:

Can we at some point simply get this in (if it is in a state to attach to the build) and make further enhancements in git and have history for these changes rather than trying to polish the rather large review to perfection? I know there is a tradeoff at some point of how much stuff is outstanding...

My view is that we should fix the known bad things, i.e. lack of synchronous sysctl and lack of proper jailing, and then iterate in tree on merging the two implementations.

Note that all of the specific issues I've brought up are things that I have patched downstream in CheriBSD, because we are trying to ship this any day now and don't want to ship things that don't work (the lack of synchrony was being hit 100% of the time in my bhyve wrapper scripts for handling reboots, and the lack of proper jailing isn't a good idea to knowingly ship with).

[jrtc27]

Also, vmm_dev.c is a fork of the amd64 code yet ditches its copyright (NetApp), which surely isn’t right, and I imagine other files are derived from amd64’s. Has there been any care put into verifying the copyright on any of these files?

[andrew]

• Reduce the diff with amd64 (from @markj)
• Clean up the vgicv3 driver based on a more careful reading of the spec:
  • Calculate gicr_typer as needed
  • Handle unsupported accesses
  • Require the CPU to have started before accessing the redistributor as if they are in the same power domain