Hi Andre,

On 03/06/2018 06:01 PM, Andre Przywara wrote:
On 06/03/18 17:46, Julien Grall wrote:
On 05/03/18 16:03, Andre Przywara wrote:
Add a new header file for the new and improved GIC implementation.
The big change is that we now have a struct vgic_irq per IRQ instead
of spreading all the information over various bitmaps in the ranks.

We include this new header conditionally from within the old header
file for the time being to avoid touching all the users.

This is based on Linux commit b18b57787f5e, written by Christoffer Dall.

Signed-off-by: Andre Przywara <andre.przyw...@linaro.org>
---
Changelog RFC ... v1:
- rename header file to new_vgic.h
- drop unneeded data structures (vgic_its, vgic_v<x>_cpu_if)
- reorder members in vgic_irq to avoid padding
- move flags members into bool bitfields
- drop prototypes
- use unsigned and uint<x>_t data types
- keep arch_vcpu member name as "vgic"

   xen/include/asm-arm/new_vgic.h | 198
+++++++++++++++++++++++++++++++++++++++++
   xen/include/asm-arm/vgic.h     |   6 ++
   2 files changed, 204 insertions(+)
   create mode 100644 xen/include/asm-arm/new_vgic.h

diff --git a/xen/include/asm-arm/new_vgic.h
b/xen/include/asm-arm/new_vgic.h
new file mode 100644
index 0000000000..54be5aa3eb
--- /dev/null
+++ b/xen/include/asm-arm/new_vgic.h
@@ -0,0 +1,198 @@
+/*
+ * 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/>.
+ */
+#ifndef __ASM_ARM_NEW_VGIC_H
+#define __ASM_ARM_NEW_VGIC_H
+
+#include <asm/atomic.h>
+#include <asm/mmio.h>
+#include <xen/list.h>
+#include <xen/mm.h>
+#include <xen/spinlock.h>
+
+#define VGIC_V3_MAX_CPUS        255
+#define VGIC_V2_MAX_CPUS        8
+#define VGIC_NR_SGIS            16
+#define VGIC_NR_PPIS            16
+#define VGIC_NR_PRIVATE_IRQS    (VGIC_NR_SGIS + VGIC_NR_PPIS)
+#define VGIC_MAX_PRIVATE        (VGIC_NR_PRIVATE_IRQS - 1)
+#define VGIC_MAX_SPI            1019
+#define VGIC_MAX_RESERVED       1023
+#define VGIC_MIN_LPI            8192
+
+#define irq_is_ppi(irq) ((irq) >= VGIC_NR_SGIS && (irq) <
VGIC_NR_PRIVATE_IRQS)
+#define irq_is_spi(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \
+                         (irq) <= VGIC_MAX_SPI)
+
+enum vgic_type {
+    VGIC_V2,        /* Good ol' GICv2 */
+    VGIC_V3,        /* New fancy GICv3 */
+};
+
+#define VGIC_V2_MAX_LRS         (1 << 6)
+#define VGIC_V3_MAX_LRS         16
+#define VGIC_V3_LR_INDEX(lr)    (VGIC_V3_MAX_LRS - 1 - lr)
+
+enum vgic_irq_config {
+    VGIC_CONFIG_EDGE = 0,

Again, I don't think it is necessary to set to 0 here as IIRC an enum
always start at 0 if not specified.

But I consider this more readable, especially since this is a bool now.
So it's immediately obvious that EDGE is 0.

Fair enough, I probably know to much the C spec :).

Shall I replace this with:
#define VGIC_CONFIG_EDGE false
#define VGIC_CONFIG_LEVEL true
now? Or using 0 and 1?

The enum vgic_irq_config type is now not used anywhere.

I don't mind to keep the enum with a comment above. If you switch to define, then I would prefer to use false/true.

Cheers,

--
Julien Grall

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

Reply via email to