On Wed, Jun 7, 2023 at 5:53 PM Nicholas Piggin <npiggin@xxxxxxxxx> wrote:
[snip]
>
> The general idea is fine, some of the names could use a bit of
> improvement. What's a BOOK3S_WRAPPER for example, is it not a
> VCPU_WRAPPER, or alternatively why isn't a VCORE_WRAPPER Book3S
> as well?
Yeah the names are not great.
I didn't call it VCPU_WRAPPER because I wanted to keep separate
BOOK3S_WRAPPER for book3s registers
HV_WRAPPER for hv specific registers
I will change it to something like you suggested.
[snip]
>
> Stray hunk I think.
Yep.
>
> > @@ -957,10 +957,32 @@ static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val) \
> > vcpu->arch.shared->reg = cpu_to_le##size(val); \
> > } \
> >
> > +#define SHARED_CACHE_WRAPPER_GET(reg, size) \
> > +static inline u##size kvmppc_get_##reg(struct kvm_vcpu *vcpu) \
> > +{ \
> > + if (kvmppc_shared_big_endian(vcpu)) \
> > + return be##size##_to_cpu(vcpu->arch.shared->reg); \
> > + else \
> > + return le##size##_to_cpu(vcpu->arch.shared->reg); \
> > +} \
> > +
> > +#define SHARED_CACHE_WRAPPER_SET(reg, size) \
> > +static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val) \
> > +{ \
> > + if (kvmppc_shared_big_endian(vcpu)) \
> > + vcpu->arch.shared->reg = cpu_to_be##size(val); \
> > + else \
> > + vcpu->arch.shared->reg = cpu_to_le##size(val); \
> > +} \
> > +
> > #define SHARED_WRAPPER(reg, size) \
> > SHARED_WRAPPER_GET(reg, size) \
> > SHARED_WRAPPER_SET(reg, size) \
> >
> > +#define SHARED_CACHE_WRAPPER(reg, size) \
> > + SHARED_CACHE_WRAPPER_GET(reg, size) \
> > + SHARED_CACHE_WRAPPER_SET(reg, size) \
>
> SHARED_CACHE_WRAPPER that does the same thing as SHARED_WRAPPER.
That changes once the guest state buffer IDs are included in a later
patch.
>
> I know some of the names are a but crufty but it's probably a good time
> to rethink them a bit.
>
> KVMPPC_VCPU_SHARED_REG_ACCESSOR or something like that. A few
> more keystrokes could help imensely.
Yes, I will do something like that, for the BOOK3S_WRAPPER and
HV_WRAPPER
too.
>
> > diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
> > index 34f1db212824..34bc0a8a1288 100644
> > --- a/arch/powerpc/kvm/book3s_hv_p9_entry.c
> > +++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
> > @@ -305,7 +305,7 @@ static void switch_mmu_to_guest_radix(struct kvm *kvm, struct kvm_vcpu *vcpu, u6
> > u32 pid;
> >
> > lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
> > - pid = vcpu->arch.pid;
> > + pid = kvmppc_get_pid(vcpu);
> >
> > /*
> > * Prior memory accesses to host PID Q3 must be completed before we
>
> Could add some accessors for get_lpid / get_guest_id which check for the
> correct KVM mode maybe.
True.
Thanks,
Jordan
>
> Thanks,
> Nick
>
