Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand <david@redhat.com>]

On 28.08.2017 10:07, Christian Borntraeger wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> kvm has always supported the concept of starting in z/Arch mode so let's
> reflect the feature bit to the guest.
> 
> Also, we change sigp set architecture to reject any request to change
> architecture modes.

Hm ... this seems to imply that CZAM is always set, but what about
running on old user space (possibly on old hw)? Old QEMU will not enable
CZAM.

And especially old user space will rely on SET ARCHITECTURE being
handled in the kernel.

I agree, that for QEMU we can always do that change (as we control which
features are indicated to the guest). But I am wondering if we would
have to stick to existing behavior unless test_kvm_facility(vcpu->kvm, 138).

> 
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
>  arch/s390/tools/gen_facilities.c |  1 +
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
> index 1a252f5..11d3845 100644
> --- a/arch/s390/kvm/sigp.c
> +++ b/arch/s390/kvm/sigp.c
> @@ -155,29 +155,26 @@ static int __sigp_stop_and_store_status(struct kvm_vcpu *vcpu,
>  	return rc;
>  }
>  
[...]

>  	}
> -	return rc;
> +
> +	*status_reg &= 0xffffffff00000000ULL;

s/ULL/UL/

> +
> +	/* Reject set arch order, with czam we're always in z/Arch mode. */
> +	*status_reg |= (all_stopped ? SIGP_STATUS_INVALID_PARAMETER :
> +					SIGP_STATUS_INCORRECT_STATE);
> +	return SIGP_CC_STATUS_STORED;
>  }
>  
>  static int __sigp_set_prefix(struct kvm_vcpu *vcpu, struct kvm_vcpu *dst_vcpu,
> @@ -446,7 +443,8 @@ int kvm_s390_handle_sigp(struct kvm_vcpu *vcpu)
>  	switch (order_code) {
>  	case SIGP_SET_ARCHITECTURE:
>  		vcpu->stat.instruction_sigp_arch++;
> -		rc = __sigp_set_arch(vcpu, parameter);
> +		rc = __sigp_set_arch(vcpu, parameter,
> +				     &vcpu->run->s.regs.gprs[r1]);
>  		break;
>  	default:
>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 025ea20..181db5b 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>  			78, /* enhanced-DAT 2 */
>  			130, /* instruction-execution-protection */
>  			131, /* enhanced-SOP 2 and side-effect */
> +			138, /* configuration z/architecture mode (czam) */
>  			146, /* msa extension 8 */
>  			-1  /* END */
>  		}
> 


-- 

Thanks,

David