Re: [PATCH 14/23] KVM: PPC: Book3S HV: Nested: Context switch slb for nested hpt guest

[Paul Mackerras <paulus@ozlabs.org>]

On Mon, Aug 26, 2019 at 04:21:00PM +1000, Suraj Jitindar Singh wrote:
> A version 2 of the H_ENTER_NESTED hcall was added with an argument to
> specify the slb entries which should be used to run the nested guest.
> 
> Add support for this version of the hcall structures to
> kvmhv_enter_nested_guest() and context switch the slb when the nested
> guest being run is a hpt (hash page table) guest.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Question below...

> @@ -307,6 +335,26 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
>  	vcpu->arch.regs.msr = vcpu->arch.shregs.msr;
>  	saved_l1_regs = vcpu->arch.regs;
>  	kvmhv_save_hv_regs(vcpu, &saved_l1_hv);
> +	/* if running hpt then context switch the slb in the vcpu struct */
> +	if (!radix) {
> +		slb_ptr = kvmppc_get_gpr(vcpu, 6);
> +		l2_slb = kzalloc(sizeof(*l2_slb), GFP_KERNEL);
> +		saved_l1_slb = kzalloc(sizeof(*saved_l1_slb), GFP_KERNEL);
> +
> +		if ((!l2_slb) || (!saved_l1_slb)) {
> +			ret = H_HARDWARE;
> +			goto out_free;
> +		}
> +		err = kvm_vcpu_read_guest(vcpu, slb_ptr, l2_slb,
> +					  sizeof(struct guest_slb));
> +		if (err) {
> +			ret = H_PARAMETER;
> +			goto out_free;
> +		}
> +		if (kvmppc_need_byteswap(vcpu))
> +			byteswap_guest_slb(l2_slb);
> +		kvmhv_save_guest_slb(vcpu, saved_l1_slb);

Why are we bothering to save the SLB state of the L1 guest, which has
to be a radix guest?  Won't the L1 SLB state always just have 0
entries?

> @@ -354,6 +409,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
>  		vcpu->arch.shregs.msr |= MSR_TS_S;
>  	vc->tb_offset = saved_l1_hv.tb_offset;
>  	restore_hv_regs(vcpu, &saved_l1_hv);
> +	if (!radix)
> +		kvmhv_restore_guest_slb(vcpu, saved_l1_slb);

Likewise here can't we just set vcpu->arch.slb_max and
vcpu->arch.slb_nr to zero?

Paul.