| Message ID | 20230216160012.272345-4-kristina.martsenko@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: support Armv8.8 memcpy instructions in userspace | expand |
On Thu, Feb 16, 2023 at 04:00:05PM +0000, Kristina Martsenko wrote: > After this change, SMPME is no longer set for guests, which should have > no effect as SME is currently disabled for guests. SMPME is mainly set for the benefit of any future guests, it is used to turn off SME priorities for guests. While it's true that we don't have any guests support yet it's there as safety to ensure we don't forget to enable it later on or something otherwise goes wrong. We don't need priority mapping on the host since we don't have any priority mapping support at all, and it's difficult to see a reason why we would want to remap our own priorities in the host.
On Thu, 16 Feb 2023 16:00:05 +0000, Kristina Martsenko <kristina.martsenko@arm.com> wrote: > > Switch the HCRX_EL2 register between host and guest configurations, in > order to enable different features in the host and guest. > > Note that the guest flags are only set if all CPUs have HCRX_EL2. > Asymmetric systems where only some CPUs have HCRX_EL2 are not supported > and will result in guests running with the host flags set (and a "SANITY > CHECK" warning printed for the host). > > After this change, SMPME is no longer set for guests, which should have > no effect as SME is currently disabled for guests. Why not preserve the behaviour by propagating the flag into the guest setup? > > Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> > --- > > I wasn't sure what to do about asymmetric systems. It seems a bit > fragile, maybe someone has a better idea? I would simply prevent these CPUs from booting if they come after a primary CPU that has the feature. These hypothetical asymmetric setups put a huge complexity on the kernel, and I'm worried that we're just giving implementers too much freedom. If someone comes up with that sort of stuff, they can write the patches themselves... Or do you know of any braindead setup involving an asymmetric FEAT_HCX implementation? Thanks, M.
On 16/02/2023 16:20, Mark Brown wrote: > On Thu, Feb 16, 2023 at 04:00:05PM +0000, Kristina Martsenko wrote: > >> After this change, SMPME is no longer set for guests, which should have >> no effect as SME is currently disabled for guests. > > SMPME is mainly set for the benefit of any future guests, it is > used to turn off SME priorities for guests. While it's true that > we don't have any guests support yet it's there as safety to > ensure we don't forget to enable it later on or something > otherwise goes wrong. We don't need priority mapping on the host > since we don't have any priority mapping support at all, and it's > difficult to see a reason why we would want to remap our own > priorities in the host. That makes sense. I'll fix this in v2 so that priority mapping is enabled for guests and disabled for the host. Thanks, Kristina
On 16/02/2023 16:35, Marc Zyngier wrote: > On Thu, 16 Feb 2023 16:00:05 +0000, > Kristina Martsenko <kristina.martsenko@arm.com> wrote: >> >> Switch the HCRX_EL2 register between host and guest configurations, in >> order to enable different features in the host and guest. >> >> Note that the guest flags are only set if all CPUs have HCRX_EL2. >> Asymmetric systems where only some CPUs have HCRX_EL2 are not supported >> and will result in guests running with the host flags set (and a "SANITY >> CHECK" warning printed for the host). >> >> After this change, SMPME is no longer set for guests, which should have >> no effect as SME is currently disabled for guests. > > Why not preserve the behaviour by propagating the flag into the guest > setup? I thought it made more sense to disable SMPME given that SME is not supported in guests yet (and that the existing behavior was just a side effect of not having support for switching HCRX), but I'd misunderstood what SMPME is for, and following Mark's explanation I'll actually preserve the behavior for guests, but now disable SMPME for the host instead (as SME priority mapping has no benefit in the host and is not intended to be used there). > >> >> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> >> --- >> >> I wasn't sure what to do about asymmetric systems. It seems a bit >> fragile, maybe someone has a better idea? > > I would simply prevent these CPUs from booting if they come after a > primary CPU that has the feature. I considered that but the concern I heard was that since virtualization is an optional feature then people may still want to use the system without it. > These hypothetical asymmetric setups > put a huge complexity on the kernel, and I'm worried that we're just > giving implementers too much freedom. > > If someone comes up with that sort of stuff, they can write the > patches themselves... I'll make it panic on a mismatch for now and it can be revisited in the future if such a system actually appears (which does seem very unlikely). > Or do you know of any braindead setup involving > an asymmetric FEAT_HCX implementation? Nope don't know of one, it was just hypothetical. Thanks, Kristina
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index caa31f4ab1cd..cd8dd307aaba 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -93,6 +93,7 @@ #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC) #define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H) +#define HCRX_GUEST_FLAGS 0 #define HCRX_HOST_FLAGS (HCRX_EL2_SMPME) /* TCR_EL2 Registers bits */ diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 07d37ff88a3f..a1bf2d879db5 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -129,6 +129,9 @@ static inline void ___activate_traps(struct kvm_vcpu *vcpu) if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN) && (hcr & HCR_VSE)) write_sysreg_s(vcpu->arch.vsesr_el2, SYS_VSESR_EL2); + + if (cpus_have_final_cap(ARM64_HAS_HCX)) + write_sysreg_s(HCRX_GUEST_FLAGS, SYS_HCRX_EL2); } static inline void ___deactivate_traps(struct kvm_vcpu *vcpu) @@ -143,6 +146,9 @@ static inline void ___deactivate_traps(struct kvm_vcpu *vcpu) vcpu->arch.hcr_el2 &= ~HCR_VSE; vcpu->arch.hcr_el2 |= read_sysreg(hcr_el2) & HCR_VSE; } + + if (cpus_have_final_cap(ARM64_HAS_HCX)) + write_sysreg_s(HCRX_HOST_FLAGS, SYS_HCRX_EL2); } static inline bool __populate_fault_info(struct kvm_vcpu *vcpu)
Switch the HCRX_EL2 register between host and guest configurations, in order to enable different features in the host and guest. Note that the guest flags are only set if all CPUs have HCRX_EL2. Asymmetric systems where only some CPUs have HCRX_EL2 are not supported and will result in guests running with the host flags set (and a "SANITY CHECK" warning printed for the host). After this change, SMPME is no longer set for guests, which should have no effect as SME is currently disabled for guests. Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com> --- I wasn't sure what to do about asymmetric systems. It seems a bit fragile, maybe someone has a better idea? arch/arm64/include/asm/kvm_arm.h | 1 + arch/arm64/kvm/hyp/include/hyp/switch.h | 6 ++++++ 2 files changed, 7 insertions(+)