ARM: DT: Add binding for GIC virtualization extentions (VGIC)
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
--- Documentation/devicetree/bindings/arm/gic.txt | 29 +++++++++++++++++++++++++ 1 files changed, 29 insertions(+), 0 deletions(-)
Documentation/devicetree/bindings/arm/gic.txt +29 -0
@@ -57,3 +57,32 @@ Example: <0xfff10100 0x100>; }; + +* GIC virtualization extensions (VGIC) + +For ARM cores that support the virtualization extensions, an +additional node describes the VGIC hardware block. Only exists if the +GIC is the primary interrupt controller. + +Required properties: +- compatible : should be one of: + "arm,cortex-a15-vgic" + "arm,cortex-a7-vgic" + "arm,vgic" + +- reg : Specifies base physical address and size of the VGIC + registers. The first region is the GIC virtual interface control + register base and size. the 2nd region is the GIC virtual cpu + interface register base and size. + +- interrupts : VGIC maintainance interrupt. Must be relative to the + primary GIC. + +Example: + + vgic at 2c004000 { + compatible = "arm,cortex-a15-vgic", "arm,vgic"; + reg = <0x2c004000 0x2000>, + <0x2c006000 0x2000>; + interrupts = <1 9 0xf04>; + };
Signature · 1 line
1.7.7.1
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 3 lines · Marc Zyngier
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
David
Quoted 43 lines
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/devicetree/bindings/arm/gic.txt | 29 +++++++++++++++++++++++++ 1 files changed, 29 insertions(+), 0 deletions(-)Documentation/devicetree/bindings/arm/gic.txt +29
-0@@ -57,3 +57,32 @@ Example: <0xfff10100 0x100>; }; + +* GIC virtualization extensions (VGIC) + +For ARM cores that support the virtualization extensions, an +additional node describes the VGIC hardware block. Only exists if the +GIC is the primary interrupt controller. + +Required properties: +- compatible : should be one of: + "arm,cortex-a15-vgic" + "arm,cortex-a7-vgic" + "arm,vgic" + +- reg : Specifies base physical address and size of the VGIC + registers. The first region is the GIC virtual interface control + register base and size. the 2nd region is the GIC virtual cpu + interface register base and size. + +- interrupts : VGIC maintainance interrupt. Must be relative to the + primary GIC. + +Example: + + vgic at 2c004000 { + compatible = "arm,cortex-a15-vgic", "arm,vgic"; + reg = <0x2c004000 0x2000>, + <0x2c006000 0x2000>; + interrupts = <1 9 0xf04>; + };
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
On 03/04/12 10:22, David Vrabel wrote:
Hi David,
Quoted 10 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
It also makes it easier to dynamically filter out the VGIC node if you want to derive the host DT for the guest (assuming you're emulating the same platform).
But again, I have no strong feeling about it.
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 12 lines · Marc Zyngier
On 03/04/12 10:22, David Vrabel wrote:
Hi David,
Quoted 8 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Quoted 8 lines
Quoted 2 lines
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
Don't get too hung up on the software side of things. Describe it in a way that makes sense for the hardware. There is lots of precidence for two hunks of software initializating from the same node; either by probe kicking off two init hooks, or by early init code going looking for the node manually.
g.
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
On 03/04/12 16:35, Grant Likely wrote:
Hi Grant,
Quoted 22 lines · Marc Zyngier
Quoted 12 lines · David Vrabel
Hi David,
Quoted 8 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Quoted 8 lines
Quoted 2 lines
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
Quoted 5 lines
Don't get too hung up on the software side of things. Describe it in a way that makes sense for the hardware. There is lots of precidence for two hunks of software initializating from the same node; either by probe kicking off two init hooks, or by early init code going looking for the node manually.
What I'm trying to avoid is a royal mess in the future if we get some other extension to the GIC.
Let's say we implement the following:
gic: interrupt-controller at 2c001000 {
compatible = "arm,cortex-a15-gic";
#interrupt-cells = <3>;
#address-cells = <1>;
interrupt-controller;
reg = <0x2c001000 0x1000>,
<0x2c002000 0x100>,
<0x2c004000 0x2000>,
<0x2c006000 0x2000>;
interrupts = <1 9 0xf04>;
};It's all fine (the two last regions and the interrupt are for VGIC), until someone comes up with extension FOO which requires two new regions and am interrupt. It is then impossible to distinguish between the two, short of adding more attributes.
How about this?
gic: interrupt-controller at 2c001000 {
compatible = "arm,cortex-a15-gic";
#interrupt-cells = <3>;
#address-cells = <1>;
#size-cells = <1>;
interrupt-controller;
reg = <0x2c001000 0x1000>,
<0x2c002000 0x100>;
vgic at 2c004000 {
compatible = "arm,cortex-a15-vgic", "arm,vgic";
reg = <0x2c004000 0x2000>,
<0x2c006000 0x2000>;
interrupts = <1 9 0xf04>;
};
};It cleanly separate the extension from the core GIC, and still make it part of the GIC node.
What do you think?
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 20 lines · Marc Zyngier
How about this?
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; #size-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>; vgic at 2c004000 { compatible = "arm,cortex-a15-vgic", "arm,vgic"; reg = <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>; }; };
Quoted 2 lines
It cleanly separate the extension from the core GIC, and still make it part of the GIC node.
This looks sensible to me.
David
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
In reply to Marc ZyngierQuoted 26 lines · Marc Zyngier
On 03/04/12 16:35, Grant Likely wrote:
Hi Grant,
Quoted 22 lines · Marc Zyngier
Quoted 20 lines · David Vrabel
Hi David,
Quoted 10 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
Quoted 9 lines
Quoted 5 lines
Don't get too hung up on the software side of things. Describe it in a way that makes sense for the hardware. There is lots of precidence for two hunks of software initializating from the same node; either by probe kicking off two init hooks, or by early init code going looking for the node manually.
What I'm trying to avoid is a royal mess in the future if we get some other extension to the GIC.
But that would be a new compatible string as is this case.
Quoted 12 lines
Let's say we implement the following:
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>, <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>;
Does this work having an interrupt within the parent itself? Normally this would be the connection to the next level up.
Quoted 26 lines
};It's all fine (the two last regions and the interrupt are for VGIC), until someone comes up with extension FOO which requires two new regions and am interrupt. It is then impossible to distinguish between the two, short of adding more attributes.
How about this?
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; #size-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>; vgic at 2c004000 { compatible = "arm,cortex-a15-vgic", "arm,vgic"; reg = <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>; }; };
Quoted 5 lines
It cleanly separate the extension from the core GIC, and still make it part of the GIC node.
What do you think?
I prefer the first option.
Rob
Quoted 1 line
M.
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 33 lines · Rob Herring
Quoted 32 lines · Marc Zyngier
On 03/04/12 16:35, Grant Likely wrote:
Hi Grant,
Quoted 27 lines · Marc Zyngier
Quoted 12 lines · David Vrabel
Hi David,
Quoted 8 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Quoted 8 lines
Quoted 2 lines
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
Don't get too hung up on the software side of things. Describe it in a way that makes sense for the hardware. There is lots of precidence for two hunks of software initializating from the same node; either by probe kicking off two init hooks, or by early init code going looking for the node manually.
Quoted 5 lines
Quoted 3 lines
What I'm trying to avoid is a royal mess in the future if we get some other extension to the GIC.
But that would be a new compatible string as is this case.
Yes, probably.
Quoted 13 lines
Quoted 12 lines
Let's say we implement the following:
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>, <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>;
Quoted 2 lines
Does this work having an interrupt within the parent itself? Normally this would be the connection to the next level up.
In this case we don't have an interrupt parent property, so the GIC is properly identified as the top-level interrupt controller. This is admittedly a bit fragile, and contradicts the current wording of the binding.
Quoted 26 lines
Quoted 26 lines
};It's all fine (the two last regions and the interrupt are for VGIC), until someone comes up with extension FOO which requires two new regions and am interrupt. It is then impossible to distinguish between the two, short of adding more attributes.
How about this?
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; #size-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>; vgic at 2c004000 { compatible = "arm,cortex-a15-vgic", "arm,vgic"; reg = <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>; }; };
Quoted 7 lines
Quoted 5 lines
It cleanly separate the extension from the core GIC, and still make it part of the GIC node.
What do you think?
I prefer the first option.
The first I posted (vgic node outside of the gic), or the one with everything in the same node?
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 55 lines · Marc Zyngier
Quoted 33 lines · Rob Herring
Quoted 32 lines · Marc Zyngier
On 03/04/12 16:35, Grant Likely wrote:
Hi Grant,
Quoted 27 lines · Marc Zyngier
Quoted 12 lines · David Vrabel
Hi David,
Quoted 8 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Quoted 8 lines
Quoted 2 lines
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
Don't get too hung up on the software side of things. Describe it in a way that makes sense for the hardware. There is lots of precidence for two hunks of software initializating from the same node; either by probe kicking off two init hooks, or by early init code going looking for the node manually.
Quoted 5 lines
Quoted 3 lines
What I'm trying to avoid is a royal mess in the future if we get some other extension to the GIC.
But that would be a new compatible string as is this case.
Yes, probably.
Quoted 13 lines
Quoted 12 lines
Let's say we implement the following:
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>, <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>;
Quoted 8 lines
Quoted 2 lines
Does this work having an interrupt within the parent itself? Normally this would be the connection to the next level up.
In this case we don't have an interrupt parent property, so the GIC is properly identified as the top-level interrupt controller. This is admittedly a bit fragile, and contradicts the current wording of the binding.
Actually, you typically do have interrupt parent set in the next level up if the gic is one the same bus or you globally set the parent. If the parent is itself, the code treats this as the root controller.
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
};It's all fine (the two last regions and the interrupt are for VGIC), until someone comes up with extension FOO which requires two new regions and am interrupt. It is then impossible to distinguish between the two, short of adding more attributes.
How about this?
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; #size-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>; vgic at 2c004000 { compatible = "arm,cortex-a15-vgic", "arm,vgic"; reg = <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>; }; };It cleanly separate the extension from the core GIC, and still make it part of the GIC node.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
What do you think?
I prefer the first option.
The first I posted (vgic node outside of the gic), or the one with everything in the same node?
The first one here. I don't see any advantage of a separate node.
Rob
PATCH 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
In reply to Marc ZyngierQuoted 73 lines · Marc Zyngier
On 03/04/12 16:35, Grant Likely wrote:
Hi Grant,
Quoted 22 lines · Marc Zyngier
Quoted 12 lines · David Vrabel
Hi David,
Quoted 8 lines · Marc Zyngier
Quoted 3 lines
The GICv2 can have virtualization extension support, consisting of an additional set of registers and interrupts. Add the necessary binding to the GIC DT documentation.
The Xen hypervisor's device tree support is very much incomplete so I've not looked into this is much detail.
Quoted 8 lines
Quoted 2 lines
Would it make more sense to extend the existing gic binding with the the additional information rather than adding a new node?
I'm actually torn between the two approaches. On one side, the VGIC is part of the GIC spec, hence should be part of the GIC node. On the other hand, it is logically handled by a different piece of software (the hypervisor), and would normally be probed separately. Having a separate node makes the probing more sensible.
Quoted 5 lines
Don't get too hung up on the software side of things. Describe it in a way that makes sense for the hardware. There is lots of precidence for two hunks of software initializating from the same node; either by probe kicking off two init hooks, or by early init code going looking for the node manually.
What I'm trying to avoid is a royal mess in the future if we get some other extension to the GIC.
Let's say we implement the following:
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>, <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>; };It's all fine (the two last regions and the interrupt are for VGIC), until someone comes up with extension FOO which requires two new regions and am interrupt. It is then impossible to distinguish between the two, short of adding more attributes.
How about this?
gic: interrupt-controller at 2c001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; #address-cells = <1>; #size-cells = <1>; interrupt-controller; reg = <0x2c001000 0x1000>, <0x2c002000 0x100>; vgic at 2c004000 { compatible = "arm,cortex-a15-vgic", "arm,vgic"; reg = <0x2c004000 0x2000>, <0x2c006000 0x2000>; interrupts = <1 9 0xf04>; }; };
Quoted 2 lines
It cleanly separate the extension from the core GIC, and still make it part of the GIC node.
I think that the compatible property already supports what you need to do. If an extension is added to the core binding, then suppliment it with a new compatible value:
gic: interrupt-controller at 2c001000 {
compatible = "arm,cortex-a15-vgic", "arm,cortex-a15-gic";
#interrupt-cells = <3>;
#address-cells = <1>;
#size-cells = <1>;
interrupt-controller;
reg = <0x2c001000 0x1000>,
<0x2c002000 0x100>,
<0x2c004000 0x2000>,
<0x2c006000 0x2000>;
interrupts = <1 9 0xf04>;
};Or, if it can be done in a backwards compatible way, then just extend the existing binding. In you're example above, the 'vgic' extension only gets enabled if the 3rd and 4th 'reg' tuples are added.
g.