Re: [PATCH v3 00/42] KVM: arm64: Revamp Fine Grained Trap handling

[Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>]

Hi Marc,

On 26-04-2025 17:57, Marc Zyngier wrote:
> This is yet another version of the series last posted at [1].
> 
> The eagled eye reviewer will have noticed that since v2, the series
> has more or less doubled in size for any reasonable metric (number of
> patches, number of lines added or deleted). It is therefore pretty
> urgent that this gets either merged or forgotten! ;-)
> 
> See the change log below for the details -- most of it is related to
> FGT2 (and its rather large dependencies) being added.
> 
> * From v2:
> 
>    - Added comprehensive support for FEAT_FGT2, as the host kernel is
>      now making use of these registers, without any form of context
>      switch in KVM. What could possibly go wrong?
> 
>    - Reworked some of the FGT description and handling primitives,
>      reducing the boilerplate code and tables that get added over time.
> 
>    - Rebased on 6.15-rc3.
> 
> [1]: https://lore.kernel.org/r/20250310122505.2857610-1-maz@kernel.org
> 
> Marc Zyngier (41):
>    arm64: sysreg: Add ID_AA64ISAR1_EL1.LS64 encoding for FEAT_LS64WB
>    arm64: sysreg: Update ID_AA64MMFR4_EL1 description
>    arm64: sysreg: Add layout for HCR_EL2
>    arm64: sysreg: Replace HGFxTR_EL2 with HFG{R,W}TR_EL2
>    arm64: sysreg: Update ID_AA64PFR0_EL1 description
>    arm64: sysreg: Update PMSIDR_EL1 description
>    arm64: sysreg: Update TRBIDR_EL1 description
>    arm64: sysreg: Add registers trapped by HFG{R,W}TR2_EL2
>    arm64: sysreg: Add registers trapped by HDFG{R,W}TR2_EL2
>    arm64: sysreg: Add system instructions trapped by HFGIRT2_EL2
>    arm64: Remove duplicated sysreg encodings
>    arm64: tools: Resync sysreg.h
>    arm64: Add syndrome information for trapped LD64B/ST64B{,V,V0}
>    arm64: Add FEAT_FGT2 capability
>    KVM: arm64: Tighten handling of unknown FGT groups
>    KVM: arm64: Simplify handling of negative FGT bits
>    KVM: arm64: Handle trapping of FEAT_LS64* instructions
>    KVM: arm64: Restrict ACCDATA_EL1 undef to FEAT_ST64_ACCDATA being
>      disabled
>    KVM: arm64: Don't treat HCRX_EL2 as a FGT register
>    KVM: arm64: Plug FEAT_GCS handling
>    KVM: arm64: Compute FGT masks from KVM's own FGT tables
>    KVM: arm64: Add description of FGT bits leading to EC!=0x18
>    KVM: arm64: Use computed masks as sanitisers for FGT registers
>    KVM: arm64: Propagate FGT masks to the nVHE hypervisor
>    KVM: arm64: Use computed FGT masks to setup FGT registers
>    KVM: arm64: Remove hand-crafted masks for FGT registers
>    KVM: arm64: Use KVM-specific HCRX_EL2 RES0 mask
>    KVM: arm64: Handle PSB CSYNC traps
>    KVM: arm64: Switch to table-driven FGU configuration
>    KVM: arm64: Validate FGT register descriptions against RES0 masks
>    KVM: arm64: Use FGT feature maps to drive RES0 bits
>    KVM: arm64: Allow kvm_has_feat() to take variable arguments
>    KVM: arm64: Use HCRX_EL2 feature map to drive fixed-value bits
>    KVM: arm64: Use HCR_EL2 feature map to drive fixed-value bits
>    KVM: arm64: Add FEAT_FGT2 registers to the VNCR page
>    KVM: arm64: Add sanitisation for FEAT_FGT2 registers
>    KVM: arm64: Add trap routing for FEAT_FGT2 registers
>    KVM: arm64: Add context-switch for FEAT_FGT2 registers
>    KVM: arm64: Allow sysreg ranges for FGT descriptors
>    KVM: arm64: Add FGT descriptors for FEAT_FGT2
>    KVM: arm64: Handle TSB CSYNC traps
> 
> Mark Rutland (1):
>    KVM: arm64: Unconditionally configure fine-grain traps
> 
>   arch/arm64/include/asm/el2_setup.h      |   14 +-
>   arch/arm64/include/asm/esr.h            |   10 +-
>   arch/arm64/include/asm/kvm_arm.h        |  186 ++--
>   arch/arm64/include/asm/kvm_host.h       |   56 +-
>   arch/arm64/include/asm/sysreg.h         |   26 +-
>   arch/arm64/include/asm/vncr_mapping.h   |    5 +
>   arch/arm64/kernel/cpufeature.c          |    7 +
>   arch/arm64/kvm/Makefile                 |    2 +-
>   arch/arm64/kvm/arm.c                    |   13 +
>   arch/arm64/kvm/config.c                 | 1085 +++++++++++++++++++++++
>   arch/arm64/kvm/emulate-nested.c         |  580 ++++++++----
>   arch/arm64/kvm/handle_exit.c            |   77 ++
>   arch/arm64/kvm/hyp/include/hyp/switch.h |  158 ++--
>   arch/arm64/kvm/hyp/nvhe/switch.c        |   12 +
>   arch/arm64/kvm/hyp/vgic-v3-sr.c         |    8 +-
>   arch/arm64/kvm/nested.c                 |  223 +----
>   arch/arm64/kvm/sys_regs.c               |   68 +-
>   arch/arm64/tools/cpucaps                |    1 +
>   arch/arm64/tools/sysreg                 | 1002 ++++++++++++++++++++-
>   tools/arch/arm64/include/asm/sysreg.h   |   65 +-
>   20 files changed, 2888 insertions(+), 710 deletions(-)
>   create mode 100644 arch/arm64/kvm/config.c
> 

I am trying nv-next branch and I believe these FGT related changes are 
merged. With this, selftest arm64/set_id_regs is failing. From initial 
debug it seems, the register access of SYS_CTR_EL0, SYS_MIDR_EL1, 
SYS_REVIDR_EL1 and SYS_AIDR_EL1 from guest_code is resulting in trap to 
EL2 (HCR_ID1,ID2 are set) and is getting forwarded back to EL1, since 
EL1 sync handler is not installed in the test code, resulting in 
hang(endless guest_exit/entry).

It is due to function "triage_sysreg_trap" is returning true.

When guest_code is in EL1 (default case) it is due to return in below if.

  if (tc.fgt != __NO_FGT_GROUP__ &&
             (vcpu->kvm->arch.fgu[tc.fgt] & BIT(tc.bit))) {
                 kvm_inject_undefined(vcpu);
                 return true;
         }

IMO, Host should return the value of these sysreg read instead of 
forwarding the trap to guest or something more to be added to testcode?

-- 
Thanks,
Gk