| Message ID | 20210323010305.1045293-5-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: PPC: Book3S: C-ify the P9 entry/exit code | expand |
On 23/03/2021 12:02, Nicholas Piggin wrote: > This bit only applies to hash partitions. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > arch/powerpc/kvm/book3s_hv.c | 6 ++++++ > arch/powerpc/kvm/book3s_hv_nested.c | 3 +-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index c5de7e3f22b6..1ffb0902e779 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -1645,6 +1645,12 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu, > */ > unsigned long kvmppc_filter_lpcr_hv(struct kvmppc_vcore *vc, unsigned long lpcr) > { > + struct kvm *kvm = vc->kvm; > + > + /* LPCR_TC only applies to HPT guests */ > + if (kvm_is_radix(kvm)) > + lpcr &= ~LPCR_TC; > + > /* On POWER8 and above, userspace can modify AIL */ > if (!cpu_has_feature(CPU_FTR_ARCH_207S)) > lpcr &= ~LPCR_AIL; > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c > index f7b441b3eb17..851e3f527eb2 100644 > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -140,8 +140,7 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) > /* > * Don't let L1 change LPCR bits for the L2 except these: > */ > - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD | > - LPCR_LPES | LPCR_MER; > + mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER; > hr->lpcr = kvmppc_filter_lpcr_hv(vc, > (vc->lpcr & ~mask) | (hr->lpcr & mask)); > >
On Tue, Mar 23, 2021 at 11:02:23AM +1000, Nicholas Piggin wrote: > This bit only applies to hash partitions. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/kvm/book3s_hv.c | 6 ++++++ > arch/powerpc/kvm/book3s_hv_nested.c | 3 +-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index c5de7e3f22b6..1ffb0902e779 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -1645,6 +1645,12 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu, > */ > unsigned long kvmppc_filter_lpcr_hv(struct kvmppc_vcore *vc, unsigned long lpcr) > { > + struct kvm *kvm = vc->kvm; > + > + /* LPCR_TC only applies to HPT guests */ > + if (kvm_is_radix(kvm)) > + lpcr &= ~LPCR_TC; I'm not sure I see any benefit from this, and it is a little extra complexity. > /* On POWER8 and above, userspace can modify AIL */ > if (!cpu_has_feature(CPU_FTR_ARCH_207S)) > lpcr &= ~LPCR_AIL; > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c > index f7b441b3eb17..851e3f527eb2 100644 > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -140,8 +140,7 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) > /* > * Don't let L1 change LPCR bits for the L2 except these: > */ > - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD | > - LPCR_LPES | LPCR_MER; > + mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER; Doesn't this make it completely impossible to set TC for any guest? Paul.
Excerpts from Paul Mackerras's message of March 31, 2021 2:34 pm: > On Tue, Mar 23, 2021 at 11:02:23AM +1000, Nicholas Piggin wrote: >> This bit only applies to hash partitions. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> arch/powerpc/kvm/book3s_hv.c | 6 ++++++ >> arch/powerpc/kvm/book3s_hv_nested.c | 3 +-- >> 2 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >> index c5de7e3f22b6..1ffb0902e779 100644 >> --- a/arch/powerpc/kvm/book3s_hv.c >> +++ b/arch/powerpc/kvm/book3s_hv.c >> @@ -1645,6 +1645,12 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu, >> */ >> unsigned long kvmppc_filter_lpcr_hv(struct kvmppc_vcore *vc, unsigned long lpcr) >> { >> + struct kvm *kvm = vc->kvm; >> + >> + /* LPCR_TC only applies to HPT guests */ >> + if (kvm_is_radix(kvm)) >> + lpcr &= ~LPCR_TC; > > I'm not sure I see any benefit from this, and it is a little extra > complexity. Principle of allowing a guest to mess with as little HV state as possible (but not littler), which I think is a good one to follow. > >> /* On POWER8 and above, userspace can modify AIL */ >> if (!cpu_has_feature(CPU_FTR_ARCH_207S)) >> lpcr &= ~LPCR_AIL; >> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c >> index f7b441b3eb17..851e3f527eb2 100644 >> --- a/arch/powerpc/kvm/book3s_hv_nested.c >> +++ b/arch/powerpc/kvm/book3s_hv_nested.c >> @@ -140,8 +140,7 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) >> /* >> * Don't let L1 change LPCR bits for the L2 except these: >> */ >> - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD | >> - LPCR_LPES | LPCR_MER; >> + mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER; > > Doesn't this make it completely impossible to set TC for any guest? Argh, yes Fabiano pointed this out in an earlier rev and I didn't fix this hunk after adding the filter bit. Thanks. Thanks, Nick
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index c5de7e3f22b6..1ffb0902e779 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1645,6 +1645,12 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu, */ unsigned long kvmppc_filter_lpcr_hv(struct kvmppc_vcore *vc, unsigned long lpcr) { + struct kvm *kvm = vc->kvm; + + /* LPCR_TC only applies to HPT guests */ + if (kvm_is_radix(kvm)) + lpcr &= ~LPCR_TC; + /* On POWER8 and above, userspace can modify AIL */ if (!cpu_has_feature(CPU_FTR_ARCH_207S)) lpcr &= ~LPCR_AIL; diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c index f7b441b3eb17..851e3f527eb2 100644 --- a/arch/powerpc/kvm/book3s_hv_nested.c +++ b/arch/powerpc/kvm/book3s_hv_nested.c @@ -140,8 +140,7 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) /* * Don't let L1 change LPCR bits for the L2 except these: */ - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD | - LPCR_LPES | LPCR_MER; + mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER; hr->lpcr = kvmppc_filter_lpcr_hv(vc, (vc->lpcr & ~mask) | (hr->lpcr & mask));
This bit only applies to hash partitions. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kvm/book3s_hv.c | 6 ++++++ arch/powerpc/kvm/book3s_hv_nested.c | 3 +-- 2 files changed, 7 insertions(+), 2 deletions(-)