[PATCH v2 0/3] KVM: arm64: make EL2 feature fields writable in ID_AA64MMFR1_EL1

[PATCH v2 0/3] KVM: arm64: make EL2 feature fields writable in ID_AA64MMFR1_EL1

[Jinqian Yang]

Allow userspace to downgrade EL2 features (VH, TWED, HCX) to ensure VM live
migration compatibility between hosts with varying feature support.

v1: https://lore.kernel.org/linux-arm-kernel/20250325121126.1380681-1-yangjinqian1@huawei.com/

Change Log:
v1->v2:
  -Added support for downgrading TWED and VH fields.
  -Added corresponding test cases to the set_id_regs selftest.

Jinqian Yang (3):
  KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace
  KVM: arm64: Make ID_AA64MMFR1_EL1.TWED writable from userspace
  KVM: arm64: Make ID_AA64MMFR1_EL1.VH writable from userspace

 arch/arm64/kvm/sys_regs.c                       | 3 ---
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.33.0

[PATCH v2 1/3] KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace

[Jinqian Yang]

Allow userspace to downgrade HCX in ID_AA64MMFR1_EL1. Userspace can
only change this value from high to low.

Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
---
 arch/arm64/kvm/sys_regs.c                       | 1 -
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 82ffb3b3b3cf..f2267db1c44c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3002,7 +3002,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 				      ~(ID_AA64MMFR0_EL1_RES0 |
 					ID_AA64MMFR0_EL1_ASIDBITS)),
 	ID_WRITABLE(ID_AA64MMFR1_EL1, ~(ID_AA64MMFR1_EL1_RES0 |
-					ID_AA64MMFR1_EL1_HCX |
 					ID_AA64MMFR1_EL1_TWED |
 					ID_AA64MMFR1_EL1_XNX |
 					ID_AA64MMFR1_EL1_VH |
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index d3bf9204409c..a744dee30cd2 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -165,6 +165,7 @@ static const struct reg_ftr_bits ftr_id_aa64mmfr0_el1[] = {
 static const struct reg_ftr_bits ftr_id_aa64mmfr1_el1[] = {
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, TIDCP1, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, AFP, 0),
+	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, HCX, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, ETS, 0),
 	REG_FTR_BITS(FTR_HIGHER_SAFE, ID_AA64MMFR1_EL1, SpecSEI, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, PAN, 0),
-- 
2.33.0

[PATCH v2 2/3] KVM: arm64: Make ID_AA64MMFR1_EL1.TWED writable from userspace

[Jinqian Yang]

Allow userspace to downgrade TWED in ID_AA64MMFR1_EL1. Userspace can
only change this value from high to low.

Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
---
 arch/arm64/kvm/sys_regs.c                       | 1 -
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index f2267db1c44c..db49beb8804e 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3002,7 +3002,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 				      ~(ID_AA64MMFR0_EL1_RES0 |
 					ID_AA64MMFR0_EL1_ASIDBITS)),
 	ID_WRITABLE(ID_AA64MMFR1_EL1, ~(ID_AA64MMFR1_EL1_RES0 |
-					ID_AA64MMFR1_EL1_TWED |
 					ID_AA64MMFR1_EL1_XNX |
 					ID_AA64MMFR1_EL1_VH |
 					ID_AA64MMFR1_EL1_VMIDBits)),
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index a744dee30cd2..a889550d8aa4 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -167,6 +167,7 @@ static const struct reg_ftr_bits ftr_id_aa64mmfr1_el1[] = {
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, AFP, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, HCX, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, ETS, 0),
+	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, TWED, 0),
 	REG_FTR_BITS(FTR_HIGHER_SAFE, ID_AA64MMFR1_EL1, SpecSEI, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, PAN, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, LO, 0),
-- 
2.33.0

[PATCH v2 3/3] KVM: arm64: Make ID_AA64MMFR1_EL1.VH writable from userspace

[Jinqian Yang]

Allow userspace to downgrade VH in ID_AA64MMFR1_EL1. Userspace can
only change this value from high to low.

Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
---
 arch/arm64/kvm/sys_regs.c                       | 1 -
 tools/testing/selftests/kvm/arm64/set_id_regs.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index db49beb8804e..5ff14c7ec753 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3003,7 +3003,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 					ID_AA64MMFR0_EL1_ASIDBITS)),
 	ID_WRITABLE(ID_AA64MMFR1_EL1, ~(ID_AA64MMFR1_EL1_RES0 |
 					ID_AA64MMFR1_EL1_XNX |
-					ID_AA64MMFR1_EL1_VH |
 					ID_AA64MMFR1_EL1_VMIDBits)),
 	ID_FILTERED(ID_AA64MMFR2_EL1,
 		    id_aa64mmfr2_el1, ~(ID_AA64MMFR2_EL1_RES0 |
diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index a889550d8aa4..00dd8066b46f 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -172,6 +172,7 @@ static const struct reg_ftr_bits ftr_id_aa64mmfr1_el1[] = {
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, PAN, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, LO, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, HPDS, 0),
+	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, VH, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64MMFR1_EL1, HAFDBS, 0),
 	REG_FTR_END,
 };
-- 
2.33.0

Re: [PATCH v2 3/3] KVM: arm64: Make ID_AA64MMFR1_EL1.VH writable from userspace

[Oliver Upton]

Hi Jinqian,

On Tue, Sep 09, 2025 at 11:44:15AM +0800, Jinqian Yang wrote:
> Allow userspace to downgrade VH in ID_AA64MMFR1_EL1. Userspace can
> only change this value from high to low.

I'm afraid we can't allow this. When we expose FEAT_VHE to the VM,
HCR_EL2.E2H is RES1. Meaning, vEL2 is unconditionally in a VHE
context.

I would be OK with a clarifying comment documenting why the field is
non-writable.

Thanks,
Oliver

Re: [PATCH v2 0/3] KVM: arm64: make EL2 feature fields writable in ID_AA64MMFR1_EL1

[Oliver Upton]

On Tue, Sep 09, 2025 at 11:44:12AM +0800, Jinqian Yang wrote:
> Allow userspace to downgrade EL2 features (VH, TWED, HCX) to ensure VM live
> migration compatibility between hosts with varying feature support.
> 
> v1: https://lore.kernel.org/linux-arm-kernel/20250325121126.1380681-1-yangjinqian1@huawei.com/
> 
> Change Log:
> v1->v2:
>   -Added support for downgrading TWED and VH fields.
>   -Added corresponding test cases to the set_id_regs selftest.
> 
> Jinqian Yang (3):
>   KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace
>   KVM: arm64: Make ID_AA64MMFR1_EL1.TWED writable from userspace
>   KVM: arm64: Make ID_AA64MMFR1_EL1.VH writable from userspace
> 
>  arch/arm64/kvm/sys_regs.c                       | 3 ---
>  tools/testing/selftests/kvm/arm64/set_id_regs.c | 3 +++

General comment: please make do kernel and selftest changes in separate
patches.

Thanks,
Oliver

Re: [PATCH v2 1/3] KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace

[Oliver Upton]

On Tue, Sep 09, 2025 at 11:44:13AM +0800, Jinqian Yang wrote:
> Allow userspace to downgrade HCX in ID_AA64MMFR1_EL1. Userspace can
> only change this value from high to low.
> 
> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>

I'm not sure our quality of emulation is that great in this case. We
have no way of trapping the register and it is always stateful. Better
yet, our RESx infrastructure doesn't account for the presence of
FEAT_HCX and we happily merge the contents with the host's HCRX.

We should make a reasonable attempt at upholding the architecture before
allowing userspace to de-feature FEAT_HCX.

Thanks,
Oliver

Re: [PATCH v2 1/3] KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace

[Marc Zyngier]

On Tue, 09 Sep 2025 08:07:15 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Tue, Sep 09, 2025 at 11:44:13AM +0800, Jinqian Yang wrote:
> > Allow userspace to downgrade HCX in ID_AA64MMFR1_EL1. Userspace can
> > only change this value from high to low.
> > 
> > Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
> 
> I'm not sure our quality of emulation is that great in this case. We
> have no way of trapping the register and it is always stateful. Better
> yet, our RESx infrastructure doesn't account for the presence of
> FEAT_HCX and we happily merge the contents with the host's HCRX.

Yeah, that's not good, and definitely deserves a fix.

> We should make a reasonable attempt at upholding the architecture before
> allowing userspace to de-feature FEAT_HCX.

My concern here is the transitive implications of FEAT_HCX being
disabled: a quick look shows about 20 features that depend on
FEAT_HCX, and we don't really track this. I can probably generate the
dependency graph, but that's not going to be small. Or very useful.

However, we should be able to let FEAT_HCX being disabled without
problem if the downgrading is limited to non-EL2 VMs. Same thing for
FEAT_VHE.

What do you think?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v2 1/3] KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace

[Oliver Upton]

On Tue, Sep 09, 2025 at 11:10:28AM +0100, Marc Zyngier wrote:
> On Tue, 09 Sep 2025 08:07:15 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > On Tue, Sep 09, 2025 at 11:44:13AM +0800, Jinqian Yang wrote:
> > > Allow userspace to downgrade HCX in ID_AA64MMFR1_EL1. Userspace can
> > > only change this value from high to low.
> > > 
> > > Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
> > 
> > I'm not sure our quality of emulation is that great in this case. We
> > have no way of trapping the register and it is always stateful. Better
> > yet, our RESx infrastructure doesn't account for the presence of
> > FEAT_HCX and we happily merge the contents with the host's HCRX.
> 
> Yeah, that's not good, and definitely deserves a fix.
> 
> > We should make a reasonable attempt at upholding the architecture before
> > allowing userspace to de-feature FEAT_HCX.
> 
> My concern here is the transitive implications of FEAT_HCX being
> disabled: a quick look shows about 20 features that depend on
> FEAT_HCX, and we don't really track this. I can probably generate the
> dependency graph, but that's not going to be small. Or very useful.
> 
> However, we should be able to let FEAT_HCX being disabled without
> problem if the downgrading is limited to non-EL2 VMs. Same thing for
> FEAT_VHE.
> 
> What do you think?

So I'm a bit worried about making fields sometimes-writable, it creates
a very confusing UAPI behavior. On top of that, our writable masks are
currently static.

What if we treat the entire register as RES0 in this case? It seems to
be consistent with all the underlying bits / features being NI. A
mis-described VM isn't long for this world anyway (e.g. FEAT_SCTLR2 && !FEAT_HCX)
and in that case I'd prefer an approach that keeps the KVM code as
simple as possible.

We do, after all, expect some level of sanity from userspace of feature
dependencies as we do not enforce the dependency graph at the moment.
Hell, this could be useful for someone cross-migrating a nested VM from
a machine w/o FEAT_HCX to one that has it.

Thanks,
Oliver

Re: [PATCH v2 3/3] KVM: arm64: Make ID_AA64MMFR1_EL1.VH writable from userspace

[Jinqian Yang]

On 2025/9/9 13:32, Oliver Upton wrote:
> Hi Jinqian,
> 
> On Tue, Sep 09, 2025 at 11:44:15AM +0800, Jinqian Yang wrote:
>> Allow userspace to downgrade VH in ID_AA64MMFR1_EL1. Userspace can
>> only change this value from high to low.
> 
> I'm afraid we can't allow this. When we expose FEAT_VHE to the VM,
> HCR_EL2.E2H is RES1. Meaning, vEL2 is unconditionally in a VHE
> context.
> 
> I would be OK with a clarifying comment documenting why the field is
> non-writable.
> 

Okay, v3 will keep ID_AA64MMFR1_EL1.VH non-writable.

Thanks,
Jinqian

Re: [PATCH v2 0/3] KVM: arm64: make EL2 feature fields writable in ID_AA64MMFR1_EL1

[Jinqian Yang]

On 2025/9/9 15:00, Oliver Upton wrote:
> On Tue, Sep 09, 2025 at 11:44:12AM +0800, Jinqian Yang wrote:
>> Allow userspace to downgrade EL2 features (VH, TWED, HCX) to ensure VM live
>> migration compatibility between hosts with varying feature support.
>>
>> v1: https://lore.kernel.org/linux-arm-kernel/20250325121126.1380681-1-yangjinqian1@huawei.com/
>>
>> Change Log:
>> v1->v2:
>>    -Added support for downgrading TWED and VH fields.
>>    -Added corresponding test cases to the set_id_regs selftest.
>>
>> Jinqian Yang (3):
>>    KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace
>>    KVM: arm64: Make ID_AA64MMFR1_EL1.TWED writable from userspace
>>    KVM: arm64: Make ID_AA64MMFR1_EL1.VH writable from userspace
>>
>>   arch/arm64/kvm/sys_regs.c                       | 3 ---
>>   tools/testing/selftests/kvm/arm64/set_id_regs.c | 3 +++
> 
> General comment: please make do kernel and selftest changes in separate
> patches.
> 

Will be changed in v3.

Thanks,
Jinqian

Re: [PATCH v2 1/3] KVM: arm64: Make ID_AA64MMFR1_EL1.HCX writable from userspace

[Jinqian Yang]

On 2025/9/10 5:38, Oliver Upton wrote:
> On Tue, Sep 09, 2025 at 11:10:28AM +0100, Marc Zyngier wrote:
>> On Tue, 09 Sep 2025 08:07:15 +0100,
>> Oliver Upton <oliver.upton@linux.dev> wrote:
>>>
>>> On Tue, Sep 09, 2025 at 11:44:13AM +0800, Jinqian Yang wrote:
>>>> Allow userspace to downgrade HCX in ID_AA64MMFR1_EL1. Userspace can
>>>> only change this value from high to low.
>>>>
>>>> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
>>>
>>> I'm not sure our quality of emulation is that great in this case. We
>>> have no way of trapping the register and it is always stateful. Better
>>> yet, our RESx infrastructure doesn't account for the presence of
>>> FEAT_HCX and we happily merge the contents with the host's HCRX.
>>
>> Yeah, that's not good, and definitely deserves a fix.
>>
>>> We should make a reasonable attempt at upholding the architecture before
>>> allowing userspace to de-feature FEAT_HCX.
>>
>> My concern here is the transitive implications of FEAT_HCX being
>> disabled: a quick look shows about 20 features that depend on
>> FEAT_HCX, and we don't really track this. I can probably generate the
>> dependency graph, but that's not going to be small. Or very useful.
>>
>> However, we should be able to let FEAT_HCX being disabled without
>> problem if the downgrading is limited to non-EL2 VMs. Same thing for
>> FEAT_VHE.
>>
>> What do you think?
> 
> So I'm a bit worried about making fields sometimes-writable, it creates
> a very confusing UAPI behavior. On top of that, our writable masks are
> currently static.
> 
> What if we treat the entire register as RES0 in this case? It seems to
> be consistent with all the underlying bits / features being NI. A
> mis-described VM isn't long for this world anyway (e.g. FEAT_SCTLR2 && !FEAT_HCX)
> and in that case I'd prefer an approach that keeps the KVM code as
> simple as possible.
> 
> We do, after all, expect some level of sanity from userspace of feature
> dependencies as we do not enforce the dependency graph at the moment.
> Hell, this could be useful for someone cross-migrating a nested VM from
> a machine w/o FEAT_HCX to one that has it.
> 

Making HCX writable is very useful, as it allows VMs to migrate between 
machines that support and do not support FEAT_HCX. For non-EL2 VMs, we 
have tested that migration works without issues :)

Thanks,
Jinqian