Re: [RFC PATCH v2 1/6] KVM: PPC: Use getters and setters for vcpu register state
On Wed, Jun 7, 2023 at 5:53 PM Nicholas Piggin 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
>
Re: [RFC PATCH v2 1/6] KVM: PPC: Use getters and setters for vcpu register state
On Mon Jun 5, 2023 at 4:48 PM AEST, Jordan Niethe wrote:
> There are already some getter and setter functions used for accessing
> vcpu register state, e.g. kvmppc_get_pc(). There are also more
> complicated examples that are generated by macros like
> kvmppc_get_sprg0() which are generated by the SHARED_SPRNG_WRAPPER()
> macro.
>
> In the new PAPR API for nested guest partitions the L1 is required to
> communicate with the L0 to modify and read nested guest state.
>
> Prepare to support this by replacing direct accesses to vcpu register
> state with wrapper functions. Follow the existing pattern of using
> macros to generate individual wrappers. These wrappers will
> be augmented for supporting PAPR nested guests later.
>
> Signed-off-by: Jordan Niethe
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 68 +++-
> arch/powerpc/include/asm/kvm_ppc.h | 48 --
> arch/powerpc/kvm/book3s.c | 22 +--
> arch/powerpc/kvm/book3s_64_mmu_hv.c| 4 +-
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 9 +-
> arch/powerpc/kvm/book3s_64_vio.c | 4 +-
> arch/powerpc/kvm/book3s_hv.c | 222 +
> arch/powerpc/kvm/book3s_hv.h | 59 +++
> arch/powerpc/kvm/book3s_hv_builtin.c | 10 +-
> arch/powerpc/kvm/book3s_hv_p9_entry.c | 4 +-
> arch/powerpc/kvm/book3s_hv_ras.c | 5 +-
> arch/powerpc/kvm/book3s_hv_rm_mmu.c| 8 +-
> arch/powerpc/kvm/book3s_hv_rm_xics.c | 4 +-
> arch/powerpc/kvm/book3s_xive.c | 9 +-
> arch/powerpc/kvm/powerpc.c | 4 +-
> 15 files changed, 322 insertions(+), 158 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
> b/arch/powerpc/include/asm/kvm_book3s.h
> index bbf5e2c5fe09..4e91f54a3f9f 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -392,6 +392,16 @@ static inline ulong kvmppc_get_pc(struct kvm_vcpu *vcpu)
> return vcpu->arch.regs.nip;
> }
>
> +static inline void kvmppc_set_pid(struct kvm_vcpu *vcpu, u32 val)
> +{
> + vcpu->arch.pid = val;
> +}
> +
> +static inline u32 kvmppc_get_pid(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->arch.pid;
> +}
> +
> static inline u64 kvmppc_get_msr(struct kvm_vcpu *vcpu);
> static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
> {
> @@ -403,10 +413,66 @@ static inline ulong kvmppc_get_fault_dar(struct
> kvm_vcpu *vcpu)
> return vcpu->arch.fault_dar;
> }
>
> +#define BOOK3S_WRAPPER_SET(reg, size)
> \
> +static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)
> \
> +{\
> + \
> + vcpu->arch.reg = val; \
> +}
> +
> +#define BOOK3S_WRAPPER_GET(reg, size)
> \
> +static inline u##size kvmppc_get_##reg(struct kvm_vcpu *vcpu)
> \
> +{\
> + return vcpu->arch.reg; \
> +}
> +
> +#define BOOK3S_WRAPPER(reg, size)\
> + BOOK3S_WRAPPER_SET(reg, size) \
> + BOOK3S_WRAPPER_GET(reg, size) \
> +
> +BOOK3S_WRAPPER(tar, 64)
> +BOOK3S_WRAPPER(ebbhr, 64)
> +BOOK3S_WRAPPER(ebbrr, 64)
> +BOOK3S_WRAPPER(bescr, 64)
> +BOOK3S_WRAPPER(ic, 64)
> +BOOK3S_WRAPPER(vrsave, 64)
> +
> +
> +#define VCORE_WRAPPER_SET(reg, size) \
> +static inline void kvmppc_set_##reg ##_hv(struct kvm_vcpu *vcpu, u##size
> val)\
> +{\
> + vcpu->arch.vcore->reg = val;\
> +}
> +
> +#define VCORE_WRAPPER_GET(reg, size) \
> +static inline u##size kvmppc_get_##reg ##_hv(struct kvm_vcpu *vcpu) \
> +{\
> + return vcpu->arch.vcore->reg; \
> +}
> +
> +#define VCORE_WRAPPER(reg, size) \
> + VCORE_WRAPPER_SET(reg, size)\
> + VCORE_WRAPPER_GET(reg, size)\
> +
> +
> +VCORE_WRAPPER(vtb, 64)
> +VCORE_WRAPPER(tb_offset, 64)
> +VCORE_WRAPPER(lpcr, 64)
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?
> +
> +static inline u64 kvmppc_get_dec_expires(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->arch.dec_expires;
> +}
> +
> +static inline void kvmppc_set_dec_expires(struct kvm_vcpu *vcpu, u64 val)
> +{
> + vcpu->arch.dec_expires = val;
> +}
[RFC PATCH v2 1/6] KVM: PPC: Use getters and setters for vcpu register state
There are already some getter and setter functions used for accessing
vcpu register state, e.g. kvmppc_get_pc(). There are also more
complicated examples that are generated by macros like
kvmppc_get_sprg0() which are generated by the SHARED_SPRNG_WRAPPER()
macro.
In the new PAPR API for nested guest partitions the L1 is required to
communicate with the L0 to modify and read nested guest state.
Prepare to support this by replacing direct accesses to vcpu register
state with wrapper functions. Follow the existing pattern of using
macros to generate individual wrappers. These wrappers will
be augmented for supporting PAPR nested guests later.
Signed-off-by: Jordan Niethe
---
arch/powerpc/include/asm/kvm_book3s.h | 68 +++-
arch/powerpc/include/asm/kvm_ppc.h | 48 --
arch/powerpc/kvm/book3s.c | 22 +--
arch/powerpc/kvm/book3s_64_mmu_hv.c| 4 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 9 +-
arch/powerpc/kvm/book3s_64_vio.c | 4 +-
arch/powerpc/kvm/book3s_hv.c | 222 +
arch/powerpc/kvm/book3s_hv.h | 59 +++
arch/powerpc/kvm/book3s_hv_builtin.c | 10 +-
arch/powerpc/kvm/book3s_hv_p9_entry.c | 4 +-
arch/powerpc/kvm/book3s_hv_ras.c | 5 +-
arch/powerpc/kvm/book3s_hv_rm_mmu.c| 8 +-
arch/powerpc/kvm/book3s_hv_rm_xics.c | 4 +-
arch/powerpc/kvm/book3s_xive.c | 9 +-
arch/powerpc/kvm/powerpc.c | 4 +-
15 files changed, 322 insertions(+), 158 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index bbf5e2c5fe09..4e91f54a3f9f 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -392,6 +392,16 @@ static inline ulong kvmppc_get_pc(struct kvm_vcpu *vcpu)
return vcpu->arch.regs.nip;
}
+static inline void kvmppc_set_pid(struct kvm_vcpu *vcpu, u32 val)
+{
+ vcpu->arch.pid = val;
+}
+
+static inline u32 kvmppc_get_pid(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.pid;
+}
+
static inline u64 kvmppc_get_msr(struct kvm_vcpu *vcpu);
static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
{
@@ -403,10 +413,66 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu
*vcpu)
return vcpu->arch.fault_dar;
}
+#define BOOK3S_WRAPPER_SET(reg, size) \
+static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)
\
+{ \
+ \
+ vcpu->arch.reg = val; \
+}
+
+#define BOOK3S_WRAPPER_GET(reg, size) \
+static inline u##size kvmppc_get_##reg(struct kvm_vcpu *vcpu) \
+{ \
+ return vcpu->arch.reg; \
+}
+
+#define BOOK3S_WRAPPER(reg, size) \
+ BOOK3S_WRAPPER_SET(reg, size) \
+ BOOK3S_WRAPPER_GET(reg, size) \
+
+BOOK3S_WRAPPER(tar, 64)
+BOOK3S_WRAPPER(ebbhr, 64)
+BOOK3S_WRAPPER(ebbrr, 64)
+BOOK3S_WRAPPER(bescr, 64)
+BOOK3S_WRAPPER(ic, 64)
+BOOK3S_WRAPPER(vrsave, 64)
+
+
+#define VCORE_WRAPPER_SET(reg, size) \
+static inline void kvmppc_set_##reg ##_hv(struct kvm_vcpu *vcpu, u##size val)
\
+{ \
+ vcpu->arch.vcore->reg = val;\
+}
+
+#define VCORE_WRAPPER_GET(reg, size) \
+static inline u##size kvmppc_get_##reg ##_hv(struct kvm_vcpu *vcpu)\
+{ \
+ return vcpu->arch.vcore->reg; \
+}
+
+#define VCORE_WRAPPER(reg, size) \
+ VCORE_WRAPPER_SET(reg, size)\
+ VCORE_WRAPPER_GET(reg, size)\
+
+
+VCORE_WRAPPER(vtb, 64)
+VCORE_WRAPPER(tb_offset, 64)
+VCORE_WRAPPER(lpcr, 64)
+
+static inline u64 kvmppc_get_dec_expires(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.dec_expires;
+}
+
+static inline void kvmppc_set_dec_expires(struct kvm_vcpu *vcpu, u64 val)
+{
+ vcpu->arch.dec_expires = val;
+}
+
/* Expiry time of vcpu DEC relative to host TB */
static inline u64 kvmppc_dec_expires_host_tb(struct kvm_vcpu *vcpu)
{
- return vcpu->arch.dec_expires - vcpu->arch.vcore->tb_offset;
+ return kvmppc_get_dec_expires(vcpu) - kvmppc_get_tb_offset_hv(vcpu);
}
static inline bool is_kvmppc_resume_guest(int r)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 79a9c0bb8bba..fbac353ac46b 100644
---
