Commit 464f2164da7e ("arm64: Add missing ERX*_EL1 encodings") add some
new RAS registers. Trap them to kvm.
Signed-off-by: Ruidong Tian <[email protected]>
---
arch/arm64/kvm/sys_regs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 30253bd19917..76a9ba155d58 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2389,8 +2389,13 @@ static const struct sys_reg_desc sys_reg_descs[] = {
{ SYS_DESC(SYS_ERXCTLR_EL1), trap_raz_wi },
{ SYS_DESC(SYS_ERXSTATUS_EL1), trap_raz_wi },
{ SYS_DESC(SYS_ERXADDR_EL1), trap_raz_wi },
+ { SYS_DESC(SYS_ERXPFGF_EL1), trap_raz_wi },
+ { SYS_DESC(SYS_ERXPFGCTL_EL1), trap_raz_wi },
+ { SYS_DESC(SYS_ERXPFGCDN_EL1), trap_raz_wi },
{ SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi },
{ SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi },
+ { SYS_DESC(SYS_ERXMISC2_EL1), trap_raz_wi },
+ { SYS_DESC(SYS_ERXMISC3_EL1), trap_raz_wi },
MTE_REG(TFSR_EL1),
MTE_REG(TFSRE0_EL1),
--
2.33.1
On Wed, 10 Jan 2024 07:57:39 +0000,
Ruidong Tian <[email protected]> wrote:
>
> Commit 464f2164da7e ("arm64: Add missing ERX*_EL1 encodings") add some
> new RAS registers. Trap them to kvm.
Well, they *are* already trapped by virtue of HCR_EL2.FIEN being
0. They are lacking a trap handler though.
>
> Signed-off-by: Ruidong Tian <[email protected]>
> ---
> arch/arm64/kvm/sys_regs.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 30253bd19917..76a9ba155d58 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2389,8 +2389,13 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> { SYS_DESC(SYS_ERXCTLR_EL1), trap_raz_wi },
> { SYS_DESC(SYS_ERXSTATUS_EL1), trap_raz_wi },
> { SYS_DESC(SYS_ERXADDR_EL1), trap_raz_wi },
> + { SYS_DESC(SYS_ERXPFGF_EL1), trap_raz_wi },
> + { SYS_DESC(SYS_ERXPFGCTL_EL1), trap_raz_wi },
> + { SYS_DESC(SYS_ERXPFGCDN_EL1), trap_raz_wi },
> { SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi },
> { SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi },
> + { SYS_DESC(SYS_ERXMISC2_EL1), trap_raz_wi },
> + { SYS_DESC(SYS_ERXMISC3_EL1), trap_raz_wi },
>
> MTE_REG(TFSR_EL1),
> MTE_REG(TFSRE0_EL1),
If my reading of the ARM ARM is correct, these registers only exist if
FEAT_RASv1p1 is implemented. Which means that we shouldn't handle
those as RAZ/WI unconditionally, but instead check for what we
advertise to the guest and handle it accordingly.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Jan 10, 2024 at 12:20:30PM +0000, Marc Zyngier wrote:
> If my reading of the ARM ARM is correct, these registers only exist if
> FEAT_RASv1p1 is implemented. Which means that we shouldn't handle
> those as RAZ/WI unconditionally, but instead check for what we
> advertise to the guest and handle it accordingly.
Can we go a step further and just stop advertising RAS to guests? I don't
expect VMs to gain much from our RAZ/WI implementation. Conditional
RAZ/WI would still be helpful in this case for migrated VMs that have
'seen' the feature.
--
Thanks,
Oliver
Hi Oliver,
On 15/01/2024 14:47, Oliver Upton wrote:
> On Wed, Jan 10, 2024 at 12:20:30PM +0000, Marc Zyngier wrote:
>> If my reading of the ARM ARM is correct, these registers only exist if
>> FEAT_RASv1p1 is implemented. Which means that we shouldn't handle
>> those as RAZ/WI unconditionally, but instead check for what we
>> advertise to the guest and handle it accordingly.
>
> Can we go a step further and just stop advertising RAS to guests? I don't
> expect VMs to gain much from our RAZ/WI implementation.
These CPU registers would describe the error in a kernel-first setup, but firmware-first
has its own in-memory way of doing that.
The CPU features indicates the IESB feature and ESB-instruction exist to fence errors, and
that the CPU uses the ESR_ELx.{S,A}ET bits to describe the CPU state after an error. These
are all useful as part of the notification of an error, be that kernel-first or
firmware-first.
When its supported by the hardware, the VMM can inject an asynchronous external abort
using KVM_GET_VCPU_EVENTS - otherwise the ESR_ELx.ISS bits are all imp-def, meaning all
errors are catastrophic.
Doing this would skip save/restore of VDISR_EL2, is there any other reason to do it?
> Conditional
> RAZ/WI would still be helpful in this case for migrated VMs that have
> 'seen' the feature.
Thanks,
James
Hi James,
On Mon, Jan 15, 2024 at 05:21:19PM +0000, James Morse wrote:
> Hi Oliver,
>
> On 15/01/2024 14:47, Oliver Upton wrote:
> > On Wed, Jan 10, 2024 at 12:20:30PM +0000, Marc Zyngier wrote:
> >> If my reading of the ARM ARM is correct, these registers only exist if
> >> FEAT_RASv1p1 is implemented. Which means that we shouldn't handle
> >> those as RAZ/WI unconditionally, but instead check for what we
> >> advertise to the guest and handle it accordingly.
> >
> > Can we go a step further and just stop advertising RAS to guests? I don't
> > expect VMs to gain much from our RAZ/WI implementation.
>
> These CPU registers would describe the error in a kernel-first setup, but firmware-first
> has its own in-memory way of doing that.
>
> The CPU features indicates the IESB feature and ESB-instruction exist to fence errors, and
> that the CPU uses the ESR_ELx.{S,A}ET bits to describe the CPU state after an error. These
> are all useful as part of the notification of an error, be that kernel-first or
> firmware-first.
>
> When its supported by the hardware, the VMM can inject an asynchronous external abort
> using KVM_GET_VCPU_EVENTS - otherwise the ESR_ELx.ISS bits are all imp-def, meaning all
> errors are catastrophic.
>
> Doing this would skip save/restore of VDISR_EL2, is there any other reason to do it?
Forgive me, had the blinders on and was thinking only of the error
record interface, not ESB/DISR. In that context it makes a lot less
sense to hide RAS from guests, especially if the guest depends on ESB
being a NOP if the hardware doesn't support it.
--
Thanks,
Oliver