[Xen-devel,RFC,44/49] ARM: new VGIC: vgic-init: register VGIC

Message ID 20180209143937.28866-45-andre.przywara@linaro.org
State New
Headers show
Series
  • New VGIC(-v2) implementation
Related show

Commit Message

Andre Przywara Feb. 9, 2018, 2:39 p.m.
This patch implements the function which is called by Xen when it wants
to register the virtual GIC.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 xen/arch/arm/vgic/vgic-init.c | 62 +++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/vgic/vgic.h      |  3 +++
 2 files changed, 65 insertions(+)
 create mode 100644 xen/arch/arm/vgic/vgic-init.c

Comments

Julien Grall Feb. 19, 2018, 12:39 p.m. | #1
Hi Andre,

On 09/02/18 14:39, Andre Przywara wrote:
> This patch implements the function which is called by Xen when it wants
> to register the virtual GIC.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>   xen/arch/arm/vgic/vgic-init.c | 62 +++++++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/vgic/vgic.h      |  3 +++
>   2 files changed, 65 insertions(+)
>   create mode 100644 xen/arch/arm/vgic/vgic-init.c
> 
> diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
> new file mode 100644
> index 0000000000..b5f1183a50
> --- /dev/null
> +++ b/xen/arch/arm/vgic/vgic-init.c
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (C) 2015, 2016 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/sched.h>
> +#include <asm/arm_vgic.h>
> +
> +#include "vgic.h"
> +
> +/* CREATION */
> +
> +/**
> + * domain_vgic_register: create a virtual GIC
> + * @d: domain pointer
> + * @mmio_count: pointer to add number of required MMIO regions
> + *
> + * was: kvm_vgic_create
> + */
> +int domain_vgic_register(struct domain *d, int *mmio_count)

mmio_count should be set to the number of I/O region you will register.

> +{
> +    switch ( d->arch.vgic.version )
> +    {
> +#ifdef CONFIG_HAS_GICV3
> +    case GIC_V3:
> +        d->arch.max_vcpus = VGIC_V3_MAX_CPUS;
> +        break;
> +#endif
> +    case GIC_V2:
> +        d->arch.max_vcpus = VGIC_V2_MAX_CPUS;
> +        break;
> +    }
> +
> +    if ( d->max_vcpus > d->arch.max_vcpus )
> +        return -E2BIG;
> +
> +    d->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
> +    d->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
> +    d->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;

Is there any reason to store an address rather than a frame? The latter 
would add a be more safety.

> +
> +    return 0;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
> index b104f8e964..205ce10ffa 100644
> --- a/xen/arch/arm/vgic/vgic.h
> +++ b/xen/arch/arm/vgic/vgic.h
> @@ -20,6 +20,9 @@
>   #define PRODUCT_ID_KVM      0x4b    /* ASCII code K */
>   #define IMPLEMENTER_ARM     0x43b
>   
> +#define VGIC_ADDR_UNDEF     (-1)

Please use INVALID_PADDR here.

> +#define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
> +
>   #define VGIC_PRI_BITS       5
>   
>   #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
> 

Cheers,
Andre Przywara Feb. 26, 2018, 5:33 p.m. | #2
Hi,

On 19/02/18 12:39, Julien Grall wrote:
> Hi Andre,
> 
> On 09/02/18 14:39, Andre Przywara wrote:
>> This patch implements the function which is called by Xen when it wants
>> to register the virtual GIC.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/arch/arm/vgic/vgic-init.c | 62
>> +++++++++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/vgic/vgic.h      |  3 +++
>>   2 files changed, 65 insertions(+)
>>   create mode 100644 xen/arch/arm/vgic/vgic-init.c
>>
>> diff --git a/xen/arch/arm/vgic/vgic-init.c
>> b/xen/arch/arm/vgic/vgic-init.c
>> new file mode 100644
>> index 0000000000..b5f1183a50
>> --- /dev/null
>> +++ b/xen/arch/arm/vgic/vgic-init.c
>> @@ -0,0 +1,62 @@
>> +/*
>> + * Copyright (C) 2015, 2016 ARM Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <xen/sched.h>
>> +#include <asm/arm_vgic.h>
>> +
>> +#include "vgic.h"
>> +
>> +/* CREATION */
>> +
>> +/**
>> + * domain_vgic_register: create a virtual GIC
>> + * @d: domain pointer
>> + * @mmio_count: pointer to add number of required MMIO regions
>> + *
>> + * was: kvm_vgic_create
>> + */
>> +int domain_vgic_register(struct domain *d, int *mmio_count)
> 
> mmio_count should be set to the number of I/O region you will register.
> 
>> +{
>> +    switch ( d->arch.vgic.version )
>> +    {
>> +#ifdef CONFIG_HAS_GICV3
>> +    case GIC_V3:
>> +        d->arch.max_vcpus = VGIC_V3_MAX_CPUS;
>> +        break;
>> +#endif
>> +    case GIC_V2:
>> +        d->arch.max_vcpus = VGIC_V2_MAX_CPUS;
>> +        break;
>> +    }
>> +
>> +    if ( d->max_vcpus > d->arch.max_vcpus )
>> +        return -E2BIG;
>> +
>> +    d->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
>> +    d->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
>> +    d->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
> 
> Is there any reason to store an address rather than a frame? The latter
> would add a be more safety.

Possibly, but the existing VGIC doesn't do it as well, which means that
at the moment we deal with addresses (and not frame numbers) everywhere
(for instance GUEST_GICD_BASE).
Changing this for the distributor internally already (as requested for
some other patch) caused more changes than I liked, and at the moment I
am trying to make the TODO list smaller, not bigger ;-)

Cheers,
Andre.

>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
>> index b104f8e964..205ce10ffa 100644
>> --- a/xen/arch/arm/vgic/vgic.h
>> +++ b/xen/arch/arm/vgic/vgic.h
>> @@ -20,6 +20,9 @@
>>   #define PRODUCT_ID_KVM      0x4b    /* ASCII code K */
>>   #define IMPLEMENTER_ARM     0x43b
>>   +#define VGIC_ADDR_UNDEF     (-1)
> 
> Please use INVALID_PADDR here.
> 
>> +#define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
>> +
>>   #define VGIC_PRI_BITS       5
>>     #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
>>
> 
> Cheers,
>

Patch

diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
new file mode 100644
index 0000000000..b5f1183a50
--- /dev/null
+++ b/xen/arch/arm/vgic/vgic-init.c
@@ -0,0 +1,62 @@ 
+/*
+ * Copyright (C) 2015, 2016 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/sched.h>
+#include <asm/arm_vgic.h>
+
+#include "vgic.h"
+
+/* CREATION */
+
+/**
+ * domain_vgic_register: create a virtual GIC
+ * @d: domain pointer
+ * @mmio_count: pointer to add number of required MMIO regions
+ *
+ * was: kvm_vgic_create
+ */
+int domain_vgic_register(struct domain *d, int *mmio_count)
+{
+    switch ( d->arch.vgic.version )
+    {
+#ifdef CONFIG_HAS_GICV3
+    case GIC_V3:
+        d->arch.max_vcpus = VGIC_V3_MAX_CPUS;
+        break;
+#endif
+    case GIC_V2:
+        d->arch.max_vcpus = VGIC_V2_MAX_CPUS;
+        break;
+    }
+
+    if ( d->max_vcpus > d->arch.max_vcpus )
+        return -E2BIG;
+
+    d->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
+    d->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
+    d->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
index b104f8e964..205ce10ffa 100644
--- a/xen/arch/arm/vgic/vgic.h
+++ b/xen/arch/arm/vgic/vgic.h
@@ -20,6 +20,9 @@ 
 #define PRODUCT_ID_KVM      0x4b    /* ASCII code K */
 #define IMPLEMENTER_ARM     0x43b
 
+#define VGIC_ADDR_UNDEF     (-1)
+#define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
+
 #define VGIC_PRI_BITS       5
 
 #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)