[v9,21/22] KVM: s390: CPU model support for AP virtualization

Commit Message

Tony Krowiak Aug. 13, 2018, 9:48 p.m. UTC

From: Tony Krowiak <[e-mail]>

Introduces a new CPU model feature and two CPU model
facilities to support AP virtualization for KVM guests.

CPU model feature:

The KVM_S390_VM_CPU_FEAT_AP feature indicates that
AP instructions are available on the guest. This
feature will be enabled by the kernel only if the AP
instructions are installed on the linux host. This feature
must be specifically turned on for the KVM guest from
userspace to use the VFIO AP device driver for guest
access to AP devices.

CPU model facilities:

1. AP Query Configuration Information (QCI) facility is installed.

   This is indicated by setting facilities bit 12 for
   the guest. The kernel will not enable this facility
   for the guest if it is not set on the host.

   If this facility is not set for the KVM guest, then only
   APQNs with an APQI less than 16 will be used by a Linux
   guest regardless of the matrix configuration for the virtual
   machine. This is a limitation of the Linux AP bus.

2. AP Facilities Test facility (APFT) is installed.

   This is indicated by setting facilities bit 15 for
   the guest. The kernel will not enable this facility for
   the guest if it is not set on the host.

   If this facility is not set for the KVM guest, then no
   AP devices will be available to the guest regardless of
   the guest's matrix configuration for the virtual
   machine. This is a limitation of the Linux AP bus.

Signed-off-by: Tony Krowiak <[e-mail]>
Reviewed-by: Christian Borntraeger <[e-mail]>
Reviewed-by: Halil Pasic <[e-mail]>
Tested-by: Michael Mueller <[e-mail]>
Tested-by: Farhan Ali <[e-mail]>
Signed-off-by: Christian Borntraeger <[e-mail]>
---
 arch/s390/kvm/kvm-s390.c         |    5 +++++
 arch/s390/tools/gen_facilities.c |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

Comments

Cornelia Huck Aug. 20, 2018, 2:26 p.m. UTC | #1

On Mon, 13 Aug 2018 17:48:18 -0400
Tony Krowiak <[e-mail]> wrote:

> From: Tony Krowiak <[e-mail]>
> 
> Introduces a new CPU model feature and two CPU model
> facilities to support AP virtualization for KVM guests.
> 
> CPU model feature:
> 
> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
> AP instructions are available on the guest. This
> feature will be enabled by the kernel only if the AP
> instructions are installed on the linux host. This feature
> must be specifically turned on for the KVM guest from
> userspace to use the VFIO AP device driver for guest
> access to AP devices.
> 
> CPU model facilities:
> 
> 1. AP Query Configuration Information (QCI) facility is installed.
> 
>    This is indicated by setting facilities bit 12 for
>    the guest. The kernel will not enable this facility
>    for the guest if it is not set on the host.
> 
>    If this facility is not set for the KVM guest, then only
>    APQNs with an APQI less than 16 will be used by a Linux
>    guest regardless of the matrix configuration for the virtual
>    machine. This is a limitation of the Linux AP bus.
> 
> 2. AP Facilities Test facility (APFT) is installed.
> 
>    This is indicated by setting facilities bit 15 for
>    the guest. The kernel will not enable this facility for
>    the guest if it is not set on the host.
> 
>    If this facility is not set for the KVM guest, then no
>    AP devices will be available to the guest regardless of
>    the guest's matrix configuration for the virtual
>    machine. This is a limitation of the Linux AP bus.
> 
> Signed-off-by: Tony Krowiak <[e-mail]>
> Reviewed-by: Christian Borntraeger <[e-mail]>
> Reviewed-by: Halil Pasic <[e-mail]>
> Tested-by: Michael Mueller <[e-mail]>
> Tested-by: Farhan Ali <[e-mail]>
> Signed-off-by: Christian Borntraeger <[e-mail]>
> ---
>  arch/s390/kvm/kvm-s390.c         |    5 +++++
>  arch/s390/tools/gen_facilities.c |    2 ++
>  2 files changed, 7 insertions(+), 0 deletions(-)

Looks sane to me.

David Hildenbrand Aug. 21, 2018, 8:03 a.m. UTC | #2

On 13.08.2018 23:48, Tony Krowiak wrote:
> From: Tony Krowiak <[e-mail]>
> 
> Introduces a new CPU model feature and two CPU model
> facilities to support AP virtualization for KVM guests.
> 
> CPU model feature:
> 
> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
> AP instructions are available on the guest. This
> feature will be enabled by the kernel only if the AP
> instructions are installed on the linux host. This feature
> must be specifically turned on for the KVM guest from
> userspace to use the VFIO AP device driver for guest
> access to AP devices.
> 
> CPU model facilities:
> 
> 1. AP Query Configuration Information (QCI) facility is installed.
> 
>    This is indicated by setting facilities bit 12 for
>    the guest. The kernel will not enable this facility
>    for the guest if it is not set on the host.
> 
>    If this facility is not set for the KVM guest, then only
>    APQNs with an APQI less than 16 will be used by a Linux
>    guest regardless of the matrix configuration for the virtual
>    machine. This is a limitation of the Linux AP bus.
> 
> 2. AP Facilities Test facility (APFT) is installed.
> 
>    This is indicated by setting facilities bit 15 for
>    the guest. The kernel will not enable this facility for
>    the guest if it is not set on the host.
> 
>    If this facility is not set for the KVM guest, then no
>    AP devices will be available to the guest regardless of
>    the guest's matrix configuration for the virtual
>    machine. This is a limitation of the Linux AP bus.
> 
> Signed-off-by: Tony Krowiak <[e-mail]>
> Reviewed-by: Christian Borntraeger <[e-mail]>
> Reviewed-by: Halil Pasic <[e-mail]>
> Tested-by: Michael Mueller <[e-mail]>
> Tested-by: Farhan Ali <[e-mail]>
> Signed-off-by: Christian Borntraeger <[e-mail]>
> ---
>  arch/s390/kvm/kvm-s390.c         |    5 +++++
>  arch/s390/tools/gen_facilities.c |    2 ++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 1e8cb67..d5e04d2 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>  
>  	if (MACHINE_HAS_ESOP)
>  		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
> +
> +	/* Check if AP instructions installed on host */
> +	if (ap_instructions_available())
> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
> +
>  	/*
>  	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>  	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 90a8c9e..a52290b 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -106,6 +106,8 @@ struct facility_def {
>  
>  		.name = "FACILITIES_KVM_CPUMODEL",
>  		.bits = (int[]){
> +			12, /* AP Query Configuration Information */
> +			15, /* AP Facilities Test */
>  			-1  /* END */
>  		}
>  	},
> 

Reviewed-by: David Hildenbrand <[e-mail]>

David Hildenbrand Aug. 22, 2018, 11:19 a.m. UTC | #3

On 13.08.2018 23:48, Tony Krowiak wrote:
> From: Tony Krowiak <[e-mail]>
> 
> Introduces a new CPU model feature and two CPU model
> facilities to support AP virtualization for KVM guests.
> 
> CPU model feature:
> 
> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
> AP instructions are available on the guest. This
> feature will be enabled by the kernel only if the AP
> instructions are installed on the linux host. This feature
> must be specifically turned on for the KVM guest from
> userspace to use the VFIO AP device driver for guest
> access to AP devices.
> 
> CPU model facilities:
> 
> 1. AP Query Configuration Information (QCI) facility is installed.
> 
>    This is indicated by setting facilities bit 12 for
>    the guest. The kernel will not enable this facility
>    for the guest if it is not set on the host.
> 
>    If this facility is not set for the KVM guest, then only
>    APQNs with an APQI less than 16 will be used by a Linux
>    guest regardless of the matrix configuration for the virtual
>    machine. This is a limitation of the Linux AP bus.
> 
> 2. AP Facilities Test facility (APFT) is installed.
> 
>    This is indicated by setting facilities bit 15 for
>    the guest. The kernel will not enable this facility for
>    the guest if it is not set on the host.
> 
>    If this facility is not set for the KVM guest, then no
>    AP devices will be available to the guest regardless of
>    the guest's matrix configuration for the virtual
>    machine. This is a limitation of the Linux AP bus.
> 
> Signed-off-by: Tony Krowiak <[e-mail]>
> Reviewed-by: Christian Borntraeger <[e-mail]>
> Reviewed-by: Halil Pasic <[e-mail]>
> Tested-by: Michael Mueller <[e-mail]>
> Tested-by: Farhan Ali <[e-mail]>
> Signed-off-by: Christian Borntraeger <[e-mail]>
> ---
>  arch/s390/kvm/kvm-s390.c         |    5 +++++
>  arch/s390/tools/gen_facilities.c |    2 ++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 1e8cb67..d5e04d2 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>  
>  	if (MACHINE_HAS_ESOP)
>  		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
> +
> +	/* Check if AP instructions installed on host */
> +	if (ap_instructions_available())
> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
> +
>  	/*
>  	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>  	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 90a8c9e..a52290b 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -106,6 +106,8 @@ struct facility_def {
>  
>  		.name = "FACILITIES_KVM_CPUMODEL",
>  		.bits = (int[]){
> +			12, /* AP Query Configuration Information */
> +			15, /* AP Facilities Test */
>  			-1  /* END */
>  		}
>  	},
> 

I really wonder if we should also export the APXA facility.

We can probe and allow that CPU feature. However, we cannot disable it
(as of now).

We have other CPU features where it is the same case (basically all
subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
export them, but support to disable them has never been implemented.

On a high level, we could then e.g. deny to start a QEMU guest if APXA
is available but has been disabled. (until we know that disabling it
actually works - if ever).

This helps to catch nasty migration bugs (e.g. APXA suddenly
disappearing). Although unlikely, definitely possible.


Are there any other AP related facilities that the guest can from now on
probe that should also become part of the CPU model?

David Hildenbrand Aug. 22, 2018, 11:24 a.m. UTC | #4

On 22.08.2018 13:19, David Hildenbrand wrote:
> On 13.08.2018 23:48, Tony Krowiak wrote:
>> From: Tony Krowiak <[e-mail]>
>>
>> Introduces a new CPU model feature and two CPU model
>> facilities to support AP virtualization for KVM guests.
>>
>> CPU model feature:
>>
>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>> AP instructions are available on the guest. This
>> feature will be enabled by the kernel only if the AP
>> instructions are installed on the linux host. This feature
>> must be specifically turned on for the KVM guest from
>> userspace to use the VFIO AP device driver for guest
>> access to AP devices.
>>
>> CPU model facilities:
>>
>> 1. AP Query Configuration Information (QCI) facility is installed.
>>
>>    This is indicated by setting facilities bit 12 for
>>    the guest. The kernel will not enable this facility
>>    for the guest if it is not set on the host.
>>
>>    If this facility is not set for the KVM guest, then only
>>    APQNs with an APQI less than 16 will be used by a Linux
>>    guest regardless of the matrix configuration for the virtual
>>    machine. This is a limitation of the Linux AP bus.
>>
>> 2. AP Facilities Test facility (APFT) is installed.
>>
>>    This is indicated by setting facilities bit 15 for
>>    the guest. The kernel will not enable this facility for
>>    the guest if it is not set on the host.
>>
>>    If this facility is not set for the KVM guest, then no
>>    AP devices will be available to the guest regardless of
>>    the guest's matrix configuration for the virtual
>>    machine. This is a limitation of the Linux AP bus.
>>
>> Signed-off-by: Tony Krowiak <[e-mail]>
>> Reviewed-by: Christian Borntraeger <[e-mail]>
>> Reviewed-by: Halil Pasic <[e-mail]>
>> Tested-by: Michael Mueller <[e-mail]>
>> Tested-by: Farhan Ali <[e-mail]>
>> Signed-off-by: Christian Borntraeger <[e-mail]>
>> ---
>>  arch/s390/kvm/kvm-s390.c         |    5 +++++
>>  arch/s390/tools/gen_facilities.c |    2 ++
>>  2 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 1e8cb67..d5e04d2 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>  
>>  	if (MACHINE_HAS_ESOP)
>>  		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>> +
>> +	/* Check if AP instructions installed on host */
>> +	if (ap_instructions_available())
>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>> +
>>  	/*
>>  	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>  	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>> index 90a8c9e..a52290b 100644
>> --- a/arch/s390/tools/gen_facilities.c
>> +++ b/arch/s390/tools/gen_facilities.c
>> @@ -106,6 +106,8 @@ struct facility_def {
>>  
>>  		.name = "FACILITIES_KVM_CPUMODEL",
>>  		.bits = (int[]){
>> +			12, /* AP Query Configuration Information */
>> +			15, /* AP Facilities Test */
>>  			-1  /* END */
>>  		}
>>  	},
>>
> 
> I really wonder if we should also export the APXA facility.
> 
> We can probe and allow that CPU feature. However, we cannot disable it
> (as of now).
> 
> We have other CPU features where it is the same case (basically all
> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
> export them, but support to disable them has never been implemented.
> 
> On a high level, we could then e.g. deny to start a QEMU guest if APXA
> is available but has been disabled. (until we know that disabling it
> actually works - if ever).
> 
> This helps to catch nasty migration bugs (e.g. APXA suddenly
> disappearing). Although unlikely, definitely possible.
> 
> 
> Are there any other AP related facilities that the guest can from now on
> probe that should also become part of the CPU model?
> 

To be more precise, shouldn't PQAP(QCI) be handled just like other
subfunctions? (I remember it should)

That would imply that there is actually theoretically a way to fake away
certain AP facilities.

Pierre Morel Aug. 22, 2018, 2:33 p.m. UTC | #5

On 22/08/2018 13:19, David Hildenbrand wrote:
> On 13.08.2018 23:48, Tony Krowiak wrote:
>> From: Tony Krowiak <[e-mail]>
>>
>> Introduces a new CPU model feature and two CPU model
>> facilities to support AP virtualization for KVM guests.
>>
>> CPU model feature:
>>
>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>> AP instructions are available on the guest. This
>> feature will be enabled by the kernel only if the AP
>> instructions are installed on the linux host. This feature
>> must be specifically turned on for the KVM guest from
>> userspace to use the VFIO AP device driver for guest
>> access to AP devices.
>>
>> CPU model facilities:
>>
>> 1. AP Query Configuration Information (QCI) facility is installed.
>>
>>     This is indicated by setting facilities bit 12 for
>>     the guest. The kernel will not enable this facility
>>     for the guest if it is not set on the host.
>>
>>     If this facility is not set for the KVM guest, then only
>>     APQNs with an APQI less than 16 will be used by a Linux
>>     guest regardless of the matrix configuration for the virtual
>>     machine. This is a limitation of the Linux AP bus.
>>
>> 2. AP Facilities Test facility (APFT) is installed.
>>
>>     This is indicated by setting facilities bit 15 for
>>     the guest. The kernel will not enable this facility for
>>     the guest if it is not set on the host.
>>
>>     If this facility is not set for the KVM guest, then no
>>     AP devices will be available to the guest regardless of
>>     the guest's matrix configuration for the virtual
>>     machine. This is a limitation of the Linux AP bus.
>>
>> Signed-off-by: Tony Krowiak <[e-mail]>
>> Reviewed-by: Christian Borntraeger <[e-mail]>
>> Reviewed-by: Halil Pasic <[e-mail]>
>> Tested-by: Michael Mueller <[e-mail]>
>> Tested-by: Farhan Ali <[e-mail]>
>> Signed-off-by: Christian Borntraeger <[e-mail]>
>> ---
>>   arch/s390/kvm/kvm-s390.c         |    5 +++++
>>   arch/s390/tools/gen_facilities.c |    2 ++
>>   2 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 1e8cb67..d5e04d2 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>   
>>   	if (MACHINE_HAS_ESOP)
>>   		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>> +
>> +	/* Check if AP instructions installed on host */
>> +	if (ap_instructions_available())
>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>> +
>>   	/*
>>   	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>   	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>> index 90a8c9e..a52290b 100644
>> --- a/arch/s390/tools/gen_facilities.c
>> +++ b/arch/s390/tools/gen_facilities.c
>> @@ -106,6 +106,8 @@ struct facility_def {
>>   
>>   		.name = "FACILITIES_KVM_CPUMODEL",
>>   		.bits = (int[]){
>> +			12, /* AP Query Configuration Information */
>> +			15, /* AP Facilities Test */
>>   			-1  /* END */
>>   		}
>>   	},
>>
> 
> I really wonder if we should also export the APXA facility.
> 
> We can probe and allow that CPU feature. However, we cannot disable it
> (as of now).
> 
> We have other CPU features where it is the same case (basically all
> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
> export them, but support to disable them has never been implemented.
> 
> On a high level, we could then e.g. deny to start a QEMU guest if APXA
> is available but has been disabled. (until we know that disabling it
> actually works - if ever).
> 
> This helps to catch nasty migration bugs (e.g. APXA suddenly
> disappearing). Although unlikely, definitely possible. >
> 
> Are there any other AP related facilities that the guest can from now on
> probe that should also become part of the CPU model?
> 



Before going too far in a discussion on features which we do not really 
need, we can make clear that we only support beginning with z13 and only 
in the Z architecture mode as host and as guest.

We then need to abort the VFIO driver if APXA is not installed.

In this case we will have no problem with older guests not having idea 
about APXA.

Would it be a solution?


Regards,
Pierre

David Hildenbrand Aug. 22, 2018, 3:04 p.m. UTC | #6

On 22.08.2018 16:33, Pierre Morel wrote:
> On 22/08/2018 13:19, David Hildenbrand wrote:
>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>> From: Tony Krowiak <[e-mail]>
>>>
>>> Introduces a new CPU model feature and two CPU model
>>> facilities to support AP virtualization for KVM guests.
>>>
>>> CPU model feature:
>>>
>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>> AP instructions are available on the guest. This
>>> feature will be enabled by the kernel only if the AP
>>> instructions are installed on the linux host. This feature
>>> must be specifically turned on for the KVM guest from
>>> userspace to use the VFIO AP device driver for guest
>>> access to AP devices.
>>>
>>> CPU model facilities:
>>>
>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>
>>>     This is indicated by setting facilities bit 12 for
>>>     the guest. The kernel will not enable this facility
>>>     for the guest if it is not set on the host.
>>>
>>>     If this facility is not set for the KVM guest, then only
>>>     APQNs with an APQI less than 16 will be used by a Linux
>>>     guest regardless of the matrix configuration for the virtual
>>>     machine. This is a limitation of the Linux AP bus.
>>>
>>> 2. AP Facilities Test facility (APFT) is installed.
>>>
>>>     This is indicated by setting facilities bit 15 for
>>>     the guest. The kernel will not enable this facility for
>>>     the guest if it is not set on the host.
>>>
>>>     If this facility is not set for the KVM guest, then no
>>>     AP devices will be available to the guest regardless of
>>>     the guest's matrix configuration for the virtual
>>>     machine. This is a limitation of the Linux AP bus.
>>>
>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>> Reviewed-by: Halil Pasic <[e-mail]>
>>> Tested-by: Michael Mueller <[e-mail]>
>>> Tested-by: Farhan Ali <[e-mail]>
>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>> ---
>>>   arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>   arch/s390/tools/gen_facilities.c |    2 ++
>>>   2 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 1e8cb67..d5e04d2 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>   
>>>   	if (MACHINE_HAS_ESOP)
>>>   		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>> +
>>> +	/* Check if AP instructions installed on host */
>>> +	if (ap_instructions_available())
>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>> +
>>>   	/*
>>>   	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>   	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>> index 90a8c9e..a52290b 100644
>>> --- a/arch/s390/tools/gen_facilities.c
>>> +++ b/arch/s390/tools/gen_facilities.c
>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>   
>>>   		.name = "FACILITIES_KVM_CPUMODEL",
>>>   		.bits = (int[]){
>>> +			12, /* AP Query Configuration Information */
>>> +			15, /* AP Facilities Test */
>>>   			-1  /* END */
>>>   		}
>>>   	},
>>>
>>
>> I really wonder if we should also export the APXA facility.
>>
>> We can probe and allow that CPU feature. However, we cannot disable it
>> (as of now).
>>
>> We have other CPU features where it is the same case (basically all
>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>> export them, but support to disable them has never been implemented.
>>
>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>> is available but has been disabled. (until we know that disabling it
>> actually works - if ever).
>>
>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>> disappearing). Although unlikely, definitely possible. >
>>
>> Are there any other AP related facilities that the guest can from now on
>> probe that should also become part of the CPU model?
>>
> 
> 
> 
> Before going too far in a discussion on features which we do not really 
> need, we can make clear that we only support beginning with z13 and only 
> in the Z architecture mode as host and as guest.

Easy answer:

The CPU model should be prepared for all eventualities. We have handled
it that way since the beginning.

The minimal thing I expect to have is all relevant features probed and
exported to user space. Just like we do with all the MSA/PTFF/PLO
subfunctions. I expect (and remember) this to be the same for PQAP.

(there are some very special cases regarding subfunctions that are not
indicated)

Why? The CPU model is not KVM specific.

> 
> We then need to abort the VFIO driver if APXA is not installed.

While you should do that, the CPU model is more generic. This would only
imply that as of now, the APXA feature would always be available if the
AP feature is available.

In addition, it makes the vSIE handling code easier - there is always
APXA and for now it cannot be disabled.

> 
> In this case we will have no problem with older guests not having idea 
> about APXA.
> 
> Would it be a solution?

Any feature the guest sees, should be part of the CPU model. The whole
environment for cpu subfunctions is already in place both in KVM and
QEMU. Only disabling subfunctions in KVM is not implemented yet.

You can exclude any subfunctions/facilities that are only valid on LPAR
level and cannot be used in some guest either way. (that makes life
sometimes easier)


I know that this might sound a little bit complicated, but it really
isn't. Boils down to modifying kvm_s390_cpu_feat_init() and specifying
some features+feature groups in QEMU.

> 
> 
> Regards,
> Pierre
>

Pierre Morel Aug. 22, 2018, 3:50 p.m. UTC | #7

On 22/08/2018 17:04, David Hildenbrand wrote:
> On 22.08.2018 16:33, Pierre Morel wrote:
>> On 22/08/2018 13:19, David Hildenbrand wrote:
>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>> From: Tony Krowiak <[e-mail]>
>>>>
>>>> Introduces a new CPU model feature and two CPU model
>>>> facilities to support AP virtualization for KVM guests.
>>>>
>>>> CPU model feature:
>>>>
>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>> AP instructions are available on the guest. This
>>>> feature will be enabled by the kernel only if the AP
>>>> instructions are installed on the linux host. This feature
>>>> must be specifically turned on for the KVM guest from
>>>> userspace to use the VFIO AP device driver for guest
>>>> access to AP devices.
>>>>
>>>> CPU model facilities:
>>>>
>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>
>>>>      This is indicated by setting facilities bit 12 for
>>>>      the guest. The kernel will not enable this facility
>>>>      for the guest if it is not set on the host.
>>>>
>>>>      If this facility is not set for the KVM guest, then only
>>>>      APQNs with an APQI less than 16 will be used by a Linux
>>>>      guest regardless of the matrix configuration for the virtual
>>>>      machine. This is a limitation of the Linux AP bus.
>>>>
>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>
>>>>      This is indicated by setting facilities bit 15 for
>>>>      the guest. The kernel will not enable this facility for
>>>>      the guest if it is not set on the host.
>>>>
>>>>      If this facility is not set for the KVM guest, then no
>>>>      AP devices will be available to the guest regardless of
>>>>      the guest's matrix configuration for the virtual
>>>>      machine. This is a limitation of the Linux AP bus.
>>>>
>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>> Tested-by: Michael Mueller <[e-mail]>
>>>> Tested-by: Farhan Ali <[e-mail]>
>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>> ---
>>>>    arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>    arch/s390/tools/gen_facilities.c |    2 ++
>>>>    2 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>> index 1e8cb67..d5e04d2 100644
>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>    
>>>>    	if (MACHINE_HAS_ESOP)
>>>>    		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>> +
>>>> +	/* Check if AP instructions installed on host */
>>>> +	if (ap_instructions_available())
>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>> +
>>>>    	/*
>>>>    	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>    	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>> index 90a8c9e..a52290b 100644
>>>> --- a/arch/s390/tools/gen_facilities.c
>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>    
>>>>    		.name = "FACILITIES_KVM_CPUMODEL",
>>>>    		.bits = (int[]){
>>>> +			12, /* AP Query Configuration Information */
>>>> +			15, /* AP Facilities Test */
>>>>    			-1  /* END */
>>>>    		}
>>>>    	},
>>>>
>>>
>>> I really wonder if we should also export the APXA facility.
>>>
>>> We can probe and allow that CPU feature. However, we cannot disable it
>>> (as of now).
>>>
>>> We have other CPU features where it is the same case (basically all
>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>> export them, but support to disable them has never been implemented.
>>>
>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>> is available but has been disabled. (until we know that disabling it
>>> actually works - if ever).
>>>
>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>> disappearing). Although unlikely, definitely possible. >
>>>
>>> Are there any other AP related facilities that the guest can from now on
>>> probe that should also become part of the CPU model?
>>>
>>
>>
>>
>> Before going too far in a discussion on features which we do not really
>> need, we can make clear that we only support beginning with z13 and only
>> in the Z architecture mode as host and as guest.
> 
> Easy answer:
> 
> The CPU model should be prepared for all eventualities. We have handled
> it that way since the beginning.
> 
> The minimal thing I expect to have is all relevant features probed and
> exported to user space. Just like we do with all the MSA/PTFF/PLO
> subfunctions. I expect (and remember) this to be the same for PQAP.

OK, we will need a separate patch.

> 
> (there are some very special cases regarding subfunctions that are not
> indicated)
> 
> Why? The CPU model is not KVM specific.

Is it true for privilege instructions?

> 
>>
>> We then need to abort the VFIO driver if APXA is not installed.
> 
> While you should do that, the CPU model is more generic. This would only
> imply that as of now, the APXA feature would always be available if the
> AP feature is available.

yes

> 
> In addition, it makes the vSIE handling code easier - there is always
> APXA and for now it cannot be disabled.
> 

yes :)

>>
>> In this case we will have no problem with older guests not having idea
>> about APXA.
>>
>> Would it be a solution?
> 
> Any feature the guest sees, should be part of the CPU model. The whole
> environment for cpu subfunctions is already in place both in KVM and
> QEMU. Only disabling subfunctions in KVM is not implemented yet.
> 
> You can exclude any subfunctions/facilities that are only valid on LPAR
> level and cannot be used in some guest either way. (that makes life
> sometimes easier)
> 
> 
> I know that this might sound a little bit complicated, but it really
> isn't. Boils down to modifying kvm_s390_cpu_feat_init() and specifying
> some features+feature groups in QEMU.

OK, we definitively need another patch/patch-set, to handle this.
Do you think it can be done in another series since if we always support 
APXA when we have AP instructions, we already have an indication that
APXA exist: the AP facility.

Regards,
Pierre

David Hildenbrand Aug. 22, 2018, 4:57 p.m. UTC | #8

>>>
>>> In this case we will have no problem with older guests not having idea
>>> about APXA.
>>>
>>> Would it be a solution?
>>
>> Any feature the guest sees, should be part of the CPU model. The whole
>> environment for cpu subfunctions is already in place both in KVM and
>> QEMU. Only disabling subfunctions in KVM is not implemented yet.
>>
>> You can exclude any subfunctions/facilities that are only valid on LPAR
>> level and cannot be used in some guest either way. (that makes life
>> sometimes easier)
>>
>>
>> I know that this might sound a little bit complicated, but it really
>> isn't. Boils down to modifying kvm_s390_cpu_feat_init() and specifying
>> some features+feature groups in QEMU.
> 
> OK, we definitively need another patch/patch-set, to handle this.
> Do you think it can be done in another series since if we always support 
> APXA when we have AP instructions, we already have an indication that
> APXA exist: the AP facility.
> 

Please implement the subfunction stuff right away. This will allow to
handle all future facilities transparently from a kernel POV.

Implementing that should be easy - and I don't like gluing features
together in such a way.

You can always assure that consistent data (e.g. AP + APXA availability)
is reported from KVM to QEMU.

> Regards,
> Pierre
> 
> 
> 
>

Anthony Krowiak Aug. 22, 2018, 8:16 p.m. UTC | #9

On 08/22/2018 07:24 AM, David Hildenbrand wrote:
> On 22.08.2018 13:19, David Hildenbrand wrote:
>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>> From: Tony Krowiak <[e-mail]>
>>>
>>> Introduces a new CPU model feature and two CPU model
>>> facilities to support AP virtualization for KVM guests.
>>>
>>> CPU model feature:
>>>
>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>> AP instructions are available on the guest. This
>>> feature will be enabled by the kernel only if the AP
>>> instructions are installed on the linux host. This feature
>>> must be specifically turned on for the KVM guest from
>>> userspace to use the VFIO AP device driver for guest
>>> access to AP devices.
>>>
>>> CPU model facilities:
>>>
>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>
>>>     This is indicated by setting facilities bit 12 for
>>>     the guest. The kernel will not enable this facility
>>>     for the guest if it is not set on the host.
>>>
>>>     If this facility is not set for the KVM guest, then only
>>>     APQNs with an APQI less than 16 will be used by a Linux
>>>     guest regardless of the matrix configuration for the virtual
>>>     machine. This is a limitation of the Linux AP bus.
>>>
>>> 2. AP Facilities Test facility (APFT) is installed.
>>>
>>>     This is indicated by setting facilities bit 15 for
>>>     the guest. The kernel will not enable this facility for
>>>     the guest if it is not set on the host.
>>>
>>>     If this facility is not set for the KVM guest, then no
>>>     AP devices will be available to the guest regardless of
>>>     the guest's matrix configuration for the virtual
>>>     machine. This is a limitation of the Linux AP bus.
>>>
>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>> Reviewed-by: Halil Pasic <[e-mail]>
>>> Tested-by: Michael Mueller <[e-mail]>
>>> Tested-by: Farhan Ali <[e-mail]>
>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>> ---
>>>   arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>   arch/s390/tools/gen_facilities.c |    2 ++
>>>   2 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 1e8cb67..d5e04d2 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>   
>>>   	if (MACHINE_HAS_ESOP)
>>>   		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>> +
>>> +	/* Check if AP instructions installed on host */
>>> +	if (ap_instructions_available())
>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>> +
>>>   	/*
>>>   	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>   	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>> index 90a8c9e..a52290b 100644
>>> --- a/arch/s390/tools/gen_facilities.c
>>> +++ b/arch/s390/tools/gen_facilities.c
>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>   
>>>   		.name = "FACILITIES_KVM_CPUMODEL",
>>>   		.bits = (int[]){
>>> +			12, /* AP Query Configuration Information */
>>> +			15, /* AP Facilities Test */
>>>   			-1  /* END */
>>>   		}
>>>   	},
>>>
>> I really wonder if we should also export the APXA facility.
>>
>> We can probe and allow that CPU feature. However, we cannot disable it
>> (as of now).
>>
>> We have other CPU features where it is the same case (basically all
>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>> export them, but support to disable them has never been implemented.
>>
>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>> is available but has been disabled. (until we know that disabling it
>> actually works - if ever).
>>
>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>> disappearing). Although unlikely, definitely possible.
>>
>>
>> Are there any other AP related facilities that the guest can from now on
>> probe that should also become part of the CPU model?
>>
> To be more precise, shouldn't PQAP(QCI) be handled just like other
> subfunctions? (I remember it should)

When you suggest PQAP(QCI) be handled like other subfunctions, are you
suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
with a bit indicating the QCI subfunction is available? The availability
of the QCI subfunction of the PQAP instruction is determined by facilities
bit 12. Is it not enough to export facilities bit 12?

>
> That would imply that there is actually theoretically a way to fake away
> certain AP facilities.
>

Anthony Krowiak Aug. 22, 2018, 8:54 p.m. UTC | #10

On 08/22/2018 07:19 AM, David Hildenbrand wrote:
> On 13.08.2018 23:48, Tony Krowiak wrote:
>> From: Tony Krowiak <[e-mail]>
>>
>> Introduces a new CPU model feature and two CPU model
>> facilities to support AP virtualization for KVM guests.
>>
>> CPU model feature:
>>
>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>> AP instructions are available on the guest. This
>> feature will be enabled by the kernel only if the AP
>> instructions are installed on the linux host. This feature
>> must be specifically turned on for the KVM guest from
>> userspace to use the VFIO AP device driver for guest
>> access to AP devices.
>>
>> CPU model facilities:
>>
>> 1. AP Query Configuration Information (QCI) facility is installed.
>>
>>     This is indicated by setting facilities bit 12 for
>>     the guest. The kernel will not enable this facility
>>     for the guest if it is not set on the host.
>>
>>     If this facility is not set for the KVM guest, then only
>>     APQNs with an APQI less than 16 will be used by a Linux
>>     guest regardless of the matrix configuration for the virtual
>>     machine. This is a limitation of the Linux AP bus.
>>
>> 2. AP Facilities Test facility (APFT) is installed.
>>
>>     This is indicated by setting facilities bit 15 for
>>     the guest. The kernel will not enable this facility for
>>     the guest if it is not set on the host.
>>
>>     If this facility is not set for the KVM guest, then no
>>     AP devices will be available to the guest regardless of
>>     the guest's matrix configuration for the virtual
>>     machine. This is a limitation of the Linux AP bus.
>>
>> Signed-off-by: Tony Krowiak <[e-mail]>
>> Reviewed-by: Christian Borntraeger <[e-mail]>
>> Reviewed-by: Halil Pasic <[e-mail]>
>> Tested-by: Michael Mueller <[e-mail]>
>> Tested-by: Farhan Ali <[e-mail]>
>> Signed-off-by: Christian Borntraeger <[e-mail]>
>> ---
>>   arch/s390/kvm/kvm-s390.c         |    5 +++++
>>   arch/s390/tools/gen_facilities.c |    2 ++
>>   2 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 1e8cb67..d5e04d2 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>   
>>   	if (MACHINE_HAS_ESOP)
>>   		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>> +
>> +	/* Check if AP instructions installed on host */
>> +	if (ap_instructions_available())
>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>> +
>>   	/*
>>   	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>   	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>> index 90a8c9e..a52290b 100644
>> --- a/arch/s390/tools/gen_facilities.c
>> +++ b/arch/s390/tools/gen_facilities.c
>> @@ -106,6 +106,8 @@ struct facility_def {
>>   
>>   		.name = "FACILITIES_KVM_CPUMODEL",
>>   		.bits = (int[]){
>> +			12, /* AP Query Configuration Information */
>> +			15, /* AP Facilities Test */
>>   			-1  /* END */
>>   		}
>>   	},
>>
> I really wonder if we should also export the APXA facility.

Given this comment is made within the context of the
FACILITIES_KVM_CPUMODEL I might point out that APXA is not
indicated by a facilities bit. It is indicated by a bit in
the QCI control block returned from the PQAP(QCI)
instruction to indicate that APXA is installed on all CPUs.

> We can probe and allow that CPU feature. However, we cannot disable it
> (as of now).

Given this patch series implements passthrough devices,
the output of the PQAP(QCI) will always be from a real
device - i.e., there will be no way to disable it.

>
> We have other CPU features where it is the same case (basically all
> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
> export them, but support to disable them has never been implemented.
>
> On a high level, we could then e.g. deny to start a QEMU guest if APXA
> is available but has been disabled. (until we know that disabling it
> actually works - if ever).
>
> This helps to catch nasty migration bugs (e.g. APXA suddenly
> disappearing). Although unlikely, definitely possible.

Migration of AP devices is not supported by this patch series, so this 
should
not be an issue.

>
>
> Are there any other AP related facilities that the guest can from now on
> probe that should also become part of the CPU model?
>

Anthony Krowiak Aug. 22, 2018, 9:05 p.m. UTC | #11

On 08/22/2018 12:57 PM, David Hildenbrand wrote:
>>>> In this case we will have no problem with older guests not having idea
>>>> about APXA.
>>>>
>>>> Would it be a solution?
>>> Any feature the guest sees, should be part of the CPU model. The whole
>>> environment for cpu subfunctions is already in place both in KVM and
>>> QEMU. Only disabling subfunctions in KVM is not implemented yet.
>>>
>>> You can exclude any subfunctions/facilities that are only valid on LPAR
>>> level and cannot be used in some guest either way. (that makes life
>>> sometimes easier)
>>>
>>>
>>> I know that this might sound a little bit complicated, but it really
>>> isn't. Boils down to modifying kvm_s390_cpu_feat_init() and specifying
>>> some features+feature groups in QEMU.
>> OK, we definitively need another patch/patch-set, to handle this.
>> Do you think it can be done in another series since if we always support
>> APXA when we have AP instructions, we already have an indication that
>> APXA exist: the AP facility.
>>
> Please implement the subfunction stuff right away. This will allow to
> handle all future facilities transparently from a kernel POV.

I find your use of the term 'subfunction' confusing here. In the
kvm_s390_cpu_feat_init(void) function, it looks like the
kvm_s390_available_subfunc structure is filled in with bits
returned from CPACF queries of various MSA facilities to indicate
which CPACF functions are supported. APXA is not a subfunction but
a facility that is indicated by a bit returned from the PQAP(QCI)
instruction. If we are to implement this, wouldn't it be done as
a CPU model feature as opposed to a subfunction? Am I
misunderstanding what you are asking for?

>
> Implementing that should be easy - and I don't like gluing features
> together in such a way.
>
> You can always assure that consistent data (e.g. AP + APXA availability)
> is reported from KVM to QEMU.
>
>> Regards,
>> Pierre
>>
>>
>>
>>
>

David Hildenbrand Aug. 23, 2018, 7:42 a.m. UTC | #12

On 22.08.2018 23:05, Tony Krowiak wrote:
> On 08/22/2018 12:57 PM, David Hildenbrand wrote:
>>>>> In this case we will have no problem with older guests not having idea
>>>>> about APXA.
>>>>>
>>>>> Would it be a solution?
>>>> Any feature the guest sees, should be part of the CPU model. The whole
>>>> environment for cpu subfunctions is already in place both in KVM and
>>>> QEMU. Only disabling subfunctions in KVM is not implemented yet.
>>>>
>>>> You can exclude any subfunctions/facilities that are only valid on LPAR
>>>> level and cannot be used in some guest either way. (that makes life
>>>> sometimes easier)
>>>>
>>>>
>>>> I know that this might sound a little bit complicated, but it really
>>>> isn't. Boils down to modifying kvm_s390_cpu_feat_init() and specifying
>>>> some features+feature groups in QEMU.
>>> OK, we definitively need another patch/patch-set, to handle this.
>>> Do you think it can be done in another series since if we always support
>>> APXA when we have AP instructions, we already have an indication that
>>> APXA exist: the AP facility.
>>>
>> Please implement the subfunction stuff right away. This will allow to
>> handle all future facilities transparently from a kernel POV.
> 
> I find your use of the term 'subfunction' confusing here. In the
> kvm_s390_cpu_feat_init(void) function, it looks like the
> kvm_s390_available_subfunc structure is filled in with bits
> returned from CPACF queries of various MSA facilities to indicate
> which CPACF functions are supported. APXA is not a subfunction but
> a facility that is indicated by a bit returned from the PQAP(QCI)
> instruction. If we are to implement this, wouldn't it be done as
> a CPU model feature as opposed to a subfunction? Am I
> misunderstanding what you are asking for?

Yes, "subfunction" is a confusing terminology. (I once called it
subfeature/sufacility, but ended up using subfunction).

From a high level perspective, these are just feature bits - "can I use
feature X" / "is feature X available".

What all of these "query" blocks (MSA, PLO, PQAP(QCI) ...) have in
common is:
- in contrast to STFL(E), they are as a default not modified by the
  hypervisor but silently passed through
- dropping one of the bits (e.g. APXA) can break the guest - guest
  visible ABI
- any newly added feature/facility in such a block (new HW generation)
  should be transparently handled by HW and not require modifications in
  the hypervisor - because they are right away presented to the guest.
  (unfortunately with minor exceptions - e.g. APXA might be such a
  candidate, but that was rather a design error back then)
- there is a way we can overwrite which features are presented to the
  guest

The nice thing about that "blob" exported to user space (in contrast to
features) is that it does not have to be fixed up in KVM every time a
new feature/facility is added. As they should be transparently handled.
Only QEMU has to be thought about the new feature - which can be done
right away when introducing the new CPU model.

That's why the natural choice for PQAP(QCI) is also exposing it as
subfunctions, and not as CPU model features (kvm interface).

David Hildenbrand Aug. 23, 2018, 7:44 a.m. UTC | #13

On 22.08.2018 22:16, Tony Krowiak wrote:
> On 08/22/2018 07:24 AM, David Hildenbrand wrote:
>> On 22.08.2018 13:19, David Hildenbrand wrote:
>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>> From: Tony Krowiak <[e-mail]>
>>>>
>>>> Introduces a new CPU model feature and two CPU model
>>>> facilities to support AP virtualization for KVM guests.
>>>>
>>>> CPU model feature:
>>>>
>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>> AP instructions are available on the guest. This
>>>> feature will be enabled by the kernel only if the AP
>>>> instructions are installed on the linux host. This feature
>>>> must be specifically turned on for the KVM guest from
>>>> userspace to use the VFIO AP device driver for guest
>>>> access to AP devices.
>>>>
>>>> CPU model facilities:
>>>>
>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>
>>>>     This is indicated by setting facilities bit 12 for
>>>>     the guest. The kernel will not enable this facility
>>>>     for the guest if it is not set on the host.
>>>>
>>>>     If this facility is not set for the KVM guest, then only
>>>>     APQNs with an APQI less than 16 will be used by a Linux
>>>>     guest regardless of the matrix configuration for the virtual
>>>>     machine. This is a limitation of the Linux AP bus.
>>>>
>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>
>>>>     This is indicated by setting facilities bit 15 for
>>>>     the guest. The kernel will not enable this facility for
>>>>     the guest if it is not set on the host.
>>>>
>>>>     If this facility is not set for the KVM guest, then no
>>>>     AP devices will be available to the guest regardless of
>>>>     the guest's matrix configuration for the virtual
>>>>     machine. This is a limitation of the Linux AP bus.
>>>>
>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>> Tested-by: Michael Mueller <[e-mail]>
>>>> Tested-by: Farhan Ali <[e-mail]>
>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>> ---
>>>>   arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>   arch/s390/tools/gen_facilities.c |    2 ++
>>>>   2 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>> index 1e8cb67..d5e04d2 100644
>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>   
>>>>   	if (MACHINE_HAS_ESOP)
>>>>   		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>> +
>>>> +	/* Check if AP instructions installed on host */
>>>> +	if (ap_instructions_available())
>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>> +
>>>>   	/*
>>>>   	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>   	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>> index 90a8c9e..a52290b 100644
>>>> --- a/arch/s390/tools/gen_facilities.c
>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>   
>>>>   		.name = "FACILITIES_KVM_CPUMODEL",
>>>>   		.bits = (int[]){
>>>> +			12, /* AP Query Configuration Information */
>>>> +			15, /* AP Facilities Test */
>>>>   			-1  /* END */
>>>>   		}
>>>>   	},
>>>>
>>> I really wonder if we should also export the APXA facility.
>>>
>>> We can probe and allow that CPU feature. However, we cannot disable it
>>> (as of now).
>>>
>>> We have other CPU features where it is the same case (basically all
>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>> export them, but support to disable them has never been implemented.
>>>
>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>> is available but has been disabled. (until we know that disabling it
>>> actually works - if ever).
>>>
>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>> disappearing). Although unlikely, definitely possible.
>>>
>>>
>>> Are there any other AP related facilities that the guest can from now on
>>> probe that should also become part of the CPU model?
>>>
>> To be more precise, shouldn't PQAP(QCI) be handled just like other
>> subfunctions? (I remember it should)
> 
> When you suggest PQAP(QCI) be handled like other subfunctions, are you
> suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
> with a bit indicating the QCI subfunction is available? The availability
> of the QCI subfunction of the PQAP instruction is determined by facilities
> bit 12. Is it not enough to export facilities bit 12?

The feature block (128 bit) from PQAP(QCI) should be passed through a
subfunction block to QEMU.

So it is about passing e.g. APXA availability, not QCI itself. (as you
correctly said, that is stfl 12)

David Hildenbrand Aug. 23, 2018, 7:48 a.m. UTC | #14

>>>
>> I really wonder if we should also export the APXA facility.
> 
> Given this comment is made within the context of the
> FACILITIES_KVM_CPUMODEL I might point out that APXA is not
> indicated by a facilities bit. It is indicated by a bit in
> the QCI control block returned from the PQAP(QCI)
> instruction to indicate that APXA is installed on all CPUs.
> 
>> We can probe and allow that CPU feature. However, we cannot disable it
>> (as of now).
> 
> Given this patch series implements passthrough devices,
> the output of the PQAP(QCI) will always be from a real
> device - i.e., there will be no way to disable it.
> 

see below

>>
>> We have other CPU features where it is the same case (basically all
>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>> export them, but support to disable them has never been implemented.
>>
>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>> is available but has been disabled. (until we know that disabling it
>> actually works - if ever).
>>
>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>> disappearing). Although unlikely, definitely possible.
> 
> Migration of AP devices is not supported by this patch series, so this 
> should
> not be an issue.

Might not be a problem now, but could be later. As I said in a different
reply, the CPU model in QEMU does not care about KVM.

I want the QEMU CPU model and the KVM interfaces to be clean and future
proof. That's why my opinion is to handle PQAP(QCI) just like all the
other "feature blocks" we already have.

Cornelia Huck Aug. 23, 2018, 8:24 a.m. UTC | #15

On Thu, 23 Aug 2018 09:48:48 +0200
David Hildenbrand <[e-mail]> wrote:

> > Migration of AP devices is not supported by this patch series, so this 
> > should
> > not be an issue.  
> 
> Might not be a problem now, but could be later. As I said in a different
> reply, the CPU model in QEMU does not care about KVM.
> 
> I want the QEMU CPU model and the KVM interfaces to be clean and future
> proof. That's why my opinion is to handle PQAP(QCI) just like all the
> other "feature blocks" we already have.

+1 to that sentiment.

It's better to try to get this correct now than having to hack around
should we want to implement things in the future.

Pierre Morel Aug. 23, 2018, 8:26 a.m. UTC | #16

On 23/08/2018 09:48, David Hildenbrand wrote:
>>>>
>>> I really wonder if we should also export the APXA facility.
>>
>> Given this comment is made within the context of the
>> FACILITIES_KVM_CPUMODEL I might point out that APXA is not
>> indicated by a facilities bit. It is indicated by a bit in
>> the QCI control block returned from the PQAP(QCI)
>> instruction to indicate that APXA is installed on all CPUs.
>>
>>> We can probe and allow that CPU feature. However, we cannot disable it
>>> (as of now).
>>
>> Given this patch series implements passthrough devices,
>> the output of the PQAP(QCI) will always be from a real
>> device - i.e., there will be no way to disable it.
>>
> 
> see below
> 
>>>
>>> We have other CPU features where it is the same case (basically all
>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>> export them, but support to disable them has never been implemented.
>>>
>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>> is available but has been disabled. (until we know that disabling it
>>> actually works - if ever).
>>>
>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>> disappearing). Although unlikely, definitely possible.
>>
>> Migration of AP devices is not supported by this patch series, so this
>> should
>> not be an issue.
> 
> Might not be a problem now, but could be later. As I said in a different
> reply, the CPU model in QEMU does not care about KVM.
> 
> I want the QEMU CPU model and the KVM interfaces to be clean and future
> proof. That's why my opinion is to handle PQAP(QCI) just like all the
> other "feature blocks" we already have.
> 

Don't you mix with the TAPQ instruction which has
a T bit to specify interception.
It indeed is not in the subfunction list even it
has a T bit to indicate interception.

TAPQ-t is indicated through the APFT facility.

We can use this bit as an indication of the presence
of APXA, the documentation mention that both are implemented together.

regards,
Pierre

Halil Pasic Aug. 23, 2018, 10 a.m. UTC | #17

On 08/23/2018 09:44 AM, David Hildenbrand wrote:
> On 22.08.2018 22:16, Tony Krowiak wrote:
>> On 08/22/2018 07:24 AM, David Hildenbrand wrote:
>>> On 22.08.2018 13:19, David Hildenbrand wrote:
>>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>>> From: Tony Krowiak <[e-mail]>
>>>>>
>>>>> Introduces a new CPU model feature and two CPU model
>>>>> facilities to support AP virtualization for KVM guests.
>>>>>
>>>>> CPU model feature:
>>>>>
>>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>>> AP instructions are available on the guest. This
>>>>> feature will be enabled by the kernel only if the AP
>>>>> instructions are installed on the linux host. This feature
>>>>> must be specifically turned on for the KVM guest from
>>>>> userspace to use the VFIO AP device driver for guest
>>>>> access to AP devices.
>>>>>
>>>>> CPU model facilities:
>>>>>
>>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>>
>>>>>      This is indicated by setting facilities bit 12 for
>>>>>      the guest. The kernel will not enable this facility
>>>>>      for the guest if it is not set on the host.
>>>>>
>>>>>      If this facility is not set for the KVM guest, then only
>>>>>      APQNs with an APQI less than 16 will be used by a Linux
>>>>>      guest regardless of the matrix configuration for the virtual
>>>>>      machine. This is a limitation of the Linux AP bus.
>>>>>
>>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>>
>>>>>      This is indicated by setting facilities bit 15 for
>>>>>      the guest. The kernel will not enable this facility for
>>>>>      the guest if it is not set on the host.
>>>>>
>>>>>      If this facility is not set for the KVM guest, then no
>>>>>      AP devices will be available to the guest regardless of
>>>>>      the guest's matrix configuration for the virtual
>>>>>      machine. This is a limitation of the Linux AP bus.
>>>>>
>>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>>> Tested-by: Michael Mueller <[e-mail]>
>>>>> Tested-by: Farhan Ali <[e-mail]>
>>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>>> ---
>>>>>    arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>>    arch/s390/tools/gen_facilities.c |    2 ++
>>>>>    2 files changed, 7 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>> index 1e8cb67..d5e04d2 100644
>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>>    
>>>>>    	if (MACHINE_HAS_ESOP)
>>>>>    		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>>> +
>>>>> +	/* Check if AP instructions installed on host */
>>>>> +	if (ap_instructions_available())
>>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>>> +
>>>>>    	/*
>>>>>    	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>>    	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>>> index 90a8c9e..a52290b 100644
>>>>> --- a/arch/s390/tools/gen_facilities.c
>>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>>    
>>>>>    		.name = "FACILITIES_KVM_CPUMODEL",
>>>>>    		.bits = (int[]){
>>>>> +			12, /* AP Query Configuration Information */
>>>>> +			15, /* AP Facilities Test */
>>>>>    			-1  /* END */
>>>>>    		}
>>>>>    	},
>>>>>
>>>> I really wonder if we should also export the APXA facility.
>>>>
>>>> We can probe and allow that CPU feature. However, we cannot disable it
>>>> (as of now).
>>>>
>>>> We have other CPU features where it is the same case (basically all
>>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>>> export them, but support to disable them has never been implemented.
>>>>
>>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>>> is available but has been disabled. (until we know that disabling it
>>>> actually works - if ever).
>>>>
>>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>>> disappearing). Although unlikely, definitely possible.
>>>>
>>>>
>>>> Are there any other AP related facilities that the guest can from now on
>>>> probe that should also become part of the CPU model?
>>>>
>>> To be more precise, shouldn't PQAP(QCI) be handled just like other
>>> subfunctions? (I remember it should)
>>
>> When you suggest PQAP(QCI) be handled like other subfunctions, are you
>> suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
>> with a bit indicating the QCI subfunction is available? The availability
>> of the QCI subfunction of the PQAP instruction is determined by facilities
>> bit 12. Is it not enough to export facilities bit 12?
> 
> The feature block (128 bit) from PQAP(QCI) should be passed through a
> subfunction block to QEMU.
> 

I'm confused, which 128 bit?

> So it is about passing e.g. APXA availability, not QCI itself. (as you
> correctly said, that is stfl 12)
>

David Hildenbrand Aug. 23, 2018, 10:28 a.m. UTC | #18

On 23.08.2018 12:00, Halil Pasic wrote:
> 
> 
> On 08/23/2018 09:44 AM, David Hildenbrand wrote:
>> On 22.08.2018 22:16, Tony Krowiak wrote:
>>> On 08/22/2018 07:24 AM, David Hildenbrand wrote:
>>>> On 22.08.2018 13:19, David Hildenbrand wrote:
>>>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>>>> From: Tony Krowiak <[e-mail]>
>>>>>>
>>>>>> Introduces a new CPU model feature and two CPU model
>>>>>> facilities to support AP virtualization for KVM guests.
>>>>>>
>>>>>> CPU model feature:
>>>>>>
>>>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>>>> AP instructions are available on the guest. This
>>>>>> feature will be enabled by the kernel only if the AP
>>>>>> instructions are installed on the linux host. This feature
>>>>>> must be specifically turned on for the KVM guest from
>>>>>> userspace to use the VFIO AP device driver for guest
>>>>>> access to AP devices.
>>>>>>
>>>>>> CPU model facilities:
>>>>>>
>>>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>>>
>>>>>>      This is indicated by setting facilities bit 12 for
>>>>>>      the guest. The kernel will not enable this facility
>>>>>>      for the guest if it is not set on the host.
>>>>>>
>>>>>>      If this facility is not set for the KVM guest, then only
>>>>>>      APQNs with an APQI less than 16 will be used by a Linux
>>>>>>      guest regardless of the matrix configuration for the virtual
>>>>>>      machine. This is a limitation of the Linux AP bus.
>>>>>>
>>>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>>>
>>>>>>      This is indicated by setting facilities bit 15 for
>>>>>>      the guest. The kernel will not enable this facility for
>>>>>>      the guest if it is not set on the host.
>>>>>>
>>>>>>      If this facility is not set for the KVM guest, then no
>>>>>>      AP devices will be available to the guest regardless of
>>>>>>      the guest's matrix configuration for the virtual
>>>>>>      machine. This is a limitation of the Linux AP bus.
>>>>>>
>>>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>>>> Tested-by: Michael Mueller <[e-mail]>
>>>>>> Tested-by: Farhan Ali <[e-mail]>
>>>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>>>> ---
>>>>>>    arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>>>    arch/s390/tools/gen_facilities.c |    2 ++
>>>>>>    2 files changed, 7 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>> index 1e8cb67..d5e04d2 100644
>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>>>    
>>>>>>    	if (MACHINE_HAS_ESOP)
>>>>>>    		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>>>> +
>>>>>> +	/* Check if AP instructions installed on host */
>>>>>> +	if (ap_instructions_available())
>>>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>>>> +
>>>>>>    	/*
>>>>>>    	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>>>    	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>>>> index 90a8c9e..a52290b 100644
>>>>>> --- a/arch/s390/tools/gen_facilities.c
>>>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>>>    
>>>>>>    		.name = "FACILITIES_KVM_CPUMODEL",
>>>>>>    		.bits = (int[]){
>>>>>> +			12, /* AP Query Configuration Information */
>>>>>> +			15, /* AP Facilities Test */
>>>>>>    			-1  /* END */
>>>>>>    		}
>>>>>>    	},
>>>>>>
>>>>> I really wonder if we should also export the APXA facility.
>>>>>
>>>>> We can probe and allow that CPU feature. However, we cannot disable it
>>>>> (as of now).
>>>>>
>>>>> We have other CPU features where it is the same case (basically all
>>>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>>>> export them, but support to disable them has never been implemented.
>>>>>
>>>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>>>> is available but has been disabled. (until we know that disabling it
>>>>> actually works - if ever).
>>>>>
>>>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>>>> disappearing). Although unlikely, definitely possible.
>>>>>
>>>>>
>>>>> Are there any other AP related facilities that the guest can from now on
>>>>> probe that should also become part of the CPU model?
>>>>>
>>>> To be more precise, shouldn't PQAP(QCI) be handled just like other
>>>> subfunctions? (I remember it should)
>>>
>>> When you suggest PQAP(QCI) be handled like other subfunctions, are you
>>> suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
>>> with a bit indicating the QCI subfunction is available? The availability
>>> of the QCI subfunction of the PQAP instruction is determined by facilities
>>> bit 12. Is it not enough to export facilities bit 12?
>>
>> The feature block (128 bit) from PQAP(QCI) should be passed through a
>> subfunction block to QEMU.
>>
> 
> I'm confused, which 128 bit?


Me too :) , I was assuming this block to be 128bit, but the qci block
has 128 bytes....

And looking at arch/s390/include/asm/ap.h, there is a lot of information
contained that is definitely not of interest for CPU models...

I wonder if there is somewhere defined which bits are reserved for
future features/facilities, compared to ap masks and such.

This is really hard to understand/plan without access to documentation.

You (Halil, Tony, Pier, ...) should have a look if what I described
related to PQAP(QCI) containing features that should get part of the CPU
model makes sense or not. For now I was thinking that there is some part
inside of QCI that is strictly reserved for facilities/features that we
can use.

Pierre Morel Aug. 23, 2018, 11:10 a.m. UTC | #19

On 23/08/2018 12:28, David Hildenbrand wrote:
> On 23.08.2018 12:00, Halil Pasic wrote:
>>
>>
>> On 08/23/2018 09:44 AM, David Hildenbrand wrote:
>>> On 22.08.2018 22:16, Tony Krowiak wrote:
>>>> On 08/22/2018 07:24 AM, David Hildenbrand wrote:
>>>>> On 22.08.2018 13:19, David Hildenbrand wrote:
>>>>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>>>>> From: Tony Krowiak <[e-mail]>
>>>>>>>
>>>>>>> Introduces a new CPU model feature and two CPU model
>>>>>>> facilities to support AP virtualization for KVM guests.
>>>>>>>
>>>>>>> CPU model feature:
>>>>>>>
>>>>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>>>>> AP instructions are available on the guest. This
>>>>>>> feature will be enabled by the kernel only if the AP
>>>>>>> instructions are installed on the linux host. This feature
>>>>>>> must be specifically turned on for the KVM guest from
>>>>>>> userspace to use the VFIO AP device driver for guest
>>>>>>> access to AP devices.
>>>>>>>
>>>>>>> CPU model facilities:
>>>>>>>
>>>>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>>>>
>>>>>>>       This is indicated by setting facilities bit 12 for
>>>>>>>       the guest. The kernel will not enable this facility
>>>>>>>       for the guest if it is not set on the host.
>>>>>>>
>>>>>>>       If this facility is not set for the KVM guest, then only
>>>>>>>       APQNs with an APQI less than 16 will be used by a Linux
>>>>>>>       guest regardless of the matrix configuration for the virtual
>>>>>>>       machine. This is a limitation of the Linux AP bus.
>>>>>>>
>>>>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>>>>
>>>>>>>       This is indicated by setting facilities bit 15 for
>>>>>>>       the guest. The kernel will not enable this facility for
>>>>>>>       the guest if it is not set on the host.
>>>>>>>
>>>>>>>       If this facility is not set for the KVM guest, then no
>>>>>>>       AP devices will be available to the guest regardless of
>>>>>>>       the guest's matrix configuration for the virtual
>>>>>>>       machine. This is a limitation of the Linux AP bus.
>>>>>>>
>>>>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>>>>> Tested-by: Michael Mueller <[e-mail]>
>>>>>>> Tested-by: Farhan Ali <[e-mail]>
>>>>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>>>>> ---
>>>>>>>     arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>>>>     arch/s390/tools/gen_facilities.c |    2 ++
>>>>>>>     2 files changed, 7 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>>> index 1e8cb67..d5e04d2 100644
>>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>>>>     
>>>>>>>     	if (MACHINE_HAS_ESOP)
>>>>>>>     		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>>>>> +
>>>>>>> +	/* Check if AP instructions installed on host */
>>>>>>> +	if (ap_instructions_available())
>>>>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>>>>> +
>>>>>>>     	/*
>>>>>>>     	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>>>>     	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>>>>> index 90a8c9e..a52290b 100644
>>>>>>> --- a/arch/s390/tools/gen_facilities.c
>>>>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>>>>     
>>>>>>>     		.name = "FACILITIES_KVM_CPUMODEL",
>>>>>>>     		.bits = (int[]){
>>>>>>> +			12, /* AP Query Configuration Information */
>>>>>>> +			15, /* AP Facilities Test */
>>>>>>>     			-1  /* END */
>>>>>>>     		}
>>>>>>>     	},
>>>>>>>
>>>>>> I really wonder if we should also export the APXA facility.
>>>>>>
>>>>>> We can probe and allow that CPU feature. However, we cannot disable it
>>>>>> (as of now).
>>>>>>
>>>>>> We have other CPU features where it is the same case (basically all
>>>>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>>>>> export them, but support to disable them has never been implemented.
>>>>>>
>>>>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>>>>> is available but has been disabled. (until we know that disabling it
>>>>>> actually works - if ever).
>>>>>>
>>>>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>>>>> disappearing). Although unlikely, definitely possible.
>>>>>>
>>>>>>
>>>>>> Are there any other AP related facilities that the guest can from now on
>>>>>> probe that should also become part of the CPU model?
>>>>>>
>>>>> To be more precise, shouldn't PQAP(QCI) be handled just like other
>>>>> subfunctions? (I remember it should)
>>>>
>>>> When you suggest PQAP(QCI) be handled like other subfunctions, are you
>>>> suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
>>>> with a bit indicating the QCI subfunction is available? The availability
>>>> of the QCI subfunction of the PQAP instruction is determined by facilities
>>>> bit 12. Is it not enough to export facilities bit 12?
>>>
>>> The feature block (128 bit) from PQAP(QCI) should be passed through a
>>> subfunction block to QEMU.
>>>
>>
>> I'm confused, which 128 bit?
> 
> 
> Me too :) , I was assuming this block to be 128bit, but the qci block
> has 128 bytes....
> 
> And looking at arch/s390/include/asm/ap.h, there is a lot of information
> contained that is definitely not of interest for CPU models...
> 
> I wonder if there is somewhere defined which bits are reserved for
> future features/facilities, compared to ap masks and such.
> 
> This is really hard to understand/plan without access to documentation.
> 
> You (Halil, Tony, Pier, ...) should have a look if what I described
> related to PQAP(QCI) containing features that should get part of the CPU
> model makes sense or not. For now I was thinking that there is some part
> inside of QCI that is strictly reserved for facilities/features that we
> can use.
> 

David,
I already answered to you on this subject.

First,
Are you sure you do not mistake QCI for TAPQ which has the t bit
instruction interception bit as all the instructions you use as
subfunctions?

Second,
The TAPQ interception bit is exposed through the facility bit 15
and is documented as being installed when the APXA facility is installed.

If we have the APFT, we have the APXA, problem seems solved to me.

Regards,
Pierre

David Hildenbrand Aug. 23, 2018, 11:12 a.m. UTC | #20

On 23.08.2018 13:10, Pierre Morel wrote:
> On 23/08/2018 12:28, David Hildenbrand wrote:
>> On 23.08.2018 12:00, Halil Pasic wrote:
>>>
>>>
>>> On 08/23/2018 09:44 AM, David Hildenbrand wrote:
>>>> On 22.08.2018 22:16, Tony Krowiak wrote:
>>>>> On 08/22/2018 07:24 AM, David Hildenbrand wrote:
>>>>>> On 22.08.2018 13:19, David Hildenbrand wrote:
>>>>>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>>>>>> From: Tony Krowiak <[e-mail]>
>>>>>>>>
>>>>>>>> Introduces a new CPU model feature and two CPU model
>>>>>>>> facilities to support AP virtualization for KVM guests.
>>>>>>>>
>>>>>>>> CPU model feature:
>>>>>>>>
>>>>>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>>>>>> AP instructions are available on the guest. This
>>>>>>>> feature will be enabled by the kernel only if the AP
>>>>>>>> instructions are installed on the linux host. This feature
>>>>>>>> must be specifically turned on for the KVM guest from
>>>>>>>> userspace to use the VFIO AP device driver for guest
>>>>>>>> access to AP devices.
>>>>>>>>
>>>>>>>> CPU model facilities:
>>>>>>>>
>>>>>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>>>>>
>>>>>>>>       This is indicated by setting facilities bit 12 for
>>>>>>>>       the guest. The kernel will not enable this facility
>>>>>>>>       for the guest if it is not set on the host.
>>>>>>>>
>>>>>>>>       If this facility is not set for the KVM guest, then only
>>>>>>>>       APQNs with an APQI less than 16 will be used by a Linux
>>>>>>>>       guest regardless of the matrix configuration for the virtual
>>>>>>>>       machine. This is a limitation of the Linux AP bus.
>>>>>>>>
>>>>>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>>>>>
>>>>>>>>       This is indicated by setting facilities bit 15 for
>>>>>>>>       the guest. The kernel will not enable this facility for
>>>>>>>>       the guest if it is not set on the host.
>>>>>>>>
>>>>>>>>       If this facility is not set for the KVM guest, then no
>>>>>>>>       AP devices will be available to the guest regardless of
>>>>>>>>       the guest's matrix configuration for the virtual
>>>>>>>>       machine. This is a limitation of the Linux AP bus.
>>>>>>>>
>>>>>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>>>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>>>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>>>>>> Tested-by: Michael Mueller <[e-mail]>
>>>>>>>> Tested-by: Farhan Ali <[e-mail]>
>>>>>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>>>>>> ---
>>>>>>>>     arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>>>>>     arch/s390/tools/gen_facilities.c |    2 ++
>>>>>>>>     2 files changed, 7 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>>>> index 1e8cb67..d5e04d2 100644
>>>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>>>>>     
>>>>>>>>     	if (MACHINE_HAS_ESOP)
>>>>>>>>     		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>>>>>> +
>>>>>>>> +	/* Check if AP instructions installed on host */
>>>>>>>> +	if (ap_instructions_available())
>>>>>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>>>>>> +
>>>>>>>>     	/*
>>>>>>>>     	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>>>>>     	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>>>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>>>>>> index 90a8c9e..a52290b 100644
>>>>>>>> --- a/arch/s390/tools/gen_facilities.c
>>>>>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>>>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>>>>>     
>>>>>>>>     		.name = "FACILITIES_KVM_CPUMODEL",
>>>>>>>>     		.bits = (int[]){
>>>>>>>> +			12, /* AP Query Configuration Information */
>>>>>>>> +			15, /* AP Facilities Test */
>>>>>>>>     			-1  /* END */
>>>>>>>>     		}
>>>>>>>>     	},
>>>>>>>>
>>>>>>> I really wonder if we should also export the APXA facility.
>>>>>>>
>>>>>>> We can probe and allow that CPU feature. However, we cannot disable it
>>>>>>> (as of now).
>>>>>>>
>>>>>>> We have other CPU features where it is the same case (basically all
>>>>>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>>>>>> export them, but support to disable them has never been implemented.
>>>>>>>
>>>>>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>>>>>> is available but has been disabled. (until we know that disabling it
>>>>>>> actually works - if ever).
>>>>>>>
>>>>>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>>>>>> disappearing). Although unlikely, definitely possible.
>>>>>>>
>>>>>>>
>>>>>>> Are there any other AP related facilities that the guest can from now on
>>>>>>> probe that should also become part of the CPU model?
>>>>>>>
>>>>>> To be more precise, shouldn't PQAP(QCI) be handled just like other
>>>>>> subfunctions? (I remember it should)
>>>>>
>>>>> When you suggest PQAP(QCI) be handled like other subfunctions, are you
>>>>> suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
>>>>> with a bit indicating the QCI subfunction is available? The availability
>>>>> of the QCI subfunction of the PQAP instruction is determined by facilities
>>>>> bit 12. Is it not enough to export facilities bit 12?
>>>>
>>>> The feature block (128 bit) from PQAP(QCI) should be passed through a
>>>> subfunction block to QEMU.
>>>>
>>>
>>> I'm confused, which 128 bit?
>>
>>
>> Me too :) , I was assuming this block to be 128bit, but the qci block
>> has 128 bytes....
>>
>> And looking at arch/s390/include/asm/ap.h, there is a lot of information
>> contained that is definitely not of interest for CPU models...
>>
>> I wonder if there is somewhere defined which bits are reserved for
>> future features/facilities, compared to ap masks and such.
>>
>> This is really hard to understand/plan without access to documentation.
>>
>> You (Halil, Tony, Pier, ...) should have a look if what I described
>> related to PQAP(QCI) containing features that should get part of the CPU
>> model makes sense or not. For now I was thinking that there is some part
>> inside of QCI that is strictly reserved for facilities/features that we
>> can use.
>>
> 
> David,
> I already answered to you on this subject.
> 
> First,
> Are you sure you do not mistake QCI for TAPQ which has the t bit
> instruction interception bit as all the instructions you use as
> subfunctions?

Yes, I am pretty sure it is PQAP(QCI), please check with Christian /
architecture documentations.

> 
> Second,
> The TAPQ interception bit is exposed through the facility bit 15
> and is documented as being installed when the APXA facility is installed.
> 
> If we have the APFT, we have the APXA, problem seems solved to me.

What is apsc, qact, rc8a in the qci blocks? are the facility bits?

> 
> Regards,
> Pierre
>

Pierre Morel Aug. 23, 2018, 12:47 p.m. UTC | #21

On 23/08/2018 13:12, David Hildenbrand wrote:
> On 23.08.2018 13:10, Pierre Morel wrote:
>> On 23/08/2018 12:28, David Hildenbrand wrote:
>>> On 23.08.2018 12:00, Halil Pasic wrote:
>>>>
>>>>
>>>> On 08/23/2018 09:44 AM, David Hildenbrand wrote:
>>>>> On 22.08.2018 22:16, Tony Krowiak wrote:
>>>>>> On 08/22/2018 07:24 AM, David Hildenbrand wrote:
>>>>>>> On 22.08.2018 13:19, David Hildenbrand wrote:
>>>>>>>> On 13.08.2018 23:48, Tony Krowiak wrote:
>>>>>>>>> From: Tony Krowiak <[e-mail]>
>>>>>>>>>
>>>>>>>>> Introduces a new CPU model feature and two CPU model
>>>>>>>>> facilities to support AP virtualization for KVM guests.
>>>>>>>>>
>>>>>>>>> CPU model feature:
>>>>>>>>>
>>>>>>>>> The KVM_S390_VM_CPU_FEAT_AP feature indicates that
>>>>>>>>> AP instructions are available on the guest. This
>>>>>>>>> feature will be enabled by the kernel only if the AP
>>>>>>>>> instructions are installed on the linux host. This feature
>>>>>>>>> must be specifically turned on for the KVM guest from
>>>>>>>>> userspace to use the VFIO AP device driver for guest
>>>>>>>>> access to AP devices.
>>>>>>>>>
>>>>>>>>> CPU model facilities:
>>>>>>>>>
>>>>>>>>> 1. AP Query Configuration Information (QCI) facility is installed.
>>>>>>>>>
>>>>>>>>>        This is indicated by setting facilities bit 12 for
>>>>>>>>>        the guest. The kernel will not enable this facility
>>>>>>>>>        for the guest if it is not set on the host.
>>>>>>>>>
>>>>>>>>>        If this facility is not set for the KVM guest, then only
>>>>>>>>>        APQNs with an APQI less than 16 will be used by a Linux
>>>>>>>>>        guest regardless of the matrix configuration for the virtual
>>>>>>>>>        machine. This is a limitation of the Linux AP bus.
>>>>>>>>>
>>>>>>>>> 2. AP Facilities Test facility (APFT) is installed.
>>>>>>>>>
>>>>>>>>>        This is indicated by setting facilities bit 15 for
>>>>>>>>>        the guest. The kernel will not enable this facility for
>>>>>>>>>        the guest if it is not set on the host.
>>>>>>>>>
>>>>>>>>>        If this facility is not set for the KVM guest, then no
>>>>>>>>>        AP devices will be available to the guest regardless of
>>>>>>>>>        the guest's matrix configuration for the virtual
>>>>>>>>>        machine. This is a limitation of the Linux AP bus.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tony Krowiak <[e-mail]>
>>>>>>>>> Reviewed-by: Christian Borntraeger <[e-mail]>
>>>>>>>>> Reviewed-by: Halil Pasic <[e-mail]>
>>>>>>>>> Tested-by: Michael Mueller <[e-mail]>
>>>>>>>>> Tested-by: Farhan Ali <[e-mail]>
>>>>>>>>> Signed-off-by: Christian Borntraeger <[e-mail]>
>>>>>>>>> ---
>>>>>>>>>      arch/s390/kvm/kvm-s390.c         |    5 +++++
>>>>>>>>>      arch/s390/tools/gen_facilities.c |    2 ++
>>>>>>>>>      2 files changed, 7 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>>>>> index 1e8cb67..d5e04d2 100644
>>>>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>>>>> @@ -367,6 +367,11 @@ static void kvm_s390_cpu_feat_init(void)
>>>>>>>>>      
>>>>>>>>>      	if (MACHINE_HAS_ESOP)
>>>>>>>>>      		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>>>>>>>>> +
>>>>>>>>> +	/* Check if AP instructions installed on host */
>>>>>>>>> +	if (ap_instructions_available())
>>>>>>>>> +		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
>>>>>>>>> +
>>>>>>>>>      	/*
>>>>>>>>>      	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>>>>>>>>>      	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>>>>>>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>>>>>>> index 90a8c9e..a52290b 100644
>>>>>>>>> --- a/arch/s390/tools/gen_facilities.c
>>>>>>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>>>>>>> @@ -106,6 +106,8 @@ struct facility_def {
>>>>>>>>>      
>>>>>>>>>      		.name = "FACILITIES_KVM_CPUMODEL",
>>>>>>>>>      		.bits = (int[]){
>>>>>>>>> +			12, /* AP Query Configuration Information */
>>>>>>>>> +			15, /* AP Facilities Test */
>>>>>>>>>      			-1  /* END */
>>>>>>>>>      		}
>>>>>>>>>      	},
>>>>>>>>>
>>>>>>>> I really wonder if we should also export the APXA facility.
>>>>>>>>
>>>>>>>> We can probe and allow that CPU feature. However, we cannot disable it
>>>>>>>> (as of now).
>>>>>>>>
>>>>>>>> We have other CPU features where it is the same case (basically all
>>>>>>>> subfunctions). See kvm_s390_get_processor_subfunc(). We probe them and
>>>>>>>> export them, but support to disable them has never been implemented.
>>>>>>>>
>>>>>>>> On a high level, we could then e.g. deny to start a QEMU guest if APXA
>>>>>>>> is available but has been disabled. (until we know that disabling it
>>>>>>>> actually works - if ever).
>>>>>>>>
>>>>>>>> This helps to catch nasty migration bugs (e.g. APXA suddenly
>>>>>>>> disappearing). Although unlikely, definitely possible.
>>>>>>>>
>>>>>>>>
>>>>>>>> Are there any other AP related facilities that the guest can from now on
>>>>>>>> probe that should also become part of the CPU model?
>>>>>>>>
>>>>>>> To be more precise, shouldn't PQAP(QCI) be handled just like other
>>>>>>> subfunctions? (I remember it should)
>>>>>>
>>>>>> When you suggest PQAP(QCI) be handled like other subfunctions, are you
>>>>>> suggesting that there should be a field in struct kvm_s390_vm_cpu_subfunc
>>>>>> with a bit indicating the QCI subfunction is available? The availability
>>>>>> of the QCI subfunction of the PQAP instruction is determined by facilities
>>>>>> bit 12. Is it not enough to export facilities bit 12?
>>>>>
>>>>> The feature block (128 bit) from PQAP(QCI) should be passed through a
>>>>> subfunction block to QEMU.
>>>>>
>>>>
>>>> I'm confused, which 128 bit?
>>>
>>>
>>> Me too :) , I was assuming this block to be 128bit, but the qci block
>>> has 128 bytes....
>>>
>>> And looking at arch/s390/include/asm/ap.h, there is a lot of information
>>> contained that is definitely not of interest for CPU models...
>>>
>>> I wonder if there is somewhere defined which bits are reserved for
>>> future features/facilities, compared to ap masks and such.
>>>
>>> This is really hard to understand/plan without access to documentation.
>>>
>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>> related to PQAP(QCI) containing features that should get part of the CPU
>>> model makes sense or not. For now I was thinking that there is some part
>>> inside of QCI that is strictly reserved for facilities/features that we
>>> can use.
>>>
>>
>> David,
>> I already answered to you on this subject.
>>
>> First,
>> Are you sure you do not mistake QCI for TAPQ which has the t bit
>> instruction interception bit as all the instructions you use as
>> subfunctions?
> 
> Yes, I am pretty sure it is PQAP(QCI), please check with Christian /
> architecture documentations.

OK.

> 
>>
>> Second,
>> The TAPQ interception bit is exposed through the facility bit 15
>> and is documented as being installed when the APXA facility is installed.
>>
>> If we have the APFT, we have the APXA, problem seems solved to me.

hum. wrong, sorry, the assertion is in the wrong way...

> 
> What is apsc, qact, rc8a in the qci blocks? are the facility bits?

Yes, facility bits concerning the AP instructions

> 
>>
>> Regards,
>> Pierre
>>
> 
>

Halil Pasic Aug. 23, 2018, 1:22 p.m. UTC | #22

On 08/23/2018 02:47 PM, Pierre Morel wrote:
> On 23/08/2018 13:12, David Hildenbrand wrote:
[..]
>>>>>
>>>>> I'm confused, which 128 bit?
>>>>
>>>>
>>>> Me too :) , I was assuming this block to be 128bit, but the qci block
>>>> has 128 bytes....
>>>>
>>>> And looking at arch/s390/include/asm/ap.h, there is a lot of information
>>>> contained that is definitely not of interest for CPU models...
>>>>
>>>> I wonder if there is somewhere defined which bits are reserved for
>>>> future features/facilities, compared to ap masks and such.
>>>>
>>>> This is really hard to understand/plan without access to documentation.
>>>>
>>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>>> related to PQAP(QCI) containing features that should get part of the CPU
>>>> model makes sense or not. For now I was thinking that there is some part
>>>> inside of QCI that is strictly reserved for facilities/features that we
>>>> can use.

No there is no such part. The architecture documentation is quite confusing
with some aspects (e.g. persistence) of how exactly some of these features
work and are indicated. I'm having a hard time finding my opinion. I may
end up asking some questions later, but for now i have to think first.

Just one hint. There is a programming note stating that if bit 2 of the
QCI block is one there is at least one AP card in the machine that actually
has APXA installed.

I read the architecture so that the APXA has a 'cpu part' (if we are
doing APXA the cpu can't spec exception on certain bits not being zor9)
and a 'card(s) part'.

Since the stuff seems quite difficult to sort out properly, I ask myself
are there real problems we must solve?

This ultimately seems to be  about the migration, right? You say 'This helps
to catch nasty migration bugs (e.g. APXA suddenly disappearing).' at the very
beginning of the discussion. Yes, we don't have to have an vfio_ap device,
he guest can and will start looking for AP resources if
only the cpu model features installed. So the guest could observe
a disappearing APXA, but I don't think that would lead to problems (with
Linux at least).

And there ain't much AP a guest can sanely do without if no AP resources
are there.

I would really prefer not rushing a solution if we don't have to.

>
> 
>>
>> What is apsc, qact, rc8a in the qci blocks? are the facility bits?
> 
> Yes, facility bits concerning the AP instructions
> 

According to the current AR document rc8a ain't a facility but bits
0-2 and 4-7 kind of are.

Regards,
Halil

David Hildenbrand Aug. 23, 2018, 1:38 p.m. UTC | #23

On 23.08.2018 15:22, Halil Pasic wrote:
> 
> 
> On 08/23/2018 02:47 PM, Pierre Morel wrote:
>> On 23/08/2018 13:12, David Hildenbrand wrote:
> [..]
>>>>>>
>>>>>> I'm confused, which 128 bit?
>>>>>
>>>>>
>>>>> Me too :) , I was assuming this block to be 128bit, but the qci block
>>>>> has 128 bytes....
>>>>>
>>>>> And looking at arch/s390/include/asm/ap.h, there is a lot of information
>>>>> contained that is definitely not of interest for CPU models...
>>>>>
>>>>> I wonder if there is somewhere defined which bits are reserved for
>>>>> future features/facilities, compared to ap masks and such.
>>>>>
>>>>> This is really hard to understand/plan without access to documentation.
>>>>>
>>>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>>>> related to PQAP(QCI) containing features that should get part of the CPU
>>>>> model makes sense or not. For now I was thinking that there is some part
>>>>> inside of QCI that is strictly reserved for facilities/features that we
>>>>> can use.
> 
> No there is no such part. The architecture documentation is quite confusing
> with some aspects (e.g. persistence) of how exactly some of these features
> work and are indicated. I'm having a hard time finding my opinion. I may
> end up asking some questions later, but for now i have to think first.
> 
> Just one hint. There is a programming note stating that if bit 2 of the
> QCI block is one there is at least one AP card in the machine that actually
> has APXA installed.
> 
> I read the architecture so that the APXA has a 'cpu part' (if we are
> doing APXA the cpu can't spec exception on certain bits not being zor9)
> and a 'card(s) part'.
> 
> Since the stuff seems quite difficult to sort out properly, I ask myself
> are there real problems we must solve?
> 
> This ultimately seems to be  about the migration, right? You say 'This helps
> to catch nasty migration bugs (e.g. APXA suddenly disappearing).' at the very
> beginning of the discussion. Yes, we don't have to have an vfio_ap device,
> he guest can and will start looking for AP resources if
> only the cpu model features installed. So the guest could observe
> a disappearing APXA, but I don't think that would lead to problems (with
> Linux at least).
> 
> And there ain't much AP a guest can sanely do without if no AP resources
> are there.
> 
> I would really prefer not rushing a solution if we don't have to.
> 
>>
>>
>>>
>>> What is apsc, qact, rc8a in the qci blocks? are the facility bits?
>>
>> Yes, facility bits concerning the AP instructions
>>
> 
> According to the current AR document rc8a ain't a facility but bits
> 0-2 and 4-7 kind of are.
> 

Easy ( :) ) answer. Everything that is the CPU part should get into the
CPU model. Everything that is AP specific not. If APXA is not a CPU
facility, fine with me to leave it out.

Ack to not rushing, but also ack to not leaving out important things.
Ack that this stuff is hard to ficure out.

> Regards,
> Halil
>

Pierre Morel Aug. 23, 2018, 2:59 p.m. UTC | #24

On 23/08/2018 15:38, David Hildenbrand wrote:
> On 23.08.2018 15:22, Halil Pasic wrote:
>>
>>
>> On 08/23/2018 02:47 PM, Pierre Morel wrote:
>>> On 23/08/2018 13:12, David Hildenbrand wrote:
>> [..]
>>>>>>>
>>>>>>> I'm confused, which 128 bit?
>>>>>>
>>>>>>
>>>>>> Me too :) , I was assuming this block to be 128bit, but the qci block
>>>>>> has 128 bytes....
>>>>>>
>>>>>> And looking at arch/s390/include/asm/ap.h, there is a lot of information
>>>>>> contained that is definitely not of interest for CPU models...
>>>>>>
>>>>>> I wonder if there is somewhere defined which bits are reserved for
>>>>>> future features/facilities, compared to ap masks and such.
>>>>>>
>>>>>> This is really hard to understand/plan without access to documentation.
>>>>>>
>>>>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>>>>> related to PQAP(QCI) containing features that should get part of the CPU
>>>>>> model makes sense or not. For now I was thinking that there is some part
>>>>>> inside of QCI that is strictly reserved for facilities/features that we
>>>>>> can use.
>>
>> No there is no such part. The architecture documentation is quite confusing
>> with some aspects (e.g. persistence) of how exactly some of these features
>> work and are indicated. I'm having a hard time finding my opinion. I may
>> end up asking some questions later, but for now i have to think first.
>>
>> Just one hint. There is a programming note stating that if bit 2 of the
>> QCI block is one there is at least one AP card in the machine that actually
>> has APXA installed.
>>
>> I read the architecture so that the APXA has a 'cpu part' (if we are
>> doing APXA the cpu can't spec exception on certain bits not being zor9)
>> and a 'card(s) part'.
>>
>> Since the stuff seems quite difficult to sort out properly, I ask myself
>> are there real problems we must solve?
>>
>> This ultimately seems to be  about the migration, right? You say 'This helps
>> to catch nasty migration bugs (e.g. APXA suddenly disappearing).' at the very
>> beginning of the discussion. Yes, we don't have to have an vfio_ap device,
>> he guest can and will start looking for AP resources if
>> only the cpu model features installed. So the guest could observe
>> a disappearing APXA, but I don't think that would lead to problems (with
>> Linux at least).
>>
>> And there ain't much AP a guest can sanely do without if no AP resources
>> are there.
>>
>> I would really prefer not rushing a solution if we don't have to.
>>
>>>
>>>
>>>>
>>>> What is apsc, qact, rc8a in the qci blocks? are the facility bits?
>>>
>>> Yes, facility bits concerning the AP instructions
>>>
>>
>> According to the current AR document rc8a ain't a facility but bits
>> 0-2 and 4-7 kind of are.
>>
> 
> Easy ( :) ) answer. Everything that is the CPU part should get into the
> CPU model. Everything that is AP specific not. If APXA is not a CPU
> facility, fine with me to leave it out.
> 
> Ack to not rushing, but also ack to not leaving out important things.
> Ack that this stuff is hard to ficure out.

APXA is not a CPU part, it is a machine part (SIE) and a AP part (QCI,TAPQ),
it has no influence on CPU instructions but on the AP instructions.
Consequently, if I understood the definition correctly, it should not go 
in the CPU model.

Regards,
Pierre

Anthony Krowiak Aug. 23, 2018, 5:35 p.m. UTC | #25

On 08/23/2018 10:59 AM, Pierre Morel wrote:
> On 23/08/2018 15:38, David Hildenbrand wrote:
>> On 23.08.2018 15:22, Halil Pasic wrote:
>>>
>>>
>>> On 08/23/2018 02:47 PM, Pierre Morel wrote:
>>>> On 23/08/2018 13:12, David Hildenbrand wrote:
>>> [..]
>>>>>>>>
>>>>>>>> I'm confused, which 128 bit?
>>>>>>>
>>>>>>>
>>>>>>> Me too :) , I was assuming this block to be 128bit, but the qci 
>>>>>>> block
>>>>>>> has 128 bytes....
>>>>>>>
>>>>>>> And looking at arch/s390/include/asm/ap.h, there is a lot of 
>>>>>>> information
>>>>>>> contained that is definitely not of interest for CPU models...
>>>>>>>
>>>>>>> I wonder if there is somewhere defined which bits are reserved for
>>>>>>> future features/facilities, compared to ap masks and such.
>>>>>>>
>>>>>>> This is really hard to understand/plan without access to 
>>>>>>> documentation.
>>>>>>>
>>>>>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>>>>>> related to PQAP(QCI) containing features that should get part of 
>>>>>>> the CPU
>>>>>>> model makes sense or not. For now I was thinking that there is 
>>>>>>> some part
>>>>>>> inside of QCI that is strictly reserved for facilities/features 
>>>>>>> that we
>>>>>>> can use.
>>>
>>> No there is no such part. The architecture documentation is quite 
>>> confusing
>>> with some aspects (e.g. persistence) of how exactly some of these 
>>> features
>>> work and are indicated. I'm having a hard time finding my opinion. I 
>>> may
>>> end up asking some questions later, but for now i have to think first.
>>>
>>> Just one hint. There is a programming note stating that if bit 2 of the
>>> QCI block is one there is at least one AP card in the machine that 
>>> actually
>>> has APXA installed.
>>>
>>> I read the architecture so that the APXA has a 'cpu part' (if we are
>>> doing APXA the cpu can't spec exception on certain bits not being zor9)
>>> and a 'card(s) part'.
>>>
>>> Since the stuff seems quite difficult to sort out properly, I ask 
>>> myself
>>> are there real problems we must solve?
>>>
>>> This ultimately seems to be  about the migration, right? You say 
>>> 'This helps
>>> to catch nasty migration bugs (e.g. APXA suddenly disappearing).' at 
>>> the very
>>> beginning of the discussion. Yes, we don't have to have an vfio_ap 
>>> device,
>>> he guest can and will start looking for AP resources if
>>> only the cpu model features installed. So the guest could observe
>>> a disappearing APXA, but I don't think that would lead to problems 
>>> (with
>>> Linux at least).
>>>
>>> And there ain't much AP a guest can sanely do without if no AP 
>>> resources
>>> are there.
>>>
>>> I would really prefer not rushing a solution if we don't have to.
>>>
>>>>
>>>>
>>>>>
>>>>> What is apsc, qact, rc8a in the qci blocks? are the facility bits?
>>>>
>>>> Yes, facility bits concerning the AP instructions
>>>>
>>>
>>> According to the current AR document rc8a ain't a facility but bits
>>> 0-2 and 4-7 kind of are.
>>>
>>
>> Easy ( :) ) answer. Everything that is the CPU part should get into the
>> CPU model. Everything that is AP specific not. If APXA is not a CPU
>> facility, fine with me to leave it out.
>>
>> Ack to not rushing, but also ack to not leaving out important things.
>> Ack that this stuff is hard to ficure out.
>
> APXA is not a CPU part, it is a machine part (SIE) and a AP part 
> (QCI,TAPQ),
> it has no influence on CPU instructions but on the AP instructions.
> Consequently, if I understood the definition correctly, it should not 
> go in the CPU model.

The APXA bit returned via the PQAP(QCI) instruction indicates the APXA 
facility is
installed in the CPUs of the configuration. This means that the facility is
installed in one or more adjunct processors but not necessarily all. 
Given that
it indicates a CPU property, maybe it does belong in the CPU model?

>
> Regards,
> Pierre
>
>
>
>

David Hildenbrand Aug. 23, 2018, 5:40 p.m. UTC | #26

On 23.08.2018 19:35, Tony Krowiak wrote:
> On 08/23/2018 10:59 AM, Pierre Morel wrote:
>> On 23/08/2018 15:38, David Hildenbrand wrote:
>>> On 23.08.2018 15:22, Halil Pasic wrote:
>>>>
>>>>
>>>> On 08/23/2018 02:47 PM, Pierre Morel wrote:
>>>>> On 23/08/2018 13:12, David Hildenbrand wrote:
>>>> [..]
>>>>>>>>>
>>>>>>>>> I'm confused, which 128 bit?
>>>>>>>>
>>>>>>>>
>>>>>>>> Me too :) , I was assuming this block to be 128bit, but the qci 
>>>>>>>> block
>>>>>>>> has 128 bytes....
>>>>>>>>
>>>>>>>> And looking at arch/s390/include/asm/ap.h, there is a lot of 
>>>>>>>> information
>>>>>>>> contained that is definitely not of interest for CPU models...
>>>>>>>>
>>>>>>>> I wonder if there is somewhere defined which bits are reserved for
>>>>>>>> future features/facilities, compared to ap masks and such.
>>>>>>>>
>>>>>>>> This is really hard to understand/plan without access to 
>>>>>>>> documentation.
>>>>>>>>
>>>>>>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>>>>>>> related to PQAP(QCI) containing features that should get part of 
>>>>>>>> the CPU
>>>>>>>> model makes sense or not. For now I was thinking that there is 
>>>>>>>> some part
>>>>>>>> inside of QCI that is strictly reserved for facilities/features 
>>>>>>>> that we
>>>>>>>> can use.
>>>>
>>>> No there is no such part. The architecture documentation is quite 
>>>> confusing
>>>> with some aspects (e.g. persistence) of how exactly some of these 
>>>> features
>>>> work and are indicated. I'm having a hard time finding my opinion. I 
>>>> may
>>>> end up asking some questions later, but for now i have to think first.
>>>>
>>>> Just one hint. There is a programming note stating that if bit 2 of the
>>>> QCI block is one there is at least one AP card in the machine that 
>>>> actually
>>>> has APXA installed.
>>>>
>>>> I read the architecture so that the APXA has a 'cpu part' (if we are
>>>> doing APXA the cpu can't spec exception on certain bits not being zor9)
>>>> and a 'card(s) part'.
>>>>
>>>> Since the stuff seems quite difficult to sort out properly, I ask 
>>>> myself
>>>> are there real problems we must solve?
>>>>
>>>> This ultimately seems to be  about the migration, right? You say 
>>>> 'This helps
>>>> to catch nasty migration bugs (e.g. APXA suddenly disappearing).' at 
>>>> the very
>>>> beginning of the discussion. Yes, we don't have to have an vfio_ap 
>>>> device,
>>>> he guest can and will start looking for AP resources if
>>>> only the cpu model features installed. So the guest could observe
>>>> a disappearing APXA, but I don't think that would lead to problems 
>>>> (with
>>>> Linux at least).
>>>>
>>>> And there ain't much AP a guest can sanely do without if no AP 
>>>> resources
>>>> are there.
>>>>
>>>> I would really prefer not rushing a solution if we don't have to.
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> What is apsc, qact, rc8a in the qci blocks? are the facility bits?
>>>>>
>>>>> Yes, facility bits concerning the AP instructions
>>>>>
>>>>
>>>> According to the current AR document rc8a ain't a facility but bits
>>>> 0-2 and 4-7 kind of are.
>>>>
>>>
>>> Easy ( :) ) answer. Everything that is the CPU part should get into the
>>> CPU model. Everything that is AP specific not. If APXA is not a CPU
>>> facility, fine with me to leave it out.
>>>
>>> Ack to not rushing, but also ack to not leaving out important things.
>>> Ack that this stuff is hard to ficure out.
>>
>> APXA is not a CPU part, it is a machine part (SIE) and a AP part 
>> (QCI,TAPQ),
>> it has no influence on CPU instructions but on the AP instructions.
>> Consequently, if I understood the definition correctly, it should not 
>> go in the CPU model.
> 
> The APXA bit returned via the PQAP(QCI) instruction indicates the APXA 
> facility is
> installed in the CPUs of the configuration. This means that the facility is
> installed in one or more adjunct processors but not necessarily all. 
> Given that
> it indicates a CPU property, maybe it does belong in the CPU model?
> 

Hmmm, I tend to agree - especially as it affects SIE behavior. But as
this is not a feature block (compared to what I thought), this clould be
model as a CPU feature like AP.

What about the other facilities? Do they smell more like AP card
specific stuff?

Halil Pasic Aug. 24, 2018, 10:28 a.m. UTC | #27

On 08/23/2018 07:40 PM, David Hildenbrand wrote:
> On 23.08.2018 19:35, Tony Krowiak wrote:
>> On 08/23/2018 10:59 AM, Pierre Morel wrote:
>>> On 23/08/2018 15:38, David Hildenbrand wrote:
>>>> On 23.08.2018 15:22, Halil Pasic wrote:
>>>>>
>>>>>
>>>>> On 08/23/2018 02:47 PM, Pierre Morel wrote:
>>>>>> On 23/08/2018 13:12, David Hildenbrand wrote:
>>>>> [..]
>>>>>>>>>>
>>>>>>>>>> I'm confused, which 128 bit?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Me too :) , I was assuming this block to be 128bit, but the qci
>>>>>>>>> block
>>>>>>>>> has 128 bytes....
>>>>>>>>>
>>>>>>>>> And looking at arch/s390/include/asm/ap.h, there is a lot of
>>>>>>>>> information
>>>>>>>>> contained that is definitely not of interest for CPU models...
>>>>>>>>>
>>>>>>>>> I wonder if there is somewhere defined which bits are reserved for
>>>>>>>>> future features/facilities, compared to ap masks and such.
>>>>>>>>>
>>>>>>>>> This is really hard to understand/plan without access to
>>>>>>>>> documentation.
>>>>>>>>>
>>>>>>>>> You (Halil, Tony, Pier, ...) should have a look if what I described
>>>>>>>>> related to PQAP(QCI) containing features that should get part of
>>>>>>>>> the CPU
>>>>>>>>> model makes sense or not. For now I was thinking that there is
>>>>>>>>> some part
>>>>>>>>> inside of QCI that is strictly reserved for facilities/features
>>>>>>>>> that we
>>>>>>>>> can use.
>>>>>
>>>>> No there is no such part. The architecture documentation is quite
>>>>> confusing
>>>>> with some aspects (e.g. persistence) of how exactly some of these
>>>>> features
>>>>> work and are indicated. I'm having a hard time finding my opinion. I
>>>>> may
>>>>> end up asking some questions later, but for now i have to think first.
>>>>>
>>>>> Just one hint. There is a programming note stating that if bit 2 of the
>>>>> QCI block is one there is at least one AP card in the machine that
>>>>> actually
>>>>> has APXA installed.
>>>>>
>>>>> I read the architecture so that the APXA has a 'cpu part' (if we are
>>>>> doing APXA the cpu can't spec exception on certain bits not being zor9)
>>>>> and a 'card(s) part'.
>>>>>
>>>>> Since the stuff seems quite difficult to sort out properly, I ask
>>>>> myself
>>>>> are there real problems we must solve?
>>>>>
>>>>> This ultimately seems to be  about the migration, right? You say
>>>>> 'This helps
>>>>> to catch nasty migration bugs (e.g. APXA suddenly disappearing).' at
>>>>> the very
>>>>> beginning of the discussion. Yes, we don't have to have an vfio_ap
>>>>> device,
>>>>> he guest can and will start looking for AP resources if
>>>>> only the cpu model features installed. So the guest could observe
>>>>> a disappearing APXA, but I don't think that would lead to problems
>>>>> (with
>>>>> Linux at least).
>>>>>
>>>>> And there ain't much AP a guest can sanely do without if no AP
>>>>> resources
>>>>> are there.
>>>>>
>>>>> I would really prefer not rushing a solution if we don't have to.
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> What is apsc, qact, rc8a in the qci blocks? are the facility bits?
>>>>>>
>>>>>> Yes, facility bits concerning the AP instructions
>>>>>>
>>>>>
>>>>> According to the current AR document rc8a ain't a facility but bits
>>>>> 0-2 and 4-7 kind of are.
>>>>>
>>>>
>>>> Easy ( :) ) answer. Everything that is the CPU part should get into the
>>>> CPU model. Everything that is AP specific not. If APXA is not a CPU
>>>> facility, fine with me to leave it out.
>>>>
>>>> Ack to not rushing, but also ack to not leaving out important things.
>>>> Ack that this stuff is hard to ficure out.
>>>
>>> APXA is not a CPU part, it is a machine part (SIE) and a AP part
>>> (QCI,TAPQ),
>>> it has no influence on CPU instructions but on the AP instructions.
>>> Consequently, if I understood the definition correctly, it should not
>>> go in the CPU model.
>>
>> The APXA bit returned via the PQAP(QCI) instruction indicates the APXA
>> facility is
>> installed in the CPUs of the configuration. This means that the facility is
>> installed in one or more adjunct processors but not necessarily all.
>> Given that
>> it indicates a CPU property, maybe it does belong in the CPU model?
>>
> 
> Hmmm, I tend to agree - especially as it affects SIE behavior. But as
> this is not a feature block (compared to what I thought), this clould be
> model as a CPU feature like AP.
> 

There is certainly a CPU aspect to APXA: before APXA the APQN had to
have zeros in certain bits (otherwise specification exception). When
running with APXA we have a guarantee that there won't be any
specification exception flying because such an bit is set. The interesting
question is, is APXA constant let's say as long as an LPAR partition is
activated?

Regards,
Halil

Anthony Krowiak Sept. 12, 2018, 5:42 p.m. UTC | #28

On 08/23/2018 04:24 AM, Cornelia Huck wrote:
> On Thu, 23 Aug 2018 09:48:48 +0200
> David Hildenbrand <[e-mail]> wrote:
>
>>> Migration of AP devices is not supported by this patch series, so this
>>> should
>>> not be an issue.
>> Might not be a problem now, but could be later. As I said in a different
>> reply, the CPU model in QEMU does not care about KVM.
>>
>> I want the QEMU CPU model and the KVM interfaces to be clean and future
>> proof. That's why my opinion is to handle PQAP(QCI) just like all the
>> other "feature blocks" we already have.
> +1 to that sentiment.
>
> It's better to try to get this correct now than having to hack around
> should we want to implement things in the future.

Just so we're on the same page here as far as what to expect for v10 of
this patch series, let me summarize the the very long series of private
exchanges as well as this thread:

* The APXA facility indicated by a bit returned in the response to the
   PQAP(QCI) function indicates only whether the APXA facility is available
   on one or more APs installed on the system.
* The only way to change the bit returned from PQAP(QCI) is to intercept the
   instruction and emulate it, so it makes no sense for passthrough devices.
* The AP(s) with APXA installed may not necessarily even be in the 
configuration.
* The only way to determine whether APXA is installed in a given AP is to
   query it using the PQAP(TAPQ) instruction.

It was decided that APXA is better modeled as device configuration. If 
and when
emulation is implemented, APXA can be configured for any AP devices assigned
to a guest. Since AP instructions will be intercepted when emulating AP,
the PQAP(QCI) instruction can return the APXA bit according to whether any
AP is configured with APXA installed. That matches the real architecture 
much
more closely. So, the bottom line is that we will not introduce a new 
CPU model
feature for APXA in v10 of this series.

>

David Hildenbrand Sept. 17, 2018, 7:57 a.m. UTC | #29

Am 12.09.18 um 19:42 schrieb Tony Krowiak:
> On 08/23/2018 04:24 AM, Cornelia Huck wrote:
>> On Thu, 23 Aug 2018 09:48:48 +0200
>> David Hildenbrand <[e-mail]> wrote:
>>
>>>> Migration of AP devices is not supported by this patch series, so this
>>>> should
>>>> not be an issue.
>>> Might not be a problem now, but could be later. As I said in a different
>>> reply, the CPU model in QEMU does not care about KVM.
>>>
>>> I want the QEMU CPU model and the KVM interfaces to be clean and future
>>> proof. That's why my opinion is to handle PQAP(QCI) just like all the
>>> other "feature blocks" we already have.
>> +1 to that sentiment.
>>
>> It's better to try to get this correct now than having to hack around
>> should we want to implement things in the future.
> 
> Just so we're on the same page here as far as what to expect for v10 of
> this patch series, let me summarize the the very long series of private
> exchanges as well as this thread:
> 
> * The APXA facility indicated by a bit returned in the response to the
>    PQAP(QCI) function indicates only whether the APXA facility is available
>    on one or more APs installed on the system.
> * The only way to change the bit returned from PQAP(QCI) is to intercept the
>    instruction and emulate it, so it makes no sense for passthrough devices.
> * The AP(s) with APXA installed may not necessarily even be in the 
> configuration.
> * The only way to determine whether APXA is installed in a given AP is to
>    query it using the PQAP(TAPQ) instruction.
> 
> It was decided that APXA is better modeled as device configuration. If 
> and when
> emulation is implemented, APXA can be configured for any AP devices assigned
> to a guest. Since AP instructions will be intercepted when emulating AP,
> the PQAP(QCI) instruction can return the APXA bit according to whether any
> AP is configured with APXA installed. That matches the real architecture 
> much
> more closely. So, the bottom line is that we will not introduce a new 
> CPU model
> feature for APXA in v10 of this series.

Yes, that sounds sane to me. In addition, all other QCI indicated
"features/facilitites" are handled on a per-device basis and not on a
CPU-model basis.

Patch

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 1e8cb67..d5e04d2 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -367,6 +367,11 @@  static void kvm_s390_cpu_feat_init(void)
 
 	if (MACHINE_HAS_ESOP)
 		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
+
+	/* Check if AP instructions installed on host */
+	if (ap_instructions_available())
+		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_AP);
+
 	/*
 	 * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
 	 * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index 90a8c9e..a52290b 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -106,6 +106,8 @@  struct facility_def {
 
 		.name = "FACILITIES_KVM_CPUMODEL",
 		.bits = (int[]){
+			12, /* AP Query Configuration Information */
+			15, /* AP Facilities Test */
 			-1  /* END */
 		}
 	},