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 <jpn@xxxxxxxxxxxxxxxxxx>
> ---
> 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;
> +}
> +
> /* 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
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -936,7 +936,7 @@ static inline ulong kvmppc_get_##reg(struct kvm_vcpu *vcpu) \
> #define SPRNG_WRAPPER_SET(reg, bookehv_spr) \
> static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, ulong val) \
> { \
> - mtspr(bookehv_spr, val); \
> + mtspr(bookehv_spr, val); \
> } \
>
> #define SHARED_WRAPPER_GET(reg, size) \
Stray hunk I think.
> @@ -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.
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.
> 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.
Thanks,
Nick
