mbox series

[v3,00/11] KVM: arm64: Fix handling of host fpsimd/sve state in protected mode

Message ID 20240528125914.277057-1-tabba@google.com (mailing list archive)
Headers show
Series KVM: arm64: Fix handling of host fpsimd/sve state in protected mode | expand

Message

Fuad Tabba May 28, 2024, 12:59 p.m. UTC
Changes since v2 [1]
- Rebased on Linux 6.10-rc1 (1613e604df0c)
- Apply suggestions/fixes suggested for V2 (Marc)
- Add an isb() to __hyp_sve_restore_guest()
- Squash patch that introduces kvm_host_sve_max_vl with following
  patch, since it's used there
- Some refactoring and tidying up
- Introduce and use sve_cond_update_zcr_vq_isb(), which only does
  an isb() if ZCR is updated (RFC, next to last patch)
- Remove sve_cond_update_zcr_vq_*, since it's not likely to help
  much (RFC, last patch)

With the KVM host data rework [2], handling of fpsimd and sve
state in protected mode is done at hyp. For protected VMs, we
don't want to leak any guest state to the host, including whether
a guest has used fpsimd/sve.

To complete the work started with the host data rework, in
regards to protected mode, ensure that the host's fpsimd context
and its sve context are restored on guest exit, since the rework
has hidden the fpsimd/sve state from the host.

This patch series eagerly restores the host fpsimd/sve state on
guest exit when running in protected mode, which happens only if
the guest has used fpsimd/sve. This means that the saving of the
state is lazy, similar to the behavior of KVM in other modes, but
the restoration of the host state is eager.

The last two patches are not essential to this patch series, and
the last one undoes the next-to-last. Please consider only one
(or neither) of these two patches for inclusion.

This series is based on Linux 6.10-rc1 (1613e604df0c).

Tested on qemu, with the kernel sve stress tests.

Cheers,
/fuad

[1] https://lore.kernel.org/all/20240521163720.3812851-1-tabba@google.com/
[2] https://lore.kernel.org/all/20240322170945.3292593-1-maz@kernel.org/

Fuad Tabba (11):
  KVM: arm64: Reintroduce __sve_save_state
  KVM: arm64: Abstract set/clear of CPTR_EL2 bits behind helper
  KVM: arm64: Specialize handling of host fpsimd state on trap
  KVM: arm64: Allocate memory mapped at hyp for host sve state in pKVM
  KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM
  KVM: arm64: Consolidate initializing the host data's fpsimd_state/sve
    in pKVM
  KVM: arm64: Refactor CPACR trap bit setting/clearing to use ELx format
  KVM: arm64: Add an isb before restoring guest sve state
  KVM: arm64: Do not use sve_cond_update_zcr updating with
    ZCR_ELx_LEN_MASK
  KVM: arm64: Do not perform an isb() if ZCR_EL2 isn't updated
  KVM: arm64: Drop sve_cond_update_zcr_vq_*

 arch/arm64/include/asm/el2_setup.h      |  6 +-
 arch/arm64/include/asm/fpsimd.h         | 11 ----
 arch/arm64/include/asm/kvm_arm.h        |  6 ++
 arch/arm64/include/asm/kvm_emulate.h    | 71 +++++++++++++++++++++--
 arch/arm64/include/asm/kvm_host.h       | 25 +++++++-
 arch/arm64/include/asm/kvm_hyp.h        |  2 +
 arch/arm64/include/asm/kvm_pkvm.h       |  9 +++
 arch/arm64/kvm/arm.c                    | 76 +++++++++++++++++++++++++
 arch/arm64/kvm/fpsimd.c                 |  8 +--
 arch/arm64/kvm/hyp/fpsimd.S             |  6 ++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 36 ++++++------
 arch/arm64/kvm/hyp/include/nvhe/pkvm.h  |  1 -
 arch/arm64/kvm/hyp/nvhe/hyp-main.c      | 75 +++++++++++++++++++++---
 arch/arm64/kvm/hyp/nvhe/pkvm.c          | 17 ++----
 arch/arm64/kvm/hyp/nvhe/setup.c         | 25 +++++++-
 arch/arm64/kvm/hyp/nvhe/switch.c        | 24 +++++++-
 arch/arm64/kvm/hyp/vhe/switch.c         | 12 ++--
 arch/arm64/kvm/reset.c                  |  3 +
 18 files changed, 342 insertions(+), 71 deletions(-)


base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0

Comments

Fuad Tabba May 28, 2024, 1:13 p.m. UTC | #1
Hi,

On Tue, May 28, 2024 at 1:59 PM Fuad Tabba <tabba@google.com> wrote:
>
> Changes since v2 [1]
> - Rebased on Linux 6.10-rc1 (1613e604df0c)
> - Apply suggestions/fixes suggested for V2 (Marc)
> - Add an isb() to __hyp_sve_restore_guest()
> - Squash patch that introduces kvm_host_sve_max_vl with following
>   patch, since it's used there
> - Some refactoring and tidying up
> - Introduce and use sve_cond_update_zcr_vq_isb(), which only does
>   an isb() if ZCR is updated (RFC, next to last patch)

Just realized that

"An indirect read of ZCR_EL1.LEN appears to occur in program order
relative to a direct write of the same register, without the need for
explicit synchronization."
https://developer.arm.com/documentation/ddi0595/2021-03/AArch64-Registers/ZCR-EL2--SVE-Control-Register--EL2-

I'll wait until I get comments on this series as it is before
respinning. Apologies for the spam.

Cheers,
/fuad

> - Remove sve_cond_update_zcr_vq_*, since it's not likely to help
>   much (RFC, last patch)
>
> With the KVM host data rework [2], handling of fpsimd and sve
> state in protected mode is done at hyp. For protected VMs, we
> don't want to leak any guest state to the host, including whether
> a guest has used fpsimd/sve.
>
> To complete the work started with the host data rework, in
> regards to protected mode, ensure that the host's fpsimd context
> and its sve context are restored on guest exit, since the rework
> has hidden the fpsimd/sve state from the host.
>
> This patch series eagerly restores the host fpsimd/sve state on
> guest exit when running in protected mode, which happens only if
> the guest has used fpsimd/sve. This means that the saving of the
> state is lazy, similar to the behavior of KVM in other modes, but
> the restoration of the host state is eager.
>
> The last two patches are not essential to this patch series, and
> the last one undoes the next-to-last. Please consider only one
> (or neither) of these two patches for inclusion.
>
> This series is based on Linux 6.10-rc1 (1613e604df0c).
>
> Tested on qemu, with the kernel sve stress tests.
>
> Cheers,
> /fuad
>
> [1] https://lore.kernel.org/all/20240521163720.3812851-1-tabba@google.com/
> [2] https://lore.kernel.org/all/20240322170945.3292593-1-maz@kernel.org/
>
> Fuad Tabba (11):
>   KVM: arm64: Reintroduce __sve_save_state
>   KVM: arm64: Abstract set/clear of CPTR_EL2 bits behind helper
>   KVM: arm64: Specialize handling of host fpsimd state on trap
>   KVM: arm64: Allocate memory mapped at hyp for host sve state in pKVM
>   KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM
>   KVM: arm64: Consolidate initializing the host data's fpsimd_state/sve
>     in pKVM
>   KVM: arm64: Refactor CPACR trap bit setting/clearing to use ELx format
>   KVM: arm64: Add an isb before restoring guest sve state
>   KVM: arm64: Do not use sve_cond_update_zcr updating with
>     ZCR_ELx_LEN_MASK
>   KVM: arm64: Do not perform an isb() if ZCR_EL2 isn't updated
>   KVM: arm64: Drop sve_cond_update_zcr_vq_*
>
>  arch/arm64/include/asm/el2_setup.h      |  6 +-
>  arch/arm64/include/asm/fpsimd.h         | 11 ----
>  arch/arm64/include/asm/kvm_arm.h        |  6 ++
>  arch/arm64/include/asm/kvm_emulate.h    | 71 +++++++++++++++++++++--
>  arch/arm64/include/asm/kvm_host.h       | 25 +++++++-
>  arch/arm64/include/asm/kvm_hyp.h        |  2 +
>  arch/arm64/include/asm/kvm_pkvm.h       |  9 +++
>  arch/arm64/kvm/arm.c                    | 76 +++++++++++++++++++++++++
>  arch/arm64/kvm/fpsimd.c                 |  8 +--
>  arch/arm64/kvm/hyp/fpsimd.S             |  6 ++
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 36 ++++++------
>  arch/arm64/kvm/hyp/include/nvhe/pkvm.h  |  1 -
>  arch/arm64/kvm/hyp/nvhe/hyp-main.c      | 75 +++++++++++++++++++++---
>  arch/arm64/kvm/hyp/nvhe/pkvm.c          | 17 ++----
>  arch/arm64/kvm/hyp/nvhe/setup.c         | 25 +++++++-
>  arch/arm64/kvm/hyp/nvhe/switch.c        | 24 +++++++-
>  arch/arm64/kvm/hyp/vhe/switch.c         | 12 ++--
>  arch/arm64/kvm/reset.c                  |  3 +
>  18 files changed, 342 insertions(+), 71 deletions(-)
>
>
> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
> --
> 2.45.1.288.g0e0cd299f1-goog
>
Oliver Upton May 30, 2024, 6:29 p.m. UTC | #2
On Tue, May 28, 2024 at 01:59:03PM +0100, Fuad Tabba wrote:
> Changes since v2 [1]
> - Rebased on Linux 6.10-rc1 (1613e604df0c)
> - Apply suggestions/fixes suggested for V2 (Marc)
> - Add an isb() to __hyp_sve_restore_guest()
> - Squash patch that introduces kvm_host_sve_max_vl with following
>   patch, since it's used there
> - Some refactoring and tidying up
> - Introduce and use sve_cond_update_zcr_vq_isb(), which only does
>   an isb() if ZCR is updated (RFC, next to last patch)
> - Remove sve_cond_update_zcr_vq_*, since it's not likely to help
>   much (RFC, last patch)
> 
> With the KVM host data rework [2], handling of fpsimd and sve
> state in protected mode is done at hyp. For protected VMs, we
> don't want to leak any guest state to the host, including whether
> a guest has used fpsimd/sve.
> 
> To complete the work started with the host data rework, in
> regards to protected mode, ensure that the host's fpsimd context
> and its sve context are restored on guest exit, since the rework
> has hidden the fpsimd/sve state from the host.
> 
> This patch series eagerly restores the host fpsimd/sve state on
> guest exit when running in protected mode, which happens only if
> the guest has used fpsimd/sve. This means that the saving of the
> state is lazy, similar to the behavior of KVM in other modes, but
> the restoration of the host state is eager.
> 
> The last two patches are not essential to this patch series, and
> the last one undoes the next-to-last. Please consider only one
> (or neither) of these two patches for inclusion.

For patches 1-7 (with the unnecessary isb()'s addressed):

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

I think we can do without the rest of the series for 6.10.

I also tested this on Neoverse V2.