[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[Shaokun Zhang]

From: Miao Zhong <zhongmiao@hisilicon.com>

When PRI queue occurs overflow, driver should update the OVACKFLG to
the PRIQ consumer register, otherwise subsequent PRI requests will not
be processed.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com> 
Signed-off-by: Miao Zhong <zhongmiao@hisilicon.com>
---
 drivers/iommu/arm-smmu-v3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d64710..deacc15 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1301,6 +1301,7 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
 
 	/* Sync our overflow flag, as we believe we're up to speed */
 	q->cons = Q_OVF(q, q->prod) | Q_WRP(q, q->cons) | Q_IDX(q, q->cons);
+	writel(q->cons, q->cons_reg);
 	return IRQ_HANDLED;
 }
 
-- 
2.7.4

[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[Jean-Philippe Brucker]

On 23/07/18 13:56, Shaokun Zhang wrote:
> From: Miao Zhong <zhongmiao@hisilicon.com>
> 
> When PRI queue occurs overflow, driver should update the OVACKFLG to
> the PRIQ consumer register, otherwise subsequent PRI requests will not
> be processed.

Since the upstream driver doesn't enable PRI in endpoints, I'm not sure
this patch makes sense on its own, but it seems correct anyway. I had a
similar patch in my initial SVA RFC, but for some reason dropped it in
later versions (https://patchwork.kernel.org/patch/9594021/)

It's worth noting that the event queue doesn't have the same problem,
because the SMMU can record new events even if the overflow hasn't been
acknowledged.

Thanks,
Jean

[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[Zhangshaokun]

Hi Will,

I saw it in your trees, is it necessary to Cc stable version?

Cheers,
Shaokun

On 2018/7/23 20:56, Shaokun Zhang wrote:
> From: Miao Zhong <zhongmiao@hisilicon.com>
> 
> When PRI queue occurs overflow, driver should update the OVACKFLG to
> the PRIQ consumer register, otherwise subsequent PRI requests will not
> be processed.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com> 
> Signed-off-by: Miao Zhong <zhongmiao@hisilicon.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d64710..deacc15 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1301,6 +1301,7 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
>  
>  	/* Sync our overflow flag, as we believe we're up to speed */
>  	q->cons = Q_OVF(q, q->prod) | Q_WRP(q, q->cons) | Q_IDX(q, q->cons);
> +	writel(q->cons, q->cons_reg);
>  	return IRQ_HANDLED;
>  }
>  
> 

[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[Will Deacon]

On Fri, Jul 27, 2018 at 05:41:46PM +0800, Zhangshaokun wrote:
> I saw it in your trees, is it necessary to Cc stable version?

I don't think so, given that we don't actually support PRI upstream.

Will

> On 2018/7/23 20:56, Shaokun Zhang wrote:
> > From: Miao Zhong <zhongmiao@hisilicon.com>
> > 
> > When PRI queue occurs overflow, driver should update the OVACKFLG to
> > the PRIQ consumer register, otherwise subsequent PRI requests will not
> > be processed.
> > 
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Robin Murphy <robin.murphy@arm.com> 
> > Signed-off-by: Miao Zhong <zhongmiao@hisilicon.com>
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> > index 1d64710..deacc15 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -1301,6 +1301,7 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
> >  
> >  	/* Sync our overflow flag, as we believe we're up to speed */
> >  	q->cons = Q_OVF(q, q->prod) | Q_WRP(q, q->cons) | Q_IDX(q, q->cons);
> > +	writel(q->cons, q->cons_reg);
> >  	return IRQ_HANDLED;
> >  }
> >  
> > 
> 

[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[Zhangshaokun]

On 2018/7/27 17:48, Will Deacon wrote:
> On Fri, Jul 27, 2018 at 05:41:46PM +0800, Zhangshaokun wrote:
>> I saw it in your trees, is it necessary to Cc stable version?
> 
> I don't think so, given that we don't actually support PRI upstream.
> 

Got it, thanks your reply.

Shaokun

> Will
> 
>> On 2018/7/23 20:56, Shaokun Zhang wrote:
>>> From: Miao Zhong <zhongmiao@hisilicon.com>
>>>
>>> When PRI queue occurs overflow, driver should update the OVACKFLG to
>>> the PRIQ consumer register, otherwise subsequent PRI requests will not
>>> be processed.
>>>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Cc: Robin Murphy <robin.murphy@arm.com> 
>>> Signed-off-by: Miao Zhong <zhongmiao@hisilicon.com>
>>> ---
>>>  drivers/iommu/arm-smmu-v3.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>>> index 1d64710..deacc15 100644
>>> --- a/drivers/iommu/arm-smmu-v3.c
>>> +++ b/drivers/iommu/arm-smmu-v3.c
>>> @@ -1301,6 +1301,7 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev)
>>>  
>>>  	/* Sync our overflow flag, as we believe we're up to speed */
>>>  	q->cons = Q_OVF(q, q->prod) | Q_WRP(q, q->cons) | Q_IDX(q, q->cons);
>>> +	writel(q->cons, q->cons_reg);
>>>  	return IRQ_HANDLED;
>>>  }
>>>  
>>>
>>
> 
> .
> 

[PATCH] iommu/arm-smmu-v3: sync the OVACKFLG to PRIQ consumer register

[Jean-Philippe Brucker]

Hi,

On 23/07/18 15:43, Zhongmiao wrote:
> Yeah,    I haven't tested smmu  eventq  overflow ,  so i'm not sure if there is any problem with smmu eventq.  However, the code shows that there is also a problem after the smmu eventq overflows. ^_^??.

There really shouldn't be any problem with the eventq. Handling the
event overflow in my patch is mostly cosmetic and can be removed. See
the Event queue overflow section of the SMMUv3 specification (IHI0070B):

"Note: In terms of delivering events, a queue in an unacknowledged
overflow state does not behave any differently to normal queue state. If
software were to consume events and free space but leave overflow
unacknowledged, new events could be recorded."

Thanks,
Jean