To allow the direct injection of SGIs into a guest, the GICv4.1
architecture has to sacrifice the Active state so that SGIs look
a lot like LPIs (they are injected by the same mechanism).
In order not to break existing software, the architecture gives
offers guests OSs the choice: SGIs with or without an active
state. It is the hypervisors duty to honor the guest's choice.
For this, the architecture offers a discovery bit indicating whether
the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
bit indicating whether the guest wants Active-less SGIs or not
(controlled by GICD_CTLR.nASSGIreq).
A hypervisor not supporting GICv4.1 SGIs would leave nASSGIcap
clear, and a guest not knowing about GICv4.1 SGIs (or definitely
wanting an Active state) would leave nASSGIreq clear (both being
thankfully backward compatible with older revisions of the GIC).
Since Linux is perfectly happy without an active state on SGIs,
inform the hypervisor that we'll use that if offered.
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Zenghui Yu <[email protected]>
---
drivers/irqchip/irq-gic-v3.c | 10 ++++++++--
include/linux/irqchip/arm-gic-v3.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index cd76435c4a31..73e87e176d76 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -724,6 +724,7 @@ static void __init gic_dist_init(void)
unsigned int i;
u64 affinity;
void __iomem *base = gic_data.dist_base;
+ u32 val;
/* Disable the distributor */
writel_relaxed(0, base + GICD_CTLR);
@@ -756,9 +757,14 @@ static void __init gic_dist_init(void)
/* Now do the common stuff, and wait for the distributor to drain */
gic_dist_config(base, GIC_LINE_NR, gic_dist_wait_for_rwp);
+ val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
+ if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
+ pr_info("Enabling SGIs without active state\n");
+ val |= GICD_CTLR_nASSGIreq;
+ }
+
/* Enable distributor with ARE, Group1 */
- writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1,
- base + GICD_CTLR);
+ writel_relaxed(val, base + GICD_CTLR);
/*
* Set all global interrupts to the boot CPU only. ARE must be
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 83439bfb6c5b..c29a02678a6f 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -57,6 +57,7 @@
#define GICD_SPENDSGIR 0x0F20
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_CTLR_nASSGIreq (1U << 8)
#define GICD_CTLR_DS (1U << 6)
#define GICD_CTLR_ARE_NS (1U << 4)
#define GICD_CTLR_ENABLE_G1A (1U << 1)
@@ -90,6 +91,7 @@
#define GICD_TYPER_ESPIS(typer) \
(((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0)
+#define GICD_TYPER2_nASSGIcap (1U << 8)
#define GICD_TYPER2_VIL (1U << 7)
#define GICD_TYPER2_VID GENMASK(4, 0)
--
2.20.1
Hi Marc,
On 2020/3/5 4:33, Marc Zyngier wrote:
> To allow the direct injection of SGIs into a guest, the GICv4.1
> architecture has to sacrifice the Active state so that SGIs look
> a lot like LPIs (they are injected by the same mechanism).
>
> In order not to break existing software, the architecture gives
> offers guests OSs the choice: SGIs with or without an active
> state. It is the hypervisors duty to honor the guest's choice.
>
> For this, the architecture offers a discovery bit indicating whether
> the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
> bit indicating whether the guest wants Active-less SGIs or not
> (controlled by GICD_CTLR.nASSGIreq).
I still can't find the description of these two bits in IHI0069F.
Are they actually architected and will be available in the future
version of the spec? I want to confirm it again since this has a
great impact on the KVM code, any pointers?
Thanks,
Zenghui
Hi Zenghui,
On 2020-03-12 06:30, Zenghui Yu wrote:
> Hi Marc,
>
> On 2020/3/5 4:33, Marc Zyngier wrote:
>> To allow the direct injection of SGIs into a guest, the GICv4.1
>> architecture has to sacrifice the Active state so that SGIs look
>> a lot like LPIs (they are injected by the same mechanism).
>>
>> In order not to break existing software, the architecture gives
>> offers guests OSs the choice: SGIs with or without an active
>> state. It is the hypervisors duty to honor the guest's choice.
>>
>> For this, the architecture offers a discovery bit indicating whether
>> the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
>> bit indicating whether the guest wants Active-less SGIs or not
>> (controlled by GICD_CTLR.nASSGIreq).
>
> I still can't find the description of these two bits in IHI0069F.
> Are they actually architected and will be available in the future
> version of the spec? I want to confirm it again since this has a
> great impact on the KVM code, any pointers?
Damn. The bits *are* in the engineering spec version 19 (unfortunately
not a public document, but I believe you should have access to it).
If the bits have effectively been removed from the spec, I'll drop the
GICv4.1 code from the 5.7 queue until we find a way to achieve the same
level of support.
I've emailed people inside ARM to find out.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 2020-03-12 09:28, Marc Zyngier wrote:
> Hi Zenghui,
>
> On 2020-03-12 06:30, Zenghui Yu wrote:
>> Hi Marc,
>>
>> On 2020/3/5 4:33, Marc Zyngier wrote:
>>> To allow the direct injection of SGIs into a guest, the GICv4.1
>>> architecture has to sacrifice the Active state so that SGIs look
>>> a lot like LPIs (they are injected by the same mechanism).
>>>
>>> In order not to break existing software, the architecture gives
>>> offers guests OSs the choice: SGIs with or without an active
>>> state. It is the hypervisors duty to honor the guest's choice.
>>>
>>> For this, the architecture offers a discovery bit indicating whether
>>> the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
>>> bit indicating whether the guest wants Active-less SGIs or not
>>> (controlled by GICD_CTLR.nASSGIreq).
>>
>> I still can't find the description of these two bits in IHI0069F.
>> Are they actually architected and will be available in the future
>> version of the spec? I want to confirm it again since this has a
>> great impact on the KVM code, any pointers?
>
> Damn. The bits *are* in the engineering spec version 19 (unfortunately
> not a public document, but I believe you should have access to it).
>
> If the bits have effectively been removed from the spec, I'll drop the
> GICv4.1 code from the 5.7 queue until we find a way to achieve the same
> level of support.
>
> I've emailed people inside ARM to find out.
I've now had written confirmation that the bits are still there.
It is just that the current revision of the documentation was cut
*before*
they made it into the architecture (there seem to be a 6 month delay
between
the architecture being sampled and the documentation being released).
M.
--
Jazz is not dead. It just smells funny...
Hi Marc,
On 3/4/20 9:33 PM, Marc Zyngier wrote:
> To allow the direct injection of SGIs into a guest, the GICv4.1
> architecture has to sacrifice the Active state so that SGIs look
> a lot like LPIs (they are injected by the same mechanism).
>
> In order not to break existing software, the architecture gives
> offers guests OSs the choice: SGIs with or without an active
nit gives offers
> state. It is the hypervisors duty to honor the guest's choice.
>
> For this, the architecture offers a discovery bit indicating whether
> the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
> bit indicating whether the guest wants Active-less SGIs or not
> (controlled by GICD_CTLR.nASSGIreq).
>
> A hypervisor not supporting GICv4.1 SGIs would leave nASSGIcap
> clear, and a guest not knowing about GICv4.1 SGIs (or definitely
> wanting an Active state) would leave nASSGIreq clear (both being
> thankfully backward compatible with older revisions of the GIC).
>
> Since Linux is perfectly happy without an active state on SGIs,
> inform the hypervisor that we'll use that if offered.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> Reviewed-by: Zenghui Yu <[email protected]>
> ---
> drivers/irqchip/irq-gic-v3.c | 10 ++++++++--
> include/linux/irqchip/arm-gic-v3.h | 2 ++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index cd76435c4a31..73e87e176d76 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -724,6 +724,7 @@ static void __init gic_dist_init(void)
> unsigned int i;
> u64 affinity;
> void __iomem *base = gic_data.dist_base;
> + u32 val;
>
> /* Disable the distributor */
> writel_relaxed(0, base + GICD_CTLR);
> @@ -756,9 +757,14 @@ static void __init gic_dist_init(void)
> /* Now do the common stuff, and wait for the distributor to drain */
> gic_dist_config(base, GIC_LINE_NR, gic_dist_wait_for_rwp);
>
> + val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
> + if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
> + pr_info("Enabling SGIs without active state\n");
> + val |= GICD_CTLR_nASSGIreq;
> + }
> +
> /* Enable distributor with ARE, Group1 */
> - writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1,
> - base + GICD_CTLR);
> + writel_relaxed(val, base + GICD_CTLR);
>
> /*
> * Set all global interrupts to the boot CPU only. ARE must be
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index 83439bfb6c5b..c29a02678a6f 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -57,6 +57,7 @@
> #define GICD_SPENDSGIR 0x0F20
>
> #define GICD_CTLR_RWP (1U << 31)
> +#define GICD_CTLR_nASSGIreq (1U << 8)
I am not able to find this bit in Arm IHI 0069F (ID022020)
same for the bit in GICD_TYPER. Do we still miss part of the spec?
Thanks
Eric
> #define GICD_CTLR_DS (1U << 6)
> #define GICD_CTLR_ARE_NS (1U << 4)
> #define GICD_CTLR_ENABLE_G1A (1U << 1)
> @@ -90,6 +91,7 @@
> #define GICD_TYPER_ESPIS(typer) \
> (((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0)
>
> +#define GICD_TYPER2_nASSGIcap (1U << 8)
> #define GICD_TYPER2_VIL (1U << 7)
> #define GICD_TYPER2_VID GENMASK(4, 0)
>
>
Hi Eric,
On 2020-03-12 17:16, Auger Eric wrote:
> Hi Marc,
[...]
>> diff --git a/include/linux/irqchip/arm-gic-v3.h
>> b/include/linux/irqchip/arm-gic-v3.h
>> index 83439bfb6c5b..c29a02678a6f 100644
>> --- a/include/linux/irqchip/arm-gic-v3.h
>> +++ b/include/linux/irqchip/arm-gic-v3.h
>> @@ -57,6 +57,7 @@
>> #define GICD_SPENDSGIR 0x0F20
>>
>> #define GICD_CTLR_RWP (1U << 31)
>> +#define GICD_CTLR_nASSGIreq (1U << 8)
> I am not able to find this bit in Arm IHI 0069F (ID022020)
> same for the bit in GICD_TYPER. Do we still miss part of the spec?
See my response to Zenghui (TL;DR: this addition to the spec missed the
cut-off for revision F and will be added in the next round).
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Hi Marc,
On 2020/3/12 20:05, Marc Zyngier wrote:
> On 2020-03-12 09:28, Marc Zyngier wrote:
>> Hi Zenghui,
>>
>> On 2020-03-12 06:30, Zenghui Yu wrote:
>>> Hi Marc,
>>>
>>> On 2020/3/5 4:33, Marc Zyngier wrote:
>>>> To allow the direct injection of SGIs into a guest, the GICv4.1
>>>> architecture has to sacrifice the Active state so that SGIs look
>>>> a lot like LPIs (they are injected by the same mechanism).
>>>>
>>>> In order not to break existing software, the architecture gives
>>>> offers guests OSs the choice: SGIs with or without an active
>>>> state. It is the hypervisors duty to honor the guest's choice.
>>>>
>>>> For this, the architecture offers a discovery bit indicating whether
>>>> the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
>>>> bit indicating whether the guest wants Active-less SGIs or not
>>>> (controlled by GICD_CTLR.nASSGIreq).
>>>
>>> I still can't find the description of these two bits in IHI0069F.
>>> Are they actually architected and will be available in the future
>>> version of the spec? I want to confirm it again since this has a
>>> great impact on the KVM code, any pointers?
>>
>> Damn. The bits *are* in the engineering spec version 19 (unfortunately
>> not a public document, but I believe you should have access to it).
>>
>> If the bits have effectively been removed from the spec, I'll drop the
>> GICv4.1 code from the 5.7 queue until we find a way to achieve the same
>> level of support.
>>
>> I've emailed people inside ARM to find out.
>
> I've now had written confirmation that the bits are still there.
>
> It is just that the current revision of the documentation was cut *before*
> they made it into the architecture (there seem to be a 6 month delay
> between
> the architecture being sampled and the documentation being released).
I see. Thanks for the confirmation!
Zenghui
The following commit has been merged into the irq/core branch of tip:
Commit-ID: 0b04758b002bde9434053be2fff8064ac3d9d8bb
Gitweb: https://git.kernel.org/tip/0b04758b002bde9434053be2fff8064ac3d9d8bb
Author: Marc Zyngier <[email protected]>
AuthorDate: Wed, 04 Mar 2020 20:33:08
Committer: Marc Zyngier <[email protected]>
CommitterDate: Thu, 19 Mar 2020 11:11:21
irqchip/gic-v3: Use SGIs without active state if offered
To allow the direct injection of SGIs into a guest, the GICv4.1
architecture has to sacrifice the Active state so that SGIs look
a lot like LPIs (they are injected by the same mechanism).
In order not to break existing software, the architecture gives
offers guests OSs the choice: SGIs with or without an active
state. It is the hypervisors duty to honor the guest's choice.
For this, the architecture offers a discovery bit indicating whether
the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another
bit indicating whether the guest wants Active-less SGIs or not
(controlled by GICD_CTLR.nASSGIreq).
A hypervisor not supporting GICv4.1 SGIs would leave nASSGIcap
clear, and a guest not knowing about GICv4.1 SGIs (or definitely
wanting an Active state) would leave nASSGIreq clear (both being
thankfully backward compatible with older revisions of the GIC).
Since Linux is perfectly happy without an active state on SGIs,
inform the hypervisor that we'll use that if offered.
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Zenghui Yu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-gic-v3.c | 10 ++++++++--
include/linux/irqchip/arm-gic-v3.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index c1f7af9..b6b0f86 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -723,6 +723,7 @@ static void __init gic_dist_init(void)
unsigned int i;
u64 affinity;
void __iomem *base = gic_data.dist_base;
+ u32 val;
/* Disable the distributor */
writel_relaxed(0, base + GICD_CTLR);
@@ -755,9 +756,14 @@ static void __init gic_dist_init(void)
/* Now do the common stuff, and wait for the distributor to drain */
gic_dist_config(base, GIC_LINE_NR, gic_dist_wait_for_rwp);
+ val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
+ if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
+ pr_info("Enabling SGIs without active state\n");
+ val |= GICD_CTLR_nASSGIreq;
+ }
+
/* Enable distributor with ARE, Group1 */
- writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1,
- base + GICD_CTLR);
+ writel_relaxed(val, base + GICD_CTLR);
/*
* Set all global interrupts to the boot CPU only. ARE must be
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 83439bf..c29a026 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -57,6 +57,7 @@
#define GICD_SPENDSGIR 0x0F20
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_CTLR_nASSGIreq (1U << 8)
#define GICD_CTLR_DS (1U << 6)
#define GICD_CTLR_ARE_NS (1U << 4)
#define GICD_CTLR_ENABLE_G1A (1U << 1)
@@ -90,6 +91,7 @@
#define GICD_TYPER_ESPIS(typer) \
(((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0)
+#define GICD_TYPER2_nASSGIcap (1U << 8)
#define GICD_TYPER2_VIL (1U << 7)
#define GICD_TYPER2_VID GENMASK(4, 0)