ARM: DT: Add binding for GIC virtualization extentions (VGIC)
PATCH v2 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 | 35 +++++++++++++++++++++++- 1 files changed, 33 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/arm/gic.txt +33 -2
@@ -11,7 +11,9 @@ have PPIs or SGIs. Main node required properties: - compatible : should be one of: + "arm,cortex-a15-gic" "arm,cortex-a9-gic" + "arm,cortex-a7-gic" "arm,arm11mp-gic" - interrupt-controller : Identifies the node as an interrupt controller - #interrupt-cells : Specifies the number of cells needed to encode an
@@ -39,8 +41,9 @@ Main node required properties: the GIC cpu interface register base and size. Optional -- interrupts : Interrupt source of the parent interrupt controller. Only - present on secondary GICs. +- interrupts : Interrupt source of the parent interrupt controller on + secondary GICs, or VGIC maintainance interrupt on primary GIC (see + below). - cpu-offset : per-cpu offset within the distributor and cpu interface regions, used when the GIC doesn't have banked registers. The offset is
@@ -57,3 +60,31 @@ Example: <0xfff10100 0x100>; }; + +* GIC virtualization extensions (VGIC) + +For ARM cores that support the virtualization extensions, additional +properties must be described (they only exist if the GIC is the +primary interrupt controller). + +Required properties: + +- reg : Additional regions specifying the base physical address and + size of the VGIC registers. The first additional region is the GIC + virtual interface control register base and size. The 2nd additional + region is the GIC virtual cpu interface register base and size. + +- interrupts : VGIC maintainance interrupt. + +Example: + + interrupt-controller at 2c001000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x2c001000 0x1000>, + <0x2c002000 0x1000>, + <0x2c004000 0x2000>, + <0x2c006000 0x2000>; + interrupts = <1 9 0xf04>; + };
Signature · 1 line
1.7.7.1
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 6 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.
Signed-off-by Marc Zyngier <[email protected]>
Would it make sense to add a way to detect whether a GIC is virtual or real? Maybe an optional empty "virtual-gic" property or an additional "compatible" value. Even if we don't need it now, it might come in handy if we require it already.
Arnd
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 6 lines · Arnd Bergmann
Quoted 5 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.
Quoted 6 lines
Quoted 1 line
Signed-off-by Marc Zyngier <[email protected]>Would it make sense to add a way to detect whether a GIC is virtual or real? Maybe an optional empty "virtual-gic" property or an additional "compatible" value. Even if we don't need it now, it might come in handy if we require it already.
I don't really see a need for this. When running on a virtual machine, the kernel cannot tell if this is the real thing or not (the virtual CPU interface looks exactly like the normal one once mapped into the guest address space).
Or maybe I just didn't get your use case?
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 14 lines · Marc Zyngier
Quoted 6 lines · Arnd Bergmann
Quoted 5 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.
Quoted 6 lines
Quoted 1 line
Signed-off-by Marc Zyngier <[email protected]>Would it make sense to add a way to detect whether a GIC is virtual or real? Maybe an optional empty "virtual-gic" property or an additional "compatible" value. Even if we don't need it now, it might come in handy if we require it already.
Quoted 6 lines
I don't really see a need for this. When running on a virtual machine, the kernel cannot tell if this is the real thing or not (the virtual CPU interface looks exactly like the normal one once mapped into the guest address space).
Or maybe I just didn't get your use case?
Well, one difference seems to be the VGIC maintainance interrupt that may or may not be present, and another one the additional registers. Of course you can imply the type of GIC from the presence of this extra data, but I think it would be better to make it explicit.
Arnd
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 23 lines · Arnd Bergmann
Quoted 22 lines · Marc Zyngier
Quoted 6 lines · Arnd Bergmann
Quoted 5 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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Marc Zyngier <[email protected]>Would it make sense to add a way to detect whether a GIC is virtual or real? Maybe an optional empty "virtual-gic" property or an additional
"compatible" value. Even if we don't need it now, it might come in handy
if we require it already.I don't really see a need for this. When running on a virtual machine, the
kernel cannot tell if this is the real thing or not (the virtual CPU interface looks exactly like the normal one once mapped into the guest address space).
Quoted 3 lines
Quoted 1 line
Or maybe I just didn't get your use case?
Well, one difference seems to be the VGIC maintainance interrupt that
may
Quoted 1 line
or may not be present, and another one the additional registers. Of
course
Quoted 2 lines
you can imply the type of GIC from the presence of this extra data, but I think it would be better to make it explicit.
Ah, I see what you mean. But these registers and interrupt are actually only visible on the hypervizor side. The guest shouldn't see any of this.
I suppose we could have a "arm,has-virt-extensions" property in devices that implement the virtualization extensions (GIC, timers...).
What do you think?
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 2 lines · Marc Zyngier
Ah, I see what you mean. But these registers and interrupt are actually only visible on the hypervizor side. The guest shouldn't see any of this.
Rihgt, that makes sense.
Quoted 4 lines
I suppose we could have a "arm,has-virt-extensions" property in devices that implement the virtualization extensions (GIC, timers...).
What do you think?
Sounds good. I don't have a strong preference whether that should be a property like you suggest or a separate "compatible" value, maybe Rob or Grant can comment on what they prefer.
On the guest side, is it guaranteed that the virtual GIC looks like a real GIC without those extensions? If the actual behavior depends on the hypervisor, it might still make sense to add another flag in there to tell that it's virtual, even if we don't require it for now.
Arnd
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · Marc Zyngier
Ah, I see what you mean. But these registers and interrupt are actually only visible on the hypervizor side. The guest shouldn't see any of
this.
Quoted 11 lines
Rihgt, that makes sense.
Quoted 4 lines
I suppose we could have a "arm,has-virt-extensions" property in devices that implement the virtualization extensions (GIC, timers...).
What do you think?
Sounds good. I don't have a strong preference whether that should be a property like you suggest or a separate "compatible" value, maybe Rob or Grant can comment on what they prefer.
At the moment, the compatible string should be pretty explicit, as it is not possible to build an A7 or A15 based SoC without the virtualization extensions.
Quoted 4 lines
On the guest side, is it guaranteed that the virtual GIC looks like a real GIC without those extensions? If the actual behavior depends on the hypervisor, it might still make sense to add another flag in there to tell that it's virtual, even if we don't require it for now.
Nothing is really guaranteed, as the VGIC only exposes a GIC CPU interface to the guest, and the distributor has to be modeled by the hypervisor. But if the difference is observable by the guest, is it still a GIC? And how to differentiate between behavior A and behavior B with a single flag?
My take would be that if the guest can tell the difference, than it shouldn't be called a GIC, and have separate bindings. Or at least a different compatible string that we could use to enable quirks in the driver.
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 8 lines · Marc Zyngier
Quoted 4 lines
Sounds good. I don't have a strong preference whether that should be a property like you suggest or a separate "compatible" value, maybe Rob or Grant can comment on what they prefer.
At the moment, the compatible string should be pretty explicit, as it is not possible to build an A7 or A15 based SoC without the virtualization extensions.
Ok, makes sense. Then again, if you run an A15 guests on an A15 host, the virtual GIC would not have those extensions, right?
Quoted 9 lines
Quoted 4 lines
On the guest side, is it guaranteed that the virtual GIC looks like a real GIC without those extensions? If the actual behavior depends on the hypervisor, it might still make sense to add another flag in there to tell that it's virtual, even if we don't require it for now.
Nothing is really guaranteed, as the VGIC only exposes a GIC CPU interface to the guest, and the distributor has to be modeled by the hypervisor. But if the difference is observable by the guest, is it still a GIC? And how to differentiate between behavior A and behavior B with a single flag?
Right, in that case, we would probably want a hypervisor specific compatible property to identify it.
Quoted 4 lines
My take would be that if the guest can tell the difference, than it shouldn't be called a GIC, and have separate bindings. Or at least a different compatible string that we could use to enable quirks in the driver.
Ok, good.
Arnd
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 6 lines · Arnd Bergmann
Quoted 5 lines · Marc Zyngier
Quoted 4 lines
Sounds good. I don't have a strong preference whether that should be a property like you suggest or a separate "compatible" value, maybe Rob or Grant can comment on what they prefer.
Quoted 6 lines
Quoted 3 lines
At the moment, the compatible string should be pretty explicit, as it is not possible to build an A7 or A15 based SoC without the virtualization extensions.
Ok, makes sense. Then again, if you run an A15 guests on an A15 host, the virtual GIC would not have those extensions, right?
Indeed, they would be inaccessible (the hypervisor should not map them into the guest address space). I'd expect the guest kernel not to try to use them anyway, as it wouldn't be able to use the HYP mode.
But again, this is implementation specific.
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
In reply to Marc ZyngierQuoted 31 lines · Marc Zyngier
Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · Marc Zyngier
Ah, I see what you mean. But these registers and interrupt are actually only visible on the hypervizor side. The guest shouldn't see any of
this.
Quoted 11 lines
Rihgt, that makes sense.
Quoted 4 lines
I suppose we could have a "arm,has-virt-extensions" property in devices that implement the virtualization extensions (GIC, timers...).
What do you think?
Sounds good. I don't have a strong preference whether that should be a property like you suggest or a separate "compatible" value, maybe Rob or Grant can comment on what they prefer.
At the moment, the compatible string should be pretty explicit, as it is not possible to build an A7 or A15 based SoC without the virtualization extensions.
Quoted 4 lines
On the guest side, is it guaranteed that the virtual GIC looks like a real GIC without those extensions? If the actual behavior depends on the hypervisor, it might still make sense to add another flag in there to tell that it's virtual, even if we don't require it for now.
Nothing is really guaranteed, as the VGIC only exposes a GIC CPU interface to the guest, and the distributor has to be modeled by the hypervisor. But if the difference is observable by the guest, is it still a GIC? And how to differentiate between behavior A and behavior B with a single flag?
Quoted 4 lines
My take would be that if the guest can tell the difference, than it shouldn't be called a GIC, and have separate bindings. Or at least a different compatible string that we could use to enable quirks in the driver.
I would agree with that.
g.
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
In reply to Arnd BergmannQuoted 6 lines · Arnd Bergmann
Quoted 2 lines · Marc Zyngier
Ah, I see what you mean. But these registers and interrupt are actually only visible on the hypervizor side. The guest shouldn't see any of this.
Rihgt, that makes sense.
Quoted 8 lines
Quoted 4 lines
I suppose we could have a "arm,has-virt-extensions" property in devices that implement the virtualization extensions (GIC, timers...).
What do you think?
Sounds good. I don't have a strong preference whether that should be a property like you suggest or a separate "compatible" value, maybe Rob or Grant can comment on what they prefer.
I don't have a strong opinion either way. This approach looks fine.
g.
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
In reply to Marc ZyngierQuoted 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.
This looks fine for Xen.
David
Quoted 63 lines
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/devicetree/bindings/arm/gic.txt | 35 +++++++++++++++++++++++- 1 files changed, 33 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/arm/gic.txt +33
-2@@ -11,7 +11,9 @@ have PPIs or SGIs. Main node required properties: - compatible : should be one of: + "arm,cortex-a15-gic" "arm,cortex-a9-gic" + "arm,cortex-a7-gic" "arm,arm11mp-gic" - interrupt-controller : Identifies the node as an interrupt controller - #interrupt-cells : Specifies the number of cells needed to encode an@@ -39,8 +41,9 @@ Main node required properties: the GIC cpu interface register base and size. Optional -- interrupts : Interrupt source of the parent interrupt controller. Only - present on secondary GICs. +- interrupts : Interrupt source of the parent interrupt controller on + secondary GICs, or VGIC maintainance interrupt on primary GIC (see + below). - cpu-offset : per-cpu offset within the distributor and cpu interface regions, used when the GIC doesn't have banked registers. The offset is@@ -57,3 +60,31 @@ Example: <0xfff10100 0x100>; }; + +* GIC virtualization extensions (VGIC) + +For ARM cores that support the virtualization extensions, additional +properties must be described (they only exist if the GIC is the +primary interrupt controller). + +Required properties: + +- reg : Additional regions specifying the base physical address and + size of the VGIC registers. The first additional region is the GIC + virtual interface control register base and size. The 2nd additional + region is the GIC virtual cpu interface register base and size. + +- interrupts : VGIC maintainance interrupt. + +Example: + + interrupt-controller at 2c001000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x2c001000 0x1000>, + <0x2c002000 0x1000>, + <0x2c004000 0x2000>, + <0x2c006000 0x2000>; + interrupts = <1 9 0xf04>; + };
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 6 lines · David Vrabel
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.
This looks fine for Xen.
Thanks David. Can I add your Acked-by:?
M.
Quoted 69 lines
David
Quoted 63 lines
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/devicetree/bindings/arm/gic.txt | 35 +++++++++++++++++++++++- 1 files changed, 33 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/arm/gic.txt +33
-2@@ -11,7 +11,9 @@ have PPIs or SGIs. Main node required properties: - compatible : should be one of: + "arm,cortex-a15-gic" "arm,cortex-a9-gic" + "arm,cortex-a7-gic" "arm,arm11mp-gic" - interrupt-controller : Identifies the node as an interrupt controller - #interrupt-cells : Specifies the number of cells needed to encode an@@ -39,8 +41,9 @@ Main node required properties: the GIC cpu interface register base and size. Optional -- interrupts : Interrupt source of the parent interrupt controller. Only - present on secondary GICs. +- interrupts : Interrupt source of the parent interrupt controller on + secondary GICs, or VGIC maintainance interrupt on primary GIC (see + below). - cpu-offset : per-cpu offset within the distributor and cpu interface regions, used when the GIC doesn't have banked registers. The offset is@@ -57,3 +60,31 @@ Example: <0xfff10100 0x100>; }; + +* GIC virtualization extensions (VGIC) + +For ARM cores that support the virtualization extensions, additional +properties must be described (they only exist if the GIC is the +primary interrupt controller). + +Required properties: + +- reg : Additional regions specifying the base physical address and + size of the VGIC registers. The first additional region is the GIC + virtual interface control register base and size. The 2nd additional + region is the GIC virtual cpu interface register base and size. + +- interrupts : VGIC maintainance interrupt. + +Example: + + interrupt-controller at 2c001000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x2c001000 0x1000>, + <0x2c002000 0x1000>, + <0x2c004000 0x2000>, + <0x2c006000 0x2000>; + interrupts = <1 9 0xf04>; + };
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 9 lines · Marc Zyngier
Quoted 6 lines · David Vrabel
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.
This looks fine for Xen.
Thanks David. Can I add your Acked-by:?
Sure.
David
Grant Likely
AppliedPATCH v2 1/1 ARM: DT: Add binding for GIC virtualization extentions (VGIC)
Quoted 7 lines · David Vrabel
Quoted 6 lines · Marc Zyngier
Quoted 5 lines · David Vrabel
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.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
This looks fine for Xen.
Thanks David. Can I add your Acked-by:?
Sure.
Acked-by David Vrabel <[email protected]>
Applied, thanks.
g.