Re: [PATCH v2] KVM: s390: implement CPU model only facilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 19 Feb 2018 12:52:49 +0000
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:

> Some facilities should only provided to the guest, if these are

s/provided/be provided/
s/these/they/

> enabled by a CPU model. This allows to avoid capabilities and

s/allows/allows us/

> simply fallback to the cpumodel for deciding about a facility

s/simply fallback/to simply fall back/

> without enabling it for older QEMUs or QEMUs without a CPU
> model.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> ---
>  arch/s390/kvm/kvm-s390.c         | 54 ++++++++++++++++++++++++++--------------
>  arch/s390/kvm/kvm-s390.h         |  2 --
>  arch/s390/tools/gen_facilities.c | 20 +++++++++++++++
>  3 files changed, 55 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ba4c7092335a..9c05767c95be 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -151,13 +151,34 @@ static int nested;
>  module_param(nested, int, S_IRUGO);
>  MODULE_PARM_DESC(nested, "Nested virtualization support");
>  
> -/* upper facilities limit for kvm */
> -unsigned long kvm_s390_fac_list_mask[16] = { FACILITIES_KVM };
>  
> -unsigned long kvm_s390_fac_list_mask_size(void)
> +/*
> + * For now we handle not more than 16 double words as this is what

"For now, we handle at most..."

> + * the s390 base kernel handles and stores in prefix page. If we

s/in prefix page/in the prefix page/

> + * ever need to go beyond this, this requires changes to code, but
> + * the external uapi can stay.
> + */
> +#define SIZE_INTERNAL 16
> +
> +/*
> + * Base feature mask that defines default mask for facilities. Consists of
> + * the defines in FACILITIES_KVM and the non-hypervisor managed bits.
> + */
> +static unsigned long kvm_s390_fac_base[SIZE_INTERNAL] = { FACILITIES_KVM };
> +/*
> + * Extended feature mask. Consists of the defines in FACILITIES_KVM_CPUMODEL
> + * and defines the facilities that can be enabled via a cpu model.
> + */
> +static unsigned long kvm_s390_fac_ext[SIZE_INTERNAL] = { FACILITIES_KVM_CPUMODEL };
> +

(...)

> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 424a1ba4f874..37a77a3f280c 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -62,6 +62,13 @@ static struct facility_def facility_defs[] = {
>  		}
>  	},
>  	{
> +		/*
> +		 * FACILITIES_KVM contains the list of facilities that are
> +		 * part of the default facility mask and list passed to the

"mask and list that are passed..."

(I stumbled over this, even if the sentence is fine...)


> +		 * initial CPU model. If no CPU model is used this, together

s/is used this,/is used, this,/

> +		 * with the non-hypervisor managed bits, is the maximum list
> +		 * of guest facilities supported by KVM.
> +		 */
>  		.name = "FACILITIES_KVM",
>  		.bits = (int[]){
>  			0,  /* N3 instructions */
> @@ -89,6 +96,19 @@ static struct facility_def facility_defs[] = {
>  			-1  /* END */
>  		}
>  	},
> +	{
> +		/*
> +		 * FACILITIES_KVM_CPUMODEL contains the list of facilities
> +		 * that can be enabled by CPU model code if the host supports
> +		 * it. These facilities are not passed to the guest without
> +		 * CPU model support.
> +		 */
> +
> +		.name = "FACILITIES_KVM_CPUMODEL",
> +		.bits = (int[]){
> +			-1  /* END */
> +		}
> +	},
>  };
>  
>  static void print_facility_list(struct facility_def *def)

Only nitpickery, else

Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux