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

2018-02-26 Thread Andre Przywara
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 
>> ---
>>   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 00..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 .
>> + */
>> +
>> +#include 
>> +#include 
>> +
>> +#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,
> 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2018-02-19 Thread Julien Grall

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 
---
  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 00..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 .
+ */
+
+#include 
+#include 
+
+#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,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel