Re: [PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set
Hi Marc,
On 2019/12/18 22:39, Marc Zyngier wrote:
On 2019-11-01 11:05, Zenghui Yu wrote:
Hi Marc,
On 2019/10/27 22:42, Marc Zyngier wrote:
The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
the doorbell, so we don't need to enable/disable it all
the time
So let's escape early from the proxy related functions.
Signed-off-by: Marc Zyngier
Reviewed-by: Zenghui Yu
---
drivers/irqchip/irq-gic-v3-its.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c
b/drivers/irqchip/irq-gic-v3-its.c
index 220d490d516e..999e61a9b2c3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3069,7 +3069,7 @@ static const struct irq_domain_ops
its_domain_ops = {
/*
* This is insane.
*
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
* likely), the only way to perform an invalidate is to use a fake
* device to issue an INV command, implying that the LPI has first
* been mapped to some event on that device. Since this is not exactly
@@ -3077,9 +3077,18 @@ static const struct irq_domain_ops
its_domain_ops = {
* only issue an UNMAP if we're short on available slots.
*
* Broken by design(tm).
+ *
+ * GICv4.1 actually mandates that we're able to invalidate by
writing to a
+ * MMIO register. It doesn't implement the whole of DirectLPI, but
that's
+ * good enough. And most of the time, we don't even have to invalidate
+ * anything, so that's actually pretty good!
I can't understand the meaning of this last sentence. May I ask for an
explanation? :)
Yeah, reading this now, it feels pretty clumsy, and only remotely
connected to the patch.
What I'm trying to say here is that, although GICv4.1 doesn't have
the full spectrum of v4.0 DirectLPI (it only allows a subset of it),
this subset is more then enough for us. Here's the rational:
When a vPE exits from the hypervisor, we know whether we need to
request a doorbell or not (depending on whether we're blocking on
WFI or not). On GICv4.0, this translates into enabling the doorbell
interrupt, which generates an invalidation (costly). And whenever
we've taken a doorbell, or are scheduled again, we need to turn
the doorbell off (invalidation again).
With v4.1, we can just say *at exit time* whether we want doorbells
to be subsequently generated (see its_vpe_4_1_deschedule() and the
req_db parameter in the info structure). This is part of making
the vPE non-resident, so we have 0 overhead at this stage.
Great, and get it. Thanks for this clear explanation!
Zenghui
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set
On 2019-11-01 11:05, Zenghui Yu wrote:
Hi Marc,
On 2019/10/27 22:42, Marc Zyngier wrote:
The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
the doorbell, so we don't need to enable/disable it all
the time
So let's escape early from the proxy related functions.
Signed-off-by: Marc Zyngier
Reviewed-by: Zenghui Yu
---
drivers/irqchip/irq-gic-v3-its.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c
b/drivers/irqchip/irq-gic-v3-its.c
index 220d490d516e..999e61a9b2c3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3069,7 +3069,7 @@ static const struct irq_domain_ops
its_domain_ops = {
/*
* This is insane.
*
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
* likely), the only way to perform an invalidate is to use a fake
* device to issue an INV command, implying that the LPI has first
* been mapped to some event on that device. Since this is not
exactly
@@ -3077,9 +3077,18 @@ static const struct irq_domain_ops
its_domain_ops = {
* only issue an UNMAP if we're short on available slots.
*
* Broken by design(tm).
+ *
+ * GICv4.1 actually mandates that we're able to invalidate by
writing to a
+ * MMIO register. It doesn't implement the whole of DirectLPI, but
that's
+ * good enough. And most of the time, we don't even have to
invalidate
+ * anything, so that's actually pretty good!
I can't understand the meaning of this last sentence. May I ask for
an
explanation? :)
Yeah, reading this now, it feels pretty clumsy, and only remotely
connected to the patch.
What I'm trying to say here is that, although GICv4.1 doesn't have
the full spectrum of v4.0 DirectLPI (it only allows a subset of it),
this subset is more then enough for us. Here's the rational:
When a vPE exits from the hypervisor, we know whether we need to
request a doorbell or not (depending on whether we're blocking on
WFI or not). On GICv4.0, this translates into enabling the doorbell
interrupt, which generates an invalidation (costly). And whenever
we've taken a doorbell, or are scheduled again, we need to turn
the doorbell off (invalidation again).
With v4.1, we can just say *at exit time* whether we want doorbells
to be subsequently generated (see its_vpe_4_1_deschedule() and the
req_db parameter in the info structure). This is part of making
the vPE non-resident, so we have 0 overhead at this stage.
I'll try and update the comment here.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set
Hi Marc,
On 2019/10/27 22:42, Marc Zyngier wrote:
The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
the doorbell, so we don't need to enable/disable it all
the time
So let's escape early from the proxy related functions.
Signed-off-by: Marc Zyngier
Reviewed-by: Zenghui Yu
---
drivers/irqchip/irq-gic-v3-its.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 220d490d516e..999e61a9b2c3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3069,7 +3069,7 @@ static const struct irq_domain_ops its_domain_ops = {
/*
* This is insane.
*
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
* likely), the only way to perform an invalidate is to use a fake
* device to issue an INV command, implying that the LPI has first
* been mapped to some event on that device. Since this is not exactly
@@ -3077,9 +3077,18 @@ static const struct irq_domain_ops its_domain_ops = {
* only issue an UNMAP if we're short on available slots.
*
* Broken by design(tm).
+ *
+ * GICv4.1 actually mandates that we're able to invalidate by writing to a
+ * MMIO register. It doesn't implement the whole of DirectLPI, but that's
+ * good enough. And most of the time, we don't even have to invalidate
+ * anything, so that's actually pretty good!
I can't understand the meaning of this last sentence. May I ask for an
explanation? :)
Thanks,
Zenghui
*/
static void its_vpe_db_proxy_unmap_locked(struct its_vpe *vpe)
{
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
/* Already unmapped? */
if (vpe->vpe_proxy_event == -1)
return;
@@ -3102,6 +3111,10 @@ static void its_vpe_db_proxy_unmap_locked(struct its_vpe
*vpe)
static void its_vpe_db_proxy_unmap(struct its_vpe *vpe)
{
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
if (!gic_rdists->has_direct_lpi) {
unsigned long flags;
@@ -3113,6 +3126,10 @@ static void its_vpe_db_proxy_unmap(struct its_vpe *vpe)
static void its_vpe_db_proxy_map_locked(struct its_vpe *vpe)
{
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
/* Already mapped? */
if (vpe->vpe_proxy_event != -1)
return;
@@ -3135,6 +3152,10 @@ static void its_vpe_db_proxy_move(struct its_vpe *vpe,
int from, int to)
unsigned long flags;
struct its_collection *target_col;
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
if (gic_rdists->has_direct_lpi) {
void __iomem *rdbase;
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set
The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
the doorbell, so we don't need to enable/disable it all
the time
So let's escape early from the proxy related functions.
Signed-off-by: Marc Zyngier
---
drivers/irqchip/irq-gic-v3-its.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 220d490d516e..999e61a9b2c3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3069,7 +3069,7 @@ static const struct irq_domain_ops its_domain_ops = {
/*
* This is insane.
*
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
* likely), the only way to perform an invalidate is to use a fake
* device to issue an INV command, implying that the LPI has first
* been mapped to some event on that device. Since this is not exactly
@@ -3077,9 +3077,18 @@ static const struct irq_domain_ops its_domain_ops = {
* only issue an UNMAP if we're short on available slots.
*
* Broken by design(tm).
+ *
+ * GICv4.1 actually mandates that we're able to invalidate by writing to a
+ * MMIO register. It doesn't implement the whole of DirectLPI, but that's
+ * good enough. And most of the time, we don't even have to invalidate
+ * anything, so that's actually pretty good!
*/
static void its_vpe_db_proxy_unmap_locked(struct its_vpe *vpe)
{
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
/* Already unmapped? */
if (vpe->vpe_proxy_event == -1)
return;
@@ -3102,6 +3111,10 @@ static void its_vpe_db_proxy_unmap_locked(struct its_vpe
*vpe)
static void its_vpe_db_proxy_unmap(struct its_vpe *vpe)
{
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
if (!gic_rdists->has_direct_lpi) {
unsigned long flags;
@@ -3113,6 +3126,10 @@ static void its_vpe_db_proxy_unmap(struct its_vpe *vpe)
static void its_vpe_db_proxy_map_locked(struct its_vpe *vpe)
{
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
/* Already mapped? */
if (vpe->vpe_proxy_event != -1)
return;
@@ -3135,6 +3152,10 @@ static void its_vpe_db_proxy_move(struct its_vpe *vpe,
int from, int to)
unsigned long flags;
struct its_collection *target_col;
+ /* GICv4.1 doesn't use a proxy, so nothing to do here */
+ if (gic_rdists->has_rvpeid)
+ return;
+
if (gic_rdists->has_direct_lpi) {
void __iomem *rdbase;
--
2.20.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
