All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] kvmtool: ARM/ARM64: Misc updates
@ 2014-09-08  8:17 Anup Patel
  2014-09-08  8:17 ` [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Anup Patel @ 2014-09-08  8:17 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, patches, will.deacon, marc.zyngier, penberg,
	christoffer.dall, pranavkumar, andre.przywara, 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 v2:
- Use target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl
  for VCPU init such that we don't need to update KVMTOOL for
  every new host hardware
- Simplify DTB generation for PSCI node

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             |   52 +++++++++++++++++++++++++++++++----
 tools/kvm/arm/kvm-cpu.c         |   57 +++++++++++++++++++++++++++++++--------
 tools/kvm/kvm-cpu.c             |   19 +++++++++++++
 4 files changed, 120 insertions(+), 17 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu
  2014-09-08  8:17 [PATCH v3 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel
@ 2014-09-08  8:17 ` Anup Patel
  2014-09-11 15:54   ` Andre Przywara
  2014-09-08  8:17 ` [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-09-08  8:17 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, patches, will.deacon, marc.zyngier, penberg,
	christoffer.dall, pranavkumar, andre.przywara, 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.

If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
target type is not known to KVMTOOL then we forcefully init
VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Anup Patel <anup.patel@linaro.org>
---
 tools/kvm/arm/kvm-cpu.c |   52 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
index aeaa4cf..ba7a762 100644
--- a/tools/kvm/arm/kvm-cpu.c
+++ b/tools/kvm/arm/kvm-cpu.c
@@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
 	struct kvm_arm_target *target;
 	struct kvm_cpu *vcpu;
 	int coalesced_offset, mmap_size, err = -1;
-	unsigned int i;
+	unsigned int i, target_type;
+	struct kvm_vcpu_init preferred_init;
 	struct kvm_vcpu_init vcpu_init = {
 		.features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
 	};
@@ -55,19 +56,47 @@ 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;
-		err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
-		if (!err)
-			break;
+	/*
+	 * 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];
+				target_type = kvm_arm_targets[i]->id;
+				break;
+			}
+		}
+		if (!target) {
+			target = kvm_arm_targets[0];
+			target_type = preferred_init.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];
+			target_type = target->id;
+			vcpu_init.target = target_type;
+			err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
+			if (!err)
+				break;
+		}
+		if (err)
+			die("Unable to find matching target");
 	}
 
+	vcpu_init.target = target_type;
+	err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
 	if (err || target->init(vcpu))
-		die("Unable to initialise ARM vcpu");
+		die("Unable to initialise vcpu");
 
 	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
 				 KVM_CAP_COALESCED_MMIO);
@@ -81,6 +110,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] 14+ messages in thread

* [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64
  2014-09-08  8:17 [PATCH v3 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel
  2014-09-08  8:17 ` [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel
@ 2014-09-08  8:17 ` Anup Patel
  2014-09-11 16:07   ` Andre Przywara
  2014-09-08  8:17 ` [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel
  2014-09-08  8:17 ` [PATCH v3 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel
  3 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-09-08  8:17 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, patches, will.deacon, marc.zyngier, penberg,
	christoffer.dall, pranavkumar, andre.przywara, 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] 14+ messages in thread

* [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT
  2014-09-08  8:17 [PATCH v3 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel
  2014-09-08  8:17 ` [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel
  2014-09-08  8:17 ` [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel
@ 2014-09-08  8:17 ` Anup Patel
  2014-09-11 16:26   ` Andre Przywara
  2014-09-08  8:17 ` [PATCH v3 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel
  3 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-09-08  8:17 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, patches, will.deacon, marc.zyngier, penberg,
	christoffer.dall, pranavkumar, andre.przywara, 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] 14+ messages in thread

* [PATCH v3 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it
  2014-09-08  8:17 [PATCH v3 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel
                   ` (2 preceding siblings ...)
  2014-09-08  8:17 ` [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel
@ 2014-09-08  8:17 ` Anup Patel
  3 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-09-08  8:17 UTC (permalink / raw)
  To: kvmarm
  Cc: kvm, patches, will.deacon, marc.zyngier, penberg,
	christoffer.dall, pranavkumar, andre.przywara, 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     |   52 ++++++++++++++++++++++++++++++++++++++++++-----
 tools/kvm/arm/kvm-cpu.c |    5 +++++
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/tools/kvm/arm/fdt.c b/tools/kvm/arm/fdt.c
index 186a718..a15450e 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];
 
@@ -84,6 +85,34 @@ static void generate_irq_prop(void *fdt, u8 irq)
 	_FDT(fdt_property(fdt, "interrupts", irq_prop, sizeof(irq_prop)));
 }
 
+struct psci_fns {
+	u32 cpu_suspend;
+	u32 cpu_off;
+	u32 cpu_on;
+	u32 migrate;
+};
+
+static struct psci_fns psci_0_1_fns = {
+	.cpu_suspend = KVM_PSCI_FN_CPU_SUSPEND,
+	.cpu_off = KVM_PSCI_FN_CPU_OFF,
+	.cpu_on = KVM_PSCI_FN_CPU_ON,
+	.migrate = KVM_PSCI_FN_MIGRATE,
+};
+
+static struct psci_fns psci_0_2_aarch32_fns = {
+	.cpu_suspend = PSCI_0_2_FN_CPU_SUSPEND,
+	.cpu_off = PSCI_0_2_FN_CPU_OFF,
+	.cpu_on = PSCI_0_2_FN_CPU_ON,
+	.migrate = PSCI_0_2_FN_MIGRATE,
+};
+
+static struct psci_fns psci_0_2_aarch64_fns = {
+	.cpu_suspend = PSCI_0_2_FN64_CPU_SUSPEND,
+	.cpu_off = PSCI_0_2_FN_CPU_OFF,
+	.cpu_on = PSCI_0_2_FN64_CPU_ON,
+	.migrate = PSCI_0_2_FN64_MIGRATE,
+};
+
 static int setup_fdt(struct kvm *kvm)
 {
 	struct device_header *dev_hdr;
@@ -93,6 +122,7 @@ static int setup_fdt(struct kvm *kvm)
 		cpu_to_fdt64(kvm->arch.memory_guest_start),
 		cpu_to_fdt64(kvm->ram_size),
 	};
+	struct psci_fns *fns;
 	void *fdt		= staging_fdt;
 	void *fdt_dest		= guest_flat_to_host(kvm,
 						     kvm->arch.dtb_guest_start);
@@ -162,12 +192,24 @@ static int setup_fdt(struct kvm *kvm)
 
 	/* PSCI firmware */
 	_FDT(fdt_begin_node(fdt, "psci"));
-	_FDT(fdt_property_string(fdt, "compatible", "arm,psci"));
+	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)));
+		if (kvm->cfg.arch.aarch32_guest) {
+			fns = &psci_0_2_aarch32_fns;
+		} else {
+			fns = &psci_0_2_aarch64_fns;
+		}
+	} else {
+		_FDT(fdt_property_string(fdt, "compatible", "arm,psci"));
+		fns = &psci_0_1_fns;
+	}
 	_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_property_cell(fdt, "cpu_suspend", fns->cpu_suspend));
+	_FDT(fdt_property_cell(fdt, "cpu_off", fns->cpu_off));
+	_FDT(fdt_property_cell(fdt, "cpu_on", fns->cpu_on));
+	_FDT(fdt_property_cell(fdt, "migrate", fns->migrate));
 	_FDT(fdt_end_node(fdt));
 
 	/* Finalise. */
diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
index ba7a762..3a5f358 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] 14+ messages in thread

* Re: [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu
  2014-09-08  8:17 ` [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel
@ 2014-09-11 15:54   ` Andre Przywara
  2014-09-16 22:13     ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Andre Przywara @ 2014-09-11 15:54 UTC (permalink / raw)
  To: Anup Patel, kvmarm@lists.cs.columbia.edu
  Cc: kvm@vger.kernel.org, patches@apm.com, Will Deacon, Marc Zyngier,
	penberg@kernel.org, christoffer.dall@linaro.org,
	pranavkumar@linaro.org

Hi Anup,

On 08/09/14 09:17, 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.
> 
> If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
> target type is not known to KVMTOOL then we forcefully init
> VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.
> 
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> ---
>  tools/kvm/arm/kvm-cpu.c |   52 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
> index aeaa4cf..ba7a762 100644
> --- a/tools/kvm/arm/kvm-cpu.c
> +++ b/tools/kvm/arm/kvm-cpu.c
> @@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>  	struct kvm_arm_target *target;
>  	struct kvm_cpu *vcpu;
>  	int coalesced_offset, mmap_size, err = -1;
> -	unsigned int i;
> +	unsigned int i, target_type;
> +	struct kvm_vcpu_init preferred_init;
>  	struct kvm_vcpu_init vcpu_init = {
>  		.features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
>  	};
> @@ -55,19 +56,47 @@ 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;
> -		err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
> -		if (!err)
> -			break;
> +	/*
> +	 * 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];
> +				target_type = kvm_arm_targets[i]->id;
> +				break;
> +			}
> +		}
> +		if (!target) {
> +			target = kvm_arm_targets[0];

I think you missed the part of the patch which adds the now magic zero
member of kvm_arm_targets[]. A simple static initializer should work.

> +			target_type = preferred_init.target;

Can't you move that out of the loop, in front of it actually? Then you
can get rid of the line above setting the target_type also, since you
always use the same value now, regardless whether you found that CPU in
the list or not.

> +		}
> +	} 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];
> +			target_type = target->id;
> +			vcpu_init.target = target_type;
> +			err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
> +			if (!err)
> +				break;
> +		}
> +		if (err)
> +			die("Unable to find matching target");
>  	}
>  
> +	vcpu_init.target = target_type;
> +	err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);

You should do this only in the if-branch above, since you (try to) call
KVM_ARM_VCPU_INIT already in the else branch before. Otherwise in the
latter case you would do it twice.

Regards,
Andre.

>  	if (err || target->init(vcpu))
> -		die("Unable to initialise ARM vcpu");
> +		die("Unable to initialise vcpu");
>  
>  	coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
>  				 KVM_CAP_COALESCED_MMIO);
> @@ -81,6 +110,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] 14+ messages in thread

* Re: [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64
  2014-09-08  8:17 ` [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel
@ 2014-09-11 16:07   ` Andre Przywara
  2014-09-16 22:24     ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Andre Przywara @ 2014-09-11 16:07 UTC (permalink / raw)
  To: Anup Patel, kvmarm@lists.cs.columbia.edu
  Cc: kvm@vger.kernel.org, patches@apm.com, Will Deacon, Marc Zyngier,
	penberg@kernel.org, christoffer.dall@linaro.org,
	pranavkumar@linaro.org

Anup,

On 08/09/14 09:17, Anup Patel wrote:
> 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.

Why do you need this still if the previous patch got rid of the need for
naming each and every CPU in kvmtool?
Do you care about kernels older than 3.12? I wouldn't bother so much
since you'd need a much newer kvmtool anyway.

Can you consider dropping this patch then?
I'd rather avoid adding CPUs to this list needlessly from now on.

Regards,
Andre.

> 
> 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);
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT
  2014-09-08  8:17 ` [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel
@ 2014-09-11 16:26   ` Andre Przywara
  2014-09-16 22:29     ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Andre Przywara @ 2014-09-11 16:26 UTC (permalink / raw)
  To: Anup Patel, kvmarm@lists.cs.columbia.edu
  Cc: kvm@vger.kernel.org, patches@apm.com, Will Deacon, Marc Zyngier,
	penberg@kernel.org, christoffer.dall@linaro.org,
	pranavkumar@linaro.org


On 08/09/14 09:17, Anup Patel wrote:
> 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.

Is that really a good idea to default to exit_kvm?
I find a shutdown a rather drastic default.
Also I'd like to see RESET not easily mapped to shutdown. If the user
resets the box explicitly, it's probably expected to come up again (to
load an updated kernel or proceed with an install).
So what about a more explicit message like: "... please restart the VM"
until we gain proper reboot support in kvmtool?

Regards,
Andre.

> 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;
>  
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu
  2014-09-11 15:54   ` Andre Przywara
@ 2014-09-16 22:13     ` Anup Patel
  2014-09-16 22:46       ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-09-16 22:13 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Anup Patel, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	patches@apm.com, Will Deacon, Marc Zyngier, penberg@kernel.org,
	christoffer.dall@linaro.org, pranavkumar@linaro.org

On Thu, Sep 11, 2014 at 9:24 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi Anup,
>
> On 08/09/14 09:17, 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.
>>
>> If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
>> target type is not known to KVMTOOL then we forcefully init
>> VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.
>>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> ---
>>  tools/kvm/arm/kvm-cpu.c |   52 +++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
>> index aeaa4cf..ba7a762 100644
>> --- a/tools/kvm/arm/kvm-cpu.c
>> +++ b/tools/kvm/arm/kvm-cpu.c
>> @@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>>       struct kvm_arm_target *target;
>>       struct kvm_cpu *vcpu;
>>       int coalesced_offset, mmap_size, err = -1;
>> -     unsigned int i;
>> +     unsigned int i, target_type;
>> +     struct kvm_vcpu_init preferred_init;
>>       struct kvm_vcpu_init vcpu_init = {
>>               .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
>>       };
>> @@ -55,19 +56,47 @@ 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;
>> -             err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
>> -             if (!err)
>> -                     break;
>> +     /*
>> +      * 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];
>> +                             target_type = kvm_arm_targets[i]->id;
>> +                             break;
>> +                     }
>> +             }
>> +             if (!target) {
>> +                     target = kvm_arm_targets[0];
>
> I think you missed the part of the patch which adds the now magic zero
> member of kvm_arm_targets[]. A simple static initializer should work.

Yes, good catch. I will fix this.

>
>> +                     target_type = preferred_init.target;
>
> Can't you move that out of the loop, in front of it actually? Then you
> can get rid of the line above setting the target_type also, since you
> always use the same value now, regardless whether you found that CPU in
> the list or not.

Sure, I'll rearrange this.

>
>> +             }
>> +     } 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];
>> +                     target_type = target->id;
>> +                     vcpu_init.target = target_type;
>> +                     err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
>> +                     if (!err)
>> +                             break;
>> +             }
>> +             if (err)
>> +                     die("Unable to find matching target");
>>       }
>>
>> +     vcpu_init.target = target_type;
>> +     err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
>
> You should do this only in the if-branch above, since you (try to) call
> KVM_ARM_VCPU_INIT already in the else branch before. Otherwise in the
> latter case you would do it twice.

I was trying to follow the old code which was doing KVM_ARM_VCPU_INIT
twice for the same VCPU but it makes sense to do it only once.

Regards,
Anup

>
> Regards,
> Andre.
>
>>       if (err || target->init(vcpu))
>> -             die("Unable to initialise ARM vcpu");
>> +             die("Unable to initialise vcpu");
>>
>>       coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
>>                                KVM_CAP_COALESCED_MMIO);
>> @@ -81,6 +110,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;
>>  }
>>
>>
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and contains information
> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
> It is to be used solely for the purpose of furthering the parties' business relationship.
> All unauthorized review, use, disclosure or distribution is prohibited.
> If you are not the intended recipient, please contact the sender by reply e-mail
> and destroy all copies of the original message.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64
  2014-09-11 16:07   ` Andre Przywara
@ 2014-09-16 22:24     ` Anup Patel
  2014-09-16 22:46       ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-09-16 22:24 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Anup Patel, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	patches@apm.com, Will Deacon, Marc Zyngier, penberg@kernel.org,
	christoffer.dall@linaro.org, pranavkumar@linaro.org

On Thu, Sep 11, 2014 at 9:37 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Anup,
>
> On 08/09/14 09:17, Anup Patel wrote:
>> 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.
>
> Why do you need this still if the previous patch got rid of the need for
> naming each and every CPU in kvmtool?
> Do you care about kernels older than 3.12? I wouldn't bother so much
> since you'd need a much newer kvmtool anyway.

Yes, actually APM SW team uses 3.12 kernel.

>
> Can you consider dropping this patch then?
> I'd rather avoid adding CPUs to this list needlessly from now on.

I think lets keep this patch because there are quite a few X-Gene
users using older kernel.

Regards,
Anup

>
> Regards,
> Andre.
>
>>
>> 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);
>>
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and contains information
> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
> It is to be used solely for the purpose of furthering the parties' business relationship.
> All unauthorized review, use, disclosure or distribution is prohibited.
> If you are not the intended recipient, please contact the sender by reply e-mail
> and destroy all copies of the original message.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT
  2014-09-11 16:26   ` Andre Przywara
@ 2014-09-16 22:29     ` Anup Patel
  2014-09-16 22:47       ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-09-16 22:29 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Anup Patel, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	patches@apm.com, Will Deacon, Marc Zyngier, penberg@kernel.org,
	christoffer.dall@linaro.org, pranavkumar@linaro.org

On Thu, Sep 11, 2014 at 9:56 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>
> On 08/09/14 09:17, Anup Patel wrote:
>> 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.
>
> Is that really a good idea to default to exit_kvm?
> I find a shutdown a rather drastic default.
> Also I'd like to see RESET not easily mapped to shutdown. If the user
> resets the box explicitly, it's probably expected to come up again (to
> load an updated kernel or proceed with an install).

Absolutely correct but we don't have VM reboot API in KVMTOOL
so I choose this route.

> So what about a more explicit message like: "... please restart the VM"
> until we gain proper reboot support in kvmtool?

Sure, I will print additional info for reset event such as:
INFO: VM reboot support not available
INFO: Please restart the VM manually

Regards,
Anup

>
> Regards,
> Andre.
>
>> 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;
>>
>>
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and contains information
> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
> It is to be used solely for the purpose of furthering the parties' business relationship.
> All unauthorized review, use, disclosure or distribution is prohibited.
> If you are not the intended recipient, please contact the sender by reply e-mail
> and destroy all copies of the original message.
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu
  2014-09-16 22:13     ` Anup Patel
@ 2014-09-16 22:46       ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-09-16 22:46 UTC (permalink / raw)
  To: Anup Patel
  Cc: Andre Przywara, kvm@vger.kernel.org, patches@apm.com, Will Deacon,
	penberg@kernel.org, kvmarm@lists.cs.columbia.edu

On Wed, Sep 17, 2014 at 3:43 AM, Anup Patel <apatel@apm.com> wrote:
> On Thu, Sep 11, 2014 at 9:24 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi Anup,
>>
>> On 08/09/14 09:17, 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.
>>>
>>> If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
>>> target type is not known to KVMTOOL then we forcefully init
>>> VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.
>>>
>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>> ---
>>>  tools/kvm/arm/kvm-cpu.c |   52 +++++++++++++++++++++++++++++++++++++----------
>>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
>>> index aeaa4cf..ba7a762 100644
>>> --- a/tools/kvm/arm/kvm-cpu.c
>>> +++ b/tools/kvm/arm/kvm-cpu.c
>>> @@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
>>>       struct kvm_arm_target *target;
>>>       struct kvm_cpu *vcpu;
>>>       int coalesced_offset, mmap_size, err = -1;
>>> -     unsigned int i;
>>> +     unsigned int i, target_type;
>>> +     struct kvm_vcpu_init preferred_init;
>>>       struct kvm_vcpu_init vcpu_init = {
>>>               .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
>>>       };
>>> @@ -55,19 +56,47 @@ 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;
>>> -             err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
>>> -             if (!err)
>>> -                     break;
>>> +     /*
>>> +      * 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];
>>> +                             target_type = kvm_arm_targets[i]->id;
>>> +                             break;
>>> +                     }
>>> +             }
>>> +             if (!target) {
>>> +                     target = kvm_arm_targets[0];
>>
>> I think you missed the part of the patch which adds the now magic zero
>> member of kvm_arm_targets[]. A simple static initializer should work.
>
> Yes, good catch. I will fix this.
>
>>
>>> +                     target_type = preferred_init.target;
>>
>> Can't you move that out of the loop, in front of it actually? Then you
>> can get rid of the line above setting the target_type also, since you
>> always use the same value now, regardless whether you found that CPU in
>> the list or not.
>
> Sure, I'll rearrange this.
>
>>
>>> +             }
>>> +     } 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];
>>> +                     target_type = target->id;
>>> +                     vcpu_init.target = target_type;
>>> +                     err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
>>> +                     if (!err)
>>> +                             break;
>>> +             }
>>> +             if (err)
>>> +                     die("Unable to find matching target");
>>>       }
>>>
>>> +     vcpu_init.target = target_type;
>>> +     err = ioctl(vcpu->vcpu_fd, KVM_ARM_VCPU_INIT, &vcpu_init);
>>
>> You should do this only in the if-branch above, since you (try to) call
>> KVM_ARM_VCPU_INIT already in the else branch before. Otherwise in the
>> latter case you would do it twice.
>
> I was trying to follow the old code which was doing KVM_ARM_VCPU_INIT
> twice for the same VCPU but it makes sense to do it only once.
>
> Regards,
> Anup
>
>>
>> Regards,
>> Andre.
>>
>>>       if (err || target->init(vcpu))
>>> -             die("Unable to initialise ARM vcpu");
>>> +             die("Unable to initialise vcpu");
>>>
>>>       coalesced_offset = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION,
>>>                                KVM_CAP_COALESCED_MMIO);
>>> @@ -81,6 +110,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;
>>>  }
>>>
>>>
>> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
>> is for the sole use of the intended recipient(s) and contains information
>> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
>> It is to be used solely for the purpose of furthering the parties' business relationship.
>> All unauthorized review, use, disclosure or distribution is prohibited.
>> If you are not the intended recipient, please contact the sender by reply e-mail
>> and destroy all copies of the original message.
>>

Please ignore this notice. I used wrong email in my last reply.

Regards,
Anup

> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64
  2014-09-16 22:24     ` Anup Patel
@ 2014-09-16 22:46       ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-09-16 22:46 UTC (permalink / raw)
  To: Anup Patel
  Cc: Andre Przywara, kvm@vger.kernel.org, patches@apm.com, Will Deacon,
	penberg@kernel.org, kvmarm@lists.cs.columbia.edu

On Wed, Sep 17, 2014 at 3:54 AM, Anup Patel <apatel@apm.com> wrote:
> On Thu, Sep 11, 2014 at 9:37 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Anup,
>>
>> On 08/09/14 09:17, Anup Patel wrote:
>>> 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.
>>
>> Why do you need this still if the previous patch got rid of the need for
>> naming each and every CPU in kvmtool?
>> Do you care about kernels older than 3.12? I wouldn't bother so much
>> since you'd need a much newer kvmtool anyway.
>
> Yes, actually APM SW team uses 3.12 kernel.
>
>>
>> Can you consider dropping this patch then?
>> I'd rather avoid adding CPUs to this list needlessly from now on.
>
> I think lets keep this patch because there are quite a few X-Gene
> users using older kernel.
>
> Regards,
> Anup
>
>>
>> Regards,
>> Andre.
>>
>>>
>>> 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);
>>>
>> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
>> is for the sole use of the intended recipient(s) and contains information
>> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
>> It is to be used solely for the purpose of furthering the parties' business relationship.
>> All unauthorized review, use, disclosure or distribution is prohibited.
>> If you are not the intended recipient, please contact the sender by reply e-mail
>> and destroy all copies of the original message.
>>

Please ignore this notice. I used wrong email in my last reply.

Regards,
Anup

> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT
  2014-09-16 22:29     ` Anup Patel
@ 2014-09-16 22:47       ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-09-16 22:47 UTC (permalink / raw)
  To: Anup Patel
  Cc: Andre Przywara, kvm@vger.kernel.org, patches@apm.com, Will Deacon,
	penberg@kernel.org, kvmarm@lists.cs.columbia.edu

On Wed, Sep 17, 2014 at 3:59 AM, Anup Patel <apatel@apm.com> wrote:
> On Thu, Sep 11, 2014 at 9:56 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> On 08/09/14 09:17, Anup Patel wrote:
>>> 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.
>>
>> Is that really a good idea to default to exit_kvm?
>> I find a shutdown a rather drastic default.
>> Also I'd like to see RESET not easily mapped to shutdown. If the user
>> resets the box explicitly, it's probably expected to come up again (to
>> load an updated kernel or proceed with an install).
>
> Absolutely correct but we don't have VM reboot API in KVMTOOL
> so I choose this route.
>
>> So what about a more explicit message like: "... please restart the VM"
>> until we gain proper reboot support in kvmtool?
>
> Sure, I will print additional info for reset event such as:
> INFO: VM reboot support not available
> INFO: Please restart the VM manually
>
> Regards,
> Anup
>
>>
>> Regards,
>> Andre.
>>
>>> 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;
>>>
>>>
>> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
>> is for the sole use of the intended recipient(s) and contains information
>> that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries.
>> It is to be used solely for the purpose of furthering the parties' business relationship.
>> All unauthorized review, use, disclosure or distribution is prohibited.
>> If you are not the intended recipient, please contact the sender by reply e-mail
>> and destroy all copies of the original message.
>>

Please ignore this notice. I used wrong email in my last reply.

Regards,
Anup

> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-09-16 22:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-08  8:17 [PATCH v3 0/4] kvmtool: ARM/ARM64: Misc updates Anup Patel
2014-09-08  8:17 ` [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu Anup Patel
2014-09-11 15:54   ` Andre Przywara
2014-09-16 22:13     ` Anup Patel
2014-09-16 22:46       ` Anup Patel
2014-09-08  8:17 ` [PATCH v3 2/4] kvmtool: ARM64: Add target type potenza for aarch64 Anup Patel
2014-09-11 16:07   ` Andre Przywara
2014-09-16 22:24     ` Anup Patel
2014-09-16 22:46       ` Anup Patel
2014-09-08  8:17 ` [PATCH v3 3/4] kvmtool: Handle exit reason KVM_EXIT_SYSTEM_EVENT Anup Patel
2014-09-11 16:26   ` Andre Przywara
2014-09-16 22:29     ` Anup Patel
2014-09-16 22:47       ` Anup Patel
2014-09-08  8:17 ` [PATCH v3 4/4] kvmtool: ARM/ARM64: Provide PSCI-0.2 to guest when KVM supports it Anup Patel

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.