From: Mark Rutland <mark.rutland@arm.com> To: Steven Price <steven.price@arm.com> Cc: Marc Zyngier <marc.zyngier@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call Date: Mon, 10 Dec 2018 10:39:44 +0000 [thread overview] Message-ID: <20181210103943.g5txylpjvfxdolpi@lakrids.cambridge.arm.com> (raw) In-Reply-To: <20181128144527.44710-6-steven.price@arm.com> On Wed, Nov 28, 2018 at 02:45:20PM +0000, Steven Price wrote: > This provides a mechanism for querying which paravirtualized features > are available in this hypervisor. > > Also add the header file which defines the ABI for the paravirtualized > clock features we're about to add. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > arch/arm64/include/asm/pvclock-abi.h | 32 ++++++++++++++++++++++++++++ > include/kvm/arm_pv.h | 28 ++++++++++++++++++++++++ > include/linux/arm-smccc.h | 1 + > virt/kvm/arm/hypercalls.c | 9 ++++++++ > 4 files changed, 70 insertions(+) > create mode 100644 arch/arm64/include/asm/pvclock-abi.h > create mode 100644 include/kvm/arm_pv.h > > diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h > new file mode 100644 > index 000000000000..64ce041c8922 > --- /dev/null > +++ b/arch/arm64/include/asm/pvclock-abi.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright (C) 2018 Arm Ltd. */ > + > +#ifndef __ASM_PVCLOCK_ABI_H > +#define __ASM_PVCLOCK_ABI_H > + > +#include <kvm/arm_pv.h> > + > +struct pvclock_vm_time_info { > + __le32 revision; > + __le32 attributes; > + __le64 sequence_number; > + __le64 scale_mult; > + __le32 shift; > + __le32 reserved; > + __le64 native_freq; > + __le64 pv_freq; > + __le64 div_by_pv_freq_mult; > +} __packed; > + > +struct pvclock_vcpu_stolen_time_info { > + __le32 revision; > + __le32 attributes; > + __le64 stolen_time; > + /* Structure must be 64 byte aligned, pad to that size */ > + u8 padding[48]; > +} __packed; > + > +#define PV_VM_TIME_NOT_SUPPORTED -1 > +#define PV_VM_TIME_INVALID_PARAMETERS -2 Could you please add a comment describing that these are defined in ARM DEN0057A? > + > +#endif > diff --git a/include/kvm/arm_pv.h b/include/kvm/arm_pv.h > new file mode 100644 > index 000000000000..19d2dafff31a > --- /dev/null > +++ b/include/kvm/arm_pv.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * Copyright (C) 2018 Arm Ltd. > + */ > + > +#ifndef __KVM_ARM_PV_H > +#define __KVM_ARM_PV_H > + > +#include <linux/arm-smccc.h> > + > +#define ARM_SMCCC_HV_PV_FEATURES \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_64, \ > + ARM_SMCCC_OWNER_HYP_STANDARD, \ > + 0x20) > + > +#define ARM_SMCCC_HV_PV_TIME_LPT \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_64, \ > + ARM_SMCCC_OWNER_HYP_STANDARD, \ > + 0x21) > + > +#define ARM_SMCCC_HV_PV_TIME_ST \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_64, \ > + ARM_SMCCC_OWNER_HYP_STANDARD, \ > + 0x22) > + > +#endif /* __KVM_ARM_PV_H */ Do these need to live in a separate header, away from the struct definitions? I'd be happy for these to live in <linux/arm-smccc.h>, given they're standard calls. As before, a comment referring to ARM DEN0057A would be nice. > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > index b047009e7a0a..4e0866cc48c0 100644 > --- a/include/linux/arm-smccc.h > +++ b/include/linux/arm-smccc.h > @@ -54,6 +54,7 @@ > #define ARM_SMCCC_OWNER_SIP 2 > #define ARM_SMCCC_OWNER_OEM 3 > #define ARM_SMCCC_OWNER_STANDARD 4 > +#define ARM_SMCCC_OWNER_HYP_STANDARD 5 Minor nit, but could we make that STANDARD_HYP? > #define ARM_SMCCC_OWNER_TRUSTED_APP 48 > #define ARM_SMCCC_OWNER_TRUSTED_APP_END 49 > #define ARM_SMCCC_OWNER_TRUSTED_OS 50 > diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c > index 153aa7642100..ba13b798f0f8 100644 > --- a/virt/kvm/arm/hypercalls.c > +++ b/virt/kvm/arm/hypercalls.c > @@ -5,6 +5,7 @@ > #include <linux/kvm_host.h> > > #include <asm/kvm_emulate.h> > +#include <asm/pvclock-abi.h> > > #include <kvm/arm_hypercalls.h> > #include <kvm/arm_psci.h> > @@ -40,6 +41,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) > break; > } > break; > + case ARM_SMCCC_HV_PV_FEATURES: > + val = SMCCC_RET_SUCCESS; > + break; > + } > + break; > + case ARM_SMCCC_HV_PV_FEATURES: > + feature = smccc_get_arg1(vcpu); > + switch (feature) { > } IIUC, at this point in time, this happens to always return SMCCC_RET_NOT_SUPPORTED. If you leave this part out of the patch, and add it as required, this patch is purely adding definitions, which would be a bit nicer for review. Thanks, Mark.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com> To: Steven Price <steven.price@arm.com> Cc: Marc Zyngier <marc.zyngier@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Christoffer Dall <christoffer.dall@arm.com>, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call Date: Mon, 10 Dec 2018 10:39:44 +0000 [thread overview] Message-ID: <20181210103943.g5txylpjvfxdolpi@lakrids.cambridge.arm.com> (raw) In-Reply-To: <20181128144527.44710-6-steven.price@arm.com> On Wed, Nov 28, 2018 at 02:45:20PM +0000, Steven Price wrote: > This provides a mechanism for querying which paravirtualized features > are available in this hypervisor. > > Also add the header file which defines the ABI for the paravirtualized > clock features we're about to add. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > arch/arm64/include/asm/pvclock-abi.h | 32 ++++++++++++++++++++++++++++ > include/kvm/arm_pv.h | 28 ++++++++++++++++++++++++ > include/linux/arm-smccc.h | 1 + > virt/kvm/arm/hypercalls.c | 9 ++++++++ > 4 files changed, 70 insertions(+) > create mode 100644 arch/arm64/include/asm/pvclock-abi.h > create mode 100644 include/kvm/arm_pv.h > > diff --git a/arch/arm64/include/asm/pvclock-abi.h b/arch/arm64/include/asm/pvclock-abi.h > new file mode 100644 > index 000000000000..64ce041c8922 > --- /dev/null > +++ b/arch/arm64/include/asm/pvclock-abi.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright (C) 2018 Arm Ltd. */ > + > +#ifndef __ASM_PVCLOCK_ABI_H > +#define __ASM_PVCLOCK_ABI_H > + > +#include <kvm/arm_pv.h> > + > +struct pvclock_vm_time_info { > + __le32 revision; > + __le32 attributes; > + __le64 sequence_number; > + __le64 scale_mult; > + __le32 shift; > + __le32 reserved; > + __le64 native_freq; > + __le64 pv_freq; > + __le64 div_by_pv_freq_mult; > +} __packed; > + > +struct pvclock_vcpu_stolen_time_info { > + __le32 revision; > + __le32 attributes; > + __le64 stolen_time; > + /* Structure must be 64 byte aligned, pad to that size */ > + u8 padding[48]; > +} __packed; > + > +#define PV_VM_TIME_NOT_SUPPORTED -1 > +#define PV_VM_TIME_INVALID_PARAMETERS -2 Could you please add a comment describing that these are defined in ARM DEN0057A? > + > +#endif > diff --git a/include/kvm/arm_pv.h b/include/kvm/arm_pv.h > new file mode 100644 > index 000000000000..19d2dafff31a > --- /dev/null > +++ b/include/kvm/arm_pv.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * Copyright (C) 2018 Arm Ltd. > + */ > + > +#ifndef __KVM_ARM_PV_H > +#define __KVM_ARM_PV_H > + > +#include <linux/arm-smccc.h> > + > +#define ARM_SMCCC_HV_PV_FEATURES \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_64, \ > + ARM_SMCCC_OWNER_HYP_STANDARD, \ > + 0x20) > + > +#define ARM_SMCCC_HV_PV_TIME_LPT \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_64, \ > + ARM_SMCCC_OWNER_HYP_STANDARD, \ > + 0x21) > + > +#define ARM_SMCCC_HV_PV_TIME_ST \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_64, \ > + ARM_SMCCC_OWNER_HYP_STANDARD, \ > + 0x22) > + > +#endif /* __KVM_ARM_PV_H */ Do these need to live in a separate header, away from the struct definitions? I'd be happy for these to live in <linux/arm-smccc.h>, given they're standard calls. As before, a comment referring to ARM DEN0057A would be nice. > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > index b047009e7a0a..4e0866cc48c0 100644 > --- a/include/linux/arm-smccc.h > +++ b/include/linux/arm-smccc.h > @@ -54,6 +54,7 @@ > #define ARM_SMCCC_OWNER_SIP 2 > #define ARM_SMCCC_OWNER_OEM 3 > #define ARM_SMCCC_OWNER_STANDARD 4 > +#define ARM_SMCCC_OWNER_HYP_STANDARD 5 Minor nit, but could we make that STANDARD_HYP? > #define ARM_SMCCC_OWNER_TRUSTED_APP 48 > #define ARM_SMCCC_OWNER_TRUSTED_APP_END 49 > #define ARM_SMCCC_OWNER_TRUSTED_OS 50 > diff --git a/virt/kvm/arm/hypercalls.c b/virt/kvm/arm/hypercalls.c > index 153aa7642100..ba13b798f0f8 100644 > --- a/virt/kvm/arm/hypercalls.c > +++ b/virt/kvm/arm/hypercalls.c > @@ -5,6 +5,7 @@ > #include <linux/kvm_host.h> > > #include <asm/kvm_emulate.h> > +#include <asm/pvclock-abi.h> > > #include <kvm/arm_hypercalls.h> > #include <kvm/arm_psci.h> > @@ -40,6 +41,14 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu) > break; > } > break; > + case ARM_SMCCC_HV_PV_FEATURES: > + val = SMCCC_RET_SUCCESS; > + break; > + } > + break; > + case ARM_SMCCC_HV_PV_FEATURES: > + feature = smccc_get_arg1(vcpu); > + switch (feature) { > } IIUC, at this point in time, this happens to always return SMCCC_RET_NOT_SUPPORTED. If you leave this part out of the patch, and add it as required, this patch is purely adding definitions, which would be a bit nicer for review. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2018-12-10 10:39 UTC|newest] Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-28 14:45 [PATCH 00/12] arm64: Paravirtualized time support Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 01/12] KVM: arm64: Document PV-time interface Steven Price 2018-11-28 14:45 ` Steven Price 2018-12-03 13:50 ` Andrew Jones 2018-12-03 13:50 ` Andrew Jones 2018-12-03 14:18 ` Marc Zyngier 2018-12-03 14:18 ` Marc Zyngier 2018-12-03 15:16 ` Andrew Jones 2018-12-03 15:16 ` Andrew Jones 2018-12-03 15:23 ` Marc Zyngier 2018-12-03 15:23 ` Marc Zyngier 2018-12-03 15:52 ` Andrew Jones 2018-12-03 15:52 ` Andrew Jones 2018-12-05 12:32 ` Steven Price 2018-12-05 12:32 ` Steven Price 2018-11-28 14:45 ` [PATCH 02/12] KVM: arm/arm64: Factor out hypercall handling from PSCI code Steven Price 2018-11-28 14:45 ` Steven Price 2018-12-03 16:02 ` Andrew Jones 2018-12-03 16:02 ` Andrew Jones 2018-11-28 14:45 ` [PATCH 03/12] arm/arm64: Provide a wrapper for SMCCC 1.1 calls Steven Price 2018-11-28 14:45 ` Steven Price 2018-12-10 10:27 ` Mark Rutland 2018-12-10 10:27 ` Mark Rutland 2018-12-10 13:52 ` Steven Price 2018-12-10 13:52 ` Steven Price 2018-11-28 14:45 ` [PATCH 04/12] arm/arm64: Make use of the SMCCC 1.1 wrapper Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 05/12] KVM: arm64: Implement PV_FEATURES call Steven Price 2018-11-28 14:45 ` Steven Price 2018-12-10 10:39 ` Mark Rutland [this message] 2018-12-10 10:39 ` Mark Rutland 2018-12-10 14:20 ` Steven Price 2018-12-10 14:20 ` Steven Price 2018-11-28 14:45 ` [PATCH 06/12] KVM: arm64: Support Live Physical Time reporting Steven Price 2018-11-28 14:45 ` Steven Price 2018-12-10 10:56 ` Mark Rutland 2018-12-10 10:56 ` Mark Rutland 2018-12-10 15:45 ` Steven Price 2018-12-10 15:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 07/12] clocksource: arm_arch_timer: Use paravirtualized LPT Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 08/12] KVM: Export mark_page_dirty_in_slot Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 09/12] KVM: arm64: Support stolen time reporting via shared page Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 10/12] arm64: Retrieve stolen time as paravirtualized guest Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 11/12] KVM: Allow kvm_device_ops to be const Steven Price 2018-11-28 14:45 ` Steven Price 2018-11-28 14:45 ` [PATCH 12/12] KVM: arm64: Provide a PV_TIME device to user space Steven Price 2018-11-28 14:45 ` Steven Price 2018-12-03 13:25 ` [PATCH 00/12] arm64: Paravirtualized time support Andrew Jones 2018-12-03 13:25 ` Andrew Jones 2018-12-03 14:36 ` Marc Zyngier 2018-12-03 14:36 ` Marc Zyngier 2018-12-05 12:30 ` Steven Price 2018-12-05 12:30 ` Steven Price 2018-12-10 11:40 ` Mark Rutland 2018-12-10 11:40 ` Mark Rutland 2018-12-10 16:08 ` Steven Price 2018-12-10 16:08 ` Steven Price 2019-01-08 10:36 ` Christoffer Dall 2019-01-08 10:36 ` Christoffer Dall
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=20181210103943.g5txylpjvfxdolpi@lakrids.cambridge.arm.com \ --to=mark.rutland@arm.com \ --cc=catalin.marinas@arm.com \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=marc.zyngier@arm.com \ --cc=steven.price@arm.com \ --cc=will.deacon@arm.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.