[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

[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

Re: [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-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> 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

[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

Re: [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-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> 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-5wv7dgnIgG8@public.gmane.org>
> Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> 
> Signed-off-by: Miao Zhong <zhongmiao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
>  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]

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;
>  }
>  
> 

Re: [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-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> > 
> > 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-5wv7dgnIgG8@public.gmane.org>
> > Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> 
> > Signed-off-by: Miao Zhong <zhongmiao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> > ---
> >  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;
> >  }
> >  
> > 
> 

Re: [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-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>>
>>> 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-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> 
>>> Signed-off-by: Miao Zhong <zhongmiao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>> ---
>>>  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;
>>>  }
>>>  
>>>
>>
> 
> .
> 

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

[Zhongmiao]

Hi,

On 24/07/18 12:45, 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."

In the smmu v3 protocol ,  The processing of Priq overflow is different from that of Eventq. 

Thanks,
Miao
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Zhongmiao]

Hi ,

>"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."

Yeah, I agree with you . But I test  priq , It's going to be a problem.   And  See the PRI queue overflow section of the SMMUv3 specification (IHI0070A):

" Note: On detection of overflow, ARM recommends that software processes the received PRI queue entries as quickly as possible, and follows this with a single write of SMMU_PRIQ_CONS to simultaneously move on the RD pointer and acknowledge receipt of the overflow condition by setting OVACKFLG to the value read from OVFLG."

Thanks,
Miao

Re: [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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[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

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

[Zhongmiao]

Hi Jean,

On 23/07/18 22:24, Miao  Zhong 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/)

I have verified  this patch on  HiSilicon platform  and it works.

>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.

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. ^_^…….

Thanks,
Miao

>Thanks,
>Jean



_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu