LKML Archive on lore.kernel.org
 help / color / Atom feed
From: yezengruan <yezengruan@huawei.com>
To: Steven Price <steven.price@arm.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<kvmarm@lists.cs.columbia.edu>, <kvm@vger.kernel.org>,
	<linux-doc@vger.kernel.org>,
	<virtualization@lists.linux-foundation.org>
Cc: <maz@kernel.org>, <james.morse@arm.com>, <linux@armlinux.org.uk>,
	<suzuki.poulose@arm.com>, <julien.thierry.kdev@gmail.com>,
	<catalin.marinas@arm.com>, <mark.rutland@arm.com>,
	<will@kernel.org>, <daniel.lezcano@linaro.org>,
	"Wanghaibin (D)" <wanghaibin.wang@huawei.com>
Subject: Re: [PATCH v2 3/6] KVM: arm64: Support pvlock preempted via shared structure
Date: Sat, 11 Jan 2020 15:30:42 +0800
Message-ID: <c479977c-3824-4b53-ef46-300d59ac35de@huawei.com> (raw)
In-Reply-To: <468e2bb4-8986-5e1e-8c4a-31aa56a9ae4f@arm.com>

Hi Steve,

On 2020/1/9 23:02, Steven Price wrote:
> On 26/12/2019 13:58, Zengruan Ye wrote:
>> Implement the service call for configuring a shared structure between a
>> VCPU and the hypervisor in which the hypervisor can tell the VCPU is
>> running or not.
>>
>> The preempted field is zero if 1) some old KVM deos not support this filed.
> 
> NIT: s/deos/does/

Thanks for posting this.

> 
> However, I would hope that the service call will fail if it's an old KVM not simply return zero.

Sorry, I'm not sure what you mean. The service call will fail if it's an old KVM, and the Guest will use __native_vcpu_is_preempted.

> 
>> 2) the VCPU is not preempted. Other values means the VCPU has been preempted.
>>
>> Signed-off-by: Zengruan Ye <yezengruan@huawei.com>
>> ---
>>   arch/arm/include/asm/kvm_host.h   | 18 ++++++++++++
>>   arch/arm64/include/asm/kvm_host.h | 19 +++++++++++++
>>   arch/arm64/kvm/Makefile           |  1 +
>>   virt/kvm/arm/arm.c                |  8 ++++++
>>   virt/kvm/arm/hypercalls.c         |  8 ++++++
>>   virt/kvm/arm/pvlock.c             | 46 +++++++++++++++++++++++++++++++
>>   6 files changed, 100 insertions(+)
>>   create mode 100644 virt/kvm/arm/pvlock.c
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 556cd818eccf..dfeaf9204875 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -356,6 +356,24 @@ static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
>>       return false;
>>   }
>>   +static inline void kvm_arm_pvlock_preempted_init(struct kvm_vcpu_arch *vcpu_arch)
>> +{
>> +}
>> +
>> +static inline bool kvm_arm_is_pvlock_preempted_ready(struct kvm_vcpu_arch *vcpu_arch)
>> +{
>> +    return false;
>> +}
>> +
>> +static inline gpa_t kvm_init_pvlock(struct kvm_vcpu *vcpu)
>> +{
>> +    return GPA_INVALID;
>> +}
>> +
>> +static inline void kvm_update_pvlock_preempted(struct kvm_vcpu *vcpu, u64 preempted)
>> +{
>> +}
>> +
>>   void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>>     struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index c61260cf63c5..2818a2330f92 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -354,6 +354,12 @@ struct kvm_vcpu_arch {
>>           u64 last_steal;
>>           gpa_t base;
>>       } steal;
>> +
>> +    /* Guest PV lock state */
>> +    struct {
>> +        u64 preempted;
> 
> I'm not sure why the kernel needs to (separately) track this preempted state? It doesn't appear to be used from what I can tell.

Good point, the preempted state field is not actually used, I'll remove it.

> 
> Steve
> 
>> +        gpa_t base;
>> +    } pv;
>>   };
>>     /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
>> @@ -515,6 +521,19 @@ static inline bool kvm_arm_is_pvtime_enabled(struct kvm_vcpu_arch *vcpu_arch)
>>       return (vcpu_arch->steal.base != GPA_INVALID);
>>   }
>>   +static inline void kvm_arm_pvlock_preempted_init(struct kvm_vcpu_arch *vcpu_arch)
>> +{
>> +    vcpu_arch->pv.base = GPA_INVALID;
>> +}
>> +
>> +static inline bool kvm_arm_is_pvlock_preempted_ready(struct kvm_vcpu_arch *vcpu_arch)
>> +{
>> +    return (vcpu_arch->pv.base != GPA_INVALID);
>> +}
>> +
>> +gpa_t kvm_init_pvlock(struct kvm_vcpu *vcpu);
>> +void kvm_update_pvlock_preempted(struct kvm_vcpu *vcpu, u64 preempted);
>> +
>>   void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
>>     struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
>> index 5ffbdc39e780..e4591f56d5f1 100644
>> --- a/arch/arm64/kvm/Makefile
>> +++ b/arch/arm64/kvm/Makefile
>> @@ -15,6 +15,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arm.o $(KVM)/arm/mmu.o $(KVM)/arm/mmio.
>>   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
>>   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hypercalls.o
>>   kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/pvtime.o
>> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/pvlock.o
>>     kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
>>   kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 8de4daf25097..36d57e77d3c4 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -383,6 +383,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>>         kvm_arm_pvtime_vcpu_init(&vcpu->arch);
>>   +    kvm_arm_pvlock_preempted_init(&vcpu->arch);
>> +
>>       return kvm_vgic_vcpu_init(vcpu);
>>   }
>>   @@ -421,6 +423,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>           vcpu_set_wfx_traps(vcpu);
>>         vcpu_ptrauth_setup_lazy(vcpu);
>> +
>> +    if (kvm_arm_is_pvlock_preempted_ready(&vcpu->arch))
>> +        kvm_update_pvlock_preempted(vcpu, 0);
>>   }
>>     void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>> @@ -434,6 +439,9 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>       vcpu->cpu = -1;
>>         kvm_arm_set_running_vcpu(NULL);
>> +
>> +    if (kvm_arm_is_pvlock_preempted_ready(&vcpu->arch))
>> +        kvm_update_pvlock_preempted(vcpu, 1);
>>   }
>>     static void vcpu_power_off(struct kvm_vcpu *vcpu)
>> diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c
>> index 550dfa3e53cd..1c6a11f21bb4 100644
>> --- a/virt/kvm/arm/hypercalls.c
>> +++ b/virt/kvm/arm/hypercalls.c
>> @@ -52,6 +52,9 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>>           case ARM_SMCCC_HV_PV_TIME_FEATURES:
>>               val = SMCCC_RET_SUCCESS;
>>               break;
>> +        case ARM_SMCCC_HV_PV_LOCK_FEATURES:
>> +            val = SMCCC_RET_SUCCESS;
>> +            break;
>>           }
>>           break;
>>       case ARM_SMCCC_HV_PV_TIME_FEATURES:
>> @@ -62,6 +65,11 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
>>           if (gpa != GPA_INVALID)
>>               val = gpa;
>>           break;
>> +    case ARM_SMCCC_HV_PV_LOCK_PREEMPTED:
>> +        gpa = kvm_init_pvlock(vcpu);
>> +        if (gpa != GPA_INVALID)
>> +            val = gpa;
>> +        break;
>>       default:
>>           return kvm_psci_call(vcpu);
>>       }
>> diff --git a/virt/kvm/arm/pvlock.c b/virt/kvm/arm/pvlock.c
>> new file mode 100644
>> index 000000000000..cdfd30a903b9
>> --- /dev/null
>> +++ b/virt/kvm/arm/pvlock.c
>> @@ -0,0 +1,46 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright(c) 2019 Huawei Technologies Co., Ltd
>> + * Author: Zengruan Ye <yezengruan@huawei.com>
>> + */
>> +
>> +#include <linux/arm-smccc.h>
>> +#include <linux/kvm_host.h>
>> +
>> +#include <asm/pvlock-abi.h>
>> +
>> +#include <kvm/arm_hypercalls.h>
>> +
>> +gpa_t kvm_init_pvlock(struct kvm_vcpu *vcpu)
>> +{
>> +    struct pvlock_vcpu_state init_values = {};
>> +    struct kvm *kvm = vcpu->kvm;
>> +    u64 base = vcpu->arch.pv.base;
>> +    int idx;
>> +
>> +    if (base == GPA_INVALID)
>> +        return base;
>> +
>> +    idx = srcu_read_lock(&kvm->srcu);
>> +    kvm_write_guest(kvm, base, &init_values, sizeof(init_values));
>> +    srcu_read_unlock(&kvm->srcu, idx);
>> +
>> +    return base;
>> +}
>> +
>> +void kvm_update_pvlock_preempted(struct kvm_vcpu *vcpu, u64 preempted)
>> +{
>> +    int idx;
>> +    u64 offset;
>> +    __le64 preempted_le;
>> +    struct kvm *kvm = vcpu->kvm;
>> +    u64 base = vcpu->arch.pv.base;
>> +
>> +    vcpu->arch.pv.preempted = preempted;
>> +    preempted_le = cpu_to_le64(preempted);
>> +
>> +    idx = srcu_read_lock(&kvm->srcu);
>> +    offset = offsetof(struct pvlock_vcpu_state, preempted);
>> +    kvm_put_guest(kvm, base + offset, preempted_le, u64);
>> +    srcu_read_unlock(&kvm->srcu, idx);
>> +}
>>
> 
> 
> .

Thanks,

Zengruan


  reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-26 13:58 [PATCH v2 0/6] KVM: arm64: VCPU preempted check support Zengruan Ye
2019-12-26 13:58 ` [PATCH v2 1/6] KVM: arm64: Document PV-lock interface Zengruan Ye
2020-01-09 14:53   ` Steven Price
2020-01-11  6:51     ` yezengruan
2019-12-26 13:58 ` [PATCH v2 2/6] KVM: arm64: Add SMCCC paravirtualised lock calls Zengruan Ye
2019-12-26 13:58 ` [PATCH v2 3/6] KVM: arm64: Support pvlock preempted via shared structure Zengruan Ye
2020-01-09 15:02   ` Steven Price
2020-01-11  7:30     ` yezengruan [this message]
2020-01-13 10:31       ` Steven Price
2019-12-26 13:58 ` [PATCH v2 4/6] KVM: arm64: Provide VCPU attributes for PV lock Zengruan Ye
2019-12-26 13:58 ` [PATCH v2 5/6] KVM: arm64: Add interface to support VCPU preempted check Zengruan Ye
2019-12-26 18:51   ` kbuild test robot
2019-12-27  6:52     ` yezengruan
2019-12-26 13:58 ` [PATCH v2 6/6] KVM: arm64: Support the VCPU preemption check Zengruan Ye
2020-01-09 15:09   ` Steven Price
2020-01-11  7:33     ` yezengruan
2020-01-13 12:12 ` [PATCH v2 0/6] KVM: arm64: VCPU preempted check support Will Deacon
2020-01-15 14:14   ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c479977c-3824-4b53-ef46-300d59ac35de@huawei.com \
    --to=yezengruan@huawei.com \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=james.morse@arm.com \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=steven.price@arm.com \
    --cc=suzuki.poulose@arm.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=wanghaibin.wang@huawei.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git