Re: [PATCH v2 2/6] KVM: arm/arm64: arch_timer: Rely on the arch timer to parse the firmware tables

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Christoffer,

On 03/03/16 19:38, Christoffer Dall wrote:
On Thu, Feb 11, 2016 at 03:33:20PM +0000, Julien Grall wrote:

[...]

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 6eb2c5d..3cdbefd 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -451,6 +451,8 @@ static struct arch_timer_kvm_info arch_timer_kvm_info;

  struct arch_timer_kvm_info *arch_timer_get_kvm_info(void)
  {
+	arch_timer_kvm_info.virtual_irq = arch_timer_ppi[VIRT_PPI];
+

it feels weird that we set this member on every retrieval of the pointer
to the struct?

wouldn't it be more natural to intialize the struct somewhere during
init and then this function just returns a pointer to the struct?

True, I will move the initialization into arch_timer_common_init.


  	return &arch_timer_kvm_info;
  }


[...]

diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index a669c6a..e4de549 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -17,7 +17,6 @@
   */

  #include <linux/cpu.h>
-#include <linux/of_irq.h>
  #include <linux/kvm.h>
  #include <linux/kvm_host.h>
  #include <linux/interrupt.h>
@@ -375,45 +374,28 @@ static struct notifier_block kvm_timer_cpu_nb = {
  	.notifier_call = kvm_timer_cpu_notify,
  };

-static const struct of_device_id arch_timer_of_match[] = {
-	{ .compatible	= "arm,armv7-timer",	},
-	{ .compatible	= "arm,armv8-timer",	},
-	{},
-};
-
  int kvm_timer_hyp_init(void)
  {
-	struct device_node *np;
-	unsigned int ppi;
  	struct arch_timer_kvm_info *info;
  	int err;

  	info = arch_timer_get_kvm_info();
  	timecounter = &info->timecounter;

-	np = of_find_matching_node(NULL, arch_timer_of_match);
-	if (!np) {
-		kvm_err("kvm_arch_timer: can't find DT node\n");
+	host_vtimer_irq = info->virtual_irq;
+	if (host_vtimer_irq <= 0) {
+		kvm_err("kvm_arch_timer: can't find virtual timer info or config virtual timer interrupt\n");

I don't understand the difference between the two cases in this error
message.

How about: "Invalid virtual timer IRQ: %d\n" ?

I will update the error message.


  		return -ENODEV;
  	}

-	ppi = irq_of_parse_and_map(np, 2);
-	if (!ppi) {
-		kvm_err("kvm_arch_timer: no virtual timer interrupt\n");
-		err = -EINVAL;
-		goto out;
-	}
-
-	err = request_percpu_irq(ppi, kvm_arch_timer_handler,
+	err = request_percpu_irq(host_vtimer_irq, kvm_arch_timer_handler,
  				 "kvm guest timer", kvm_get_running_vcpus());
  	if (err) {
  		kvm_err("kvm_arch_timer: can't request interrupt %d (%d)\n",
-			ppi, err);
+			host_vtimer_irq, err);
  		goto out;
  	}

-	host_vtimer_irq = ppi;
-
  	err = __register_cpu_notifier(&kvm_timer_cpu_nb);
  	if (err) {
  		kvm_err("Cannot register timer CPU notifier\n");
@@ -426,14 +408,13 @@ int kvm_timer_hyp_init(void)
  		goto out_free;
  	}

-	kvm_info("%s IRQ%d\n", np->name, ppi);
+	kvm_info("vtimer IRQ%d\n", host_vtimer_irq);

is "vtimer" a known concept anywhere?  If not, can you use some known
terms intead?

I will use "virtual timer".


  	on_each_cpu(kvm_timer_init_interrupt, NULL, 1);

  	goto out;
  out_free:
-	free_percpu_irq(ppi, kvm_get_running_vcpus());
+	free_percpu_irq(host_vtimer_irq, kvm_get_running_vcpus());
  out:
-	of_node_put(np);
  	return err;
  }

Regards,

--
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux