* [PATCH v2 0/4] kvmtool: ARM/ARM64: Misc updates
@ 2014-08-26 9:22 Anup Patel
2014-08-26 9:22 ` [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Anup Patel @ 2014-08-26 9:22 UTC (permalink / raw)
To: kvmarm
Cc: kvm, patches, will.deacon, marc.zyngier, penberg,
christoffer.dall, pranavkumar, Anup Patel
This patchset updates KVMTOOL to use some of the features
supported by Linux-3.16 KVM ARM/ARM64, such as:
1. Target CPU == Host using KVM_ARM_PREFERRED_TARGET vm ioctl
2. Target CPU type Potenza for using KVMTOOL on X-Gene
3. PSCI v0.2 support for Aarch32 and Aarch64 guest
4. System event exit reason
Changes since v1:
- Drop the patch to fix compile error for aarch64
- Fallback to old method of trying all target types if
KVM_ARM_PREFERRED_TARGET vm ioctl fails
- Print more info when handling KVM_EXIT_SYSTEM_EVENT
Anup Patel (4):
kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine
target cpu
kvmtool: ARM64: Add target type potenza for aarch64
kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT
kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it
tools/kvm/arm/aarch64/arm-cpu.c | 9 ++++++-
tools/kvm/arm/fdt.c | 39 +++++++++++++++++++++++++-----
tools/kvm/arm/kvm-cpu.c | 51 ++++++++++++++++++++++++++++++---------
tools/kvm/kvm-cpu.c | 19 +++++++++++++++
4 files changed, 100 insertions(+), 18 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu 2014-08-26 9:22 [PATCH v2 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel @ 2014-08-26 9:22 ` Anup Patel 2014-08-29 8:12 ` Reinhard Moselbach 2014-08-29 9:10 ` Andre Przywara 2014-08-26 9:22 ` [PATCH v2 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel ` (2 subsequent siblings) 3 siblings, 2 replies; 12+ messages in thread From: Anup Patel @ 2014-08-26 9:22 UTC (permalink / raw) To: kvmarm Cc: kvm, patches, will.deacon, marc.zyngier, penberg, christoffer.dall, pranavkumar, Anup Patel Instead, of trying out each and every target type we should use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type for KVM ARM/ARM64. If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to old method of trying all known target types. Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> Signed-off-by: Anup Patel <anup.patel@linaro.org> --- tools/kvm/arm/kvm-cpu.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c index aeaa4cf..c010e9c 100644 --- a/tools/kvm/arm/kvm-cpu.c +++ b/tools/kvm/arm/kvm-cpu.c @@ -34,6 +34,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) struct kvm_cpu *vcpu; int coalesced_offset, mmap_size, err = -1; unsigned int i; + struct kvm_vcpu_init preferred_init; struct kvm_vcpu_init vcpu_init = { .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id) }; @@ -55,20 +56,42 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) if (vcpu->kvm_run == MAP_FAILED) die("unable to mmap vcpu fd"); - /* Find an appropriate target CPU type. */ - for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { - if (!kvm_arm_targets[i]) - continue; - target = kvm_arm_targets[i]; - vcpu_init.target = target->id; + /* + * If preferred target ioctl successful then use preferred target + * else try each and every target type. + */ + err = ioctl(kvm->vm_fd, KVM_ARM_PREFERRED_TARGET, &preferred_init); + if (!err) { + /* Match preferred target CPU type. */ + target = NULL; + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { + if (!kvm_arm_targets[i]) + continue; + if (kvm_arm_targets[i]->id == preferred_init.target) { + target = kvm_arm_targets[i]; + break; + } + } + + vcpu_init.target = preferred_init.target; err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); - if (!err) - break; + if (err || target->init(vcpu)) + die("Unable to initialise vcpu for preferred target"); + } else { + /* Find an appropriate target CPU type. */ + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { + if (!kvm_arm_targets[i]) + continue; + target = kvm_arm_targets[i]; + vcpu_init.target = target->id; + err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); + if (!err) + break; + } + if (err || target->init(vcpu)) + die("Unable to initialise vcpu"); } - if (err || target->init(vcpu)) - die("Unable to initialise ARM vcpu"); - coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_COALESCED_MMIO); if (coalesced_offset) @@ -81,6 +104,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) vcpu->cpu_type = target->id; vcpu->cpu_compatible = target->compatible; vcpu->is_running = true; + return vcpu; } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu 2014-08-26 9:22 ` [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel @ 2014-08-29 8:12 ` Reinhard Moselbach 2014-08-29 9:10 ` Andre Przywara 1 sibling, 0 replies; 12+ messages in thread From: Reinhard Moselbach @ 2014-08-29 8:12 UTC (permalink / raw) To: Anup Patel, kvmarm; +Cc: kvm, patches, will.deacon, penberg Hi Anup, On 26/08/14 10:22, Anup Patel wrote: > Instead, of trying out each and every target type we should > use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type > for KVM ARM/ARM64. > > If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to > old method of trying all known target types. So as the algorithm currently works, it does not give us much improvement over the current behaviour. We still need to list each supported MPIDR both in kvmtool and in the kernel. Looking more closely at the code, beside the target id we only need the kvm_target_arm[] list for the compatible string and the init() function. The latter is (currently) the same for all supported type, so we could use that as a standard fallback function. The compatible string seems to be completely ignored by the ARM64 kernel, so we could as well pass "arm,armv8" all the time. In ARM(32) kernels we seem to not make any real use of it for CPUs which we care for (with virtualisation extensions). So what about the following: We keep the list as it is, but not extend it for future CPUs, expect those in need for a special compatible string or a specific init function. Instead we rely on PREFFERED_TARGET for all current and upcoming CPUs (meaning unsupported CPUs must use a 3.12 kernel or higher). If PREFERRED_TARGET works, we scan the list anyway (to find CPUs needing special treatment), but on failure of finding something in the list we just go ahead: - with the target ID the kernel returned, - an "arm,armv8" compatible string (for arm64, not sure about arm) and - call the standard kvmtool init function This should relief us from the burden of adding each supported CPU to kvmtool. Does that make sense of am I missing something? I will hack something up to prove that it works. Also there is now a race on big.LITTLE systems: if the PREFERRED_TARGET ioctl is executed on one cluster, while the KVM_ARM_VCPU_INIT call is done on another core with a different MPIDR, then the kernel will refuse to init the CPU. I don't know of a good solution for this (except the sledgehammer pinning with sched_setaffinity to the current core, which is racy, too, but should at least work somehow ;-) Any ideas? > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > Signed-off-by: Anup Patel <anup.patel@linaro.org> > --- > tools/kvm/arm/kvm-cpu.c | 46 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c > index aeaa4cf..c010e9c 100644 > --- a/tools/kvm/arm/kvm-cpu.c > +++ b/tools/kvm/arm/kvm-cpu.c > @@ -34,6 +34,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > struct kvm_cpu *vcpu; > int coalesced_offset, mmap_size, err = -1; > unsigned int i; > + struct kvm_vcpu_init preferred_init; > struct kvm_vcpu_init vcpu_init = { > .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id) > }; > @@ -55,20 +56,42 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > if (vcpu->kvm_run == MAP_FAILED) > die("unable to mmap vcpu fd"); > > - /* Find an appropriate target CPU type. */ > - for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { > - if (!kvm_arm_targets[i]) > - continue; > - target = kvm_arm_targets[i]; > - vcpu_init.target = target->id; > + /* > + * If preferred target ioctl successful then use preferred target > + * else try each and every target type. > + */ > + err = ioctl(kvm->vm_fd, KVM_ARM_PREFERRED_TARGET, &preferred_init); > + if (!err) { > + /* Match preferred target CPU type. */ > + target = NULL; > + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { > + if (!kvm_arm_targets[i]) > + continue; > + if (kvm_arm_targets[i]->id == preferred_init.target) { > + target = kvm_arm_targets[i]; > + break; > + } > + } > + > + vcpu_init.target = preferred_init.target; > err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); > - if (!err) > - break; > + if (err || target->init(vcpu)) > + die("Unable to initialise vcpu for preferred target"); So that segfaults if the CPU is not in kvmtools list (as target is still NULL). In the current implementation we should bail out (but better use the algorithm described above). Also these two line can be moved outside of the loop and joined with the last two lines from the else clause ... > + } else { > + /* Find an appropriate target CPU type. */ > + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { > + if (!kvm_arm_targets[i]) > + continue; > + target = kvm_arm_targets[i]; > + vcpu_init.target = target->id; > + err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); > + if (!err) > + break; > + } > + if (err || target->init(vcpu)) > + die("Unable to initialise vcpu"); > } .. to appear here. Cheers, Andre. > > - if (err || target->init(vcpu)) > - die("Unable to initialise ARM vcpu"); > - > coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, > KVM_CAP_COALESCED_MMIO); > if (coalesced_offset) > @@ -81,6 +104,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > vcpu->cpu_type = target->id; > vcpu->cpu_compatible = target->compatible; > vcpu->is_running = true; > + > return vcpu; > } > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu 2014-08-26 9:22 ` [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel 2014-08-29 8:12 ` Reinhard Moselbach @ 2014-08-29 9:10 ` Andre Przywara 2014-08-29 16:17 ` Will Deacon 2014-08-30 4:39 ` Anup Patel 1 sibling, 2 replies; 12+ messages in thread From: Andre Przywara @ 2014-08-29 9:10 UTC (permalink / raw) To: Anup Patel, kvmarm; +Cc: kvm, patches, will.deacon, penberg (resent, that was the wrong account before ...) Hi Anup, On 26/08/14 10:22, Anup Patel wrote: > Instead, of trying out each and every target type we should > use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type > for KVM ARM/ARM64. > > If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to > old method of trying all known target types. So as the algorithm currently works, it does not give us much improvement over the current behaviour. We still need to list each supported MPIDR both in kvmtool and in the kernel. Looking more closely at the code, beside the target id we only need the kvm_target_arm[] list for the compatible string and the init() function. The latter is (currently) the same for all supported type, so we could use that as a standard fallback function. The compatible string seems to be completely ignored by the ARM64 kernel, so we could as well pass "arm,armv8" all the time. In ARM(32) kernels we seem to not make any real use of it for CPUs which we care for (with virtualisation extensions). So what about the following: We keep the list as it is, but not extend it for future CPUs, expect those in need for a special compatible string or a specific init function. Instead we rely on PREFFERED_TARGET for all current and upcoming CPUs (meaning unsupported CPUs must use a 3.12 kernel or higher). If PREFERRED_TARGET works, we scan the list anyway (to find CPUs needing special treatment), but on failure of finding something in the list we just go ahead: - with the target ID the kernel returned, - an "arm,armv8" compatible string (for arm64, not sure about arm) and - call the standard kvmtool init function This should relief us from the burden of adding each supported CPU to kvmtool. Does that make sense of am I missing something? I will hack something up to prove that it works. Also there is now a race on big.LITTLE systems: if the PREFERRED_TARGET ioctl is executed on one cluster, while the KVM_ARM_VCPU_INIT call is done on another core with a different MPIDR, then the kernel will refuse to init the CPU. I don't know of a good solution for this (except the sledgehammer pinning with sched_setaffinity to the current core, which is racy, too, but should at least work somehow ;-) Any ideas? > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > Signed-off-by: Anup Patel <anup.patel@linaro.org> > --- > tools/kvm/arm/kvm-cpu.c | 46 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c > index aeaa4cf..c010e9c 100644 > --- a/tools/kvm/arm/kvm-cpu.c > +++ b/tools/kvm/arm/kvm-cpu.c > @@ -34,6 +34,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > struct kvm_cpu *vcpu; > int coalesced_offset, mmap_size, err = -1; > unsigned int i; > + struct kvm_vcpu_init preferred_init; > struct kvm_vcpu_init vcpu_init = { > .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id) > }; > @@ -55,20 +56,42 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > if (vcpu->kvm_run == MAP_FAILED) > die("unable to mmap vcpu fd"); > > - /* Find an appropriate target CPU type. */ > - for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { > - if (!kvm_arm_targets[i]) > - continue; > - target = kvm_arm_targets[i]; > - vcpu_init.target = target->id; > + /* > + * If preferred target ioctl successful then use preferred target > + * else try each and every target type. > + */ > + err = ioctl(kvm->vm_fd, KVM_ARM_PREFERRED_TARGET, &preferred_init); > + if (!err) { > + /* Match preferred target CPU type. */ > + target = NULL; > + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { > + if (!kvm_arm_targets[i]) > + continue; > + if (kvm_arm_targets[i]->id == preferred_init.target) { > + target = kvm_arm_targets[i]; > + break; > + } > + } > + > + vcpu_init.target = preferred_init.target; > err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); > - if (!err) > - break; > + if (err || target->init(vcpu)) > + die("Unable to initialise vcpu for preferred target"); So that segfaults if the CPU is not in kvmtools list (as target is still NULL). In the current implementation we should bail out (but better use the algorithm described above). Also these two line can be moved outside of the loop and joined with the last two lines from the else clause ... > + } else { > + /* Find an appropriate target CPU type. */ > + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { > + if (!kvm_arm_targets[i]) > + continue; > + target = kvm_arm_targets[i]; > + vcpu_init.target = target->id; > + err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); > + if (!err) > + break; > + } > + if (err || target->init(vcpu)) > + die("Unable to initialise vcpu"); > } .. to appear here. Cheers, Andre. > > - if (err || target->init(vcpu)) > - die("Unable to initialise ARM vcpu"); > - > coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, > KVM_CAP_COALESCED_MMIO); > if (coalesced_offset) > @@ -81,6 +104,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > vcpu->cpu_type = target->id; > vcpu->cpu_compatible = target->compatible; > vcpu->is_running = true; > + > return vcpu; > } > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu 2014-08-29 9:10 ` Andre Przywara @ 2014-08-29 16:17 ` Will Deacon 2014-08-29 16:21 ` Andre Przywara 2014-08-30 4:39 ` Anup Patel 1 sibling, 1 reply; 12+ messages in thread From: Will Deacon @ 2014-08-29 16:17 UTC (permalink / raw) To: Andre Przywara Cc: Anup Patel, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, patches@apm.com, penberg@kernel.org On Fri, Aug 29, 2014 at 10:10:52AM +0100, Andre Przywara wrote: > (resent, that was the wrong account before ...) Aha, and now your true identity has been revealed to all! Nice try Andre... or should I say, Rienhard? Will ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu 2014-08-29 16:17 ` Will Deacon @ 2014-08-29 16:21 ` Andre Przywara 0 siblings, 0 replies; 12+ messages in thread From: Andre Przywara @ 2014-08-29 16:21 UTC (permalink / raw) To: Will Deacon Cc: penberg@kernel.org, patches@apm.com, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org On 29/08/14 17:17, Will Deacon wrote: > On Fri, Aug 29, 2014 at 10:10:52AM +0100, Andre Przywara wrote: >> (resent, that was the wrong account before ...) > > Aha, and now your true identity has been revealed to all! Nice try Andre... > or should I say, Rienhard? Psst, don't give Google funny ideas about this (now very secret) relationship ;-) Cheers, Andre "R" P. -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu 2014-08-29 9:10 ` Andre Przywara 2014-08-29 16:17 ` Will Deacon @ 2014-08-30 4:39 ` Anup Patel 1 sibling, 0 replies; 12+ messages in thread From: Anup Patel @ 2014-08-30 4:39 UTC (permalink / raw) To: Andre Przywara Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, patches, Will Deacon, penberg Hi Andre, On 29 August 2014 14:40, Andre Przywara <andre.przywara@arm.com> wrote: > (resent, that was the wrong account before ...) > > Hi Anup, > > On 26/08/14 10:22, Anup Patel wrote: >> Instead, of trying out each and every target type we should >> use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type >> for KVM ARM/ARM64. >> >> If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to >> old method of trying all known target types. > > So as the algorithm currently works, it does not give us much > improvement over the current behaviour. We still need to list each > supported MPIDR both in kvmtool and in the kernel. > Looking more closely at the code, beside the target id we only need the > kvm_target_arm[] list for the compatible string and the init() function. > The latter is (currently) the same for all supported type, so we could > use that as a standard fallback function. > The compatible string seems to be completely ignored by the ARM64 > kernel, so we could as well pass "arm,armv8" all the time. > In ARM(32) kernels we seem to not make any real use of it for CPUs which > we care for (with virtualisation extensions). You are absolutely right here. I was just trying to keep KVMTOOL changes to minimum. > > So what about the following: > We keep the list as it is, but not extend it for future CPUs, expect > those in need for a special compatible string or a specific init > function. Instead we rely on PREFFERED_TARGET for all current and > upcoming CPUs (meaning unsupported CPUs must use a 3.12 kernel or higher). > If PREFERRED_TARGET works, we scan the list anyway (to find CPUs needing > special treatment), but on failure of finding something in the list we > just go ahead: > - with the target ID the kernel returned, > - an "arm,armv8" compatible string (for arm64, not sure about arm) and > - call the standard kvmtool init function > > This should relief us from the burden of adding each supported CPU to > kvmtool. > > Does that make sense of am I missing something? > I will hack something up to prove that it works. Yes, this makes sense. In fact, QEMU does something similar for "-cpu host -M virt" command line options. I think I should be less lazy on this one. I will rework this and make it more like QEMU "-cpu host" option. Thanks, Anup > > Also there is now a race on big.LITTLE systems: if the PREFERRED_TARGET > ioctl is executed on one cluster, while the KVM_ARM_VCPU_INIT call is > done on another core with a different MPIDR, then the kernel will refuse > to init the CPU. I don't know of a good solution for this (except the > sledgehammer pinning with sched_setaffinity to the current core, which > is racy, too, but should at least work somehow ;-) > Any ideas? > >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> Signed-off-by: Anup Patel <anup.patel@linaro.org> >> --- >> tools/kvm/arm/kvm-cpu.c | 46 +++++++++++++++++++++++++++++++++++----------- >> 1 file changed, 35 insertions(+), 11 deletions(-) >> >> diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c >> index aeaa4cf..c010e9c 100644 >> --- a/tools/kvm/arm/kvm-cpu.c >> +++ b/tools/kvm/arm/kvm-cpu.c >> @@ -34,6 +34,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) >> struct kvm_cpu *vcpu; >> int coalesced_offset, mmap_size, err = -1; >> unsigned int i; >> + struct kvm_vcpu_init preferred_init; >> struct kvm_vcpu_init vcpu_init = { >> .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id) >> }; >> @@ -55,20 +56,42 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) >> if (vcpu->kvm_run == MAP_FAILED) >> die("unable to mmap vcpu fd"); >> >> - /* Find an appropriate target CPU type. */ >> - for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { >> - if (!kvm_arm_targets[i]) >> - continue; >> - target = kvm_arm_targets[i]; >> - vcpu_init.target = target->id; >> + /* >> + * If preferred target ioctl successful then use preferred target >> + * else try each and every target type. >> + */ >> + err = ioctl(kvm->vm_fd, KVM_ARM_PREFERRED_TARGET, &preferred_init); >> + if (!err) { >> + /* Match preferred target CPU type. */ >> + target = NULL; >> + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { >> + if (!kvm_arm_targets[i]) >> + continue; >> + if (kvm_arm_targets[i]->id == preferred_init.target) { >> + target = kvm_arm_targets[i]; >> + break; >> + } >> + } >> + >> + vcpu_init.target = preferred_init.target; >> err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); >> - if (!err) >> - break; >> + if (err || target->init(vcpu)) >> + die("Unable to initialise vcpu for preferred target"); > > So that segfaults if the CPU is not in kvmtools list (as target is still > NULL). In the current implementation we should bail out (but better use > the algorithm described above). > > Also these two line can be moved outside of the loop and joined with the > last two lines from the else clause ... > >> + } else { >> + /* Find an appropriate target CPU type. */ >> + for (i = 0; i < ARRAY_SIZE(kvm_arm_targets); ++i) { >> + if (!kvm_arm_targets[i]) >> + continue; >> + target = kvm_arm_targets[i]; >> + vcpu_init.target = target->id; >> + err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init); >> + if (!err) >> + break; >> + } >> + if (err || target->init(vcpu)) >> + die("Unable to initialise vcpu"); >> } > > .. to appear here. > > Cheers, > Andre. > >> >> - if (err || target->init(vcpu)) >> - die("Unable to initialise ARM vcpu"); >> - >> coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, >> KVM_CAP_COALESCED_MMIO); >> if (coalesced_offset) >> @@ -81,6 +104,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) >> vcpu->cpu_type = target->id; >> vcpu->cpu_compatible = target->compatible; >> vcpu->is_running = true; >> + >> return vcpu; >> } >> >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] kvmtool: ARM64: Add target type potenza for aarch64 2014-08-26 9:22 [PATCH v2 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel 2014-08-26 9:22 ` [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel @ 2014-08-26 9:22 ` Anup Patel 2014-08-26 9:22 ` [PATCH v2 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel 2014-08-26 9:22 ` [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel 3 siblings, 0 replies; 12+ messages in thread From: Anup Patel @ 2014-08-26 9:22 UTC (permalink / raw) To: kvmarm Cc: kvm, patches, will.deacon, marc.zyngier, penberg, christoffer.dall, pranavkumar, Anup Patel The VCPU target type KVM_ARM_TARGET_XGENE_POTENZA is available in latest Linux-3.16-rcX or higher hence register aarch64 target type for it. This patch enables us to run KVMTOOL on X-Gene Potenza host. Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> Signed-off-by: Anup Patel <anup.patel@linaro.org> --- tools/kvm/arm/aarch64/arm-cpu.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tools/kvm/arm/aarch64/arm-cpu.c b/tools/kvm/arm/aarch64/arm-cpu.c index ce5ea2f..ce526e3 100644 --- a/tools/kvm/arm/aarch64/arm-cpu.c +++ b/tools/kvm/arm/aarch64/arm-cpu.c @@ -41,10 +41,17 @@ static struct kvm_arm_target target_cortex_a57 = { .init = arm_cpu__vcpu_init, }; +static struct kvm_arm_target target_potenza = { + .id = KVM_ARM_TARGET_XGENE_POTENZA, + .compatible = "arm,arm-v8", + .init = arm_cpu__vcpu_init, +}; + static int arm_cpu__core_init(struct kvm *kvm) { return (kvm_cpu__register_kvm_arm_target(&target_aem_v8) || kvm_cpu__register_kvm_arm_target(&target_foundation_v8) || - kvm_cpu__register_kvm_arm_target(&target_cortex_a57)); + kvm_cpu__register_kvm_arm_target(&target_cortex_a57) || + kvm_cpu__register_kvm_arm_target(&target_potenza)); } core_init(arm_cpu__core_init); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT 2014-08-26 9:22 [PATCH v2 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel 2014-08-26 9:22 ` [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel 2014-08-26 9:22 ` [PATCH v2 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel @ 2014-08-26 9:22 ` Anup Patel 2014-08-26 9:22 ` [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel 3 siblings, 0 replies; 12+ messages in thread From: Anup Patel @ 2014-08-26 9:22 UTC (permalink / raw) To: kvmarm Cc: kvm, patches, will.deacon, marc.zyngier, penberg, christoffer.dall, pranavkumar, Anup Patel The KVM_EXIT_SYSTEM_EVENT exit reason was added to define architecture independent system-wide events for a Guest. Currently, it is used by in-kernel PSCI-0.2 emulation of KVM ARM/ARM64 to inform user space about PSCI SYSTEM_OFF or PSCI SYSTEM_RESET request. For now, we simply treat all system-wide guest events as shutdown request in KVMTOOL. Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> Signed-off-by: Anup Patel <anup.patel@linaro.org> --- tools/kvm/kvm-cpu.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tools/kvm/kvm-cpu.c b/tools/kvm/kvm-cpu.c index ee0a8ec..6d01192 100644 --- a/tools/kvm/kvm-cpu.c +++ b/tools/kvm/kvm-cpu.c @@ -160,6 +160,25 @@ int kvm_cpu__start(struct kvm_cpu *cpu) goto exit_kvm; case KVM_EXIT_SHUTDOWN: goto exit_kvm; + case KVM_EXIT_SYSTEM_EVENT: + /* + * Print the type of system event and + * treat all system events as shutdown request. + */ + switch (cpu->kvm_run->system_event.type) { + case KVM_SYSTEM_EVENT_SHUTDOWN: + printf(" # Info: shutdown system event\n"); + break; + case KVM_SYSTEM_EVENT_RESET: + printf(" # Info: reset system event\n"); + break; + default: + printf(" # Warning: unknown system event type=%d\n", + cpu->kvm_run->system_event.type); + break; + }; + printf(" # Info: exiting KVMTOOL\n"); + goto exit_kvm; default: { bool ret; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it 2014-08-26 9:22 [PATCH v2 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel ` (2 preceding siblings ...) 2014-08-26 9:22 ` [PATCH v2 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel @ 2014-08-26 9:22 ` Anup Patel 2014-08-29 9:11 ` Andre Przywara 3 siblings, 1 reply; 12+ messages in thread From: Anup Patel @ 2014-08-26 9:22 UTC (permalink / raw) To: kvmarm Cc: kvm, patches, will.deacon, marc.zyngier, penberg, christoffer.dall, pranavkumar, Anup Patel If in-kernel KVM support PSCI-0.2 emulation then we should set KVM_ARM_VCPU_PSCI_0_2 feature for each guest VCPU and also provide "arm,psci-0.2","arm,psci" as PSCI compatible string. This patch updates kvm_cpu__arch_init() and setup_fdt() as per above. Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> Signed-off-by: Anup Patel <anup.patel@linaro.org> --- tools/kvm/arm/fdt.c | 39 +++++++++++++++++++++++++++++++++------ tools/kvm/arm/kvm-cpu.c | 5 +++++ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c index 186a718..93849cf2 100644 --- a/tools/kvm/arm/fdt.c +++ b/tools/kvm/arm/fdt.c @@ -13,6 +13,7 @@ #include <linux/byteorder.h> #include <linux/kernel.h> #include <linux/sizes.h> +#include <linux/psci.h> static char kern_cmdline[COMMAND_LINE_SIZE]; @@ -162,12 +163,38 @@ static int setup_fdt(struct kvm *kvm) /* PSCI firmware */ _FDT(fdt_begin_node(fdt, "psci")); - _FDT(fdt_property_string(fdt, "compatible", "arm,psci")); - _FDT(fdt_property_string(fdt, "method", "hvc")); - _FDT(fdt_property_cell(fdt, "cpu_suspend", KVM_PSCI_FN_CPU_SUSPEND)); - _FDT(fdt_property_cell(fdt, "cpu_off", KVM_PSCI_FN_CPU_OFF)); - _FDT(fdt_property_cell(fdt, "cpu_on", KVM_PSCI_FN_CPU_ON)); - _FDT(fdt_property_cell(fdt, "migrate", KVM_PSCI_FN_MIGRATE)); + if (kvm__supports_extension(kvm, KVM_CAP_ARM_PSCI_0_2)) { + const char compatible[] = "arm,psci-0.2\0arm,psci"; + _FDT(fdt_property(fdt, "compatible", + compatible, sizeof(compatible))); + _FDT(fdt_property_string(fdt, "method", "hvc")); + if (kvm->cfg.arch.aarch32_guest) { + _FDT(fdt_property_cell(fdt, "cpu_suspend", + PSCI_0_2_FN_CPU_SUSPEND)); + _FDT(fdt_property_cell(fdt, "cpu_off", + PSCI_0_2_FN_CPU_OFF)); + _FDT(fdt_property_cell(fdt, "cpu_on", + PSCI_0_2_FN_CPU_ON)); + _FDT(fdt_property_cell(fdt, "migrate", + PSCI_0_2_FN_MIGRATE)); + } else { + _FDT(fdt_property_cell(fdt, "cpu_suspend", + PSCI_0_2_FN64_CPU_SUSPEND)); + _FDT(fdt_property_cell(fdt, "cpu_off", + PSCI_0_2_FN_CPU_OFF)); + _FDT(fdt_property_cell(fdt, "cpu_on", + PSCI_0_2_FN64_CPU_ON)); + _FDT(fdt_property_cell(fdt, "migrate", + PSCI_0_2_FN64_MIGRATE)); + } + } else { + _FDT(fdt_property_string(fdt, "compatible", "arm,psci")); + _FDT(fdt_property_string(fdt, "method", "hvc")); + _FDT(fdt_property_cell(fdt, "cpu_suspend", KVM_PSCI_FN_CPU_SUSPEND)); + _FDT(fdt_property_cell(fdt, "cpu_off", KVM_PSCI_FN_CPU_OFF)); + _FDT(fdt_property_cell(fdt, "cpu_on", KVM_PSCI_FN_CPU_ON)); + _FDT(fdt_property_cell(fdt, "migrate", KVM_PSCI_FN_MIGRATE)); + } _FDT(fdt_end_node(fdt)); /* Finalise. */ diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c index c010e9c..0637e9a 100644 --- a/tools/kvm/arm/kvm-cpu.c +++ b/tools/kvm/arm/kvm-cpu.c @@ -56,6 +56,11 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) if (vcpu->kvm_run == MAP_FAILED) die("unable to mmap vcpu fd"); + /* Set KVM_ARM_VCPU_PSCI_0_2 if available */ + if (kvm__supports_extension(kvm, KVM_CAP_ARM_PSCI_0_2)) { + vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2); + } + /* * If preferred target ioctl successful then use preferred target * else try each and every target type. -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it 2014-08-26 9:22 ` [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel @ 2014-08-29 9:11 ` Andre Przywara 2014-08-30 4:46 ` Anup Patel 0 siblings, 1 reply; 12+ messages in thread From: Andre Przywara @ 2014-08-29 9:11 UTC (permalink / raw) To: Anup Patel, kvmarm; +Cc: kvm, patches, will.deacon, penberg Hi Anup, On 26/08/14 10:22, Anup Patel wrote: > If in-kernel KVM support PSCI-0.2 emulation then we should set > KVM_ARM_VCPU_PSCI_0_2 feature for each guest VCPU and also > provide "arm,psci-0.2","arm,psci" as PSCI compatible string. > > This patch updates kvm_cpu__arch_init() and setup_fdt() as > per above. > > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > Signed-off-by: Anup Patel <anup.patel@linaro.org> > --- > tools/kvm/arm/fdt.c | 39 +++++++++++++++++++++++++++++++++------ > tools/kvm/arm/kvm-cpu.c | 5 +++++ > 2 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c > index 186a718..93849cf2 100644 > --- a/tools/kvm/arm/fdt.c > +++ b/tools/kvm/arm/fdt.c > @@ -13,6 +13,7 @@ > #include <linux/byteorder.h> > #include <linux/kernel.h> > #include <linux/sizes.h> > +#include <linux/psci.h> > > static char kern_cmdline[COMMAND_LINE_SIZE]; > > @@ -162,12 +163,38 @@ static int setup_fdt(struct kvm *kvm) > > /* PSCI firmware */ > _FDT(fdt_begin_node(fdt, "psci")); > - _FDT(fdt_property_string(fdt, "compatible", "arm,psci")); > - _FDT(fdt_property_string(fdt, "method", "hvc")); > - _FDT(fdt_property_cell(fdt, "cpu_suspend", KVM_PSCI_FN_CPU_SUSPEND)); > - _FDT(fdt_property_cell(fdt, "cpu_off", KVM_PSCI_FN_CPU_OFF)); > - _FDT(fdt_property_cell(fdt, "cpu_on", KVM_PSCI_FN_CPU_ON)); > - _FDT(fdt_property_cell(fdt, "migrate", KVM_PSCI_FN_MIGRATE)); > + if (kvm__supports_extension(kvm, KVM_CAP_ARM_PSCI_0_2)) { > + const char compatible[] = "arm,psci-0.2\0arm,psci"; > + _FDT(fdt_property(fdt, "compatible", > + compatible, sizeof(compatible))); > + _FDT(fdt_property_string(fdt, "method", "hvc")); > + if (kvm->cfg.arch.aarch32_guest) { > + _FDT(fdt_property_cell(fdt, "cpu_suspend", > + PSCI_0_2_FN_CPU_SUSPEND)); > + _FDT(fdt_property_cell(fdt, "cpu_off", > + PSCI_0_2_FN_CPU_OFF)); > + _FDT(fdt_property_cell(fdt, "cpu_on", > + PSCI_0_2_FN_CPU_ON)); > + _FDT(fdt_property_cell(fdt, "migrate", > + PSCI_0_2_FN_MIGRATE)); > + } else { > + _FDT(fdt_property_cell(fdt, "cpu_suspend", > + PSCI_0_2_FN64_CPU_SUSPEND)); > + _FDT(fdt_property_cell(fdt, "cpu_off", > + PSCI_0_2_FN_CPU_OFF)); > + _FDT(fdt_property_cell(fdt, "cpu_on", > + PSCI_0_2_FN64_CPU_ON)); > + _FDT(fdt_property_cell(fdt, "migrate", > + PSCI_0_2_FN64_MIGRATE)); > + } > + } else { > + _FDT(fdt_property_string(fdt, "compatible", "arm,psci")); > + _FDT(fdt_property_string(fdt, "method", "hvc")); > + _FDT(fdt_property_cell(fdt, "cpu_suspend", KVM_PSCI_FN_CPU_SUSPEND)); > + _FDT(fdt_property_cell(fdt, "cpu_off", KVM_PSCI_FN_CPU_OFF)); > + _FDT(fdt_property_cell(fdt, "cpu_on", KVM_PSCI_FN_CPU_ON)); > + _FDT(fdt_property_cell(fdt, "migrate", KVM_PSCI_FN_MIGRATE)); > + } I guess this could be simplified much by defining three arrays with the respective function IDs and setting a pointer to the right one here. Then there would still be only one set of _FDT() calls, which reference this pointer. Like: uint32_t *psci_fn_ids; ... if (KVM_CAP_ARM_PSCI_0_2) { if (aarch32_guest) psci_fn_ids = psci_0_2_fn_ids; else psci_fn_ids = psci_0_2_fn64_ids; } else psci_fn_ids = psci_0_1_fn_ids; _FDT(fdt_property_cell(fdt, "cpu_suspend", psci_fn_ids[0])); _FDT(fdt_property_cell(fdt, "cpu_off", psci_fn_ids[1])); ... Also I wonder if we actually need those different IDs. The binding doc says that Linux' PSCI 0.2 code ignores them altogether, only using them if the "arm,psci" branch of the compatible string is actually used (on kernels not supporting PSCI 0.2) So can't we just always pass the PSCI 0.1 numbers in here? That would restrict this whole patch to just changing the compatible string, right? Regards, Andre. > _FDT(fdt_end_node(fdt)); > > /* Finalise. */ > diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c > index c010e9c..0637e9a 100644 > --- a/tools/kvm/arm/kvm-cpu.c > +++ b/tools/kvm/arm/kvm-cpu.c > @@ -56,6 +56,11 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) > if (vcpu->kvm_run == MAP_FAILED) > die("unable to mmap vcpu fd"); > > + /* Set KVM_ARM_VCPU_PSCI_0_2 if available */ > + if (kvm__supports_extension(kvm, KVM_CAP_ARM_PSCI_0_2)) { > + vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2); > + } > + > /* > * If preferred target ioctl successful then use preferred target > * else try each and every target type. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it 2014-08-29 9:11 ` Andre Przywara @ 2014-08-30 4:46 ` Anup Patel 0 siblings, 0 replies; 12+ messages in thread From: Anup Patel @ 2014-08-30 4:46 UTC (permalink / raw) To: Andre Przywara Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, patches, Will Deacon, penberg Hi Andre, On 29 August 2014 14:41, Andre Przywara <andre.przywara@arm.com> wrote: > Hi Anup, > > On 26/08/14 10:22, Anup Patel wrote: >> If in-kernel KVM support PSCI-0.2 emulation then we should set >> KVM_ARM_VCPU_PSCI_0_2 feature for each guest VCPU and also >> provide "arm,psci-0.2","arm,psci" as PSCI compatible string. >> >> This patch updates kvm_cpu__arch_init() and setup_fdt() as >> per above. >> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> Signed-off-by: Anup Patel <anup.patel@linaro.org> >> --- >> tools/kvm/arm/fdt.c | 39 +++++++++++++++++++++++++++++++++------ >> tools/kvm/arm/kvm-cpu.c | 5 +++++ >> 2 files changed, 38 insertions(+), 6 deletions(-) >> >> diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c >> index 186a718..93849cf2 100644 >> --- a/tools/kvm/arm/fdt.c >> +++ b/tools/kvm/arm/fdt.c >> @@ -13,6 +13,7 @@ >> #include <linux/byteorder.h> >> #include <linux/kernel.h> >> #include <linux/sizes.h> >> +#include <linux/psci.h> >> >> static char kern_cmdline[COMMAND_LINE_SIZE]; >> >> @@ -162,12 +163,38 @@ static int setup_fdt(struct kvm *kvm) >> >> /* PSCI firmware */ >> _FDT(fdt_begin_node(fdt, "psci")); >> - _FDT(fdt_property_string(fdt, "compatible", "arm,psci")); >> - _FDT(fdt_property_string(fdt, "method", "hvc")); >> - _FDT(fdt_property_cell(fdt, "cpu_suspend", KVM_PSCI_FN_CPU_SUSPEND)); >> - _FDT(fdt_property_cell(fdt, "cpu_off", KVM_PSCI_FN_CPU_OFF)); >> - _FDT(fdt_property_cell(fdt, "cpu_on", KVM_PSCI_FN_CPU_ON)); >> - _FDT(fdt_property_cell(fdt, "migrate", KVM_PSCI_FN_MIGRATE)); >> + if (kvm__supports_extension(kvm, KVM_CAP_ARM_PSCI_0_2)) { >> + const char compatible[] = "arm,psci-0.2\0arm,psci"; >> + _FDT(fdt_property(fdt, "compatible", >> + compatible, sizeof(compatible))); >> + _FDT(fdt_property_string(fdt, "method", "hvc")); >> + if (kvm->cfg.arch.aarch32_guest) { >> + _FDT(fdt_property_cell(fdt, "cpu_suspend", >> + PSCI_0_2_FN_CPU_SUSPEND)); >> + _FDT(fdt_property_cell(fdt, "cpu_off", >> + PSCI_0_2_FN_CPU_OFF)); >> + _FDT(fdt_property_cell(fdt, "cpu_on", >> + PSCI_0_2_FN_CPU_ON)); >> + _FDT(fdt_property_cell(fdt, "migrate", >> + PSCI_0_2_FN_MIGRATE)); >> + } else { >> + _FDT(fdt_property_cell(fdt, "cpu_suspend", >> + PSCI_0_2_FN64_CPU_SUSPEND)); >> + _FDT(fdt_property_cell(fdt, "cpu_off", >> + PSCI_0_2_FN_CPU_OFF)); >> + _FDT(fdt_property_cell(fdt, "cpu_on", >> + PSCI_0_2_FN64_CPU_ON)); >> + _FDT(fdt_property_cell(fdt, "migrate", >> + PSCI_0_2_FN64_MIGRATE)); >> + } >> + } else { >> + _FDT(fdt_property_string(fdt, "compatible", "arm,psci")); >> + _FDT(fdt_property_string(fdt, "method", "hvc")); >> + _FDT(fdt_property_cell(fdt, "cpu_suspend", KVM_PSCI_FN_CPU_SUSPEND)); >> + _FDT(fdt_property_cell(fdt, "cpu_off", KVM_PSCI_FN_CPU_OFF)); >> + _FDT(fdt_property_cell(fdt, "cpu_on", KVM_PSCI_FN_CPU_ON)); >> + _FDT(fdt_property_cell(fdt, "migrate", KVM_PSCI_FN_MIGRATE)); >> + } > > I guess this could be simplified much by defining three arrays with the > respective function IDs and setting a pointer to the right one here. > Then there would still be only one set of _FDT() calls, which reference > this pointer. Like: > uint32_t *psci_fn_ids; > ... > if (KVM_CAP_ARM_PSCI_0_2) { > if (aarch32_guest) > psci_fn_ids = psci_0_2_fn_ids; > else > psci_fn_ids = psci_0_2_fn64_ids; > } else > psci_fn_ids = psci_0_1_fn_ids; > _FDT(fdt_property_cell(fdt, "cpu_suspend", psci_fn_ids[0])); > _FDT(fdt_property_cell(fdt, "cpu_off", psci_fn_ids[1])); > ... > > Also I wonder if we actually need those different IDs. The binding doc > says that Linux' PSCI 0.2 code ignores them altogether, only using them > if the "arm,psci" branch of the compatible string is actually used (on > kernels not supporting PSCI 0.2) Your suggestion looks good to me. I will rework this patch accordingly. > So can't we just always pass the PSCI 0.1 numbers in here? > That would restrict this whole patch to just changing the compatible > string, right? If we always pass PSCI 0.1 numbers irrespective to compatible string then it breaks the case where we have latest host kernel with PSCI 0.2, latest KVMTOOL with PSCI 0.2, and older guest kernel with only PSCI 0.1 support. There was a issue in QEMU and Christoffer had send-out a patch to fix this in QEMU. (Refer, https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg01311.html) Regards, Anup > > Regards, > Andre. > >> _FDT(fdt_end_node(fdt)); >> >> /* Finalise. */ >> diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c >> index c010e9c..0637e9a 100644 >> --- a/tools/kvm/arm/kvm-cpu.c >> +++ b/tools/kvm/arm/kvm-cpu.c >> @@ -56,6 +56,11 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id) >> if (vcpu->kvm_run == MAP_FAILED) >> die("unable to mmap vcpu fd"); >> >> + /* Set KVM_ARM_VCPU_PSCI_0_2 if available */ >> + if (kvm__supports_extension(kvm, KVM_CAP_ARM_PSCI_0_2)) { >> + vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2); >> + } >> + >> /* >> * If preferred target ioctl successful then use preferred target >> * else try each and every target type. >> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-08-30 4:46 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-26 9:22 [PATCH v2 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel 2014-08-26 9:22 ` [PATCH v2 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel 2014-08-29 8:12 ` Reinhard Moselbach 2014-08-29 9:10 ` Andre Przywara 2014-08-29 16:17 ` Will Deacon 2014-08-29 16:21 ` Andre Przywara 2014-08-30 4:39 ` Anup Patel 2014-08-26 9:22 ` [PATCH v2 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel 2014-08-26 9:22 ` [PATCH v2 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel 2014-08-26 9:22 ` [PATCH v2 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel 2014-08-29 9:11 ` Andre Przywara 2014-08-30 4:46 ` Anup Patel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox