diff mbox series

[RFC,v3,08/29] KVM: arm64: Make ID_AA64MMFR0_EL1 writable

Message ID 20211117064359.2362060-9-reijiw@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: arm64: Make CPU ID registers writable by userspace | expand

Commit Message

Reiji Watanabe Nov. 17, 2021, 6:43 a.m. UTC
This patch adds id_reg_info for ID_AA64MMFR0_EL1 to make it
writable by userspace.

Since ID_AA64MMFR0_EL1 stage 2 granule size fields don't follow the
standard ID scheme, we need a special handling to validate those fields.

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 118 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

Comments

Eric Auger Nov. 25, 2021, 3:31 p.m. UTC | #1
On 11/17/21 7:43 AM, Reiji Watanabe wrote:
> This patch adds id_reg_info for ID_AA64MMFR0_EL1 to make it
> writable by userspace.
> 
> Since ID_AA64MMFR0_EL1 stage 2 granule size fields don't follow the
> standard ID scheme, we need a special handling to validate those fields.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 118 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 5812e39602fe..772e3d3067b2 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -519,6 +519,113 @@ static int validate_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> +/*
> + * Check if the requested stage2 translation granule size indicated in
> + * @mmfr0 is also indicated in @mmfr0_lim.  This function assumes that
> + * the stage1 granule size indicated in @mmfr0 has been validated already.
I would suggest: relies on the fact TGranX fields are validated before
through the arm64_check_features lookup
> + */
> +static int aa64mmfr0_tgran2_check(int field, u64 mmfr0, u64 mmfr0_lim)
> +{
> +	s64 tgran2, lim_tgran2, rtgran1;
> +	int f1;
> +	bool is_signed = true;
> +
> +	tgran2 = cpuid_feature_extract_unsigned_field(mmfr0, field);
> +	lim_tgran2 = cpuid_feature_extract_unsigned_field(mmfr0_lim, field);
> +	if (tgran2 == lim_tgran2)
> +		return 0;
> +
> +	if (tgran2 && lim_tgran2)
> +		return (tgran2 > lim_tgran2) ? -E2BIG : 0;
> +
> +	/*
> +	 * Either tgran2 or lim_tgran2 is zero.
> +	 * Need stage1 granule size to validate tgran2.
> +	 */
> +	switch (field) {
> +	case ID_AA64MMFR0_TGRAN4_2_SHIFT:
> +		f1 = ID_AA64MMFR0_TGRAN4_SHIFT;
> +		break;
> +	case ID_AA64MMFR0_TGRAN64_2_SHIFT:
> +		f1 = ID_AA64MMFR0_TGRAN64_SHIFT;
> +		break;
> +	case ID_AA64MMFR0_TGRAN16_2_SHIFT:
> +		f1 = ID_AA64MMFR0_TGRAN16_SHIFT;
> +		is_signed = false;
I don't get the is_signed setting. Don't the TGRAN_x have the same
format? Beside you can get the shift by substracting 12 to @field.

can't you directly compute if the granule is supported

> +		break;
> +	default:
> +		/* Should never happen */
> +		WARN_ONCE(1, "Unexpected stage2 granule field (%d)\n", field);
> +		return 0;
> +	}
> +
> +	/*
> +	 * If tgran2 == 0 (&& lim_tgran2 != 0), the requested stage2 granule
> +	 * size is indicated in the stage1 granule size field of @mmfr0.
> +	 * So, validate the stage1 granule size against the stage2 limit
> +	 * granule size.
> +	 * If lim_tgran2 == 0 (&& tgran2 != 0), the stage2 limit granule size
> +	 * is indicated in the stage1 granule size field of @mmfr0_lim.
> +	 * So, validate the requested stage2 granule size against the stage1
> +	 * limit granule size.
> +	 */
> +
> +	 /* Get the relevant stage1 granule size to validate tgran2 */
> +	if (tgran2 == 0)
> +		/* The requested stage1 granule size */
> +		rtgran1 = cpuid_feature_extract_field(mmfr0, f1, is_signed);
> +	else /* lim_tgran2 == 0 */
> +		/* The stage1 limit granule size */
> +		rtgran1 = cpuid_feature_extract_field(mmfr0_lim, f1, is_signed);
> +
> +	/*
> +	 * Adjust the value of rtgran1 to compare with stage2 granule size,
> +	 * which indicates: 1: Not supported, 2: Supported, etc.
> +	 */
> +	if (is_signed)
> +		/* For signed, -1: Not supported, 0: Supported, etc. */
> +		rtgran1 += 0x2;
> +	else
> +		/* For unsigned, 0: Not supported, 1: Supported, etc. */
> +		rtgran1 += 0x1;
> +
> +	if ((tgran2 == 0) && (rtgran1 > lim_tgran2))
> +		/*
> +		 * The requested stage1 granule size (== the requested stage2
> +		 * granule size) is larger than the stage2 limit granule size.
> +		 */
> +		return -E2BIG;
> +	else if ((lim_tgran2 == 0) && (tgran2 > rtgran1))
> +		/*
> +		 * The requested stage2 granule size is larger than the stage1
> +		 * limit granulze size (== the stage2 limit granule size).
> +		 */
> +		return -E2BIG;
> +
> +	return 0;
> +}
> +
> +static int validate_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
> +				     const struct id_reg_info *id_reg, u64 val)
> +{
> +	u64 limit = id_reg->vcpu_limit_val;
> +	int ret;

shouldn't you forbid reserved values for TGran4, 64?
> +
> +	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN4_2_SHIFT, val, limit);
> +	if (ret)
> +		return ret;
> +
> +	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN64_2_SHIFT, val, limit);
> +	if (ret)
> +		return ret;
> +
> +	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN16_2_SHIFT, val, limit);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
>  {
>  	u64 limit = id_reg->vcpu_limit_val;
> @@ -625,6 +732,16 @@ static struct id_reg_info id_aa64isar1_el1_info = {
>  	.get_reset_val = get_reset_id_aa64isar1_el1,
>  };
>  
> +static struct id_reg_info id_aa64mmfr0_el1_info = {
> +	.sys_reg = SYS_ID_AA64MMFR0_EL1,
> +	.ftr_check_types = S_FCT(ID_AA64MMFR0_TGRAN4_SHIFT, FCT_LOWER_SAFE) |
> +			   S_FCT(ID_AA64MMFR0_TGRAN64_SHIFT, FCT_LOWER_SAFE) |
the default?
> +			   U_FCT(ID_AA64MMFR0_TGRAN4_2_SHIFT, FCT_IGNORE) |
> +			   U_FCT(ID_AA64MMFR0_TGRAN64_2_SHIFT, FCT_IGNORE) |
> +			   U_FCT(ID_AA64MMFR0_TGRAN16_2_SHIFT, FCT_IGNORE),
maybe add comment telling the actual check is handled in the validate cb
> +	.validate = validate_id_aa64mmfr0_el1,
> +};
> +
>  /*
>   * An ID register that needs special handling to control the value for the
>   * guest must have its own id_reg_info in id_reg_info_table.
> @@ -638,6 +755,7 @@ static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
>  	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
>  	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
>  	[IDREG_IDX(SYS_ID_AA64ISAR1_EL1)] = &id_aa64isar1_el1_info,
> +	[IDREG_IDX(SYS_ID_AA64MMFR0_EL1)] = &id_aa64mmfr0_el1_info,
>  };
>  
>  static int validate_id_reg(struct kvm_vcpu *vcpu,
>
Eric Auger Nov. 25, 2021, 4:06 p.m. UTC | #2
Hi Reiji,
On 11/17/21 7:43 AM, Reiji Watanabe wrote:
> This patch adds id_reg_info for ID_AA64MMFR0_EL1 to make it
> writable by userspace.
> 
> Since ID_AA64MMFR0_EL1 stage 2 granule size fields don't follow the
> standard ID scheme, we need a special handling to validate those fields.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 118 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 5812e39602fe..772e3d3067b2 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -519,6 +519,113 @@ static int validate_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
>  	return 0;
>  }
>  
> +/*
> + * Check if the requested stage2 translation granule size indicated in
> + * @mmfr0 is also indicated in @mmfr0_lim.  This function assumes that
> + * the stage1 granule size indicated in @mmfr0 has been validated already.
> + */
> +static int aa64mmfr0_tgran2_check(int field, u64 mmfr0, u64 mmfr0_lim)
> +{
> +	s64 tgran2, lim_tgran2, rtgran1;
> +	int f1;
> +	bool is_signed = true;
> +
> +	tgran2 = cpuid_feature_extract_unsigned_field(mmfr0, field);
> +	lim_tgran2 = cpuid_feature_extract_unsigned_field(mmfr0_lim, field);
> +	if (tgran2 == lim_tgran2)
> +		return 0;
> +
> +	if (tgran2 && lim_tgran2)
> +		return (tgran2 > lim_tgran2) ? -E2BIG : 0;
> +
> +	/*
> +	 * Either tgran2 or lim_tgran2 is zero.
> +	 * Need stage1 granule size to validate tgran2.
> +	 */
> +	switch (field) {
> +	case ID_AA64MMFR0_TGRAN4_2_SHIFT:
> +		f1 = ID_AA64MMFR0_TGRAN4_SHIFT;
> +		break;
> +	case ID_AA64MMFR0_TGRAN64_2_SHIFT:
> +		f1 = ID_AA64MMFR0_TGRAN64_SHIFT;
> +		break;
> +	case ID_AA64MMFR0_TGRAN16_2_SHIFT:
> +		f1 = ID_AA64MMFR0_TGRAN16_SHIFT;
> +		is_signed = false;
> +		break;
> +	default:
> +		/* Should never happen */
> +		WARN_ONCE(1, "Unexpected stage2 granule field (%d)\n", field);
> +		return 0;
> +	}

sorry my previous message was sent while I haven't finished :-(

if I understand correctly you forbid setting a granule that is not set
in the lim. So I would first compute whether the granule is set, would
it be through the TGranX (if _2 == 0) or though TGranX_2 if this latter
is not not. Do those computations both on val and lim and eventually
check if gran_val > gran_lim. The current code looks overly complicated
but maybe I miss the actual reason.

Eric
> +
> +	/*
> +	 * If tgran2 == 0 (&& lim_tgran2 != 0), the requested stage2 granule
> +	 * size is indicated in the stage1 granule size field of @mmfr0.
> +	 * So, validate the stage1 granule size against the stage2 limit
> +	 * granule size.
> +	 * If lim_tgran2 == 0 (&& tgran2 != 0), the stage2 limit granule size
> +	 * is indicated in the stage1 granule size field of @mmfr0_lim.
> +	 * So, validate the requested stage2 granule size against the stage1
> +	 * limit granule size.
> +	 */
> +
> +	 /* Get the relevant stage1 granule size to validate tgran2 */
> +	if (tgran2 == 0)
> +		/* The requested stage1 granule size */
> +		rtgran1 = cpuid_feature_extract_field(mmfr0, f1, is_signed);
> +	else /* lim_tgran2 == 0 */
> +		/* The stage1 limit granule size */
> +		rtgran1 = cpuid_feature_extract_field(mmfr0_lim, f1, is_signed);
> +
> +	/*
> +	 * Adjust the value of rtgran1 to compare with stage2 granule size,
> +	 * which indicates: 1: Not supported, 2: Supported, etc.
> +	 */
> +	if (is_signed)
> +		/* For signed, -1: Not supported, 0: Supported, etc. */
> +		rtgran1 += 0x2;
> +	else
> +		/* For unsigned, 0: Not supported, 1: Supported, etc. */
> +		rtgran1 += 0x1;
> +
> +	if ((tgran2 == 0) && (rtgran1 > lim_tgran2))
> +		/*
> +		 * The requested stage1 granule size (== the requested stage2
> +		 * granule size) is larger than the stage2 limit granule size.
> +		 */
> +		return -E2BIG;
> +	else if ((lim_tgran2 == 0) && (tgran2 > rtgran1))
> +		/*
> +		 * The requested stage2 granule size is larger than the stage1
> +		 * limit granulze size (== the stage2 limit granule size).
> +		 */
> +		return -E2BIG;
> +
> +	return 0;
> +}
> +
> +static int validate_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
> +				     const struct id_reg_info *id_reg, u64 val)
> +{
> +	u64 limit = id_reg->vcpu_limit_val;
> +	int ret;
> +
> +	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN4_2_SHIFT, val, limit);
> +	if (ret)
> +		return ret;
> +
> +	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN64_2_SHIFT, val, limit);
> +	if (ret)
> +		return ret;
> +
> +	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN16_2_SHIFT, val, limit);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
>  {
>  	u64 limit = id_reg->vcpu_limit_val;
> @@ -625,6 +732,16 @@ static struct id_reg_info id_aa64isar1_el1_info = {
>  	.get_reset_val = get_reset_id_aa64isar1_el1,
>  };
>  
> +static struct id_reg_info id_aa64mmfr0_el1_info = {
> +	.sys_reg = SYS_ID_AA64MMFR0_EL1,
> +	.ftr_check_types = S_FCT(ID_AA64MMFR0_TGRAN4_SHIFT, FCT_LOWER_SAFE) |
> +			   S_FCT(ID_AA64MMFR0_TGRAN64_SHIFT, FCT_LOWER_SAFE) |
> +			   U_FCT(ID_AA64MMFR0_TGRAN4_2_SHIFT, FCT_IGNORE) |
> +			   U_FCT(ID_AA64MMFR0_TGRAN64_2_SHIFT, FCT_IGNORE) |
> +			   U_FCT(ID_AA64MMFR0_TGRAN16_2_SHIFT, FCT_IGNORE),
> +	.validate = validate_id_aa64mmfr0_el1,
> +};
> +
>  /*
>   * An ID register that needs special handling to control the value for the
>   * guest must have its own id_reg_info in id_reg_info_table.
> @@ -638,6 +755,7 @@ static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
>  	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
>  	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
>  	[IDREG_IDX(SYS_ID_AA64ISAR1_EL1)] = &id_aa64isar1_el1_info,
> +	[IDREG_IDX(SYS_ID_AA64MMFR0_EL1)] = &id_aa64mmfr0_el1_info,
>  };
>  
>  static int validate_id_reg(struct kvm_vcpu *vcpu,
>
Reiji Watanabe Nov. 30, 2021, 4:43 a.m. UTC | #3
Hi Eric,

On Thu, Nov 25, 2021 at 7:31 AM Eric Auger <eauger@redhat.com> wrote:
>
>
>
> On 11/17/21 7:43 AM, Reiji Watanabe wrote:
> > This patch adds id_reg_info for ID_AA64MMFR0_EL1 to make it
> > writable by userspace.
> >
> > Since ID_AA64MMFR0_EL1 stage 2 granule size fields don't follow the
> > standard ID scheme, we need a special handling to validate those fields.
> >
> > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 118 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 118 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 5812e39602fe..772e3d3067b2 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -519,6 +519,113 @@ static int validate_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
> >       return 0;
> >  }
> >
> > +/*
> > + * Check if the requested stage2 translation granule size indicated in
> > + * @mmfr0 is also indicated in @mmfr0_lim.  This function assumes that
> > + * the stage1 granule size indicated in @mmfr0 has been validated already.
> I would suggest: relies on the fact TGranX fields are validated before
> through the arm64_check_features lookup

Thank you for the suggestion.  I will fix the comment as you suggested.

> > + */
> > +static int aa64mmfr0_tgran2_check(int field, u64 mmfr0, u64 mmfr0_lim)
> > +{
> > +     s64 tgran2, lim_tgran2, rtgran1;
> > +     int f1;
> > +     bool is_signed = true;
> > +
> > +     tgran2 = cpuid_feature_extract_unsigned_field(mmfr0, field);
> > +     lim_tgran2 = cpuid_feature_extract_unsigned_field(mmfr0_lim, field);
> > +     if (tgran2 == lim_tgran2)
> > +             return 0;
> > +
> > +     if (tgran2 && lim_tgran2)
> > +             return (tgran2 > lim_tgran2) ? -E2BIG : 0;
> > +
> > +     /*
> > +      * Either tgran2 or lim_tgran2 is zero.
> > +      * Need stage1 granule size to validate tgran2.
> > +      */
> > +     switch (field) {
> > +     case ID_AA64MMFR0_TGRAN4_2_SHIFT:
> > +             f1 = ID_AA64MMFR0_TGRAN4_SHIFT;
> > +             break;
> > +     case ID_AA64MMFR0_TGRAN64_2_SHIFT:
> > +             f1 = ID_AA64MMFR0_TGRAN64_SHIFT;
> > +             break;
> > +     case ID_AA64MMFR0_TGRAN16_2_SHIFT:
> > +             f1 = ID_AA64MMFR0_TGRAN16_SHIFT;
> > +             is_signed = false;
> I don't get the is_signed setting. Don't the TGRAN_x have the same
> format? Beside you can get the shift by substracting 12 to @field.

Yes, TGran16 is different from TGran64/TGran4.
 https://developer.arm.com/documentation/ddi0595/2021-09/AArch64-Registers/ID-AA64MMFR0-EL1--AArch64-Memory-Model-Feature-Register-0?lang=en

I didn't realize that we could get the shift by substracting 12 from
@field.  Thank you for the information.  I will use that.


> can't you directly compute if the granule is supported

No, I don't think we can because the value 0 in the TGranx_2 means
that its feature support is identified by another field (TGranx).


> > +             break;
> > +     default:
> > +             /* Should never happen */
> > +             WARN_ONCE(1, "Unexpected stage2 granule field (%d)\n", field);
> > +             return 0;
> > +     }
> > +
> > +     /*
> > +      * If tgran2 == 0 (&& lim_tgran2 != 0), the requested stage2 granule
> > +      * size is indicated in the stage1 granule size field of @mmfr0.
> > +      * So, validate the stage1 granule size against the stage2 limit
> > +      * granule size.
> > +      * If lim_tgran2 == 0 (&& tgran2 != 0), the stage2 limit granule size
> > +      * is indicated in the stage1 granule size field of @mmfr0_lim.
> > +      * So, validate the requested stage2 granule size against the stage1
> > +      * limit granule size.
> > +      */
> > +
> > +      /* Get the relevant stage1 granule size to validate tgran2 */
> > +     if (tgran2 == 0)
> > +             /* The requested stage1 granule size */
> > +             rtgran1 = cpuid_feature_extract_field(mmfr0, f1, is_signed);
> > +     else /* lim_tgran2 == 0 */
> > +             /* The stage1 limit granule size */
> > +             rtgran1 = cpuid_feature_extract_field(mmfr0_lim, f1, is_signed);
> > +
> > +     /*
> > +      * Adjust the value of rtgran1 to compare with stage2 granule size,
> > +      * which indicates: 1: Not supported, 2: Supported, etc.
> > +      */
> > +     if (is_signed)
> > +             /* For signed, -1: Not supported, 0: Supported, etc. */
> > +             rtgran1 += 0x2;
> > +     else
> > +             /* For unsigned, 0: Not supported, 1: Supported, etc. */
> > +             rtgran1 += 0x1;
> > +
> > +     if ((tgran2 == 0) && (rtgran1 > lim_tgran2))
> > +             /*
> > +              * The requested stage1 granule size (== the requested stage2
> > +              * granule size) is larger than the stage2 limit granule size.
> > +              */
> > +             return -E2BIG;
> > +     else if ((lim_tgran2 == 0) && (tgran2 > rtgran1))
> > +             /*
> > +              * The requested stage2 granule size is larger than the stage1
> > +              * limit granulze size (== the stage2 limit granule size).
> > +              */
> > +             return -E2BIG;
> > +
> > +     return 0;
> > +}
> > +
> > +static int validate_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
> > +                                  const struct id_reg_info *id_reg, u64 val)
> > +{
> > +     u64 limit = id_reg->vcpu_limit_val;
> > +     int ret;
>
> shouldn't you forbid reserved values for TGran4, 64?

I think what you meant is applied to all signed feature fields on
any ID registers.  Considering "Principles of the ID scheme for fields
in ID registers" that is described in Arm ARM, lower value than -1
should not indicate more or higher level of features than KVM on the
host can support.  In that sense, it doesn't matter (I would think
it won't cause any problems for guests).  So, I rather chose not to do
that check.


> > +
> > +     ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN4_2_SHIFT, val, limit);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN64_2_SHIFT, val, limit);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN16_2_SHIFT, val, limit);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return 0;
> > +}
> > +
> >  static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
> >  {
> >       u64 limit = id_reg->vcpu_limit_val;
> > @@ -625,6 +732,16 @@ static struct id_reg_info id_aa64isar1_el1_info = {
> >       .get_reset_val = get_reset_id_aa64isar1_el1,
> >  };
> >
> > +static struct id_reg_info id_aa64mmfr0_el1_info = {
> > +     .sys_reg = SYS_ID_AA64MMFR0_EL1,
> > +     .ftr_check_types = S_FCT(ID_AA64MMFR0_TGRAN4_SHIFT, FCT_LOWER_SAFE) |
> > +                        S_FCT(ID_AA64MMFR0_TGRAN64_SHIFT, FCT_LOWER_SAFE) |
> the default?

They are signed fields, which are not a default.

> > +                        U_FCT(ID_AA64MMFR0_TGRAN4_2_SHIFT, FCT_IGNORE) |
> > +                        U_FCT(ID_AA64MMFR0_TGRAN64_2_SHIFT, FCT_IGNORE) |
> > +                        U_FCT(ID_AA64MMFR0_TGRAN16_2_SHIFT, FCT_IGNORE),
> maybe add comment telling the actual check is handled in the validate cb

That's a good point.
I will add a brief explanation about the validate cb.

Thanks,
Reiji
diff mbox series

Patch

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 5812e39602fe..772e3d3067b2 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -519,6 +519,113 @@  static int validate_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+/*
+ * Check if the requested stage2 translation granule size indicated in
+ * @mmfr0 is also indicated in @mmfr0_lim.  This function assumes that
+ * the stage1 granule size indicated in @mmfr0 has been validated already.
+ */
+static int aa64mmfr0_tgran2_check(int field, u64 mmfr0, u64 mmfr0_lim)
+{
+	s64 tgran2, lim_tgran2, rtgran1;
+	int f1;
+	bool is_signed = true;
+
+	tgran2 = cpuid_feature_extract_unsigned_field(mmfr0, field);
+	lim_tgran2 = cpuid_feature_extract_unsigned_field(mmfr0_lim, field);
+	if (tgran2 == lim_tgran2)
+		return 0;
+
+	if (tgran2 && lim_tgran2)
+		return (tgran2 > lim_tgran2) ? -E2BIG : 0;
+
+	/*
+	 * Either tgran2 or lim_tgran2 is zero.
+	 * Need stage1 granule size to validate tgran2.
+	 */
+	switch (field) {
+	case ID_AA64MMFR0_TGRAN4_2_SHIFT:
+		f1 = ID_AA64MMFR0_TGRAN4_SHIFT;
+		break;
+	case ID_AA64MMFR0_TGRAN64_2_SHIFT:
+		f1 = ID_AA64MMFR0_TGRAN64_SHIFT;
+		break;
+	case ID_AA64MMFR0_TGRAN16_2_SHIFT:
+		f1 = ID_AA64MMFR0_TGRAN16_SHIFT;
+		is_signed = false;
+		break;
+	default:
+		/* Should never happen */
+		WARN_ONCE(1, "Unexpected stage2 granule field (%d)\n", field);
+		return 0;
+	}
+
+	/*
+	 * If tgran2 == 0 (&& lim_tgran2 != 0), the requested stage2 granule
+	 * size is indicated in the stage1 granule size field of @mmfr0.
+	 * So, validate the stage1 granule size against the stage2 limit
+	 * granule size.
+	 * If lim_tgran2 == 0 (&& tgran2 != 0), the stage2 limit granule size
+	 * is indicated in the stage1 granule size field of @mmfr0_lim.
+	 * So, validate the requested stage2 granule size against the stage1
+	 * limit granule size.
+	 */
+
+	 /* Get the relevant stage1 granule size to validate tgran2 */
+	if (tgran2 == 0)
+		/* The requested stage1 granule size */
+		rtgran1 = cpuid_feature_extract_field(mmfr0, f1, is_signed);
+	else /* lim_tgran2 == 0 */
+		/* The stage1 limit granule size */
+		rtgran1 = cpuid_feature_extract_field(mmfr0_lim, f1, is_signed);
+
+	/*
+	 * Adjust the value of rtgran1 to compare with stage2 granule size,
+	 * which indicates: 1: Not supported, 2: Supported, etc.
+	 */
+	if (is_signed)
+		/* For signed, -1: Not supported, 0: Supported, etc. */
+		rtgran1 += 0x2;
+	else
+		/* For unsigned, 0: Not supported, 1: Supported, etc. */
+		rtgran1 += 0x1;
+
+	if ((tgran2 == 0) && (rtgran1 > lim_tgran2))
+		/*
+		 * The requested stage1 granule size (== the requested stage2
+		 * granule size) is larger than the stage2 limit granule size.
+		 */
+		return -E2BIG;
+	else if ((lim_tgran2 == 0) && (tgran2 > rtgran1))
+		/*
+		 * The requested stage2 granule size is larger than the stage1
+		 * limit granulze size (== the stage2 limit granule size).
+		 */
+		return -E2BIG;
+
+	return 0;
+}
+
+static int validate_id_aa64mmfr0_el1(struct kvm_vcpu *vcpu,
+				     const struct id_reg_info *id_reg, u64 val)
+{
+	u64 limit = id_reg->vcpu_limit_val;
+	int ret;
+
+	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN4_2_SHIFT, val, limit);
+	if (ret)
+		return ret;
+
+	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN64_2_SHIFT, val, limit);
+	if (ret)
+		return ret;
+
+	ret = aa64mmfr0_tgran2_check(ID_AA64MMFR0_TGRAN16_2_SHIFT, val, limit);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static void init_id_aa64pfr0_el1_info(struct id_reg_info *id_reg)
 {
 	u64 limit = id_reg->vcpu_limit_val;
@@ -625,6 +732,16 @@  static struct id_reg_info id_aa64isar1_el1_info = {
 	.get_reset_val = get_reset_id_aa64isar1_el1,
 };
 
+static struct id_reg_info id_aa64mmfr0_el1_info = {
+	.sys_reg = SYS_ID_AA64MMFR0_EL1,
+	.ftr_check_types = S_FCT(ID_AA64MMFR0_TGRAN4_SHIFT, FCT_LOWER_SAFE) |
+			   S_FCT(ID_AA64MMFR0_TGRAN64_SHIFT, FCT_LOWER_SAFE) |
+			   U_FCT(ID_AA64MMFR0_TGRAN4_2_SHIFT, FCT_IGNORE) |
+			   U_FCT(ID_AA64MMFR0_TGRAN64_2_SHIFT, FCT_IGNORE) |
+			   U_FCT(ID_AA64MMFR0_TGRAN16_2_SHIFT, FCT_IGNORE),
+	.validate = validate_id_aa64mmfr0_el1,
+};
+
 /*
  * An ID register that needs special handling to control the value for the
  * guest must have its own id_reg_info in id_reg_info_table.
@@ -638,6 +755,7 @@  static struct id_reg_info *id_reg_info_table[KVM_ARM_ID_REG_MAX_NUM] = {
 	[IDREG_IDX(SYS_ID_AA64PFR1_EL1)] = &id_aa64pfr1_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR0_EL1)] = &id_aa64isar0_el1_info,
 	[IDREG_IDX(SYS_ID_AA64ISAR1_EL1)] = &id_aa64isar1_el1_info,
+	[IDREG_IDX(SYS_ID_AA64MMFR0_EL1)] = &id_aa64mmfr0_el1_info,
 };
 
 static int validate_id_reg(struct kvm_vcpu *vcpu,