On Mon Aug 7, 2023 at 11:45 AM 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 "Nestedv2" 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 Nestedv2 guests later.
>
> Signed-off-by: Gautam Menghani <gautam@xxxxxxxxxxxxx>
> Signed-off-by: Jordan Niethe <jniethe5@xxxxxxxxx>
> ---
> v3:
> - Do not add a helper for pvr
> - Use an expression when declaring variable in case
> - Squash in all getters and setters
> - Guatam: Pass vector registers by reference
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 123 +++++++++++++-
> arch/powerpc/include/asm/kvm_booke.h | 10 ++
> arch/powerpc/kvm/book3s.c | 38 ++---
> 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 | 220 +++++++++++++------------
> arch/powerpc/kvm/book3s_hv.h | 58 +++++++
> 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/emulate_loadstore.c | 2 +-
> arch/powerpc/kvm/powerpc.c | 76 ++++-----
> 16 files changed, 395 insertions(+), 189 deletions(-)
>
[snip]
> +
> /* 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);
> }
I don't see kvmppc_get_tb_offset_hv in this patch.
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 7f765d5ad436..738f2ecbe9b9 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -347,7 +347,7 @@ static int kvmppc_mmu_book3s_64_hv_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
> unsigned long v, orig_v, gr;
> __be64 *hptep;
> long int index;
> - int virtmode = vcpu->arch.shregs.msr & (data ? MSR_DR : MSR_IR);
> + int virtmode = kvmppc_get_msr(vcpu) & (data ? MSR_DR : MSR_IR);
>
> if (kvm_is_radix(vcpu->kvm))
> return kvmppc_mmu_radix_xlate(vcpu, eaddr, gpte, data, iswrite);
So this isn't _only_ adding new accessors. This should be functionally a
noop, but I think it introduces a branch if PR is defined.
Shared page is a slight annoyance for HV, I'd like to get rid of it...
but that's another story. I think the pattern here would be to add a
kvmppc_get_msr_hv() accessor.
And as a nitpick, for anywhere employing existing access functions, gprs
and such, could that be split into its own patch?
Looks pretty good aside from those little things.
Thanks,
Nick
