From: Marc Zyngier <maz@kernel.org>
To: t00849498 <tangnianyao@huawei.com>
Cc: <tglx@linutronix.de>, <linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <guoyang2@huawei.com>,
<wangwudi@hisilicon.com>
Subject: Re: [RESPIN PATCH] irqchip/gic-v3-its:Fix GICv4.1 needless VSYNC after unmap VPE
Date: Wed, 03 Apr 2024 11:09:56 +0100 [thread overview]
Message-ID: <86cyr6u58r.wl-maz@kernel.org> (raw)
In-Reply-To: <20240403083556.3862236-1-tangnianyao@huawei.com>
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>
>
> 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.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: t00849498 <tangnianyao@huawei.com>
Cc: <tglx@linutronix.de>, <linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <guoyang2@huawei.com>,
<wangwudi@hisilicon.com>
Subject: Re: [RESPIN PATCH] irqchip/gic-v3-its:Fix GICv4.1 needless VSYNC after unmap VPE
Date: Wed, 03 Apr 2024 11:09:56 +0100 [thread overview]
Message-ID: <86cyr6u58r.wl-maz@kernel.org> (raw)
In-Reply-To: <20240403083556.3862236-1-tangnianyao@huawei.com>
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>
>
> 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.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2024-04-03 10:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-03 8:35 [RESPIN PATCH] irqchip/gic-v3-its:Fix GICv4.1 needless VSYNC after unmap VPE t00849498
2024-04-03 8:35 ` t00849498
2024-04-03 10:09 ` Marc Zyngier [this message]
2024-04-03 10:09 ` Marc Zyngier
2024-04-06 1:55 ` Tangnianyao
2024-04-06 1:55 ` Tangnianyao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86cyr6u58r.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=guoyang2@huawei.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tangnianyao@huawei.com \
--cc=tglx@linutronix.de \
--cc=wangwudi@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.