[PATCH V2 0/5] Enable ACPI support for KVM ARM

[PATCH V2 0/5] Enable ACPI support for KVM ARM

[Wei Huang]

Initial ACPI support for ARM64 has been accepted into Linux kernel recently.
Now it is a good time to re-visit ACPI support for KVM. This patchset
enables ACPI for both arch_timer and vGIC by probing related ACPI tables
and does necessary initialization.

Note that Alexander Spyridaki submitted similar patches before. Some of
his ideas were borrowed in this patchset, but with substancial changes.
In addition we extend support for both GICv2 and GICv3.

This patchset would work better on top of recent GIC/IRQCHIP patches by
Hanjun Guo, who added support for gic_version in ACPI struct of GIC
distributor (search "ACPICA: Introduce GIC version for arm based system").

This patchset can be applied cleanly on top of kvmarm.git queue branch.

-Wei

V1->V2:
* Initialize vgic->max_gic_vcpus field in GICv2 code (found by Andrew Jones)
* Rewrite arch_timer ACPI PPI parsing code
* Verified patches on APM Mustang machine, which has arch_timer & GICv2

V1:
* Initial patchset

Wei Huang (5):
  kvm: arm64: Enable ACPI support for virt arch timer
  kvm: arm64: Dispatch virt GIC probing to device tree and ACPI
  kvm: arm64: Detect GIC version for proper ACPI vGIC probing
  kvm: arm64: Implement ACPI probing code for GICv2
  kvm: arm64: Implement ACPI probing code for GICv3

 include/kvm/arm_vgic.h    |  36 +++++++++---
 virt/kvm/arm/arch_timer.c |  75 ++++++++++++++++++++-----
 virt/kvm/arm/vgic-v2.c    |  66 ++++++++++++++++++++--
 virt/kvm/arm/vgic-v3.c    |  56 +++++++++++++++++--
 virt/kvm/arm/vgic.c       | 140 ++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 332 insertions(+), 41 deletions(-)

-- 
1.8.3.1

[PATCH V2 1/5] kvm: arm64: Enable ACPI support for virt arch timer

[Wei Huang]

This patches enables ACPI support for KVM virtual arch timer. It allows
KVM to parse ACPI table for arch timer PPI when DT table is not present.

Signed-off-by: Alexander Spyridaki <a.spyridakis@virtualopensystems.com>
Signed-off-by: Wei Huang <wei@redhat.com>
---
 virt/kvm/arm/arch_timer.c | 75 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 13 deletions(-)

diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 98c95f2..a9da75a 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -21,6 +21,7 @@
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
 #include <linux/interrupt.h>
+#include <linux/acpi.h>
 
 #include <clocksource/arm_arch_timer.h>
 #include <asm/arch_timer.h>
@@ -274,9 +275,57 @@ static const struct of_device_id arch_timer_of_match[] = {
 	{},
 };
 
-int kvm_timer_hyp_init(void)
+static int kvm_timer_ppi_dt_parse(unsigned int *ppi)
 {
 	struct device_node *np;
+
+	np = of_find_matching_node(NULL, arch_timer_of_match);
+	if (!np)
+		return -ENODEV;
+
+	*ppi = irq_of_parse_and_map(np, 2);
+	if (*ppi == 0) {
+		of_node_put(np);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_ACPI
+struct acpi_table_gtdt *gtdt_acpi;
+static void arch_timer_acpi_parse(struct acpi_table_header *table)
+{
+	gtdt_acpi = container_of(table, struct acpi_table_gtdt, header);
+}
+
+static int kvm_timer_ppi_acpi_parse(unsigned int *ppi)
+{
+	u32 flags;
+	int trigger, polarity;
+
+	/*Get the interrupt number from the GTDT table */
+	acpi_table_parse(ACPI_SIG_GTDT,
+			 (acpi_tbl_table_handler)arch_timer_acpi_parse);
+
+	if (!gtdt_acpi->virtual_timer_interrupt)
+		return -EINVAL;
+
+	flags = gtdt_acpi->virtual_timer_flags;
+	trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
+		: ACPI_LEVEL_SENSITIVE;
+	polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
+		: ACPI_ACTIVE_HIGH;
+
+	*ppi = acpi_register_gsi(NULL, gtdt_acpi->virtual_timer_interrupt,
+				 trigger, polarity);
+
+	return 0;
+}
+#endif
+
+int kvm_timer_hyp_init(void)
+{
 	unsigned int ppi;
 	int err;
 
@@ -284,19 +333,20 @@ int kvm_timer_hyp_init(void)
 	if (!timecounter)
 		return -ENODEV;
 
-	np = of_find_matching_node(NULL, arch_timer_of_match);
-	if (!np) {
-		kvm_err("kvm_arch_timer: can't find DT node\n");
-		return -ENODEV;
-	}
+	/* PPI parsing: try DT first, then ACPI */
+	err = kvm_timer_ppi_dt_parse(&ppi);
+#ifdef CONFIG_ACPI
+	if (err && !acpi_disabled)
+		err = kvm_timer_ppi_acpi_parse(&ppi);
+#endif
 
-	ppi = irq_of_parse_and_map(np, 2);
-	if (!ppi) {
-		kvm_err("kvm_arch_timer: no virtual timer interrupt\n");
-		err = -EINVAL;
-		goto out;
+	if (err) {
+		kvm_err("kvm_arch_timer: can't find virtual timer info or "
+			"config virtual timer interrupt\n");
+		return err;
 	}
 
+	/* configure IRQ handler */
 	err = request_percpu_irq(ppi, kvm_arch_timer_handler,
 				 "kvm guest timer", kvm_get_running_vcpus());
 	if (err) {
@@ -319,14 +369,13 @@ int kvm_timer_hyp_init(void)
 		goto out_free;
 	}
 
-	kvm_info("%s IRQ%d\n", np->name, ppi);
+	kvm_info("timer IRQ%d\n", ppi);
 	on_each_cpu(kvm_timer_init_interrupt, NULL, 1);
 
 	goto out;
 out_free:
 	free_percpu_irq(ppi, kvm_get_running_vcpus());
 out:
-	of_node_put(np);
 	return err;
 }
 
-- 
1.8.3.1

Re: [PATCH V2 1/5] kvm: arm64: Enable ACPI support for virt arch timer

[Andrew Jones]

On Wed, Jun 10, 2015 at 12:16:03AM -0400, Wei Huang wrote:
> This patches enables ACPI support for KVM virtual arch timer. It allows
> KVM to parse ACPI table for arch timer PPI when DT table is not present.
> 
> Signed-off-by: Alexander Spyridaki <a.spyridakis@virtualopensystems.com>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  virt/kvm/arm/arch_timer.c | 75 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 62 insertions(+), 13 deletions(-)
> 
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 98c95f2..a9da75a 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -21,6 +21,7 @@
>  #include <linux/kvm.h>
>  #include <linux/kvm_host.h>
>  #include <linux/interrupt.h>
> +#include <linux/acpi.h>
>  
>  #include <clocksource/arm_arch_timer.h>
>  #include <asm/arch_timer.h>
> @@ -274,9 +275,57 @@ static const struct of_device_id arch_timer_of_match[] = {
>  	{},
>  };
>  
> -int kvm_timer_hyp_init(void)
> +static int kvm_timer_ppi_dt_parse(unsigned int *ppi)
>  {
>  	struct device_node *np;
> +
> +	np = of_find_matching_node(NULL, arch_timer_of_match);
> +	if (!np)
> +		return -ENODEV;
> +
> +	*ppi = irq_of_parse_and_map(np, 2);
> +	if (*ppi == 0) {
> +		of_node_put(np);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_ACPI
> +struct acpi_table_gtdt *gtdt_acpi;

I think gtdt_acpi can/should be static.

> +static void arch_timer_acpi_parse(struct acpi_table_header *table)
> +{
> +	gtdt_acpi = container_of(table, struct acpi_table_gtdt, header);
> +}
> +
> +static int kvm_timer_ppi_acpi_parse(unsigned int *ppi)
> +{
> +	u32 flags;
> +	int trigger, polarity;
> +
> +	/*Get the interrupt number from the GTDT table */
> +	acpi_table_parse(ACPI_SIG_GTDT,
> +			 (acpi_tbl_table_handler)arch_timer_acpi_parse);
> +
> +	if (!gtdt_acpi->virtual_timer_interrupt)
> +		return -EINVAL;
> +
> +	flags = gtdt_acpi->virtual_timer_flags;
> +	trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
> +		: ACPI_LEVEL_SENSITIVE;
> +	polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
> +		: ACPI_ACTIVE_HIGH;
> +
> +	*ppi = acpi_register_gsi(NULL, gtdt_acpi->virtual_timer_interrupt,
> +				 trigger, polarity);
> +
> +	return 0;
> +}
> +#endif
> +
> +int kvm_timer_hyp_init(void)
> +{
>  	unsigned int ppi;
>  	int err;
>  
> @@ -284,19 +333,20 @@ int kvm_timer_hyp_init(void)
>  	if (!timecounter)
>  		return -ENODEV;
>  
> -	np = of_find_matching_node(NULL, arch_timer_of_match);
> -	if (!np) {
> -		kvm_err("kvm_arch_timer: can't find DT node\n");
> -		return -ENODEV;
> -	}
> +	/* PPI parsing: try DT first, then ACPI */
> +	err = kvm_timer_ppi_dt_parse(&ppi);
> +#ifdef CONFIG_ACPI
> +	if (err && !acpi_disabled)
> +		err = kvm_timer_ppi_acpi_parse(&ppi);
> +#endif
>  
> -	ppi = irq_of_parse_and_map(np, 2);
> -	if (!ppi) {
> -		kvm_err("kvm_arch_timer: no virtual timer interrupt\n");
> -		err = -EINVAL;
> -		goto out;
> +	if (err) {
> +		kvm_err("kvm_arch_timer: can't find virtual timer info or "
> +			"config virtual timer interrupt\n");
> +		return err;
>  	}
>  
> +	/* configure IRQ handler */
>  	err = request_percpu_irq(ppi, kvm_arch_timer_handler,
>  				 "kvm guest timer", kvm_get_running_vcpus());
>  	if (err) {
> @@ -319,14 +369,13 @@ int kvm_timer_hyp_init(void)
>  		goto out_free;
>  	}
>  
> -	kvm_info("%s IRQ%d\n", np->name, ppi);
> +	kvm_info("timer IRQ%d\n", ppi);
>  	on_each_cpu(kvm_timer_init_interrupt, NULL, 1);
>  
>  	goto out;
>  out_free:
>  	free_percpu_irq(ppi, kvm_get_running_vcpus());
>  out:
> -	of_node_put(np);
>  	return err;
>  }
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 1/5] kvm: arm64: Enable ACPI support for virt arch timer

[Hanjun Guo]

On 06/10/2015 12:16 PM, Wei Huang wrote:
> This patches enables ACPI support for KVM virtual arch timer. It allows
> KVM to parse ACPI table for arch timer PPI when DT table is not present.
>
> Signed-off-by: Alexander Spyridaki <a.spyridakis@virtualopensystems.com>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>   virt/kvm/arm/arch_timer.c | 75 +++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 62 insertions(+), 13 deletions(-)
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 98c95f2..a9da75a 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -21,6 +21,7 @@
>   #include <linux/kvm.h>
>   #include <linux/kvm_host.h>
>   #include <linux/interrupt.h>
> +#include <linux/acpi.h>
>
>   #include <clocksource/arm_arch_timer.h>
>   #include <asm/arch_timer.h>
> @@ -274,9 +275,57 @@ static const struct of_device_id arch_timer_of_match[] = {
>   	{},
>   };
>
> -int kvm_timer_hyp_init(void)
> +static int kvm_timer_ppi_dt_parse(unsigned int *ppi)
>   {
>   	struct device_node *np;
> +
> +	np = of_find_matching_node(NULL, arch_timer_of_match);
> +	if (!np)
> +		return -ENODEV;
> +
> +	*ppi = irq_of_parse_and_map(np, 2);
> +	if (*ppi == 0) {
> +		of_node_put(np);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_ACPI
> +struct acpi_table_gtdt *gtdt_acpi;
> +static void arch_timer_acpi_parse(struct acpi_table_header *table)
> +{
> +	gtdt_acpi = container_of(table, struct acpi_table_gtdt, header);
> +}
> +
> +static int kvm_timer_ppi_acpi_parse(unsigned int *ppi)
> +{
> +	u32 flags;
> +	int trigger, polarity;
> +
> +	/*Get the interrupt number from the GTDT table */
> +	acpi_table_parse(ACPI_SIG_GTDT,
> +			 (acpi_tbl_table_handler)arch_timer_acpi_parse);
> +
> +	if (!gtdt_acpi->virtual_timer_interrupt)
> +		return -EINVAL;

...

> +
> +	flags = gtdt_acpi->virtual_timer_flags;
> +	trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
> +		: ACPI_LEVEL_SENSITIVE;
> +	polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
> +		: ACPI_ACTIVE_HIGH;
> +
> +	*ppi = acpi_register_gsi(NULL, gtdt_acpi->virtual_timer_interrupt,
> +				 trigger, polarity);

Hmm, this is another duplication of getting trigger/polarity and
registering the interrupt, currently we are working on introducing
gtdt.c and put common used functions there [1], but I think this is
just some nitpick, we can refactor it later when that patch set
is accepted.

[1]: https://lkml.org/lkml/2015/6/10/367

Thanks
Hanjun

[PATCH V2 2/5] kvm: arm64: Dispatch virt GIC probing to device tree and ACPI

[Wei Huang]

This patch creates a dispatch function to support virt GIC probing
in both device tree (DT) and ACPI environment. kvm_vgic_hyp_init()
will probe DT first. If failed, it will try ACPI.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 include/kvm/arm_vgic.h | 18 +++++++++---------
 virt/kvm/arm/vgic-v2.c |  8 ++++----
 virt/kvm/arm/vgic-v3.c |  8 ++++----
 virt/kvm/arm/vgic.c    | 42 +++++++++++++++++++++++++++++++-----------
 4 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 133ea00..3ee732a 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -332,17 +332,17 @@ int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
 #define vgic_initialized(k)	(!!((k)->arch.vgic.nr_cpus))
 #define vgic_ready(k)		((k)->arch.vgic.ready)
 
-int vgic_v2_probe(struct device_node *vgic_node,
-		  const struct vgic_ops **ops,
-		  const struct vgic_params **params);
+int vgic_v2_dt_probe(struct device_node *vgic_node,
+		     const struct vgic_ops **ops,
+		     const struct vgic_params **params);
 #ifdef CONFIG_ARM_GIC_V3
-int vgic_v3_probe(struct device_node *vgic_node,
-		  const struct vgic_ops **ops,
-		  const struct vgic_params **params);
+int vgic_v3_dt_probe(struct device_node *vgic_node,
+		     const struct vgic_ops **ops,
+		     const struct vgic_params **params);
 #else
-static inline int vgic_v3_probe(struct device_node *vgic_node,
-				const struct vgic_ops **ops,
-				const struct vgic_params **params)
+static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
+				   const struct vgic_ops **ops,
+				   const struct vgic_params **params)
 {
 	return -ENODEV;
 }
diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
index f9b9c7c..295996f 100644
--- a/virt/kvm/arm/vgic-v2.c
+++ b/virt/kvm/arm/vgic-v2.c
@@ -167,7 +167,7 @@ static const struct vgic_ops vgic_v2_ops = {
 static struct vgic_params vgic_v2_params;
 
 /**
- * vgic_v2_probe - probe for a GICv2 compatible interrupt controller in DT
+ * vgic_v2_dt_probe - probe for a GICv2 compatible interrupt controller in DT
  * @node:	pointer to the DT node
  * @ops: 	address of a pointer to the GICv2 operations
  * @params:	address of a pointer to HW-specific parameters
@@ -176,9 +176,9 @@ static struct vgic_params vgic_v2_params;
  * in *ops and the HW parameters in *params. Returns an error code
  * otherwise.
  */
-int vgic_v2_probe(struct device_node *vgic_node,
-		  const struct vgic_ops **ops,
-		  const struct vgic_params **params)
+int vgic_v2_dt_probe(struct device_node *vgic_node,
+		     const struct vgic_ops **ops,
+		     const struct vgic_params **params)
 {
 	int ret;
 	struct resource vctrl_res;
diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
index dff0602..91814e2 100644
--- a/virt/kvm/arm/vgic-v3.c
+++ b/virt/kvm/arm/vgic-v3.c
@@ -211,7 +211,7 @@ static const struct vgic_ops vgic_v3_ops = {
 static struct vgic_params vgic_v3_params;
 
 /**
- * vgic_v3_probe - probe for a GICv3 compatible interrupt controller in DT
+ * vgic_v3_dt_probe - probe for a GICv3 compatible interrupt controller in DT
  * @node:	pointer to the DT node
  * @ops: 	address of a pointer to the GICv3 operations
  * @params:	address of a pointer to HW-specific parameters
@@ -220,9 +220,9 @@ static struct vgic_params vgic_v3_params;
  * in *ops and the HW parameters in *params. Returns an error code
  * otherwise.
  */
-int vgic_v3_probe(struct device_node *vgic_node,
-		  const struct vgic_ops **ops,
-		  const struct vgic_params **params)
+int vgic_v3_dt_probe(struct device_node *vgic_node,
+		     const struct vgic_ops **ops,
+		     const struct vgic_params **params)
 {
 	int ret = 0;
 	u32 gicv_idx;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 78fb820..b4010f0 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -25,6 +25,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/uaccess.h>
+#include <linux/acpi.h>
 
 #include <linux/irqchip/arm-gic.h>
 
@@ -2088,32 +2089,51 @@ static struct notifier_block vgic_cpu_nb = {
 };
 
 static const struct of_device_id vgic_ids[] = {
-	{ .compatible = "arm,cortex-a15-gic",	.data = vgic_v2_probe, },
-	{ .compatible = "arm,cortex-a7-gic",	.data = vgic_v2_probe, },
-	{ .compatible = "arm,gic-400",		.data = vgic_v2_probe, },
-	{ .compatible = "arm,gic-v3",		.data = vgic_v3_probe, },
+	{ .compatible = "arm,cortex-a15-gic",	.data = vgic_v2_dt_probe, },
+	{ .compatible = "arm,cortex-a7-gic",	.data = vgic_v2_dt_probe, },
+	{ .compatible = "arm,gic-400",		.data = vgic_v2_dt_probe, },
+	{ .compatible = "arm,gic-v3",		.data = vgic_v3_dt_probe, },
 	{},
 };
 
-int kvm_vgic_hyp_init(void)
+static int kvm_vgic_dt_probe(void)
 {
 	const struct of_device_id *matched_id;
 	const int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
 				const struct vgic_params **);
 	struct device_node *vgic_node;
-	int ret;
 
 	vgic_node = of_find_matching_node_and_match(NULL,
 						    vgic_ids, &matched_id);
-	if (!vgic_node) {
-		kvm_err("error: no compatible GIC node found\n");
+	if (!vgic_node)
 		return -ENODEV;
-	}
 
 	vgic_probe = matched_id->data;
-	ret = vgic_probe(vgic_node, &vgic_ops, &vgic);
-	if (ret)
+
+	return vgic_probe(vgic_node, &vgic_ops, &vgic);
+}
+
+#ifdef CONFIG_ACPI
+static int kvm_vgic_acpi_probe(void)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_ACPI */
+
+int kvm_vgic_hyp_init(void)
+{
+	int ret;
+
+	ret = kvm_vgic_dt_probe();
+#ifdef CONFIG_ACPI
+	if (ret && !acpi_disabled)
+		ret = kvm_vgic_acpi_probe();
+#endif
+
+	if (ret) {
+		kvm_err("error: KVM vGIC probing failed\n");
 		return ret;
+	}
 
 	ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
 				 "vgic", kvm_get_running_vcpus());
-- 
1.8.3.1

Re: [PATCH V2 2/5] kvm: arm64: Dispatch virt GIC probing to device tree and ACPI

[Hanjun Guo]

On 06/10/2015 12:16 PM, Wei Huang wrote:
> This patch creates a dispatch function to support virt GIC probing
> in both device tree (DT) and ACPI environment. kvm_vgic_hyp_init()
> will probe DT first. If failed, it will try ACPI.
>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>   include/kvm/arm_vgic.h | 18 +++++++++---------
>   virt/kvm/arm/vgic-v2.c |  8 ++++----
>   virt/kvm/arm/vgic-v3.c |  8 ++++----
>   virt/kvm/arm/vgic.c    | 42 +++++++++++++++++++++++++++++++-----------
>   4 files changed, 48 insertions(+), 28 deletions(-)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 133ea00..3ee732a 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -332,17 +332,17 @@ int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
>   #define vgic_initialized(k)	(!!((k)->arch.vgic.nr_cpus))
>   #define vgic_ready(k)		((k)->arch.vgic.ready)
>
> -int vgic_v2_probe(struct device_node *vgic_node,
> -		  const struct vgic_ops **ops,
> -		  const struct vgic_params **params);
> +int vgic_v2_dt_probe(struct device_node *vgic_node,
> +		     const struct vgic_ops **ops,
> +		     const struct vgic_params **params);
>   #ifdef CONFIG_ARM_GIC_V3
> -int vgic_v3_probe(struct device_node *vgic_node,
> -		  const struct vgic_ops **ops,
> -		  const struct vgic_params **params);
> +int vgic_v3_dt_probe(struct device_node *vgic_node,
> +		     const struct vgic_ops **ops,
> +		     const struct vgic_params **params);
>   #else
> -static inline int vgic_v3_probe(struct device_node *vgic_node,
> -				const struct vgic_ops **ops,
> -				const struct vgic_params **params)
> +static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
> +				   const struct vgic_ops **ops,
> +				   const struct vgic_params **params)
>   {
>   	return -ENODEV;
>   }
> diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
> index f9b9c7c..295996f 100644
> --- a/virt/kvm/arm/vgic-v2.c
> +++ b/virt/kvm/arm/vgic-v2.c
> @@ -167,7 +167,7 @@ static const struct vgic_ops vgic_v2_ops = {
>   static struct vgic_params vgic_v2_params;
>
>   /**
> - * vgic_v2_probe - probe for a GICv2 compatible interrupt controller in DT
> + * vgic_v2_dt_probe - probe for a GICv2 compatible interrupt controller in DT
>    * @node:	pointer to the DT node
>    * @ops: 	address of a pointer to the GICv2 operations
>    * @params:	address of a pointer to HW-specific parameters
> @@ -176,9 +176,9 @@ static struct vgic_params vgic_v2_params;
>    * in *ops and the HW parameters in *params. Returns an error code
>    * otherwise.
>    */
> -int vgic_v2_probe(struct device_node *vgic_node,
> -		  const struct vgic_ops **ops,
> -		  const struct vgic_params **params)
> +int vgic_v2_dt_probe(struct device_node *vgic_node,
> +		     const struct vgic_ops **ops,
> +		     const struct vgic_params **params)
>   {
>   	int ret;
>   	struct resource vctrl_res;
> diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
> index dff0602..91814e2 100644
> --- a/virt/kvm/arm/vgic-v3.c
> +++ b/virt/kvm/arm/vgic-v3.c
> @@ -211,7 +211,7 @@ static const struct vgic_ops vgic_v3_ops = {
>   static struct vgic_params vgic_v3_params;
>
>   /**
> - * vgic_v3_probe - probe for a GICv3 compatible interrupt controller in DT
> + * vgic_v3_dt_probe - probe for a GICv3 compatible interrupt controller in DT
>    * @node:	pointer to the DT node
>    * @ops: 	address of a pointer to the GICv3 operations
>    * @params:	address of a pointer to HW-specific parameters
> @@ -220,9 +220,9 @@ static struct vgic_params vgic_v3_params;
>    * in *ops and the HW parameters in *params. Returns an error code
>    * otherwise.
>    */
> -int vgic_v3_probe(struct device_node *vgic_node,
> -		  const struct vgic_ops **ops,
> -		  const struct vgic_params **params)
> +int vgic_v3_dt_probe(struct device_node *vgic_node,
> +		     const struct vgic_ops **ops,
> +		     const struct vgic_params **params)
>   {
>   	int ret = 0;
>   	u32 gicv_idx;
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 78fb820..b4010f0 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -25,6 +25,7 @@
>   #include <linux/of_address.h>
>   #include <linux/of_irq.h>
>   #include <linux/uaccess.h>
> +#include <linux/acpi.h>
>
>   #include <linux/irqchip/arm-gic.h>
>
> @@ -2088,32 +2089,51 @@ static struct notifier_block vgic_cpu_nb = {
>   };
>
>   static const struct of_device_id vgic_ids[] = {
> -	{ .compatible = "arm,cortex-a15-gic",	.data = vgic_v2_probe, },
> -	{ .compatible = "arm,cortex-a7-gic",	.data = vgic_v2_probe, },
> -	{ .compatible = "arm,gic-400",		.data = vgic_v2_probe, },
> -	{ .compatible = "arm,gic-v3",		.data = vgic_v3_probe, },
> +	{ .compatible = "arm,cortex-a15-gic",	.data = vgic_v2_dt_probe, },
> +	{ .compatible = "arm,cortex-a7-gic",	.data = vgic_v2_dt_probe, },
> +	{ .compatible = "arm,gic-400",		.data = vgic_v2_dt_probe, },
> +	{ .compatible = "arm,gic-v3",		.data = vgic_v3_dt_probe, },
>   	{},
>   };
>
> -int kvm_vgic_hyp_init(void)
> +static int kvm_vgic_dt_probe(void)
>   {
>   	const struct of_device_id *matched_id;
>   	const int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
>   				const struct vgic_params **);
>   	struct device_node *vgic_node;
> -	int ret;
>
>   	vgic_node = of_find_matching_node_and_match(NULL,
>   						    vgic_ids, &matched_id);
> -	if (!vgic_node) {
> -		kvm_err("error: no compatible GIC node found\n");
> +	if (!vgic_node)
>   		return -ENODEV;
> -	}
>
>   	vgic_probe = matched_id->data;
> -	ret = vgic_probe(vgic_node, &vgic_ops, &vgic);
> -	if (ret)
> +
> +	return vgic_probe(vgic_node, &vgic_ops, &vgic);
> +}
> +
> +#ifdef CONFIG_ACPI
> +static int kvm_vgic_acpi_probe(void)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_ACPI */
> +
> +int kvm_vgic_hyp_init(void)
> +{
> +	int ret;
> +
> +	ret = kvm_vgic_dt_probe();
> +#ifdef CONFIG_ACPI
> +	if (ret && !acpi_disabled)
> +		ret = kvm_vgic_acpi_probe();
> +#endif

How about:

#ifdef CONFIG_ACPI
static int kvm_vgic_acpi_probe(void)
{
	return -EINVAL;
}
#else
static inline int kvm_vgic_acpi_probe(void) { }
#endif /* CONFIG_ACPI */

Then no need for #ifdef CONFIG_ACPI ... #endif in the function.

Thanks
Hanjun

[PATCH V2 3/5] kvm: arm64: Detect GIC version for proper ACPI vGIC probing

[Wei Huang]

There are two GICs (GICv2 and GICv3) supported by KVM. So it is necessary
to find out GIC version before calling ACPI probing functions defined
in vgic-v2.c and vgic-v3.c.

This patch detects GIC version by checking gic_version field of GIC
distributor, which was defined  since ACPI 6.0. In case of ACPI 5.1,
we use manual hardware discovery to find out GIC version.

NOTE: This patch is based on a recent patch by Hanjun Guo.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
 include/kvm/arm_vgic.h |  18 +++++++++
 virt/kvm/arm/vgic-v2.c |  10 +++++
 virt/kvm/arm/vgic-v3.c |  10 +++++
 virt/kvm/arm/vgic.c    | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 3ee732a..7a44b08 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -24,6 +24,7 @@
 #include <linux/irqreturn.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <linux/acpi.h>
 #include <kvm/iodev.h>
 
 #define VGIC_NR_IRQS_LEGACY	256
@@ -335,10 +336,18 @@ int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
 int vgic_v2_dt_probe(struct device_node *vgic_node,
 		     const struct vgic_ops **ops,
 		     const struct vgic_params **params);
+#ifdef CONFIG_ACPI
+int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *,
+		       const struct vgic_ops **ops,
+		       const struct vgic_params **params);
+#endif /* CONFIG_ACPI */
 #ifdef CONFIG_ARM_GIC_V3
 int vgic_v3_dt_probe(struct device_node *vgic_node,
 		     const struct vgic_ops **ops,
 		     const struct vgic_params **params);
+int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
+		       const struct vgic_ops **ops,
+		       const struct vgic_params **params);
 #else
 static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
 				   const struct vgic_ops **ops,
@@ -346,6 +355,15 @@ static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
 {
 	return -ENODEV;
 }
+
+#ifdef CONFIG_ACPI
+int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
+		       const struct vgic_ops **ops,
+		       const struct vgic_params **params)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_ACPI */
 #endif
 
 #endif
diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
index 295996f..711de82 100644
--- a/virt/kvm/arm/vgic-v2.c
+++ b/virt/kvm/arm/vgic-v2.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/acpi.h>
 
 #include <linux/irqchip/arm-gic.h>
 
@@ -257,3 +258,12 @@ out:
 	of_node_put(vgic_node);
 	return ret;
 }
+
+#ifdef CONFIG_ACPI
+int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
+		       const struct vgic_ops **ops,
+		       const struct vgic_params **params)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_ACPI */
diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
index 91814e2..99d0f9f 100644
--- a/virt/kvm/arm/vgic-v3.c
+++ b/virt/kvm/arm/vgic-v3.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/acpi.h>
 
 #include <linux/irqchip/arm-gic-v3.h>
 
@@ -285,3 +286,12 @@ out:
 	of_node_put(vgic_node);
 	return ret;
 }
+
+#ifdef CONFIG_ACPI
+int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
+		       const struct vgic_ops **ops,
+		       const struct vgic_params **params)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_ACPI */
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index b4010f0..cd09877 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -28,6 +28,7 @@
 #include <linux/acpi.h>
 
 #include <linux/irqchip/arm-gic.h>
+#include <linux/irqchip/arm-gic-v3.h>
 
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_arm.h>
@@ -2114,9 +2115,106 @@ static int kvm_vgic_dt_probe(void)
 }
 
 #ifdef CONFIG_ACPI
+u8 gic_version = ACPI_MADT_GIC_VER_UNKNOWN;
+phys_addr_t dist_phy_base;
+static struct acpi_madt_generic_interrupt *vgic_acpi;
+
+static void gic_get_acpi_header(struct acpi_subtable_header *header)
+{
+	vgic_acpi = (struct acpi_madt_generic_interrupt *)header;
+}
+
+static int gic_parse_distributor(struct acpi_subtable_header *header,
+				 const unsigned long end)
+{
+	struct acpi_madt_generic_distributor *dist;
+
+	dist = (struct acpi_madt_generic_distributor *)header;
+
+	if (BAD_MADT_ENTRY(dist, end))
+		return -EINVAL;
+
+	gic_version = dist->gic_version;
+	dist_phy_base = dist->base_address;
+
+	return 0;
+}
+
+static int gic_match_redist(struct acpi_subtable_header *header,
+			    const unsigned long end)
+{
+	return 0;
+}
+
+static bool gic_redist_is_present(void)
+{
+	int count;
+
+	/* scan MADT table to find if we have redistributor entries */
+	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
+				      gic_match_redist, 0);
+
+	return (count > 0) ? true : false;
+}
+
 static int kvm_vgic_acpi_probe(void)
 {
-	return -EINVAL;
+	u32 reg;
+	int count;
+	void __iomem *dist_base;
+	int ret;
+
+	/* MADT table */
+	ret = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
+			(acpi_tbl_entry_handler)gic_get_acpi_header, 0);
+	if (!ret) {
+		pr_err("Failed to get MADT VGIC CPU entry\n");
+		return -ENODEV;
+	}
+
+	/* detect GIC version */
+	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
+				      gic_parse_distributor, 0);
+	if (count <= 0) {
+		pr_err("No valid GIC distributor entry exists\n");
+		return -ENODEV;
+	}
+	if (gic_version >= ACPI_MADT_GIC_VER_RESERVED) {
+		pr_err("Invalid GIC version %d in MADT\n", gic_version);
+		return -EINVAL;
+	}
+
+	/* falls back to manual hardware discovery under ACPI 5.1 */
+	if (gic_version == ACPI_MADT_GIC_VER_UNKNOWN) {
+		if (gic_redist_is_present()) {
+			dist_base = ioremap(dist_phy_base, SZ_64K);
+			if (!dist_base)
+				return -ENOMEM;
+
+			reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
+			if (reg == GIC_PIDR2_ARCH_GICv3)
+				gic_version = ACPI_MADT_GIC_VER_V3;
+			else
+				gic_version = ACPI_MADT_GIC_VER_V4;
+
+			iounmap(dist_base);
+		} else {
+			gic_version = ACPI_MADT_GIC_VER_V2;
+		}
+	}
+
+	switch (gic_version) {
+	case ACPI_MADT_GIC_VER_V2:
+		ret = vgic_v2_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
+		break;
+	case ACPI_MADT_GIC_VER_V3:
+		ret = vgic_v3_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
+		break;
+	default:
+		ret = -ENODEV;
+	}
+
+	return ret;
 }
 #endif /* CONFIG_ACPI */
 
-- 
1.8.3.1

Re: [PATCH V2 3/5] kvm: arm64: Detect GIC version for proper ACPI vGIC probing

[Andrew Jones]

On Wed, Jun 10, 2015 at 12:16:05AM -0400, Wei Huang wrote:
> There are two GICs (GICv2 and GICv3) supported by KVM. So it is necessary
> to find out GIC version before calling ACPI probing functions defined
> in vgic-v2.c and vgic-v3.c.
> 
> This patch detects GIC version by checking gic_version field of GIC
> distributor, which was defined  since ACPI 6.0. In case of ACPI 5.1,
> we use manual hardware discovery to find out GIC version.
> 
> NOTE: This patch is based on a recent patch by Hanjun Guo.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  include/kvm/arm_vgic.h |  18 +++++++++
>  virt/kvm/arm/vgic-v2.c |  10 +++++
>  virt/kvm/arm/vgic-v3.c |  10 +++++
>  virt/kvm/arm/vgic.c    | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 137 insertions(+), 1 deletion(-)
> 
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 3ee732a..7a44b08 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -24,6 +24,7 @@
>  #include <linux/irqreturn.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
> +#include <linux/acpi.h>
>  #include <kvm/iodev.h>
>  
>  #define VGIC_NR_IRQS_LEGACY	256
> @@ -335,10 +336,18 @@ int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
>  int vgic_v2_dt_probe(struct device_node *vgic_node,
>  		     const struct vgic_ops **ops,
>  		     const struct vgic_params **params);
> +#ifdef CONFIG_ACPI
> +int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params);
> +#endif /* CONFIG_ACPI */
>  #ifdef CONFIG_ARM_GIC_V3
>  int vgic_v3_dt_probe(struct device_node *vgic_node,
>  		     const struct vgic_ops **ops,
>  		     const struct vgic_params **params);
> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params);
>  #else
>  static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
>  				   const struct vgic_ops **ops,
> @@ -346,6 +355,15 @@ static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
>  {
>  	return -ENODEV;
>  }
> +
> +#ifdef CONFIG_ACPI
> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params)
> +{
> +	return -ENODEV;
> +}
> +#endif /* CONFIG_ACPI */
>  #endif
>  
>  #endif
> diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
> index 295996f..711de82 100644
> --- a/virt/kvm/arm/vgic-v2.c
> +++ b/virt/kvm/arm/vgic-v2.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/acpi.h>
>  
>  #include <linux/irqchip/arm-gic.h>
>  
> @@ -257,3 +258,12 @@ out:
>  	of_node_put(vgic_node);
>  	return ret;
>  }
> +
> +#ifdef CONFIG_ACPI
> +int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_ACPI */
> diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
> index 91814e2..99d0f9f 100644
> --- a/virt/kvm/arm/vgic-v3.c
> +++ b/virt/kvm/arm/vgic-v3.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/acpi.h>
>  
>  #include <linux/irqchip/arm-gic-v3.h>
>  
> @@ -285,3 +286,12 @@ out:
>  	of_node_put(vgic_node);
>  	return ret;
>  }
> +
> +#ifdef CONFIG_ACPI
> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_ACPI */
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index b4010f0..cd09877 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -28,6 +28,7 @@
>  #include <linux/acpi.h>
>  
>  #include <linux/irqchip/arm-gic.h>
> +#include <linux/irqchip/arm-gic-v3.h>
>  
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_arm.h>
> @@ -2114,9 +2115,106 @@ static int kvm_vgic_dt_probe(void)
>  }
>  
>  #ifdef CONFIG_ACPI
> +u8 gic_version = ACPI_MADT_GIC_VER_UNKNOWN;
> +phys_addr_t dist_phy_base;

I think these can/should be static, and adding acpi to the name
might be nice.

> +static struct acpi_madt_generic_interrupt *vgic_acpi;
> +
> +static void gic_get_acpi_header(struct acpi_subtable_header *header)
> +{
> +	vgic_acpi = (struct acpi_madt_generic_interrupt *)header;
> +}
> +
> +static int gic_parse_distributor(struct acpi_subtable_header *header,
> +				 const unsigned long end)
> +{
> +	struct acpi_madt_generic_distributor *dist;
> +
> +	dist = (struct acpi_madt_generic_distributor *)header;
> +
> +	if (BAD_MADT_ENTRY(dist, end))
> +		return -EINVAL;
> +
> +	gic_version = dist->gic_version;
> +	dist_phy_base = dist->base_address;
> +
> +	return 0;
> +}
> +
> +static int gic_match_redist(struct acpi_subtable_header *header,
> +			    const unsigned long end)
> +{
> +	return 0;
> +}
> +
> +static bool gic_redist_is_present(void)
> +{
> +	int count;
> +
> +	/* scan MADT table to find if we have redistributor entries */
> +	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
> +				      gic_match_redist, 0);
> +
> +	return (count > 0) ? true : false;

nit: no need for the '? true : false'

> +}
> +
>  static int kvm_vgic_acpi_probe(void)
>  {
> -	return -EINVAL;
> +	u32 reg;
> +	int count;
> +	void __iomem *dist_base;
> +	int ret;
> +
> +	/* MADT table */
> +	ret = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
> +			(acpi_tbl_entry_handler)gic_get_acpi_header, 0);
> +	if (!ret) {
> +		pr_err("Failed to get MADT VGIC CPU entry\n");
> +		return -ENODEV;
> +	}
> +
> +	/* detect GIC version */
> +	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> +				      gic_parse_distributor, 0);
> +	if (count <= 0) {
> +		pr_err("No valid GIC distributor entry exists\n");
> +		return -ENODEV;
> +	}
> +	if (gic_version >= ACPI_MADT_GIC_VER_RESERVED) {
> +		pr_err("Invalid GIC version %d in MADT\n", gic_version);
> +		return -EINVAL;
> +	}
> +
> +	/* falls back to manual hardware discovery under ACPI 5.1 */
> +	if (gic_version == ACPI_MADT_GIC_VER_UNKNOWN) {
> +		if (gic_redist_is_present()) {
> +			dist_base = ioremap(dist_phy_base, SZ_64K);
> +			if (!dist_base)
> +				return -ENOMEM;
> +
> +			reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> +			if (reg == GIC_PIDR2_ARCH_GICv3)
> +				gic_version = ACPI_MADT_GIC_VER_V3;
> +			else
> +				gic_version = ACPI_MADT_GIC_VER_V4;
> +
> +			iounmap(dist_base);
> +		} else {
> +			gic_version = ACPI_MADT_GIC_VER_V2;
> +		}
> +	}
> +
> +	switch (gic_version) {
> +	case ACPI_MADT_GIC_VER_V2:
> +		ret = vgic_v2_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
> +		break;
> +	case ACPI_MADT_GIC_VER_V3:
> +		ret = vgic_v3_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
> +		break;
> +	default:
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;
>  }
>  #endif /* CONFIG_ACPI */
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/5] kvm: arm64: Detect GIC version for proper ACPI vGIC probing

[Marc Zyngier]

On 10/06/15 05:16, Wei Huang wrote:
> There are two GICs (GICv2 and GICv3) supported by KVM. So it is necessary
> to find out GIC version before calling ACPI probing functions defined
> in vgic-v2.c and vgic-v3.c.
> 
> This patch detects GIC version by checking gic_version field of GIC
> distributor, which was defined  since ACPI 6.0. In case of ACPI 5.1,
> we use manual hardware discovery to find out GIC version.
> 
> NOTE: This patch is based on a recent patch by Hanjun Guo.

Well, this is really a duplicate of Hanjun's patch, and I'd rather have
some common infrastructure.

Surely it should be possible for the ACPI GIC code to export the
necessary entry points, or even to provide the required information?

Thanks,

	M.

> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  include/kvm/arm_vgic.h |  18 +++++++++
>  virt/kvm/arm/vgic-v2.c |  10 +++++
>  virt/kvm/arm/vgic-v3.c |  10 +++++
>  virt/kvm/arm/vgic.c    | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 137 insertions(+), 1 deletion(-)
> 
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 3ee732a..7a44b08 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -24,6 +24,7 @@
>  #include <linux/irqreturn.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
> +#include <linux/acpi.h>
>  #include <kvm/iodev.h>
>  
>  #define VGIC_NR_IRQS_LEGACY	256
> @@ -335,10 +336,18 @@ int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
>  int vgic_v2_dt_probe(struct device_node *vgic_node,
>  		     const struct vgic_ops **ops,
>  		     const struct vgic_params **params);
> +#ifdef CONFIG_ACPI
> +int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params);
> +#endif /* CONFIG_ACPI */
>  #ifdef CONFIG_ARM_GIC_V3
>  int vgic_v3_dt_probe(struct device_node *vgic_node,
>  		     const struct vgic_ops **ops,
>  		     const struct vgic_params **params);
> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params);
>  #else
>  static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
>  				   const struct vgic_ops **ops,
> @@ -346,6 +355,15 @@ static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
>  {
>  	return -ENODEV;
>  }
> +
> +#ifdef CONFIG_ACPI
> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params)
> +{
> +	return -ENODEV;
> +}
> +#endif /* CONFIG_ACPI */
>  #endif
>  
>  #endif
> diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
> index 295996f..711de82 100644
> --- a/virt/kvm/arm/vgic-v2.c
> +++ b/virt/kvm/arm/vgic-v2.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/acpi.h>
>  
>  #include <linux/irqchip/arm-gic.h>
>  
> @@ -257,3 +258,12 @@ out:
>  	of_node_put(vgic_node);
>  	return ret;
>  }
> +
> +#ifdef CONFIG_ACPI
> +int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_ACPI */
> diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
> index 91814e2..99d0f9f 100644
> --- a/virt/kvm/arm/vgic-v3.c
> +++ b/virt/kvm/arm/vgic-v3.c
> @@ -23,6 +23,7 @@
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/acpi.h>
>  
>  #include <linux/irqchip/arm-gic-v3.h>
>  
> @@ -285,3 +286,12 @@ out:
>  	of_node_put(vgic_node);
>  	return ret;
>  }
> +
> +#ifdef CONFIG_ACPI
> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
> +		       const struct vgic_ops **ops,
> +		       const struct vgic_params **params)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_ACPI */
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index b4010f0..cd09877 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -28,6 +28,7 @@
>  #include <linux/acpi.h>
>  
>  #include <linux/irqchip/arm-gic.h>
> +#include <linux/irqchip/arm-gic-v3.h>
>  
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_arm.h>
> @@ -2114,9 +2115,106 @@ static int kvm_vgic_dt_probe(void)
>  }
>  
>  #ifdef CONFIG_ACPI
> +u8 gic_version = ACPI_MADT_GIC_VER_UNKNOWN;
> +phys_addr_t dist_phy_base;
> +static struct acpi_madt_generic_interrupt *vgic_acpi;
> +
> +static void gic_get_acpi_header(struct acpi_subtable_header *header)
> +{
> +	vgic_acpi = (struct acpi_madt_generic_interrupt *)header;
> +}
> +
> +static int gic_parse_distributor(struct acpi_subtable_header *header,
> +				 const unsigned long end)
> +{
> +	struct acpi_madt_generic_distributor *dist;
> +
> +	dist = (struct acpi_madt_generic_distributor *)header;
> +
> +	if (BAD_MADT_ENTRY(dist, end))
> +		return -EINVAL;
> +
> +	gic_version = dist->gic_version;
> +	dist_phy_base = dist->base_address;
> +
> +	return 0;
> +}
> +
> +static int gic_match_redist(struct acpi_subtable_header *header,
> +			    const unsigned long end)
> +{
> +	return 0;
> +}
> +
> +static bool gic_redist_is_present(void)
> +{
> +	int count;
> +
> +	/* scan MADT table to find if we have redistributor entries */
> +	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
> +				      gic_match_redist, 0);
> +
> +	return (count > 0) ? true : false;
> +}
> +
>  static int kvm_vgic_acpi_probe(void)
>  {
> -	return -EINVAL;
> +	u32 reg;
> +	int count;
> +	void __iomem *dist_base;
> +	int ret;
> +
> +	/* MADT table */
> +	ret = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
> +			(acpi_tbl_entry_handler)gic_get_acpi_header, 0);
> +	if (!ret) {
> +		pr_err("Failed to get MADT VGIC CPU entry\n");
> +		return -ENODEV;
> +	}
> +
> +	/* detect GIC version */
> +	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> +				      gic_parse_distributor, 0);
> +	if (count <= 0) {
> +		pr_err("No valid GIC distributor entry exists\n");
> +		return -ENODEV;
> +	}
> +	if (gic_version >= ACPI_MADT_GIC_VER_RESERVED) {
> +		pr_err("Invalid GIC version %d in MADT\n", gic_version);
> +		return -EINVAL;
> +	}
> +
> +	/* falls back to manual hardware discovery under ACPI 5.1 */
> +	if (gic_version == ACPI_MADT_GIC_VER_UNKNOWN) {
> +		if (gic_redist_is_present()) {
> +			dist_base = ioremap(dist_phy_base, SZ_64K);
> +			if (!dist_base)
> +				return -ENOMEM;
> +
> +			reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> +			if (reg == GIC_PIDR2_ARCH_GICv3)
> +				gic_version = ACPI_MADT_GIC_VER_V3;
> +			else
> +				gic_version = ACPI_MADT_GIC_VER_V4;
> +
> +			iounmap(dist_base);
> +		} else {
> +			gic_version = ACPI_MADT_GIC_VER_V2;
> +		}
> +	}
> +
> +	switch (gic_version) {
> +	case ACPI_MADT_GIC_VER_V2:
> +		ret = vgic_v2_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
> +		break;
> +	case ACPI_MADT_GIC_VER_V3:
> +		ret = vgic_v3_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
> +		break;
> +	default:
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;
>  }
>  #endif /* CONFIG_ACPI */
>  
> 


-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2 3/5] kvm: arm64: Detect GIC version for proper ACPI vGIC probing

[Wei Huang]

On 06/10/2015 11:43 AM, Marc Zyngier wrote:
> On 10/06/15 05:16, Wei Huang wrote:
>> There are two GICs (GICv2 and GICv3) supported by KVM. So it is necessary
>> to find out GIC version before calling ACPI probing functions defined
>> in vgic-v2.c and vgic-v3.c.
>>
>> This patch detects GIC version by checking gic_version field of GIC
>> distributor, which was defined  since ACPI 6.0. In case of ACPI 5.1,
>> we use manual hardware discovery to find out GIC version.
>>
>> NOTE: This patch is based on a recent patch by Hanjun Guo.
> 
> Well, this is really a duplicate of Hanjun's patch, and I'd rather have
> some common infrastructure.
> 
> Surely it should be possible for the ACPI GIC code to export the
> necessary entry points, or even to provide the required information?
> 

I agreed that this is a duplication and should be avoided if possible.
One easy solution is to export from GIC driver to KVM (using
EXPORT_SYMBOL or something similar). Is this acceptable?

Anyway the difficulty is to find a common place to store and share info
between other modules & KVM.

Thanks,
-Wei


> Thanks,
> 
> 	M.
> 
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Signed-off-by: Wei Huang <wei@redhat.com>
>> ---
>>  include/kvm/arm_vgic.h |  18 +++++++++
>>  virt/kvm/arm/vgic-v2.c |  10 +++++
>>  virt/kvm/arm/vgic-v3.c |  10 +++++
>>  virt/kvm/arm/vgic.c    | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>  4 files changed, 137 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index 3ee732a..7a44b08 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -24,6 +24,7 @@
>>  #include <linux/irqreturn.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/types.h>
>> +#include <linux/acpi.h>
>>  #include <kvm/iodev.h>
>>  
>>  #define VGIC_NR_IRQS_LEGACY	256
>> @@ -335,10 +336,18 @@ int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
>>  int vgic_v2_dt_probe(struct device_node *vgic_node,
>>  		     const struct vgic_ops **ops,
>>  		     const struct vgic_params **params);
>> +#ifdef CONFIG_ACPI
>> +int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *,
>> +		       const struct vgic_ops **ops,
>> +		       const struct vgic_params **params);
>> +#endif /* CONFIG_ACPI */
>>  #ifdef CONFIG_ARM_GIC_V3
>>  int vgic_v3_dt_probe(struct device_node *vgic_node,
>>  		     const struct vgic_ops **ops,
>>  		     const struct vgic_params **params);
>> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
>> +		       const struct vgic_ops **ops,
>> +		       const struct vgic_params **params);
>>  #else
>>  static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
>>  				   const struct vgic_ops **ops,
>> @@ -346,6 +355,15 @@ static inline int vgic_v3_dt_probe(struct device_node *vgic_node,
>>  {
>>  	return -ENODEV;
>>  }
>> +
>> +#ifdef CONFIG_ACPI
>> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *,
>> +		       const struct vgic_ops **ops,
>> +		       const struct vgic_params **params)
>> +{
>> +	return -ENODEV;
>> +}
>> +#endif /* CONFIG_ACPI */
>>  #endif
>>  
>>  #endif
>> diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
>> index 295996f..711de82 100644
>> --- a/virt/kvm/arm/vgic-v2.c
>> +++ b/virt/kvm/arm/vgic-v2.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>> +#include <linux/acpi.h>
>>  
>>  #include <linux/irqchip/arm-gic.h>
>>  
>> @@ -257,3 +258,12 @@ out:
>>  	of_node_put(vgic_node);
>>  	return ret;
>>  }
>> +
>> +#ifdef CONFIG_ACPI
>> +int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
>> +		       const struct vgic_ops **ops,
>> +		       const struct vgic_params **params)
>> +{
>> +	return -EINVAL;
>> +}
>> +#endif /* CONFIG_ACPI */
>> diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
>> index 91814e2..99d0f9f 100644
>> --- a/virt/kvm/arm/vgic-v3.c
>> +++ b/virt/kvm/arm/vgic-v3.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>> +#include <linux/acpi.h>
>>  
>>  #include <linux/irqchip/arm-gic-v3.h>
>>  
>> @@ -285,3 +286,12 @@ out:
>>  	of_node_put(vgic_node);
>>  	return ret;
>>  }
>> +
>> +#ifdef CONFIG_ACPI
>> +int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
>> +		       const struct vgic_ops **ops,
>> +		       const struct vgic_params **params)
>> +{
>> +	return -EINVAL;
>> +}
>> +#endif /* CONFIG_ACPI */
>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>> index b4010f0..cd09877 100644
>> --- a/virt/kvm/arm/vgic.c
>> +++ b/virt/kvm/arm/vgic.c
>> @@ -28,6 +28,7 @@
>>  #include <linux/acpi.h>
>>  
>>  #include <linux/irqchip/arm-gic.h>
>> +#include <linux/irqchip/arm-gic-v3.h>
>>  
>>  #include <asm/kvm_emulate.h>
>>  #include <asm/kvm_arm.h>
>> @@ -2114,9 +2115,106 @@ static int kvm_vgic_dt_probe(void)
>>  }
>>  
>>  #ifdef CONFIG_ACPI
>> +u8 gic_version = ACPI_MADT_GIC_VER_UNKNOWN;
>> +phys_addr_t dist_phy_base;
>> +static struct acpi_madt_generic_interrupt *vgic_acpi;
>> +
>> +static void gic_get_acpi_header(struct acpi_subtable_header *header)
>> +{
>> +	vgic_acpi = (struct acpi_madt_generic_interrupt *)header;
>> +}
>> +
>> +static int gic_parse_distributor(struct acpi_subtable_header *header,
>> +				 const unsigned long end)
>> +{
>> +	struct acpi_madt_generic_distributor *dist;
>> +
>> +	dist = (struct acpi_madt_generic_distributor *)header;
>> +
>> +	if (BAD_MADT_ENTRY(dist, end))
>> +		return -EINVAL;
>> +
>> +	gic_version = dist->gic_version;
>> +	dist_phy_base = dist->base_address;
>> +
>> +	return 0;
>> +}
>> +
>> +static int gic_match_redist(struct acpi_subtable_header *header,
>> +			    const unsigned long end)
>> +{
>> +	return 0;
>> +}
>> +
>> +static bool gic_redist_is_present(void)
>> +{
>> +	int count;
>> +
>> +	/* scan MADT table to find if we have redistributor entries */
>> +	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
>> +				      gic_match_redist, 0);
>> +
>> +	return (count > 0) ? true : false;
>> +}
>> +
>>  static int kvm_vgic_acpi_probe(void)
>>  {
>> -	return -EINVAL;
>> +	u32 reg;
>> +	int count;
>> +	void __iomem *dist_base;
>> +	int ret;
>> +
>> +	/* MADT table */
>> +	ret = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
>> +			(acpi_tbl_entry_handler)gic_get_acpi_header, 0);
>> +	if (!ret) {
>> +		pr_err("Failed to get MADT VGIC CPU entry\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	/* detect GIC version */
>> +	count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
>> +				      gic_parse_distributor, 0);
>> +	if (count <= 0) {
>> +		pr_err("No valid GIC distributor entry exists\n");
>> +		return -ENODEV;
>> +	}
>> +	if (gic_version >= ACPI_MADT_GIC_VER_RESERVED) {
>> +		pr_err("Invalid GIC version %d in MADT\n", gic_version);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* falls back to manual hardware discovery under ACPI 5.1 */
>> +	if (gic_version == ACPI_MADT_GIC_VER_UNKNOWN) {
>> +		if (gic_redist_is_present()) {
>> +			dist_base = ioremap(dist_phy_base, SZ_64K);
>> +			if (!dist_base)
>> +				return -ENOMEM;
>> +
>> +			reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
>> +			if (reg == GIC_PIDR2_ARCH_GICv3)
>> +				gic_version = ACPI_MADT_GIC_VER_V3;
>> +			else
>> +				gic_version = ACPI_MADT_GIC_VER_V4;
>> +
>> +			iounmap(dist_base);
>> +		} else {
>> +			gic_version = ACPI_MADT_GIC_VER_V2;
>> +		}
>> +	}
>> +
>> +	switch (gic_version) {
>> +	case ACPI_MADT_GIC_VER_V2:
>> +		ret = vgic_v2_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
>> +		break;
>> +	case ACPI_MADT_GIC_VER_V3:
>> +		ret = vgic_v3_acpi_probe(vgic_acpi, &vgic_ops, &vgic);
>> +		break;
>> +	default:
>> +		ret = -ENODEV;
>> +	}
>> +
>> +	return ret;
>>  }
>>  #endif /* CONFIG_ACPI */
>>  
>>
> 
> 

Re: [PATCH V2 3/5] kvm: arm64: Detect GIC version for proper ACPI vGIC probing

[Marc Zyngier]

Hi Wei,

On 11/06/15 05:51, Wei Huang wrote:
> 
> 
> On 06/10/2015 11:43 AM, Marc Zyngier wrote:
>> On 10/06/15 05:16, Wei Huang wrote:
>>> There are two GICs (GICv2 and GICv3) supported by KVM. So it is necessary
>>> to find out GIC version before calling ACPI probing functions defined
>>> in vgic-v2.c and vgic-v3.c.
>>>
>>> This patch detects GIC version by checking gic_version field of GIC
>>> distributor, which was defined  since ACPI 6.0. In case of ACPI 5.1,
>>> we use manual hardware discovery to find out GIC version.
>>>
>>> NOTE: This patch is based on a recent patch by Hanjun Guo.
>>
>> Well, this is really a duplicate of Hanjun's patch, and I'd rather have
>> some common infrastructure.
>>
>> Surely it should be possible for the ACPI GIC code to export the
>> necessary entry points, or even to provide the required information?
>>
> 
> I agreed that this is a duplication and should be avoided if possible.
> One easy solution is to export from GIC driver to KVM (using
> EXPORT_SYMBOL or something similar). Is this acceptable?

I don't think EXPORT_SYMBOL is required, as we currently don't support
having KVM as a module.

Simply making available a global structure containing the base addresses
and interrupt should be enough, and could be shared with both DT and
ACPI. You could start your series by letting both GIC drivers expose
that information obtained through DT, convert KVM to use this structure,
and later on let ACPI fill in this structure too.

> Anyway the difficulty is to find a common place to store and share info
> between other modules & KVM.

Indeed. As a rule of thumb, I want to minimize the amount of gratuitous
divergence between DT and ACPI. So the sooner we extract the required
information from whatever firmware we have, the better.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH V2 4/5] kvm: arm64: Implement ACPI probing code for GICv2

[Wei Huang]

This patches enables ACPI support for KVM virtual GICv2. KVM parses
ACPI table for virt GIC related information and initializes resources.

Signed-off-by: Alexander Spyridaki <a.spyridakis@virtualopensystems.com>
Signed-off-by: Wei Huang <wei@redhat.com>
---
 virt/kvm/arm/vgic-v2.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
index 711de82..f6e56e9 100644
--- a/virt/kvm/arm/vgic-v2.c
+++ b/virt/kvm/arm/vgic-v2.c
@@ -264,6 +264,54 @@ int vgic_v2_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
 		       const struct vgic_ops **ops,
 		       const struct vgic_params **params)
 {
-	return -EINVAL;
+	struct vgic_params *vgic = &vgic_v2_params;
+	int irq_mode, ret;
+
+	/* IRQ trigger mode */
+	irq_mode = (vgic_acpi->flags & ACPI_MADT_VGIC_IRQ_MODE) ?
+		ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;
+	vgic->maint_irq = acpi_register_gsi(NULL, vgic_acpi->vgic_interrupt,
+					    irq_mode, ACPI_ACTIVE_HIGH);
+	if (!vgic->maint_irq) {
+		kvm_err("Cannot register VGIC ACPI maintenance irq\n");
+		ret = -ENXIO;
+		goto out;
+	}
+
+	/* GICH resource */
+	vgic->vctrl_base = ioremap(vgic_acpi->gich_base_address, SZ_8K);
+	if (!vgic->vctrl_base) {
+		kvm_err("cannot ioremap GICH memory\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	vgic->nr_lr = readl_relaxed(vgic->vctrl_base + GICH_VTR);
+	vgic->nr_lr = (vgic->nr_lr & 0x3f) + 1;
+
+	ret = create_hyp_io_mappings(vgic->vctrl_base,
+				     vgic->vctrl_base + SZ_8K,
+				     vgic_acpi->gich_base_address);
+	if (ret) {
+		kvm_err("Cannot map GICH into hyp\n");
+		goto out;
+	}
+
+	vgic->vcpu_base = vgic_acpi->gicv_base_address;
+	vgic->can_emulate_gicv2 = true;
+	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
+
+	kvm_info("GICH base=0x%llx, GICV base=0x%llx, IRQ=%d\n",
+		 (unsigned long long)vgic_acpi->gich_base_address,
+		 (unsigned long long)vgic_acpi->gicv_base_address,
+		 vgic->maint_irq);
+
+	vgic->type = VGIC_V2;
+	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
+	*ops = &vgic_v2_ops;
+	*params = vgic;
+
+out:
+	return ret;
 }
 #endif /* CONFIG_ACPI */
-- 
1.8.3.1

[PATCH V2 5/5] kvm: arm64: Implement ACPI probing code for GICv3

[Wei Huang]

This patches enables ACPI support for KVM virtual GICv3. KVM parses
ACPI table for virt GIC related information and initializes resources.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 virt/kvm/arm/vgic-v3.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
index 99d0f9f..2e4df78 100644
--- a/virt/kvm/arm/vgic-v3.c
+++ b/virt/kvm/arm/vgic-v3.c
@@ -292,6 +292,44 @@ int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
 		       const struct vgic_ops **ops,
 		       const struct vgic_params **params)
 {
-	return -EINVAL;
+	int ret = 0;
+	struct vgic_params *vgic = &vgic_v3_params;
+	int irq_mode;
+
+	/* IRQ trigger mode */
+	irq_mode = (vgic_acpi->flags & ACPI_MADT_VGIC_IRQ_MODE) ?
+		ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;
+	vgic->maint_irq = acpi_register_gsi(NULL, vgic_acpi->vgic_interrupt,
+					    irq_mode, ACPI_ACTIVE_HIGH);
+	if (!vgic->maint_irq) {
+		kvm_err("Cannot register VGIC ACPI maintenance irq\n");
+		ret = -ENXIO;
+		goto out;
+	}
+
+	ich_vtr_el2 = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
+	vgic->nr_lr = (ich_vtr_el2 & 0xf) + 1;
+	vgic->can_emulate_gicv2 = false;
+
+	vgic->vcpu_base = vgic_acpi->gicv_base_address;
+
+	if (vgic->vcpu_base == 0)
+		kvm_info("disabling GICv2 emulation\n");
+	else {
+		vgic->can_emulate_gicv2 = true;
+		kvm_register_device_ops(&kvm_arm_vgic_v2_ops,
+					KVM_DEV_TYPE_ARM_VGIC_V2);
+	}
+
+	kvm_register_device_ops(&kvm_arm_vgic_v3_ops, KVM_DEV_TYPE_ARM_VGIC_V3);
+
+	vgic->vctrl_base = NULL;
+	vgic->type = VGIC_V3;
+	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
+
+	*ops = &vgic_v3_ops;
+	*params = vgic;
+out:
+	return ret;
 }
 #endif /* CONFIG_ACPI */
-- 
1.8.3.1

Re: [PATCH V2 5/5] kvm: arm64: Implement ACPI probing code for GICv3

[Andrew Jones]

On Wed, Jun 10, 2015 at 12:16:07AM -0400, Wei Huang wrote:
> This patches enables ACPI support for KVM virtual GICv3. KVM parses
> ACPI table for virt GIC related information and initializes resources.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  virt/kvm/arm/vgic-v3.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
> index 99d0f9f..2e4df78 100644
> --- a/virt/kvm/arm/vgic-v3.c
> +++ b/virt/kvm/arm/vgic-v3.c
> @@ -292,6 +292,44 @@ int vgic_v3_acpi_probe(struct acpi_madt_generic_interrupt *vgic_acpi,
>  		       const struct vgic_ops **ops,
>  		       const struct vgic_params **params)
>  {
> -	return -EINVAL;
> +	int ret = 0;
> +	struct vgic_params *vgic = &vgic_v3_params;
> +	int irq_mode;
> +
> +	/* IRQ trigger mode */
> +	irq_mode = (vgic_acpi->flags & ACPI_MADT_VGIC_IRQ_MODE) ?
> +		ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;
> +	vgic->maint_irq = acpi_register_gsi(NULL, vgic_acpi->vgic_interrupt,
> +					    irq_mode, ACPI_ACTIVE_HIGH);
> +	if (!vgic->maint_irq) {
> +		kvm_err("Cannot register VGIC ACPI maintenance irq\n");
> +		ret = -ENXIO;
> +		goto out;

nit:
This is the other return besides the end of the function. Why
not drop the 'ret' variable, do return -ENXIO here and return 0
at the end.

> +	}
> +
> +	ich_vtr_el2 = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
> +	vgic->nr_lr = (ich_vtr_el2 & 0xf) + 1;
> +	vgic->can_emulate_gicv2 = false;
> +
> +	vgic->vcpu_base = vgic_acpi->gicv_base_address;
> +
> +	if (vgic->vcpu_base == 0)
> +		kvm_info("disabling GICv2 emulation\n");
> +	else {
> +		vgic->can_emulate_gicv2 = true;
> +		kvm_register_device_ops(&kvm_arm_vgic_v2_ops,
> +					KVM_DEV_TYPE_ARM_VGIC_V2);
> +	}
> +
> +	kvm_register_device_ops(&kvm_arm_vgic_v3_ops, KVM_DEV_TYPE_ARM_VGIC_V3);
> +
> +	vgic->vctrl_base = NULL;
> +	vgic->type = VGIC_V3;
> +	vgic->max_gic_vcpus = KVM_MAX_VCPUS;

Missing the kvm_info() here that is in the DT version and the v2
versions.

> +
> +	*ops = &vgic_v3_ops;
> +	*params = vgic;
> +out:
> +	return ret;
>  }
>  #endif /* CONFIG_ACPI */
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/5] Enable ACPI support for KVM ARM

[Andrew Jones]

On Wed, Jun 10, 2015 at 12:16:02AM -0400, Wei Huang wrote:
> Initial ACPI support for ARM64 has been accepted into Linux kernel recently.
> Now it is a good time to re-visit ACPI support for KVM. This patchset
> enables ACPI for both arch_timer and vGIC by probing related ACPI tables
> and does necessary initialization.
> 
> Note that Alexander Spyridaki submitted similar patches before. Some of
> his ideas were borrowed in this patchset, but with substancial changes.
> In addition we extend support for both GICv2 and GICv3.
> 
> This patchset would work better on top of recent GIC/IRQCHIP patches by
> Hanjun Guo, who added support for gic_version in ACPI struct of GIC
> distributor (search "ACPICA: Introduce GIC version for arm based system").
> 
> This patchset can be applied cleanly on top of kvmarm.git queue branch.
> 
> -Wei
> 
> V1->V2:
> * Initialize vgic->max_gic_vcpus field in GICv2 code (found by Andrew Jones)
> * Rewrite arch_timer ACPI PPI parsing code
> * Verified patches on APM Mustang machine, which has arch_timer & GICv2
> 
> V1:
> * Initial patchset
> 
> Wei Huang (5):
>   kvm: arm64: Enable ACPI support for virt arch timer
>   kvm: arm64: Dispatch virt GIC probing to device tree and ACPI
>   kvm: arm64: Detect GIC version for proper ACPI vGIC probing
>   kvm: arm64: Implement ACPI probing code for GICv2
>   kvm: arm64: Implement ACPI probing code for GICv3
> 
>  include/kvm/arm_vgic.h    |  36 +++++++++---
>  virt/kvm/arm/arch_timer.c |  75 ++++++++++++++++++++-----
>  virt/kvm/arm/vgic-v2.c    |  66 ++++++++++++++++++++--
>  virt/kvm/arm/vgic-v3.c    |  56 +++++++++++++++++--
>  virt/kvm/arm/vgic.c       | 140 ++++++++++++++++++++++++++++++++++++++++++----
>  5 files changed, 332 insertions(+), 41 deletions(-)

I've looked over the series and made a couple nits. I think we need to
wait until "ACPICA: Introduce GIC version for arm based system" is
accepted, as it may still change, and this series duplicates parts of it.

Thanks,
drew

Re: [PATCH V2 0/5] Enable ACPI support for KVM ARM

[Hanjun Guo]

On 06/10/2015 09:23 PM, Andrew Jones wrote:
> On Wed, Jun 10, 2015 at 12:16:02AM -0400, Wei Huang wrote:
>> Initial ACPI support for ARM64 has been accepted into Linux kernel recently.
>> Now it is a good time to re-visit ACPI support for KVM. This patchset
>> enables ACPI for both arch_timer and vGIC by probing related ACPI tables
>> and does necessary initialization.
>>
>> Note that Alexander Spyridaki submitted similar patches before. Some of
>> his ideas were borrowed in this patchset, but with substancial changes.
>> In addition we extend support for both GICv2 and GICv3.
>>
>> This patchset would work better on top of recent GIC/IRQCHIP patches by
>> Hanjun Guo, who added support for gic_version in ACPI struct of GIC
>> distributor (search "ACPICA: Introduce GIC version for arm based system").
>>
>> This patchset can be applied cleanly on top of kvmarm.git queue branch.
>>
>> -Wei
>>
>> V1->V2:
>> * Initialize vgic->max_gic_vcpus field in GICv2 code (found by Andrew Jones)
>> * Rewrite arch_timer ACPI PPI parsing code
>> * Verified patches on APM Mustang machine, which has arch_timer & GICv2
>>
>> V1:
>> * Initial patchset
>>
>> Wei Huang (5):
>>    kvm: arm64: Enable ACPI support for virt arch timer
>>    kvm: arm64: Dispatch virt GIC probing to device tree and ACPI
>>    kvm: arm64: Detect GIC version for proper ACPI vGIC probing
>>    kvm: arm64: Implement ACPI probing code for GICv2
>>    kvm: arm64: Implement ACPI probing code for GICv3
>>
>>   include/kvm/arm_vgic.h    |  36 +++++++++---
>>   virt/kvm/arm/arch_timer.c |  75 ++++++++++++++++++++-----
>>   virt/kvm/arm/vgic-v2.c    |  66 ++++++++++++++++++++--
>>   virt/kvm/arm/vgic-v3.c    |  56 +++++++++++++++++--
>>   virt/kvm/arm/vgic.c       | 140 ++++++++++++++++++++++++++++++++++++++++++----
>>   5 files changed, 332 insertions(+), 41 deletions(-)
>
> I've looked over the series and made a couple nits. I think we need to
> wait until "ACPICA: Introduce GIC version for arm based system" is
> accepted, as it may still change, and this series duplicates parts of it.

Yes, it was changed. Part of them is accepted by ACPICA core, but the
new release of ACPICA 20150515 missed some code below,

  include/acpi/actbl1.h | 11 +++++++++++
  1 file changed, 11 insertions(+)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index cadf21c..d382de8 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -834,6 +834,17 @@ struct acpi_madt_generic_distributor {
         u8 reserved2[3];        /* reserved - must be zero */
  };

+/* Values for Version field above */
+
+enum acpi_madt_gic_version {
+       ACPI_MADT_GIC_VERSION_NONE      = 0,
+       ACPI_MADT_GIC_VERSION_V1        = 1,
+       ACPI_MADT_GIC_VERSION_V2        = 2,
+       ACPI_MADT_GIC_VERSION_V3        = 3,
+       ACPI_MADT_GIC_VERSION_V4        = 4,
+       ACPI_MADT_GIC_VERSION_RESERVED  = 5     /* 5 and greater are 
reserved */
+};
+

it was accepted by ACPICA tools but not ACPICA for linux, I already
send them to Lv and Rafael, hope it can be merged for 4.2.

Thanks
Hanjun

Re: [PATCH V2 0/5] Enable ACPI support for KVM ARM

[Graeme Gregory]

Wei Huang <wei <at> redhat.com> writes:

> 
> Initial ACPI support for ARM64 has been accepted into Linux kernel 
recently.
> Now it is a good time to re-visit ACPI support for KVM. This patchset
> enables ACPI for both arch_timer and vGIC by probing related ACPI 
tables
> and does necessary initialization.
> 
> Note that Alexander Spyridaki submitted similar patches before. Some 
of
> his ideas were borrowed in this patchset, but with substancial 
changes.
> In addition we extend support for both GICv2 and GICv3.
> 
> This patchset would work better on top of recent GIC/IRQCHIP patches 
by
> Hanjun Guo, who added support for gic_version in ACPI struct of GIC
> distributor (search "ACPICA: Introduce GIC version for arm based 
system").
> 
> This patchset can be applied cleanly on top of kvmarm.git queue 
branch.
> 

Is there a V3 of this patchset in the works? I think GIC stuff has 
calmed down a bit since you did this V2.

Thanks

Graeme