diff mbox series

[v10,06/12] KVM: arm64: Allow userspace to change ID_AA64ISAR{0-2}_EL1

Message ID 20230920183310.1163034-7-oliver.upton@linux.dev (mailing list archive)
State New, archived
Headers show
Series KVM: arm64: Enable 'writable' ID registers | expand

Commit Message

Oliver Upton Sept. 20, 2023, 6:33 p.m. UTC 
Almost all of the features described by the ISA registers have no KVM
involvement. Allow userspace to change the value of these registers with
a couple exceptions:

 - MOPS is not writable as KVM does not currently virtualize FEAT_MOPS.

 - The PAuth fields are not writable as KVM requires both address and
   generic authentication be enabled.

 - Override the kernel's handling of BC to LOWER_SAFE.

Co-developed-by: Jing Zhang <jingzhangos@google.com>
Signed-off-by: Jing Zhang <jingzhangos@google.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 42 ++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 12 deletions(-)

Comments

Kristina Martšenko Sept. 22, 2023, 5:18 p.m. UTC | #1 
On 20/09/2023 19:33, Oliver Upton wrote:
> Almost all of the features described by the ISA registers have no KVM
> involvement. Allow userspace to change the value of these registers with
> a couple exceptions:
>
>  - MOPS is not writable as KVM does not currently virtualize FEAT_MOPS.
>
>  - The PAuth fields are not writable as KVM requires both address and
>    generic authentication be enabled.
>
>  - Override the kernel's handling of BC to LOWER_SAFE.
>
> Co-developed-by: Jing Zhang <jingzhangos@google.com>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/sys_regs.c | 42 ++++++++++++++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 10e3e6a736dc..71664bec2808 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1225,6 +1225,10 @@ static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
>                       break;
>               }
>               break;
> +     case SYS_ID_AA64ISAR2_EL1:
> +             if (kvm_ftr.shift == ID_AA64ISAR2_EL1_BC_SHIFT)
> +                     kvm_ftr.type = FTR_LOWER_SAFE;
> +             break;
>       case SYS_ID_DFR0_EL1:
>               if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
>                       kvm_ftr.type = FTR_LOWER_SAFE;

Nit: it shouldn't be necessary to override BC anymore, as it was recently fixed
in the arm64 code:
  https://lore.kernel.org/linux-arm-kernel/20230912133429.2606875-1-kristina.martsenko@arm.com/

Kristina
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Kristina Martšenko Sept. 22, 2023, 5:20 p.m. UTC | #2 
On 22/09/2023 18:18, Kristina Martsenko wrote:
> On 20/09/2023 19:33, Oliver Upton wrote:
>> Almost all of the features described by the ISA registers have no KVM
>> involvement. Allow userspace to change the value of these registers with
>> a couple exceptions:
>>
>>  - MOPS is not writable as KVM does not currently virtualize FEAT_MOPS.
>>
>>  - The PAuth fields are not writable as KVM requires both address and
>>    generic authentication be enabled.
>>
>>  - Override the kernel's handling of BC to LOWER_SAFE.
>>
>> Co-developed-by: Jing Zhang <jingzhangos@google.com>
>> Signed-off-by: Jing Zhang <jingzhangos@google.com>
>> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
>> ---
>>  arch/arm64/kvm/sys_regs.c | 42 ++++++++++++++++++++++++++++-----------
>>  1 file changed, 30 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 10e3e6a736dc..71664bec2808 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1225,6 +1225,10 @@ static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
>>                       break;
>>               }
>>               break;
>> +     case SYS_ID_AA64ISAR2_EL1:
>> +             if (kvm_ftr.shift == ID_AA64ISAR2_EL1_BC_SHIFT)
>> +                     kvm_ftr.type = FTR_LOWER_SAFE;
>> +             break;
>>       case SYS_ID_DFR0_EL1:
>>               if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
>>                       kvm_ftr.type = FTR_LOWER_SAFE;
> 
> Nit: it shouldn't be necessary to override BC anymore, as it was recently fixed
> in the arm64 code:
>   https://lore.kernel.org/linux-arm-kernel/20230912133429.2606875-1-kristina.martsenko@arm.com/
> 
> Kristina
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> 

Sorry, please ignore this disclaimer, I messed up my mail setup.

Kristina
Oliver Upton Sept. 22, 2023, 5:52 p.m. UTC | #3 
On Fri, Sep 22, 2023 at 06:18:37PM +0100, Kristina Martsenko wrote:
> On 20/09/2023 19:33, Oliver Upton wrote:
> > Almost all of the features described by the ISA registers have no KVM
> > involvement. Allow userspace to change the value of these registers with
> > a couple exceptions:
> >
> >  - MOPS is not writable as KVM does not currently virtualize FEAT_MOPS.
> >
> >  - The PAuth fields are not writable as KVM requires both address and
> >    generic authentication be enabled.
> >
> >  - Override the kernel's handling of BC to LOWER_SAFE.
> >
> > Co-developed-by: Jing Zhang <jingzhangos@google.com>
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 42 ++++++++++++++++++++++++++++-----------
> >  1 file changed, 30 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 10e3e6a736dc..71664bec2808 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1225,6 +1225,10 @@ static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
> >                       break;
> >               }
> >               break;
> > +     case SYS_ID_AA64ISAR2_EL1:
> > +             if (kvm_ftr.shift == ID_AA64ISAR2_EL1_BC_SHIFT)
> > +                     kvm_ftr.type = FTR_LOWER_SAFE;
> > +             break;
> >       case SYS_ID_DFR0_EL1:
> >               if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
> >                       kvm_ftr.type = FTR_LOWER_SAFE;
> 
> Nit: it shouldn't be necessary to override BC anymore, as it was recently fixed
> in the arm64 code:
>   https://lore.kernel.org/linux-arm-kernel/20230912133429.2606875-1-kristina.martsenko@arm.com/

Perfect, looks like that patch should go in 6.6 too. Thanks for the fix!
Oliver Upton Oct. 3, 2023, 7:41 p.m. UTC | #4 
On Fri, Sep 22, 2023 at 05:52:41PM +0000, Oliver Upton wrote:
> > Nit: it shouldn't be necessary to override BC anymore, as it was recently fixed
> > in the arm64 code:
> >   https://lore.kernel.org/linux-arm-kernel/20230912133429.2606875-1-kristina.martsenko@arm.com/
> 
> Perfect, looks like that patch should go in 6.6 too. Thanks for the fix!

Going to squash the following into the patch:

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c74920063287..179f82bd90af 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1225,10 +1225,6 @@ static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
 			break;
 		}
 		break;
-	case SYS_ID_AA64ISAR2_EL1:
-		if (kvm_ftr.shift == ID_AA64ISAR2_EL1_BC_SHIFT)
-			kvm_ftr.type = FTR_LOWER_SAFE;
-		break;
 	case SYS_ID_DFR0_EL1:
 		if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
 			kvm_ftr.type = FTR_LOWER_SAFE;
diff mbox series

Patch

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 10e3e6a736dc..71664bec2808 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1225,6 +1225,10 @@  static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
 			break;
 		}
 		break;
+	case SYS_ID_AA64ISAR2_EL1:
+		if (kvm_ftr.shift == ID_AA64ISAR2_EL1_BC_SHIFT)
+			kvm_ftr.type = FTR_LOWER_SAFE;
+		break;
 	case SYS_ID_DFR0_EL1:
 		if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
 			kvm_ftr.type = FTR_LOWER_SAFE;
@@ -1851,11 +1855,14 @@  static unsigned int elx2_visibility(const struct kvm_vcpu *vcpu,
  * from userspace.
  */
 
-/* sys_reg_desc initialiser for known cpufeature ID registers */
-#define ID_SANITISED(name) {			\
+#define ID_DESC(name)				\
 	SYS_DESC(SYS_##name),			\
 	.access	= access_id_reg,		\
-	.get_user = get_id_reg,			\
+	.get_user = get_id_reg			\
+
+/* sys_reg_desc initialiser for known cpufeature ID registers */
+#define ID_SANITISED(name) {			\
+	ID_DESC(name),				\
 	.set_user = set_id_reg,			\
 	.visibility = id_visibility,		\
 	.reset = kvm_read_sanitised_id_reg,	\
@@ -1864,15 +1871,22 @@  static unsigned int elx2_visibility(const struct kvm_vcpu *vcpu,
 
 /* sys_reg_desc initialiser for known cpufeature ID registers */
 #define AA32_ID_SANITISED(name) {		\
-	SYS_DESC(SYS_##name),			\
-	.access	= access_id_reg,		\
-	.get_user = get_id_reg,			\
+	ID_DESC(name),				\
 	.set_user = set_id_reg,			\
 	.visibility = aa32_id_visibility,	\
 	.reset = kvm_read_sanitised_id_reg,	\
 	.val = 0,				\
 }
 
+/* sys_reg_desc initialiser for writable ID registers */
+#define ID_WRITABLE(name, mask) {		\
+	ID_DESC(name),				\
+	.set_user = set_id_reg,			\
+	.visibility = id_visibility,		\
+	.reset = kvm_read_sanitised_id_reg,	\
+	.val = mask,				\
+}
+
 /*
  * sys_reg_desc initialiser for architecturally unallocated cpufeature ID
  * register with encoding Op0=3, Op1=0, CRn=0, CRm=crm, Op2=op2
@@ -1894,9 +1908,7 @@  static unsigned int elx2_visibility(const struct kvm_vcpu *vcpu,
  * RAZ for the guest.
  */
 #define ID_HIDDEN(name) {			\
-	SYS_DESC(SYS_##name),			\
-	.access = access_id_reg,		\
-	.get_user = get_id_reg,			\
+	ID_DESC(name),				\
 	.set_user = set_id_reg,			\
 	.visibility = raz_visibility,		\
 	.reset = kvm_read_sanitised_id_reg,	\
@@ -2075,9 +2087,15 @@  static const struct sys_reg_desc sys_reg_descs[] = {
 	ID_UNALLOCATED(5,7),
 
 	/* CRm=6 */
-	ID_SANITISED(ID_AA64ISAR0_EL1),
-	ID_SANITISED(ID_AA64ISAR1_EL1),
-	ID_SANITISED(ID_AA64ISAR2_EL1),
+	ID_WRITABLE(ID_AA64ISAR0_EL1, ~ID_AA64ISAR0_EL1_RES0),
+	ID_WRITABLE(ID_AA64ISAR1_EL1, ~(ID_AA64ISAR1_EL1_GPI |
+					ID_AA64ISAR1_EL1_GPA |
+					ID_AA64ISAR1_EL1_API |
+					ID_AA64ISAR1_EL1_APA)),
+	ID_WRITABLE(ID_AA64ISAR2_EL1, ~(ID_AA64ISAR2_EL1_RES0 |
+					ID_AA64ISAR2_EL1_MOPS |
+					ID_AA64ISAR2_EL1_APA3 |
+					ID_AA64ISAR2_EL1_GPA3)),
 	ID_UNALLOCATED(6,3),
 	ID_UNALLOCATED(6,4),
 	ID_UNALLOCATED(6,5),