Hi Steve,
On 2020/1/9 22:53, Steven Price wrote:
> On 26/12/2019 13:58, Zengruan Ye wrote:
>> Introduce a paravirtualization interface for KVM/arm64 to obtain the VCPU
>> is currently running or not.
>>
>> The PV lock structure of the guest is allocated by user space.
>>
>> A hypercall interface is provided for the guest to interrogate the
>> hypervisor's support for this interface and the location of the shared
>> memory structures.
>>
>> Signed-off-by: Zengruan Ye
>> ---
>> Documentation/virt/kvm/arm/pvlock.rst | 63 +
>> Documentation/virt/kvm/devices/vcpu.txt | 14 ++
>> 2 files changed, 77 insertions(+)
>> create mode 100644 Documentation/virt/kvm/arm/pvlock.rst
>>
>> diff --git a/Documentation/virt/kvm/arm/pvlock.rst
>> b/Documentation/virt/kvm/arm/pvlock.rst
>> new file mode 100644
>> index ..58b3b8ee7537
>> --- /dev/null
>> +++ b/Documentation/virt/kvm/arm/pvlock.rst
>> @@ -0,0 +1,63 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +Paravirtualized lock support for arm64
>> +==
>> +
>> +KVM/arm64 provides some hypervisor service calls to support a
>> paravirtualized
>> +guest obtaining the VCPU is currently running or not.
> NIT: ^ whether
Thanks for posting this.
>
>> +
>> +Two new SMCCC compatible hypercalls are defined:
>> +
>> +* PV_LOCK_FEATURES: 0xC620
>> +* PV_LOCK_PREEMPTED: 0xC621
>> +
>> +The existence of the PV_LOCK hypercall should be probed using the SMCCC 1.1
>> +ARCH_FEATURES mechanism before calling it.
>
> Since these are within the "vendor specific" SMCCC region ideally you should
> also check that you are talking to KVM. (Other hypervisors could allocate
> SMCCC IDs differently within this block). Will has a patch on a branch which
> gives an example of how this could work [1]
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/commit/?h=kvm/hvc&id=464f5a1741e5959c3e4d2be1966ae0093b4dce06
OK, I will add "vendor specific" check next version.
>
>> +
>> +PV_LOCK_FEATURES
>> + = ==
>> + Function ID: (uint32) 0xC620
>> + PV_call_id: (uint32) The function to query for support.
>> + Return value: (int64) NOT_SUPPORTED (-1) or SUCCESS (0) if the
>> relevant
>> + PV-lock feature is supported by the
>> hypervisor.
>> + = ==
>> +
>> +PV_LOCK_PREEMPTED
>> + = ==
>> + Function ID: (uint32) 0xC621
>> + Return value: (int64) NOT_SUPPORTED (-1) or SUCCESS (0) if the IPA
>> of
>> + this VCPU's pv data structure is configured by
>> + the hypervisor.
>> + = ==
>
> PV_LOCK_PREEMPTED also needs to return the address of this data structure.
> Either by returning this in another register, or by e.g. treating a positive
> return as an address and a negative value as an error.
This is somewhat embarrassing. The code does what you say, but the doc doesn't.
Thanks for pointing it out to me! I'll update the doc to match.
>
>> +
>> +The IPA returned by PV_LOCK_PREEMPTED should be mapped by the guest as
>> normal
>> +memory with inner and outer write back caching attributes, in the inner
>> +shareable domain.
>> +
>> +PV_LOCK_PREEMPTED returns the structure for the calling VCPU.
>> +
>> +PV lock state
>> +-
>> +
>> +The structure pointed to by the PV_LOCK_PREEMPTED hypercall is as follows:
>> +
>> ++---+-+-+-+
>> +| Field | Byte Length | Byte Offset | Description |
>> ++===+=+=+=+
>> +| preempted | 8 | 0 | Indicate the VCPU who owns this |
>
> NIT: s/Indicate/Indicates that/. Also more common English would be "the VCPU
> *that* owns"
Will update.
>
>> +| | | | struct is running or not. |
>> +| | | | Non-zero values mean the VCPU |
>> +| | | | has been preempted. Zero means |
>> +| | | | the VCPU is not preempted. |
>> ++---+-+-+-+
>> +
>> +The preempted field will be updated to 1 by the hypervisor prior to
>> scheduling
>> +a VCPU. When the VCPU is scheduled out, the preempted field will be updated
>> +to 0 by the hypervisor.
>> +
>> +The structure will be present within a reserved region of the normal memory
>> +given to the guest. The guest should not attempt to write into this memory.
>> +There is a structure per VCPU of the guest.
>
> I think it would be worth mentioning in this document that the structure is
> guaranteed to be 64-byte aligned.
Good point, I'll update the doc.
>
> St
