[v1,1/3] KVM: s390: introduce defines for control registers
diff mbox

Message ID 20180427123613.20624-2-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand April 27, 2018, 12:36 p.m. UTC
In KVM code we use masks to test/set control registers.

Let's define the ones we use in arch/s390/include/asm/ctl_reg.h and
replace all occurrences in KVM code.

As we will be needing the define for Clock-comparator sign control soon,
let's also add it.

Suggested-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/s390/include/asm/ctl_reg.h | 12 ++++++++++++
 arch/s390/kvm/guestdbg.c        |  2 +-
 arch/s390/kvm/interrupt.c       | 16 ++++++++--------
 arch/s390/kvm/kvm-s390.c        | 10 +++++++---
 4 files changed, 28 insertions(+), 12 deletions(-)

Comments

Janosch Frank April 27, 2018, 1:45 p.m. UTC | #1
On 27.04.2018 14:36, David Hildenbrand wrote:
> In KVM code we use masks to test/set control registers.
> 
> Let's define the ones we use in arch/s390/include/asm/ctl_reg.h and
> replace all occurrences in KVM code.
> 
> As we will be needing the define for Clock-comparator sign control soon,
> let's also add it.
> 
> Suggested-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  arch/s390/include/asm/ctl_reg.h | 12 ++++++++++++
>  arch/s390/kvm/guestdbg.c        |  2 +-
>  arch/s390/kvm/interrupt.c       | 16 ++++++++--------
>  arch/s390/kvm/kvm-s390.c        | 10 +++++++---
>  4 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
> index 99c93d0346f9..4600453536c2 100644
> --- a/arch/s390/include/asm/ctl_reg.h
> +++ b/arch/s390/include/asm/ctl_reg.h
> @@ -10,8 +10,20 @@
> 
>  #include <linux/const.h>
> 
> +#define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
> +#define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
> +#define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
> +#define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
> +#define CR0_CPU_TIMER_SUBMASK		_BITUL(63 - 53)
> +#define CR0_SERVICE_SIGNAL_SUBMASK	_BITUL(63 - 54)
> +#define CR0_UNUSED_56			_BITUL(63 - 56)
> +#define CR0_INTERRUPT_KEY_SUBMASK	_BITUL(63 - 57)
> +#define CR0_MEASUREMENT_ALERT_SUBMASK	_BITUL(63 - 58)
> +
>  #define CR2_GUARDED_STORAGE		_BITUL(63 - 59)
> 
> +#define CR14_UNUSED_32			_BITUL(63 - 32)
> +#define CR14_UNUSED_33			_BITUL(63 - 33)
>  #define CR14_CHANNEL_REPORT_SUBMASK	_BITUL(63 - 35)
>  #define CR14_RECOVERY_SUBMASK		_BITUL(63 - 36)
>  #define CR14_DEGRADATION_SUBMASK	_BITUL(63 - 37)
> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index b5f3e82006d0..394a5f53805b 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -153,7 +153,7 @@ void kvm_s390_patch_guest_per_regs(struct kvm_vcpu *vcpu)
> 
>  	if (guestdbg_sstep_enabled(vcpu)) {
>  		/* disable timer (clock-comparator) interrupts */
> -		vcpu->arch.sie_block->gcr[0] &= ~0x800ul;
> +		vcpu->arch.sie_block->gcr[0] &= ~CR0_CLOCK_COMPARATOR_SUBMASK;
>  		vcpu->arch.sie_block->gcr[9] |= PER_EVENT_IFETCH;
>  		vcpu->arch.sie_block->gcr[10] = 0;
>  		vcpu->arch.sie_block->gcr[11] = -1UL;
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 37d06e022238..5a30fc275a4c 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -159,7 +159,7 @@ static int psw_interrupts_disabled(struct kvm_vcpu *vcpu)
>  static int ckc_interrupts_enabled(struct kvm_vcpu *vcpu)
>  {
>  	if (psw_extint_disabled(vcpu) ||
> -	    !(vcpu->arch.sie_block->gcr[0] & 0x800ul))
> +	    !(vcpu->arch.sie_block->gcr[0] & CR0_CLOCK_COMPARATOR_SUBMASK))
>  		return 0;
>  	if (guestdbg_enabled(vcpu) && guestdbg_sstep_enabled(vcpu))
>  		/* No timer interrupts when single stepping */
> @@ -184,7 +184,7 @@ static int ckc_irq_pending(struct kvm_vcpu *vcpu)
>  static int cpu_timer_interrupts_enabled(struct kvm_vcpu *vcpu)
>  {
>  	return !psw_extint_disabled(vcpu) &&
> -	       (vcpu->arch.sie_block->gcr[0] & 0x400ul);
> +	       (vcpu->arch.sie_block->gcr[0] & CR0_CPU_TIMER_SUBMASK);
>  }
> 
>  static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu)
> @@ -285,15 +285,15 @@ static unsigned long deliverable_irqs(struct kvm_vcpu *vcpu)
>  		active_mask &= ~IRQ_PEND_IO_MASK;
>  	else
>  		active_mask = disable_iscs(vcpu, active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x2000ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_EXTERNAL_CALL_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_EXTERNAL, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x4000ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_EMERGENCY_SIGNAL_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_EMERGENCY, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x800ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_CLOCK_COMPARATOR_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_CLOCK_COMP, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x400ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_CPU_TIMER_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_CPU_TIMER, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x200ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_SERVICE_SIGNAL_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_SERVICE, &active_mask);
>  	if (psw_mchk_disabled(vcpu))
>  		active_mask &= ~IRQ_PEND_MCHK_MASK;
> @@ -1042,7 +1042,7 @@ int kvm_s390_vcpu_has_irq(struct kvm_vcpu *vcpu, int exclude_stop)
>  	/* external call pending and deliverable */
>  	if (kvm_s390_ext_call_pending(vcpu) &&
>  	    !psw_extint_disabled(vcpu) &&
> -	    (vcpu->arch.sie_block->gcr[0] & 0x2000ul))
> +	    (vcpu->arch.sie_block->gcr[0] & CR0_EXTERNAL_CALL_SUBMASK))
>  		return 1;
> 
>  	if (!exclude_stop && kvm_s390_is_stop_irq_pending(vcpu))
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 64c986243018..6efe2b0ba5e1 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2433,8 +2433,12 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
>  	vcpu->arch.sie_block->ckc       = 0UL;
>  	vcpu->arch.sie_block->todpr     = 0;
>  	memset(vcpu->arch.sie_block->gcr, 0, 16 * sizeof(__u64));
> -	vcpu->arch.sie_block->gcr[0]  = 0xE0UL;
> -	vcpu->arch.sie_block->gcr[14] = 0xC2000000UL;
> +	vcpu->arch.sie_block->gcr[0]  = CR0_UNUSED_56 |
> +					CR0_INTERRUPT_KEY_SUBMASK |
> +					CR0_MEASUREMENT_ALERT_SUBMASK;
> +	vcpu->arch.sie_block->gcr[14] = CR14_UNUSED_32 |
> +					CR14_UNUSED_33 |
> +					CR14_EXTERNAL_DAMAGE_SUBMASK;
>  	/* make sure the new fpc will be lazily loaded */
>  	save_fpu_regs();
>  	current->thread.fpu.fpc = 0;
> @@ -3192,7 +3196,7 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu)
>  		return 0;
>  	if (kvm_s390_vcpu_has_irq(vcpu, 0))
>  		return 0;
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x200ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_SERVICE_SIGNAL_SUBMASK))
>  		return 0;
>  	if (!vcpu->arch.gmap->pfault_enabled)
>  		return 0;
>
Collin Walling April 27, 2018, 4:04 p.m. UTC | #2
On 04/27/2018 08:36 AM, David Hildenbrand wrote:
> In KVM code we use masks to test/set control registers.
> 
> Let's define the ones we use in arch/s390/include/asm/ctl_reg.h and
> replace all occurrences in KVM code.
> 
> As we will be needing the define for Clock-comparator sign control soon,
> let's also add it.
> 
> Suggested-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Thanks for taking the time to do this! This code reads a lot easier.

If necessary: Reviewed-by: Collin Walling <walling@linux.ibm.com>

> ---
>  arch/s390/include/asm/ctl_reg.h | 12 ++++++++++++
>  arch/s390/kvm/guestdbg.c        |  2 +-
>  arch/s390/kvm/interrupt.c       | 16 ++++++++--------
>  arch/s390/kvm/kvm-s390.c        | 10 +++++++---
>  4 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
> index 99c93d0346f9..4600453536c2 100644
> --- a/arch/s390/include/asm/ctl_reg.h
> +++ b/arch/s390/include/asm/ctl_reg.h
> @@ -10,8 +10,20 @@
>  
>  #include <linux/const.h>
>  
> +#define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
> +#define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
> +#define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
> +#define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
> +#define CR0_CPU_TIMER_SUBMASK		_BITUL(63 - 53)
> +#define CR0_SERVICE_SIGNAL_SUBMASK	_BITUL(63 - 54)
> +#define CR0_UNUSED_56			_BITUL(63 - 56)
> +#define CR0_INTERRUPT_KEY_SUBMASK	_BITUL(63 - 57)
> +#define CR0_MEASUREMENT_ALERT_SUBMASK	_BITUL(63 - 58)
> +
>  #define CR2_GUARDED_STORAGE		_BITUL(63 - 59)
>  
> +#define CR14_UNUSED_32			_BITUL(63 - 32)
> +#define CR14_UNUSED_33			_BITUL(63 - 33)
>  #define CR14_CHANNEL_REPORT_SUBMASK	_BITUL(63 - 35)
>  #define CR14_RECOVERY_SUBMASK		_BITUL(63 - 36)
>  #define CR14_DEGRADATION_SUBMASK	_BITUL(63 - 37)
> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index b5f3e82006d0..394a5f53805b 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -153,7 +153,7 @@ void kvm_s390_patch_guest_per_regs(struct kvm_vcpu *vcpu)
>  
>  	if (guestdbg_sstep_enabled(vcpu)) {
>  		/* disable timer (clock-comparator) interrupts */
> -		vcpu->arch.sie_block->gcr[0] &= ~0x800ul;
> +		vcpu->arch.sie_block->gcr[0] &= ~CR0_CLOCK_COMPARATOR_SUBMASK;
>  		vcpu->arch.sie_block->gcr[9] |= PER_EVENT_IFETCH;
>  		vcpu->arch.sie_block->gcr[10] = 0;
>  		vcpu->arch.sie_block->gcr[11] = -1UL;
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 37d06e022238..5a30fc275a4c 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -159,7 +159,7 @@ static int psw_interrupts_disabled(struct kvm_vcpu *vcpu)
>  static int ckc_interrupts_enabled(struct kvm_vcpu *vcpu)
>  {
>  	if (psw_extint_disabled(vcpu) ||
> -	    !(vcpu->arch.sie_block->gcr[0] & 0x800ul))
> +	    !(vcpu->arch.sie_block->gcr[0] & CR0_CLOCK_COMPARATOR_SUBMASK))
>  		return 0;
>  	if (guestdbg_enabled(vcpu) && guestdbg_sstep_enabled(vcpu))
>  		/* No timer interrupts when single stepping */
> @@ -184,7 +184,7 @@ static int ckc_irq_pending(struct kvm_vcpu *vcpu)
>  static int cpu_timer_interrupts_enabled(struct kvm_vcpu *vcpu)
>  {
>  	return !psw_extint_disabled(vcpu) &&
> -	       (vcpu->arch.sie_block->gcr[0] & 0x400ul);
> +	       (vcpu->arch.sie_block->gcr[0] & CR0_CPU_TIMER_SUBMASK);
>  }
>  
>  static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu)
> @@ -285,15 +285,15 @@ static unsigned long deliverable_irqs(struct kvm_vcpu *vcpu)
>  		active_mask &= ~IRQ_PEND_IO_MASK;
>  	else
>  		active_mask = disable_iscs(vcpu, active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x2000ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_EXTERNAL_CALL_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_EXTERNAL, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x4000ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_EMERGENCY_SIGNAL_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_EMERGENCY, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x800ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_CLOCK_COMPARATOR_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_CLOCK_COMP, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x400ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_CPU_TIMER_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_CPU_TIMER, &active_mask);
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x200ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_SERVICE_SIGNAL_SUBMASK))
>  		__clear_bit(IRQ_PEND_EXT_SERVICE, &active_mask);
>  	if (psw_mchk_disabled(vcpu))
>  		active_mask &= ~IRQ_PEND_MCHK_MASK;
> @@ -1042,7 +1042,7 @@ int kvm_s390_vcpu_has_irq(struct kvm_vcpu *vcpu, int exclude_stop)
>  	/* external call pending and deliverable */
>  	if (kvm_s390_ext_call_pending(vcpu) &&
>  	    !psw_extint_disabled(vcpu) &&
> -	    (vcpu->arch.sie_block->gcr[0] & 0x2000ul))
> +	    (vcpu->arch.sie_block->gcr[0] & CR0_EXTERNAL_CALL_SUBMASK))
>  		return 1;
>  
>  	if (!exclude_stop && kvm_s390_is_stop_irq_pending(vcpu))
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 64c986243018..6efe2b0ba5e1 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2433,8 +2433,12 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
>  	vcpu->arch.sie_block->ckc       = 0UL;
>  	vcpu->arch.sie_block->todpr     = 0;
>  	memset(vcpu->arch.sie_block->gcr, 0, 16 * sizeof(__u64));
> -	vcpu->arch.sie_block->gcr[0]  = 0xE0UL;
> -	vcpu->arch.sie_block->gcr[14] = 0xC2000000UL;
> +	vcpu->arch.sie_block->gcr[0]  = CR0_UNUSED_56 |
> +					CR0_INTERRUPT_KEY_SUBMASK |
> +					CR0_MEASUREMENT_ALERT_SUBMASK;
> +	vcpu->arch.sie_block->gcr[14] = CR14_UNUSED_32 |
> +					CR14_UNUSED_33 |
> +					CR14_EXTERNAL_DAMAGE_SUBMASK;
>  	/* make sure the new fpc will be lazily loaded */
>  	save_fpu_regs();
>  	current->thread.fpu.fpc = 0;
> @@ -3192,7 +3196,7 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu)
>  		return 0;
>  	if (kvm_s390_vcpu_has_irq(vcpu, 0))
>  		return 0;
> -	if (!(vcpu->arch.sie_block->gcr[0] & 0x200ul))
> +	if (!(vcpu->arch.sie_block->gcr[0] & CR0_SERVICE_SIGNAL_SUBMASK))
>  		return 0;
>  	if (!vcpu->arch.gmap->pfault_enabled)
>  		return 0;
>
Cornelia Huck April 30, 2018, 12:40 p.m. UTC | #3
On Fri, 27 Apr 2018 14:36:11 +0200
David Hildenbrand <david@redhat.com> wrote:

> In KVM code we use masks to test/set control registers.
> 
> Let's define the ones we use in arch/s390/include/asm/ctl_reg.h and
> replace all occurrences in KVM code.
> 
> As we will be needing the define for Clock-comparator sign control soon,
> let's also add it.
> 
> Suggested-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/s390/include/asm/ctl_reg.h | 12 ++++++++++++
>  arch/s390/kvm/guestdbg.c        |  2 +-
>  arch/s390/kvm/interrupt.c       | 16 ++++++++--------
>  arch/s390/kvm/kvm-s390.c        | 10 +++++++---
>  4 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
> index 99c93d0346f9..4600453536c2 100644
> --- a/arch/s390/include/asm/ctl_reg.h
> +++ b/arch/s390/include/asm/ctl_reg.h
> @@ -10,8 +10,20 @@
>  
>  #include <linux/const.h>
>  
> +#define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
> +#define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
> +#define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
> +#define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
> +#define CR0_CPU_TIMER_SUBMASK		_BITUL(63 - 53)
> +#define CR0_SERVICE_SIGNAL_SUBMASK	_BITUL(63 - 54)
> +#define CR0_UNUSED_56			_BITUL(63 - 56)
> +#define CR0_INTERRUPT_KEY_SUBMASK	_BITUL(63 - 57)
> +#define CR0_MEASUREMENT_ALERT_SUBMASK	_BITUL(63 - 58)
> +
>  #define CR2_GUARDED_STORAGE		_BITUL(63 - 59)
>  
> +#define CR14_UNUSED_32			_BITUL(63 - 32)
> +#define CR14_UNUSED_33			_BITUL(63 - 33)

These 'unused' values look a bit odd, but keep the masks unchanged :)
(The respective values do not seem to be in the public documentation
afaics?)

>  #define CR14_CHANNEL_REPORT_SUBMASK	_BITUL(63 - 35)
>  #define CR14_RECOVERY_SUBMASK		_BITUL(63 - 36)
>  #define CR14_DEGRADATION_SUBMASK	_BITUL(63 - 37)

Looks like a real improvement in readability!

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Janosch Frank April 30, 2018, 12:49 p.m. UTC | #4
On 30.04.2018 14:40, Cornelia Huck wrote:
> On Fri, 27 Apr 2018 14:36:11 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> In KVM code we use masks to test/set control registers.
>>
>> Let's define the ones we use in arch/s390/include/asm/ctl_reg.h and
>> replace all occurrences in KVM code.
>>
>> As we will be needing the define for Clock-comparator sign control soon,
>> let's also add it.
>>
>> Suggested-by: Collin L. Walling <walling@linux.vnet.ibm.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  arch/s390/include/asm/ctl_reg.h | 12 ++++++++++++
>>  arch/s390/kvm/guestdbg.c        |  2 +-
>>  arch/s390/kvm/interrupt.c       | 16 ++++++++--------
>>  arch/s390/kvm/kvm-s390.c        | 10 +++++++---
>>  4 files changed, 28 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
>> index 99c93d0346f9..4600453536c2 100644
>> --- a/arch/s390/include/asm/ctl_reg.h
>> +++ b/arch/s390/include/asm/ctl_reg.h
>> @@ -10,8 +10,20 @@
>>  
>>  #include <linux/const.h>
>>  
>> +#define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
>> +#define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
>> +#define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
>> +#define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
>> +#define CR0_CPU_TIMER_SUBMASK		_BITUL(63 - 53)
>> +#define CR0_SERVICE_SIGNAL_SUBMASK	_BITUL(63 - 54)
>> +#define CR0_UNUSED_56			_BITUL(63 - 56)
>> +#define CR0_INTERRUPT_KEY_SUBMASK	_BITUL(63 - 57)
>> +#define CR0_MEASUREMENT_ALERT_SUBMASK	_BITUL(63 - 58)
>> +
>>  #define CR2_GUARDED_STORAGE		_BITUL(63 - 59)
>>  
>> +#define CR14_UNUSED_32			_BITUL(63 - 32)
>> +#define CR14_UNUSED_33			_BITUL(63 - 33)
> 
> These 'unused' values look a bit odd, but keep the masks unchanged :)
> (The respective values do not seem to be in the public documentation
> afaics?)

56 is in the POP and is 1 initialized for compat 370 reasons. The cr14
ones are defined like that too. Have a look at the explanations at the
bottom of the table for figure 4 - 5.

> 
>>  #define CR14_CHANNEL_REPORT_SUBMASK	_BITUL(63 - 35)
>>  #define CR14_RECOVERY_SUBMASK		_BITUL(63 - 36)
>>  #define CR14_DEGRADATION_SUBMASK	_BITUL(63 - 37)
> 
> Looks like a real improvement in readability!
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
Cornelia Huck April 30, 2018, 1:11 p.m. UTC | #5
On Mon, 30 Apr 2018 14:49:44 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 30.04.2018 14:40, Cornelia Huck wrote:
> > On Fri, 27 Apr 2018 14:36:11 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> In KVM code we use masks to test/set control registers.
> >>
> >> Let's define the ones we use in arch/s390/include/asm/ctl_reg.h and
> >> replace all occurrences in KVM code.
> >>
> >> As we will be needing the define for Clock-comparator sign control soon,
> >> let's also add it.
> >>
> >> Suggested-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >>  arch/s390/include/asm/ctl_reg.h | 12 ++++++++++++
> >>  arch/s390/kvm/guestdbg.c        |  2 +-
> >>  arch/s390/kvm/interrupt.c       | 16 ++++++++--------
> >>  arch/s390/kvm/kvm-s390.c        | 10 +++++++---
> >>  4 files changed, 28 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
> >> index 99c93d0346f9..4600453536c2 100644
> >> --- a/arch/s390/include/asm/ctl_reg.h
> >> +++ b/arch/s390/include/asm/ctl_reg.h
> >> @@ -10,8 +10,20 @@
> >>  
> >>  #include <linux/const.h>
> >>  
> >> +#define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
> >> +#define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
> >> +#define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
> >> +#define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
> >> +#define CR0_CPU_TIMER_SUBMASK		_BITUL(63 - 53)
> >> +#define CR0_SERVICE_SIGNAL_SUBMASK	_BITUL(63 - 54)
> >> +#define CR0_UNUSED_56			_BITUL(63 - 56)
> >> +#define CR0_INTERRUPT_KEY_SUBMASK	_BITUL(63 - 57)
> >> +#define CR0_MEASUREMENT_ALERT_SUBMASK	_BITUL(63 - 58)
> >> +
> >>  #define CR2_GUARDED_STORAGE		_BITUL(63 - 59)
> >>  
> >> +#define CR14_UNUSED_32			_BITUL(63 - 32)
> >> +#define CR14_UNUSED_33			_BITUL(63 - 33)  
> > 
> > These 'unused' values look a bit odd, but keep the masks unchanged :)
> > (The respective values do not seem to be in the public documentation
> > afaics?)  
> 
> 56 is in the POP and is 1 initialized for compat 370 reasons. The cr14
> ones are defined like that too. Have a look at the explanations at the
> bottom of the table for figure 4 - 5.

Ah, thanks. Did not find that table (I was looking at the actual
effects the bits are having.)

Patch
diff mbox

diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
index 99c93d0346f9..4600453536c2 100644
--- a/arch/s390/include/asm/ctl_reg.h
+++ b/arch/s390/include/asm/ctl_reg.h
@@ -10,8 +10,20 @@ 
 
 #include <linux/const.h>
 
+#define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
+#define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
+#define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
+#define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
+#define CR0_CPU_TIMER_SUBMASK		_BITUL(63 - 53)
+#define CR0_SERVICE_SIGNAL_SUBMASK	_BITUL(63 - 54)
+#define CR0_UNUSED_56			_BITUL(63 - 56)
+#define CR0_INTERRUPT_KEY_SUBMASK	_BITUL(63 - 57)
+#define CR0_MEASUREMENT_ALERT_SUBMASK	_BITUL(63 - 58)
+
 #define CR2_GUARDED_STORAGE		_BITUL(63 - 59)
 
+#define CR14_UNUSED_32			_BITUL(63 - 32)
+#define CR14_UNUSED_33			_BITUL(63 - 33)
 #define CR14_CHANNEL_REPORT_SUBMASK	_BITUL(63 - 35)
 #define CR14_RECOVERY_SUBMASK		_BITUL(63 - 36)
 #define CR14_DEGRADATION_SUBMASK	_BITUL(63 - 37)
diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
index b5f3e82006d0..394a5f53805b 100644
--- a/arch/s390/kvm/guestdbg.c
+++ b/arch/s390/kvm/guestdbg.c
@@ -153,7 +153,7 @@  void kvm_s390_patch_guest_per_regs(struct kvm_vcpu *vcpu)
 
 	if (guestdbg_sstep_enabled(vcpu)) {
 		/* disable timer (clock-comparator) interrupts */
-		vcpu->arch.sie_block->gcr[0] &= ~0x800ul;
+		vcpu->arch.sie_block->gcr[0] &= ~CR0_CLOCK_COMPARATOR_SUBMASK;
 		vcpu->arch.sie_block->gcr[9] |= PER_EVENT_IFETCH;
 		vcpu->arch.sie_block->gcr[10] = 0;
 		vcpu->arch.sie_block->gcr[11] = -1UL;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 37d06e022238..5a30fc275a4c 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -159,7 +159,7 @@  static int psw_interrupts_disabled(struct kvm_vcpu *vcpu)
 static int ckc_interrupts_enabled(struct kvm_vcpu *vcpu)
 {
 	if (psw_extint_disabled(vcpu) ||
-	    !(vcpu->arch.sie_block->gcr[0] & 0x800ul))
+	    !(vcpu->arch.sie_block->gcr[0] & CR0_CLOCK_COMPARATOR_SUBMASK))
 		return 0;
 	if (guestdbg_enabled(vcpu) && guestdbg_sstep_enabled(vcpu))
 		/* No timer interrupts when single stepping */
@@ -184,7 +184,7 @@  static int ckc_irq_pending(struct kvm_vcpu *vcpu)
 static int cpu_timer_interrupts_enabled(struct kvm_vcpu *vcpu)
 {
 	return !psw_extint_disabled(vcpu) &&
-	       (vcpu->arch.sie_block->gcr[0] & 0x400ul);
+	       (vcpu->arch.sie_block->gcr[0] & CR0_CPU_TIMER_SUBMASK);
 }
 
 static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu)
@@ -285,15 +285,15 @@  static unsigned long deliverable_irqs(struct kvm_vcpu *vcpu)
 		active_mask &= ~IRQ_PEND_IO_MASK;
 	else
 		active_mask = disable_iscs(vcpu, active_mask);
-	if (!(vcpu->arch.sie_block->gcr[0] & 0x2000ul))
+	if (!(vcpu->arch.sie_block->gcr[0] & CR0_EXTERNAL_CALL_SUBMASK))
 		__clear_bit(IRQ_PEND_EXT_EXTERNAL, &active_mask);
-	if (!(vcpu->arch.sie_block->gcr[0] & 0x4000ul))
+	if (!(vcpu->arch.sie_block->gcr[0] & CR0_EMERGENCY_SIGNAL_SUBMASK))
 		__clear_bit(IRQ_PEND_EXT_EMERGENCY, &active_mask);
-	if (!(vcpu->arch.sie_block->gcr[0] & 0x800ul))
+	if (!(vcpu->arch.sie_block->gcr[0] & CR0_CLOCK_COMPARATOR_SUBMASK))
 		__clear_bit(IRQ_PEND_EXT_CLOCK_COMP, &active_mask);
-	if (!(vcpu->arch.sie_block->gcr[0] & 0x400ul))
+	if (!(vcpu->arch.sie_block->gcr[0] & CR0_CPU_TIMER_SUBMASK))
 		__clear_bit(IRQ_PEND_EXT_CPU_TIMER, &active_mask);
-	if (!(vcpu->arch.sie_block->gcr[0] & 0x200ul))
+	if (!(vcpu->arch.sie_block->gcr[0] & CR0_SERVICE_SIGNAL_SUBMASK))
 		__clear_bit(IRQ_PEND_EXT_SERVICE, &active_mask);
 	if (psw_mchk_disabled(vcpu))
 		active_mask &= ~IRQ_PEND_MCHK_MASK;
@@ -1042,7 +1042,7 @@  int kvm_s390_vcpu_has_irq(struct kvm_vcpu *vcpu, int exclude_stop)
 	/* external call pending and deliverable */
 	if (kvm_s390_ext_call_pending(vcpu) &&
 	    !psw_extint_disabled(vcpu) &&
-	    (vcpu->arch.sie_block->gcr[0] & 0x2000ul))
+	    (vcpu->arch.sie_block->gcr[0] & CR0_EXTERNAL_CALL_SUBMASK))
 		return 1;
 
 	if (!exclude_stop && kvm_s390_is_stop_irq_pending(vcpu))
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 64c986243018..6efe2b0ba5e1 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2433,8 +2433,12 @@  static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
 	vcpu->arch.sie_block->ckc       = 0UL;
 	vcpu->arch.sie_block->todpr     = 0;
 	memset(vcpu->arch.sie_block->gcr, 0, 16 * sizeof(__u64));
-	vcpu->arch.sie_block->gcr[0]  = 0xE0UL;
-	vcpu->arch.sie_block->gcr[14] = 0xC2000000UL;
+	vcpu->arch.sie_block->gcr[0]  = CR0_UNUSED_56 |
+					CR0_INTERRUPT_KEY_SUBMASK |
+					CR0_MEASUREMENT_ALERT_SUBMASK;
+	vcpu->arch.sie_block->gcr[14] = CR14_UNUSED_32 |
+					CR14_UNUSED_33 |
+					CR14_EXTERNAL_DAMAGE_SUBMASK;
 	/* make sure the new fpc will be lazily loaded */
 	save_fpu_regs();
 	current->thread.fpu.fpc = 0;
@@ -3192,7 +3196,7 @@  static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu)
 		return 0;
 	if (kvm_s390_vcpu_has_irq(vcpu, 0))
 		return 0;
-	if (!(vcpu->arch.sie_block->gcr[0] & 0x200ul))
+	if (!(vcpu->arch.sie_block->gcr[0] & CR0_SERVICE_SIGNAL_SUBMASK))
 		return 0;
 	if (!vcpu->arch.gmap->pfault_enabled)
 		return 0;