[PATCH v4 28/28] arm/arm64: Add hyp-stub API documentation

On Fri, Mar 24, 2017 at 03:57:41PM +0000, Marc Zyngier wrote:

On 24/03/17 15:23, Christoffer Dall wrote:

quoted

On Fri, Mar 24, 2017 at 02:42:13PM +0000, Marc Zyngier wrote:

quoted

On 24/03/17 14:33, Christoffer Dall wrote:

quoted

On Tue, Mar 21, 2017 at 07:20:58PM +0000, Marc Zyngier wrote:

quoted

In order to help people understanding the hyp-stub API that exists
between the host kernel and the hypervisor mode (whether a hypervisor
has been installed or not), let's document said API.

As with any form of documentation, I expect it to become obsolete
and completely misleading within 20 minutes after having being merged.

I don't think this last sentence belongs in the commit message.

quoted

Acked-by: Russell King <redacted>
Signed-off-by: Marc Zyngier <redacted>
---
 Documentation/virtual/kvm/arm/hyp-abi.txt | 45 +++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/virtual/kvm/arm/hyp-abi.txt

diff --git a/Documentation/virtual/kvm/arm/hyp-abi.txt b/Documentation/virtual/kvm/arm/hyp-abi.txt
new file mode 100644
index 000000000000..a1e0314d2249
--- /dev/null
+++ b/Documentation/virtual/kvm/arm/hyp-abi.txt

@@ -0,0 +1,45 @@
+* Internal ABI between the kernel and HYP
+
+This file documents the interaction between the Linux kernel and the
+hypervisor layer when running Linux as a hypervisor (for example
+KVM). It doesn't cover the interaction of the kernel with the
+hypervisor when running as a guest (under Xen, KVM or any other
+hypervisor), or any hypervisor-specific interaction when the kernel is
+used as a host.
+
+On arm and arm64 (without VHE), the kernel doesn't run in hypervisor
+mode, but still needs to interact with it, allowing a built-in
+hypervisor to be either installed or torn down.
+
+In order to achieve this, the kernel must be booted at HYP (arm) or
+EL2 (arm64), allowing it to install a set of stubs before dropping to
+SVC/EL1. These stubs are accessible by using a 'hvc #0' instruction,
+and only act on individual CPUs.
+
+Unless specified otherwise, any built-in hypervisor must implement
+these functions (see arch/arm{,64}/include/asm/virt.h):
+
+* r0/x0 = HVC_SET_VECTORS
+  r1/x1 = vectors
+
+  Set HVBAR/VBAR_EL2 to 'vectors' to enable a hypervisor. 'vectors'
+  must be a physical address, and respect the alignment requirements
+  of the architecture. Only implemented by the initial stubs.

Does this last sentence mean that KVM doesn't implement this function?

Indeed. 

I think the wording could be improved to "Only implemented by the
initial stubs, not by Linux hypervisors."

Looks good to me, I'll use that.

quoted

quoted

Directly setting the vectors is inherently unsafe (MMU could
still be ON, for example), hence the HVC_SET_VECTORS operation that
allow this to be done safely (RESET followed by SET).

Hmm, is it any less safe than setting the vectors using the hyp stubs?
In the initial case, we're relying on the caller knowing that the MMU is
off, and that the stubs are in place, otherwise it doesn't make sense.

That's why I was implying here that RESET+SET is the safe way of doing
it. Another solution may be to make SET imply RESET, and keep it
implemented always?

Meh, I think these patches are as stable as is required for the use
cases at hand, so let's not introduce more churn in these patches.

quoted

But I think the point is just that we don't have a need to change the
vector from within KVM; we only have a need to set the vector when going
from stub->kvm, and we have a need to reset the whole thing (including
turning off the MMU) when going from kvm->stub.

Exactly. The current API doesn't try to cater for all possible cases. It
just make sure that KVM can be installed and torn down safely.

Fair enough.

quoted

Anyhow, my comment was just about the wording, as I had some nagging
doubt when I read the patch.

quoted

quoted

quoted

+
+* r0/x0 = HVC_RESET_VECTORS
+
+  Turn HYP/EL2 MMU off, and reset HVBAR/VBAR_EL2 to the default
+  value. This effectively disables an existing hypervisor.

What's the 'default value' ?  Could we say to the physical address of
the hypervisor stub's exception vector?

Shouldn't that be the kernel stub's exception vector?

Yeah, the "initials stubs' exception vector" is probably the most
coherent thing we can use here.

I'll use that when respinning it.

Thanks,
-Christoffer