Messages in this thread | | | Subject | Re: [RESPIN PATCH] irqchip/gic-v3-its:Fix GICv4.1 needless VSYNC after unmap VPE | From | Tangnianyao <> | Date | Sat, 6 Apr 2024 09:55:18 +0800 |
| |
On 4/3/2024 18:09, Marc Zyngier wrote: > Thanks for respinning this. > > A few remarks: > > The subject line could be improved. Something like: > > "irqchip/gic-v4: Don't issue a VSYNC after VMAPP with V=0" > > On Wed, 03 Apr 2024 09:35:56 +0100, > t00849498 <tangnianyao@huawei.com> wrote: >> From: Nianyao Tang <tangnianyao@huawei.com> >> >> Quote from GIC spec 5.3.19, a VMAPP with {V, Alloc}=={0, x} >> is self-synchronizing, This means the ITS command queue does not >> show the command as consumed until all of its effects are completed. > Since this is a direct quote, make it clear that it is so. > >> We don't need VSYNC to guarantee unmap finish. And VSYNC after unmap VPE >> will reach an invalid vpe table entry, which may trigger exception >> like SError or RAS. Let's fix it. > This should be much stronger. It's not that we don't need VSYNC. It is > that VSYNC is actively wrong. I suggest that you rewrite the commit > message along these lines: > > <msg> > As per the GICv4.1 spec (Arm IHI 0069H, 5.3.19): > > "A VMAPP with {V, Alloc}=={0, x} is self-synchronizing. This means the > ITS command queue does not show the command as consumed until all of > its effects are completed." > > Furthermore, VSYNC is allowed to deliver an SError when referencing a > non existent VPE. > > By these definitions, a VMAPP followed by a VSYNC is a bug, as the > later references a VPE that has been unmapped by the former. > > Fix it by eliding the VSYNC in this scenario. > </msg>
Thanks for the above comments, I will resend later. > >> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com> > Please also add: > > Fixes: 64edfaa9a234 ("irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP") > > With the above fixed: > > Reviewed-by: Marc Zyngier <maz@kernel.org> > > Thanks, > > M. >
| |