Excerpts from Nicholas Piggin's message of April 1, 2021 7:32 pm: > Excerpts from Paul Mackerras's message of March 31, 2021 2:08 pm: >> On Tue, Mar 23, 2021 at 11:02:21AM +1000, Nicholas Piggin wrote: >>> Guest LPCR depends on hardware type, and future changes will add >>> restrictions based on errata and guest MMU mode. Move this logic >>> to a common function and use it for the cases where the guest >>> wants to update its LPCR (or the LPCR of a nested guest). >> >> [snip] >> >>> @@ -4641,8 +4662,9 @@ void kvmppc_update_lpcr(struct kvm *kvm, unsigned long lpcr, unsigned long mask) >>> struct kvmppc_vcore *vc = kvm->arch.vcores[i]; >>> if (!vc) >>> continue; >>> + >>> spin_lock(&vc->lock); >>> - vc->lpcr = (vc->lpcr & ~mask) | lpcr; >>> + vc->lpcr = kvmppc_filter_lpcr_hv(vc, (vc->lpcr & ~mask) | lpcr); >> >> This change seems unnecessary, since kvmppc_update_lpcr is called only >> to update MMU configuration bits, not as a result of any action by >> userspace or a nested hypervisor. It's also beyond the scope of what >> was mentioned in the commit message. > > I didn't think it was outside the spirit of the patch, but yes > only the guest update LPCR case was enumerated. Would it be more > consistent to add it to the changelog and leave it in here or would > you prefer it left out until there is a real use? On second thoughts, I already left at least one other place without such a check, so I now tend to agree with you. But I instead added a test that just ensures the host is not out of synch with itself in terms of what it can set the LPCR to. Thanks, Nick