| Message ID | 1444142228-6696-2-git-send-email-drjones@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 6 October 2015 at 15:37, Andrew Jones <drjones@redhat.com> wrote: > I'm not sure if arm_gic_common.h is supposed to be common, not > only between tcg and kvm, but also v2 and v3, but it currently > is (arm_gicv3_common.h includes it, and it's the only gic header > included by hw/arm/virt.c). I don't think it should be included. We're doing completely separate implementations of the GICv3 from the old GICv1/2 code. It looks like the only thing in the gicv3 code that is using a define from the arm_gic_common.h file is "GIC_INTERNAL", so we can just put a suitable define of that into the v3 header (maybe giving it a better name in the process). > If it should be the super-common > header, then it's unfortunate that the define names are too > generic. This patch doesn't help much, as it doesn't rename > anything, but it does start heading down the right path. With > it, code including the super-common header can start using more > appropriate names for a couple very gic-version-specific defines. All the things in this header should be GICv2 only, really. It's unfortunate that all our naming (including the source files and the device names) uses just "gic", but a mass renaming doesn't seem like a great idea to me. Given that, perhaps having a few symbols with a "GICV2" prefix is going to be more confusing than helpful. thanks -- PMM
Hi! > It looks like the only thing in the gicv3 code that is using > a define from the arm_gic_common.h file is "GIC_INTERNAL", > so we can just put a suitable define of that into the v3 header > (maybe giving it a better name in the process). Yes, indeed. Actually, first versions of my GICv3 patches did use own #define, but i was criticized for using GICV3_INTERNAL in my code and having GIC_INTERNAL in kvm_arm_gic_set_irq(), which is shared by both KVM implementations. So, i decided to use GIC_INTERNAL everywhere and inherited it from v2. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index 564a72b2cf77f..299226064a30f 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -25,11 +25,13 @@ /* Maximum number of possible interrupts, determined by the GIC architecture */ #define GIC_MAXIRQ 1020 +#define GICV2_MAXIRQ GIC_MAXIRQ /* First 32 are private to each CPU (SGIs and PPIs). */ #define GIC_INTERNAL 32 #define GIC_NR_SGIS 16 /* Maximum number of possible CPU interfaces, determined by GIC architecture */ #define GIC_NCPU 8 +#define GICV2_NCPU GIC_NCPU #define MAX_NR_GROUP_PRIO 128 #define GIC_NR_APRS (MAX_NR_GROUP_PRIO / 32)
I'm not sure if arm_gic_common.h is supposed to be common, not only between tcg and kvm, but also v2 and v3, but it currently is (arm_gicv3_common.h includes it, and it's the only gic header included by hw/arm/virt.c). If it should be the super-common header, then it's unfortunate that the define names are too generic. This patch doesn't help much, as it doesn't rename anything, but it does start heading down the right path. With it, code including the super-common header can start using more appropriate names for a couple very gic-version-specific defines. Signed-off-by: Andrew Jones <drjones@redhat.com> --- include/hw/intc/arm_gic_common.h | 2 ++ 1 file changed, 2 insertions(+)