| Message ID | 20240612023553.127813-2-shahuang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Making BT Field in ID_AA64PFR1_EL1 writable | expand |
On Tue, Jun 11, 2024 at 10:35:51PM -0400, Shaoqin Huang wrote: > When migrating from MtCollins to AmpereOne, the BT field value in > ID_AA64PFR1_EL1 register is different and not writable. This causes > the migration to fail. > > The BT field means Branch Target Identification mechanism support in > AArch64 state. The value 0 means BT is not implemented, the value 1 > means BT is implemented. > > On MtCollins(Migration Src), the BT value is 0. > On AmpereOne(Migration Dst), the BT value is 1. > > As it defined in the ftr_id_aa64dfr0, the samller value is safe. So if typo: smaller > we make the BT field writable, on the AmpereOne(Migration Dst) the BT > field will be overrided with value 0. > > Signed-off-by: Shaoqin Huang <shahuang@redhat.com> > --- > But there is a question, the ARM DDI mentions from Armv8.5, the only > permitted value is 0b01. Do you guys know if there are any consequence > if the userspace write value 0b0 into this field? Or we should restrict > that at some level, like in VMM or kernel level? There's no directly visible attribute in the CPU registers to determine what level of the architecture the implementation supports, and I don't really want KVM to go about policing this. The general guidance for ID register fields is that we allow userspace to select a subset of CPU features supported by KVM / the implementation, which in this case would include the _NI encoding for the field. This has been slightly opinionated so far, leaving features that userspace selects via a separate mechanism (e.g. KVM_ARM_VCPU_INIT) read only. Userspace can (and should) come up with its own heuristics for determining the feature set for the vCPU. > --- > arch/arm64/kvm/sys_regs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index c9f4f387155f..8e0ea62e14e1 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2292,7 +2292,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > ID_AA64PFR0_EL1_GIC | > ID_AA64PFR0_EL1_AdvSIMD | > ID_AA64PFR0_EL1_FP), }, > - ID_SANITISED(ID_AA64PFR1_EL1), > + ID_WRITABLE(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_EL1_BT), This doesn't compile. The macro prefixes "SYS_" to the register name.
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index c9f4f387155f..8e0ea62e14e1 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2292,7 +2292,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_AA64PFR0_EL1_GIC | ID_AA64PFR0_EL1_AdvSIMD | ID_AA64PFR0_EL1_FP), }, - ID_SANITISED(ID_AA64PFR1_EL1), + ID_WRITABLE(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_EL1_BT), ID_UNALLOCATED(4,2), ID_UNALLOCATED(4,3), ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0),
When migrating from MtCollins to AmpereOne, the BT field value in ID_AA64PFR1_EL1 register is different and not writable. This causes the migration to fail. The BT field means Branch Target Identification mechanism support in AArch64 state. The value 0 means BT is not implemented, the value 1 means BT is implemented. On MtCollins(Migration Src), the BT value is 0. On AmpereOne(Migration Dst), the BT value is 1. As it defined in the ftr_id_aa64dfr0, the samller value is safe. So if we make the BT field writable, on the AmpereOne(Migration Dst) the BT field will be overrided with value 0. Signed-off-by: Shaoqin Huang <shahuang@redhat.com> --- But there is a question, the ARM DDI mentions from Armv8.5, the only permitted value is 0b01. Do you guys know if there are any consequence if the userspace write value 0b0 into this field? Or we should restrict that at some level, like in VMM or kernel level? --- arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)