Re: [Qemu-arm] [Qemu-devel] [kvm-unit-tests PATCH v4 06/11] arm/arm64: add initial gicv2 support

[Andrew Jones]

On Wed, Nov 09, 2016 at 11:53:39AM +0000, Andre Przywara wrote:
[...]
> > diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> > new file mode 100644
> > index 000000000000..ec92f1064dc0
> > --- /dev/null
> > +++ b/lib/arm/asm/gic.h
> > @@ -0,0 +1,44 @@
> > +/*
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#ifndef _ASMARM_GIC_H_
> > +#define _ASMARM_GIC_H_
> > +
> > +#include <asm/gic-v2.h>
> > +
> > +#define GIC_CPU_CTRL                       0x00
> > +#define GIC_CPU_PRIMASK                    0x04
> > +
> > +#define GICC_ENABLE                        0x1
> > +#define GICC_INT_PRI_THRESHOLD             0xf0
> > +
> > +#define GIC_DIST_CTRL                      0x000
> > +#define GIC_DIST_CTR                       0x004
> 
> I think we shouldn't copy this old name here, which stems from pre-GICv2
> times (PL390?), IIUC. Both GIC specs talk of TYPER here.
> 
> Also if we now use the same defines for both the GICv2 and GICv3
> distributor, we should really stick with the (modern) spec naming, which
> is GICD_CTRL, GICD_TYPER and so on. Same for the CPU interface (GICC_CTRL).
> In the kernel these names are used for GICv3, but we didn't bother to
> adjust the existing GICv2 names to avoid pointless churn.
> Also that would line up with the bit field defines below.
>

Yeah, I noticed the CTR vs. TYPER naming weirdness. I considered
exclusively defining the gicv3/modern/better name, but wasn't sure
I should deviate from the kernel names. Keeping the names consistent
is nice for grepping kernel sources, which needs to be done when a
test fails and we want to find the bug.

I'd personally prefer "pointless" churn in the kernel to get it
straightened out there. If it ever is, then I'd obviously update
this code too. Until then, I think maintaining easy greppability
is the better choice.

Thanks,
drew