Re: [PATCH v15 07/11] target/s390x/cpu topology: activating CPU topology

[Thomas Huth]

On 01/02/2023 14.20, Pierre Morel wrote:
> The KVM capability KVM_CAP_S390_CPU_TOPOLOGY is used to
> activate the S390_FEAT_CONFIGURATION_TOPOLOGY feature and
> the topology facility in the host CPU model for the guest
> in the case the topology is available in QEMU and in KVM.
>
> The feature is disabled by default and fenced for SE
> (secure execution).
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   hw/s390x/cpu-topology.c   |  2 +-
>   target/s390x/cpu_models.c |  1 +
>   target/s390x/kvm/kvm.c    | 12 ++++++++++++
>   3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
> index 1028bf4476..c33378577b 100644
> --- a/hw/s390x/cpu-topology.c
> +++ b/hw/s390x/cpu-topology.c
> @@ -55,7 +55,7 @@ int s390_socket_nb(S390CPU *cpu)
>    */
>   bool s390_has_topology(void)
>   {
> -    return false;
> +    return s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY);
>   }
>   
>   /**
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 065ec6d66c..aca2c5c96b 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -254,6 +254,7 @@ bool s390_has_feat(S390Feat feat)
>           case S390_FEAT_SIE_CMMA:
>           case S390_FEAT_SIE_PFMFI:
>           case S390_FEAT_SIE_IBS:
> +        case S390_FEAT_CONFIGURATION_TOPOLOGY:
>               return false;
>               break;
>           default:
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index fb63be41b7..808e35a7bd 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -2470,6 +2470,18 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, 
> Error **errp)
>           set_bit(S390_FEAT_UNPACK, model->features);
>       }
>   
> +    /*
> +     * If we have kernel support for CPU Topology indicate the
> +     * configuration-topology facility.
> +     */
> +    if (kvm_check_extension(kvm_state, KVM_CAP_S390_CPU_TOPOLOGY)) {
> +        if (kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_CPU_TOPOLOGY, 0) < 0) {
> +            error_setg(errp, "KVM: Error enabling KVM_CAP_S390_CPU_TOPOLOGY");
> +            return;
> +        }
> +        set_bit(S390_FEAT_CONFIGURATION_TOPOLOGY, model->features);
> +    }

Not sure, but for the other capabilities, the kvm_vm_enable_cap() is rather 
done in kvm_arch_init() instead ... likely that it is properly available in 
case you don't run with the "host" cpu model? So should the 
kvm_vm_enable_cap(KVM_CAP_S390_CPU_TOPOLOGY) also be moved there (but of 
course keep the set_bit() here in kvm_s390_get_host_cpu_model())?

 Thomas