* Re: [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs
2023-10-13 22:33 [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs Marc Zyngier
@ 2023-10-14 3:32 ` Oliver Upton
2023-10-17 18:50 ` Marc Zyngier
2023-10-16 14:15 ` Miguel Luis
2023-10-16 14:58 ` Eric Auger
2 siblings, 1 reply; 7+ messages in thread
From: Oliver Upton @ 2023-10-14 3:32 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, kvm, James Morse, Suzuki K Poulose, Zenghui Yu
On Fri, Oct 13, 2023 at 11:33:11PM +0100, Marc Zyngier wrote:
> DBGVCR32_EL2, DACR32_EL2, IFSR32_EL2 and FPEXC32_EL2 are required to
> UNDEF when AArch32 isn't implemented, which is definitely the case when
> running NV.
>
> Given that this is the only case where these registers can trap,
> unconditionally inject an UNDEF exception.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
If you intend to send this as a fix for 6.6:
Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
Otherwise it is on the stack of patches I'll pick up for 6.7
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs
2023-10-14 3:32 ` Oliver Upton
@ 2023-10-17 18:50 ` Marc Zyngier
0 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2023-10-17 18:50 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, linux-arm-kernel, kvm, James Morse, Suzuki K Poulose, Zenghui Yu
On Sat, 14 Oct 2023 04:32:46 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Fri, Oct 13, 2023 at 11:33:11PM +0100, Marc Zyngier wrote:
> > DBGVCR32_EL2, DACR32_EL2, IFSR32_EL2 and FPEXC32_EL2 are required to
> > UNDEF when AArch32 isn't implemented, which is definitely the case when
> > running NV.
> >
> > Given that this is the only case where these registers can trap,
> > unconditionally inject an UNDEF exception.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
>
> If you intend to send this as a fix for 6.6:
>
> Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
>
> Otherwise it is on the stack of patches I'll pick up for 6.7
Thanks.
I've queued it as a fix for now, together with Miguel's NV fix
series.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs
2023-10-13 22:33 [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs Marc Zyngier
2023-10-14 3:32 ` Oliver Upton
@ 2023-10-16 14:15 ` Miguel Luis
2023-10-16 14:28 ` Marc Zyngier
2023-10-16 14:58 ` Eric Auger
2 siblings, 1 reply; 7+ messages in thread
From: Miguel Luis @ 2023-10-16 14:15 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, kvm, James Morse, Suzuki K Poulose,
Oliver Upton, Zenghui Yu
Hi Marc,
> On 13 Oct 2023, at 22:33, Marc Zyngier <maz@kernel.org> wrote:
>
> DBGVCR32_EL2, DACR32_EL2, IFSR32_EL2 and FPEXC32_EL2 are required to
> UNDEF when AArch32 isn't implemented, which is definitely the case when
> running NV.
>
> Given that this is the only case where these registers can trap,
> unconditionally inject an UNDEF exception.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/kvm/sys_regs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 0afd6136e275..0071ccccaf00 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1961,7 +1961,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> // DBGDTR[TR]X_EL0 share the same encoding
> { SYS_DESC(SYS_DBGDTRTX_EL0), trap_raz_wi },
>
> - { SYS_DESC(SYS_DBGVCR32_EL2), NULL, reset_val, DBGVCR32_EL2, 0 },
> + { SYS_DESC(SYS_DBGVCR32_EL2), trap_undef, reset_val, DBGVCR32_EL2, 0 },
>
> { SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
>
> @@ -2380,18 +2380,18 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> EL2_REG(VTTBR_EL2, access_rw, reset_val, 0),
> EL2_REG(VTCR_EL2, access_rw, reset_val, 0),
>
> - { SYS_DESC(SYS_DACR32_EL2), NULL, reset_unknown, DACR32_EL2 },
> + { SYS_DESC(SYS_DACR32_EL2), trap_undef, reset_unknown, DACR32_EL2 },
> EL2_REG(HDFGRTR_EL2, access_rw, reset_val, 0),
> EL2_REG(HDFGWTR_EL2, access_rw, reset_val, 0),
> EL2_REG(SPSR_EL2, access_rw, reset_val, 0),
> EL2_REG(ELR_EL2, access_rw, reset_val, 0),
> { SYS_DESC(SYS_SP_EL1), access_sp_el1},
>
> - { SYS_DESC(SYS_IFSR32_EL2), NULL, reset_unknown, IFSR32_EL2 },
> + { SYS_DESC(SYS_IFSR32_EL2), trap_undef, reset_unknown, IFSR32_EL2 },
> EL2_REG(AFSR0_EL2, access_rw, reset_val, 0),
> EL2_REG(AFSR1_EL2, access_rw, reset_val, 0),
> EL2_REG(ESR_EL2, access_rw, reset_val, 0),
> - { SYS_DESC(SYS_FPEXC32_EL2), NULL, reset_val, FPEXC32_EL2, 0x700 },
> + { SYS_DESC(SYS_FPEXC32_EL2), trap_undef, reset_val, FPEXC32_EL2, 0x700 },
>
Should SDER32_EL2 be considered to this same list?
Thanks,
Miguel
> EL2_REG(FAR_EL2, access_rw, reset_val, 0),
> EL2_REG(HPFAR_EL2, access_rw, reset_val, 0),
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs
2023-10-16 14:15 ` Miguel Luis
@ 2023-10-16 14:28 ` Marc Zyngier
2023-10-16 15:01 ` Miguel Luis
0 siblings, 1 reply; 7+ messages in thread
From: Marc Zyngier @ 2023-10-16 14:28 UTC (permalink / raw)
To: Miguel Luis
Cc: kvmarm, linux-arm-kernel, kvm, James Morse, Suzuki K Poulose,
Oliver Upton, Zenghui Yu
On 16 October 2023 15:15:02 BST, Miguel Luis <miguel.luis@oracle.com> wrote:
>Hi Marc,
>
>> On 13 Oct 2023, at 22:33, Marc Zyngier <maz@kernel.org> wrote:
>>
>> DBGVCR32_EL2, DACR32_EL2, IFSR32_EL2 and FPEXC32_EL2 are required to
>> UNDEF when AArch32 isn't implemented, which is definitely the case when
>> running NV.
>>
>> Given that this is the only case where these registers can trap,
>> unconditionally inject an UNDEF exception.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>> arch/arm64/kvm/sys_regs.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 0afd6136e275..0071ccccaf00 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1961,7 +1961,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>> // DBGDTR[TR]X_EL0 share the same encoding
>> { SYS_DESC(SYS_DBGDTRTX_EL0), trap_raz_wi },
>>
>> - { SYS_DESC(SYS_DBGVCR32_EL2), NULL, reset_val, DBGVCR32_EL2, 0 },
>> + { SYS_DESC(SYS_DBGVCR32_EL2), trap_undef, reset_val, DBGVCR32_EL2, 0 },
>>
>> { SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
>>
>> @@ -2380,18 +2380,18 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>> EL2_REG(VTTBR_EL2, access_rw, reset_val, 0),
>> EL2_REG(VTCR_EL2, access_rw, reset_val, 0),
>>
>> - { SYS_DESC(SYS_DACR32_EL2), NULL, reset_unknown, DACR32_EL2 },
>> + { SYS_DESC(SYS_DACR32_EL2), trap_undef, reset_unknown, DACR32_EL2 },
>> EL2_REG(HDFGRTR_EL2, access_rw, reset_val, 0),
>> EL2_REG(HDFGWTR_EL2, access_rw, reset_val, 0),
>> EL2_REG(SPSR_EL2, access_rw, reset_val, 0),
>> EL2_REG(ELR_EL2, access_rw, reset_val, 0),
>> { SYS_DESC(SYS_SP_EL1), access_sp_el1},
>>
>> - { SYS_DESC(SYS_IFSR32_EL2), NULL, reset_unknown, IFSR32_EL2 },
>> + { SYS_DESC(SYS_IFSR32_EL2), trap_undef, reset_unknown, IFSR32_EL2 },
>> EL2_REG(AFSR0_EL2, access_rw, reset_val, 0),
>> EL2_REG(AFSR1_EL2, access_rw, reset_val, 0),
>> EL2_REG(ESR_EL2, access_rw, reset_val, 0),
>> - { SYS_DESC(SYS_FPEXC32_EL2), NULL, reset_val, FPEXC32_EL2, 0x700 },
>> + { SYS_DESC(SYS_FPEXC32_EL2), trap_undef, reset_val, FPEXC32_EL2, 0x700 },
>>
>
>Should SDER32_EL2 be considered to this same list?
>
This wouldn't make much sense.
This register is only available when running in secure mode, and KVM is firmly non-secure.
Thanks,
M.
Jazz is not dead, it just smells funny
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs
2023-10-16 14:28 ` Marc Zyngier
@ 2023-10-16 15:01 ` Miguel Luis
0 siblings, 0 replies; 7+ messages in thread
From: Miguel Luis @ 2023-10-16 15:01 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, linux-arm-kernel, kvm, James Morse, Suzuki K Poulose,
Oliver Upton, Zenghui Yu
Hi Marc,
> On 16 Oct 2023, at 14:28, Marc Zyngier <maz@kernel.org> wrote:
>
>
>
> On 16 October 2023 15:15:02 BST, Miguel Luis <miguel.luis@oracle.com> wrote:
>> Hi Marc,
>>
>>> On 13 Oct 2023, at 22:33, Marc Zyngier <maz@kernel.org> wrote:
>>>
>>> DBGVCR32_EL2, DACR32_EL2, IFSR32_EL2 and FPEXC32_EL2 are required to
>>> UNDEF when AArch32 isn't implemented, which is definitely the case when
>>> running NV.
>>>
>>> Given that this is the only case where these registers can trap,
>>> unconditionally inject an UNDEF exception.
>>>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>> ---
>>> arch/arm64/kvm/sys_regs.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index 0afd6136e275..0071ccccaf00 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -1961,7 +1961,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>>> // DBGDTR[TR]X_EL0 share the same encoding
>>> { SYS_DESC(SYS_DBGDTRTX_EL0), trap_raz_wi },
>>>
>>> - { SYS_DESC(SYS_DBGVCR32_EL2), NULL, reset_val, DBGVCR32_EL2, 0 },
>>> + { SYS_DESC(SYS_DBGVCR32_EL2), trap_undef, reset_val, DBGVCR32_EL2, 0 },
>>>
>>> { SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
>>>
>>> @@ -2380,18 +2380,18 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>>> EL2_REG(VTTBR_EL2, access_rw, reset_val, 0),
>>> EL2_REG(VTCR_EL2, access_rw, reset_val, 0),
>>>
>>> - { SYS_DESC(SYS_DACR32_EL2), NULL, reset_unknown, DACR32_EL2 },
>>> + { SYS_DESC(SYS_DACR32_EL2), trap_undef, reset_unknown, DACR32_EL2 },
>>> EL2_REG(HDFGRTR_EL2, access_rw, reset_val, 0),
>>> EL2_REG(HDFGWTR_EL2, access_rw, reset_val, 0),
>>> EL2_REG(SPSR_EL2, access_rw, reset_val, 0),
>>> EL2_REG(ELR_EL2, access_rw, reset_val, 0),
>>> { SYS_DESC(SYS_SP_EL1), access_sp_el1},
>>>
>>> - { SYS_DESC(SYS_IFSR32_EL2), NULL, reset_unknown, IFSR32_EL2 },
>>> + { SYS_DESC(SYS_IFSR32_EL2), trap_undef, reset_unknown, IFSR32_EL2 },
>>> EL2_REG(AFSR0_EL2, access_rw, reset_val, 0),
>>> EL2_REG(AFSR1_EL2, access_rw, reset_val, 0),
>>> EL2_REG(ESR_EL2, access_rw, reset_val, 0),
>>> - { SYS_DESC(SYS_FPEXC32_EL2), NULL, reset_val, FPEXC32_EL2, 0x700 },
>>> + { SYS_DESC(SYS_FPEXC32_EL2), trap_undef, reset_val, FPEXC32_EL2, 0x700 },
>>>
>>
>> Should SDER32_EL2 be considered to this same list?
>>
>
> This wouldn't make much sense.
>
> This register is only available when running in secure mode, and KVM is firmly non-secure.
>
Thanks for clarifying.
Miguel
> Thanks,
>
> M.
>
> Jazz is not dead, it just smells funny
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs
2023-10-13 22:33 [PATCH] KVM: arm64: Do not let a L1 hypervisor access the *32_EL2 sysregs Marc Zyngier
2023-10-14 3:32 ` Oliver Upton
2023-10-16 14:15 ` Miguel Luis
@ 2023-10-16 14:58 ` Eric Auger
2 siblings, 0 replies; 7+ messages in thread
From: Eric Auger @ 2023-10-16 14:58 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, linux-arm-kernel, kvm
Cc: James Morse, Suzuki K Poulose, Oliver Upton, Zenghui Yu
Hi Marc,
On 10/14/23 00:33, Marc Zyngier wrote:
> DBGVCR32_EL2, DACR32_EL2, IFSR32_EL2 and FPEXC32_EL2 are required to
> UNDEF when AArch32 isn't implemented, which is definitely the case when
> running NV.
>
> Given that this is the only case where these registers can trap,
> unconditionally inject an UNDEF exception.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> arch/arm64/kvm/sys_regs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 0afd6136e275..0071ccccaf00 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1961,7 +1961,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> // DBGDTR[TR]X_EL0 share the same encoding
> { SYS_DESC(SYS_DBGDTRTX_EL0), trap_raz_wi },
>
> - { SYS_DESC(SYS_DBGVCR32_EL2), NULL, reset_val, DBGVCR32_EL2, 0 },
> + { SYS_DESC(SYS_DBGVCR32_EL2), trap_undef, reset_val, DBGVCR32_EL2, 0 },
>
> { SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
>
> @@ -2380,18 +2380,18 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> EL2_REG(VTTBR_EL2, access_rw, reset_val, 0),
> EL2_REG(VTCR_EL2, access_rw, reset_val, 0),
>
> - { SYS_DESC(SYS_DACR32_EL2), NULL, reset_unknown, DACR32_EL2 },
> + { SYS_DESC(SYS_DACR32_EL2), trap_undef, reset_unknown, DACR32_EL2 },
> EL2_REG(HDFGRTR_EL2, access_rw, reset_val, 0),
> EL2_REG(HDFGWTR_EL2, access_rw, reset_val, 0),
> EL2_REG(SPSR_EL2, access_rw, reset_val, 0),
> EL2_REG(ELR_EL2, access_rw, reset_val, 0),
> { SYS_DESC(SYS_SP_EL1), access_sp_el1},
>
> - { SYS_DESC(SYS_IFSR32_EL2), NULL, reset_unknown, IFSR32_EL2 },
> + { SYS_DESC(SYS_IFSR32_EL2), trap_undef, reset_unknown, IFSR32_EL2 },
> EL2_REG(AFSR0_EL2, access_rw, reset_val, 0),
> EL2_REG(AFSR1_EL2, access_rw, reset_val, 0),
> EL2_REG(ESR_EL2, access_rw, reset_val, 0),
> - { SYS_DESC(SYS_FPEXC32_EL2), NULL, reset_val, FPEXC32_EL2, 0x700 },
> + { SYS_DESC(SYS_FPEXC32_EL2), trap_undef, reset_val, FPEXC32_EL2, 0x700 },
>
> EL2_REG(FAR_EL2, access_rw, reset_val, 0),
> EL2_REG(HPFAR_EL2, access_rw, reset_val, 0),
^ permalink raw reply [flat|nested] 7+ messages in thread