[PATCH 1/2] ARM: hyp-stub: improve ABI

On Mon, Jan 09, 2017 at 12:26:39PM +0000, Russell King - ARM Linux wrote:

On Tue, Jan 03, 2017 at 10:51:49AM +0100, Christoffer Dall wrote:

quoted

Hi Russell,

On Thu, Dec 15, 2016 at 06:57:18PM +0000, Russell King - ARM Linux wrote:

quoted

What's also coming clear is that there's very few people who understand
all the interactions here, and the whole thing seems to be an undocumented
mess.  

I think the hyp stub has just served a very limited purpose so far, and
therefore is a somewhat immature implementation.  Now we've discovered a
need to clean it up, and we're all for that.  Again, I don't think the
problem is any larger than that, we just need to fix it, and it seems to
me everyone is willing to work on that.

What I want to see is some documentation of the hyp-stub, so that there
can be some element of confidence that changes there are properly
coordinated.  As I said in a follow up email:

| Either we need more people to have an understanding (so if one of them
| gets run over by a bus, we're not left floundering around) or we need
| it to be documented - even if it's just a simple comment "the ABI in
| this file is shared with XYZ, if you change the ABI here, also update
| XYZ too."

quoted

Marc even offered to work on your suggestion to support the general
hyp ABI commands in KVM.

... which is pointless, because it's a duplication of the effort I've
already put in.  My patches already do the:

#define HVC_GET_VECTORS 0
#define HVC_SET_VECTORS 1
#define HVC_SOFT_RESTART 2

thing which ARM64 does, passing the arguments in via the appropriate
registers.  However, such a change is a major revision of hyp-stub's
ABI, which completely changes the way it works.

Sorry, I'm afraid I might have been unclear.  What I meant with "general
hyp ABI commands in KVM" was, that if there's a need for KVM to support
the operations (using a unified and documented ABI) that the hyp stub
supports, then we could add that in KVM as well.  I thought your patches
added the functionality for the hyp stub, and Marc would add whichever
remaining pieces in the KVM side.


[...]

Longer term, I'd like to see the existing hypervisor documentation in
Documentation/virtual/kvm/hypercalls.txt updated with the ARM details.
According to that document, KVM effectively only exists on PPC and x86
at the present time...

I'm afraid I don't think this is right place to document this behavior.

There's a difference between an internal ABI between code running in two
CPU modes but both part of the same kernel, and a hypervisor running
a guest OS on top.

I believe that Documentation/virtual/kvm/hypercalls.txt documents the
latter case (i.e. guest hypercalls supported by the KVM host
hypervisor), not the former case, and these things should not be
combined.

I would suggest adding something like
Documentation/virtual/kvm/arm/hyp-abi.txt instead.

-Christoffer