Re: [PATCH] Loongarch: KVM: Add KVM hypercalls documentation for LoongArch

From: Mingcong Bai
Date: Wed Jul 31 2024 - 05:21:34 EST

Hi Yuli,

Thanks for submitting this documentation. I have just a couple suggestions on prose and grammar. Please see below.

在 2024-07-31 13:57，WangYuli 写道：
From: Bibo Mao <maobibo@xxxxxxxxxxx>

Add documentation topic for using pv_virt when running as a guest
on KVM hypervisor.

Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
Signed-off-by: Xianglai Li <lixianglai@xxxxxxxxxxx>
Signed-off-by: WangYuli <wangyuli@xxxxxxxxxxxxx>
---
Documentation/virt/kvm/index.rst | 1 +
.../virt/kvm/loongarch/hypercalls.rst | 79 +++++++++++++++++++
Documentation/virt/kvm/loongarch/index.rst | 10 +++
MAINTAINERS | 1 +
4 files changed, 91 insertions(+)
create mode 100644 Documentation/virt/kvm/loongarch/hypercalls.rst
create mode 100644 Documentation/virt/kvm/loongarch/index.rst

diff --git a/Documentation/virt/kvm/index.rst b/Documentation/virt/kvm/index.rst
index ad13ec55ddfe..9ca5a45c2140 100644
--- a/Documentation/virt/kvm/index.rst
+++ b/Documentation/virt/kvm/index.rst
@@ -14,6 +14,7 @@ KVM
s390/index
ppc-pv
x86/index
+ loongarch/index

locking
vcpu-requests
diff --git a/Documentation/virt/kvm/loongarch/hypercalls.rst b/Documentation/virt/kvm/loongarch/hypercalls.rst
new file mode 100644
index 000000000000..1679e48d67d2
--- /dev/null
+++ b/Documentation/virt/kvm/loongarch/hypercalls.rst
@@ -0,0 +1,79 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+The LoongArch paravirtual interface
+===================================
+
+KVM hypercalls use the HVCL instruction with code 0x100, and the hypercall
+number is put in a0 and up to five arguments may be placed in a1-a5, the
+return value is placed in v0 (alias with a0).

The original paragraph can be split into a few sentences for better readability:

"KVM hypercalls use the HVCL instruction with code 0x100 and the hypercall number is put in a0. Up to five arguments may be placed in registers a1 - a5. The return value is placed in v0 (an alias of a0)."

Not sure if "HVCL instruction with code 0x100" is a proper expression, so I have left it alone (I'm a little suspicious). Others are welcome to comment on this.

+
+The code for that interface can be found in arch/loongarch/kvm/*

It would seem that this sentence (or rather the components and structures of this whole documentation?) was borrowed from Documentation/virt/kvm/ppc-pv.rst. But nevertheless:

"Source code for this interface can be found in arch/loongarch/kvm*."

+
+Querying for existence
+======================
+
+To find out if we're running on KVM or not, cpucfg can be used with index
+CPUCFG_KVM_BASE (0x40000000), cpucfg range between 0x40000000 - 0x400000FF
+is marked as a specially reserved range. All existing and future processors
+will not implement any features in this range.

Again, please consider splitting up the sentences:

"We can use cpucfg() at index CPUCFG_KVM_BASE (0x40000000) to query whether the host is running on KVM. The CPUCPU_KVM_BASE range between 0x40000000 - 0x400000FF is marked as reserved - all existing and future processors will not implement any features in this range."

+
+When Linux is running on KVM, cpucfg with index CPUCFG_KVM_BASE (0x40000000)
+returns magic string "KVM\0"

When the Linux host is running on KVM, a read on cpucfg() at index CPUCFG_KVM_BASE (0x40000000) returns a magic string "KVM\0".

+
+Once you determined you're running under a PV capable KVM, you can now use
+hypercalls as described below.

Once you have determined that your host is running on a paravirtualization-capable KVM, you may now use hypercalls as described below.

+
+KVM hypercall ABI
+=================
+
+Hypercall ABI on KVM is simple, only one scratch register a0 (v0) and at most
+five generic registers used as input parameter. FP register and vector register
+is not used for input register and should not be modified during hypercall.
+Hypercall function can be inlined since there is only one scratch register.

"The KVM hypercall ABI is simple, with one scratch register a0 (v0) and at most five generic registers (a1 - a5) used as input parameters. The FP and vector registers are not used as input registers and should not be modified in a hypercall. Hypercall functions can be inlined, as there is only one scratch register."

It is recommended to define what the "The FP and vector registers" are with parentheses.

+
+The parameters are as follows:
+
+ ======== ================ ================
+ Register IN OUT
+ ======== ================ ================
+ a0 function number Return code

Function index?

+ a1 1st parameter -
+ a2 2nd parameter -
+ a3 3rd parameter -
+ a4 4th parameter -
+ a5 5th parameter -
+ ======== ================ ================
+
+Return codes can be as follows:

The return codes may be one of the following:

+
+ ==== =========================
+ Code Meaning
+ ==== =========================
+ 0 Success
+ -1 Hypercall not implemented
+ -2 Hypercall parameter error

Bad hypercall parameter

+ ==== =========================
+
+KVM Hypercalls Documentation
+============================
+
+The template for each hypercall is:

"The template for each hypercall is as follows:"

Also, please consider adding a blank line here (though it probably doesn't matter once rendered, but it would make the plain text more readable).

+1. Hypercall name
+2. Purpose
+
+1. KVM_HCALL_FUNC_PV_IPI
+------------------------
+
+:Purpose: Send IPIs to multiple vCPUs.
+
+- a0: KVM_HCALL_FUNC_PV_IPI
+- a1: lower part of the bitmap of destination physical CPUIDs

Lower part of the bitmap for destination physical CPUIDs.

+- a2: higher part of the bitmap of destination physical CPUIDs

Ditto, please capitalize the first letter after ":" and use "for" before "destination physical CPUIDs".

+- a3: the lowest physical CPUID in bitmap

The lowest physical CPUID in the bitmap.

+
+The hypercall lets a guest send multicast IPIs, with at most 128
+destinations per hypercall. The destinations are represented by a bitmap
+contained in the first two arguments (a1 and a2). Bit 0 of a1 corresponds
+to the physical CPUID in the third argument (a3), bit 1 corresponds to the
+physical ID a3+1, and so on.

Cleaning up sentences and dropping inconsistent expressions (such as "argument registers", which was never brought up before). I would recommend making them all consistent throughout by classifying a0 and a1 - a5.

The hypercall lets a guest send multiple IPIs (Inter-Process Interrupts) with at most 128 destinations per hypercall. The destinations are represented in a bitmap contained in the first two input registers (a1 and a2). Bit 0 of a1 corresponds to the physical CPUID in the third input register (a3) and bit 1 corresponds to the physical CPUID in a3+1 (a4), and so on.

diff --git a/Documentation/virt/kvm/loongarch/index.rst b/Documentation/virt/kvm/loongarch/index.rst
new file mode 100644
index 000000000000..83387b4c5345
--- /dev/null
+++ b/Documentation/virt/kvm/loongarch/index.rst
@@ -0,0 +1,10 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+KVM for LoongArch systems
+=========================
+
+.. toctree::
+ :maxdepth: 2
+
+ hypercalls.rst
diff --git a/MAINTAINERS b/MAINTAINERS
index 958e935449e5..8aa5d92b12ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12073,6 +12073,7 @@ L: kvm@xxxxxxxxxxxxxxx
L: loongarch@xxxxxxxxxxxxxxx
S: Maintained
T: git git://git.kernel.org/pub/scm/virt/kvm/kvm.git
+F: Documentation/virt/kvm/loongarch/
F: arch/loongarch/include/asm/kvm*
F: arch/loongarch/include/uapi/asm/kvm*
F: arch/loongarch/kvm/

Best Regards,
Mingcong Bai