* [PATCH v1 1/4] KVM: arm64: Allow userspace to get the writable masks for feature ID registers
2023-09-19 17:50 [PATCH v1 0/4] Get writable masks for feature ID from userspace Jing Zhang
@ 2023-09-19 17:50 ` Jing Zhang
2023-09-19 17:50 ` [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS Jing Zhang
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Jing Zhang @ 2023-09-19 17:50 UTC (permalink / raw)
To: KVM, KVMARM, ARMLinux, Marc Zyngier, Oliver Upton
Cc: Will Deacon, Paolo Bonzini, James Morse, Alexandru Elisei,
Suzuki K Poulose, Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck,
Shaoqin Huang, Jing Zhang
While the Feature ID range is well defined and pretty large, it isn't
inconceivable that the architecture will eventually grow some other
ranges that will need to similarly be described to userspace.
Add a VM ioctl to allow userspace to get writable masks for feature ID
registers in below system register space:
op0 = 3, op1 = {0, 1, 3}, CRn = 0, CRm = {0 - 7}, op2 = {0 - 7}
This is used to support mix-and-match userspace and kernels for writable
ID registers, where userspace may want to know upfront whether it can
actually tweak the contents of an idreg or not.
Add a new capability (KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES) that
returns a bitmap of the valid ranges, which can subsequently be
retrieved, one at a time by setting the index of the set bit as the
range identifier.
Suggested-by: Marc Zyngier <maz@kernel.org>
Suggested-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Jing Zhang <jingzhangos@google.com>
---
arch/arm64/include/asm/kvm_host.h | 2 +
arch/arm64/include/uapi/asm/kvm.h | 32 +++++++++++++++
arch/arm64/kvm/arm.c | 10 +++++
arch/arm64/kvm/sys_regs.c | 66 +++++++++++++++++++++++++++++++
include/uapi/linux/kvm.h | 2 +
5 files changed, 112 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index af06ccb7ee34..5e3f778f81db 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1078,6 +1078,8 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
struct kvm_arm_copy_mte_tags *copy_tags);
int kvm_vm_ioctl_set_counter_offset(struct kvm *kvm,
struct kvm_arm_counter_offset *offset);
+int kvm_vm_ioctl_get_reg_writable_masks(struct kvm *kvm,
+ struct reg_mask_range *range);
/* Guest/host FPSIMD coordination helpers */
int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index f7ddd73a8c0f..01242274db87 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -505,6 +505,38 @@ struct kvm_smccc_filter {
#define KVM_HYPERCALL_EXIT_SMC (1U << 0)
#define KVM_HYPERCALL_EXIT_16BIT (1U << 1)
+/*
+ * Get feature ID registers userspace writable mask.
+ *
+ * From DDI0487J.a, D19.2.66 ("ID_AA64MMFR2_EL1, AArch64 Memory Model
+ * Feature Register 2"):
+ *
+ * "The Feature ID space is defined as the System register space in
+ * AArch64 with op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7},
+ * op2=={0-7}."
+ *
+ * This covers all currently known R/O registers that indicate
+ * anything useful feature wise, including the ID registers.
+ *
+ * If we ever need to introduce a new range, it will be described as
+ * such in the range field.
+ */
+#define ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2) \
+ ({ \
+ __u64 __op1 = (op1) & 3; \
+ __op1 -= (__op1 == 3); \
+ (__op1 << 6 | ((crm) & 7) << 3 | (op2)); \
+ })
+
+#define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8)
+#define ARM64_FEATURE_ID_RANGE_IDREGS BIT(0)
+
+struct reg_mask_range {
+ __u64 addr; /* Pointer to mask array */
+ __u32 range; /* Requested range */
+ __u32 reserved[13];
+};
+
#endif
#endif /* __ARM_KVM_H__ */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 4866b3f7b4ea..1c4017a253fd 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -317,6 +317,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES:
r = kvm_supported_block_sizes();
break;
+ case KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES:
+ r = ARM64_FEATURE_ID_RANGE_IDREGS;
+ break;
default:
r = 0;
}
@@ -1629,6 +1632,13 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
return kvm_vm_set_attr(kvm, &attr);
}
+ case KVM_ARM_GET_REG_WRITABLE_MASKS: {
+ struct reg_mask_range range;
+
+ if (copy_from_user(&range, argp, sizeof(range)))
+ return -EFAULT;
+ return kvm_vm_ioctl_get_reg_writable_masks(kvm, &range);
+ }
default:
return -EINVAL;
}
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e92ec810d449..cdb9976d8091 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1373,6 +1373,13 @@ static inline bool is_id_reg(u32 id)
sys_reg_CRm(id) < 8);
}
+static inline bool is_aa32_id_reg(u32 id)
+{
+ return (sys_reg_Op0(id) == 3 && sys_reg_Op1(id) == 0 &&
+ sys_reg_CRn(id) == 0 && sys_reg_CRm(id) >= 1 &&
+ sys_reg_CRm(id) <= 3);
+}
+
static unsigned int id_visibility(const struct kvm_vcpu *vcpu,
const struct sys_reg_desc *r)
{
@@ -3572,6 +3579,65 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
return write_demux_regids(uindices);
}
+#define ARM64_FEATURE_ID_SPACE_INDEX(r) \
+ ARM64_FEATURE_ID_SPACE_IDX(sys_reg_Op0(r), \
+ sys_reg_Op1(r), \
+ sys_reg_CRn(r), \
+ sys_reg_CRm(r), \
+ sys_reg_Op2(r))
+
+static bool is_feature_id_reg(u32 encoding)
+{
+ return (sys_reg_Op0(encoding) == 3 &&
+ (sys_reg_Op1(encoding) < 2 || sys_reg_Op1(encoding) == 3) &&
+ sys_reg_CRn(encoding) == 0 &&
+ sys_reg_CRm(encoding) <= 7);
+}
+
+int kvm_vm_ioctl_get_reg_writable_masks(struct kvm *kvm, struct reg_mask_range *range)
+{
+ const void *zero_page = page_to_virt(ZERO_PAGE(0));
+ u64 __user *masks = (u64 __user *)range->addr;
+
+ /* Only feature id range is supported, reserved[13] must be zero. */
+ if (range->range != ARM64_FEATURE_ID_RANGE_IDREGS ||
+ memcmp(range->reserved, zero_page, sizeof(range->reserved)))
+ return -EINVAL;
+
+ /* Wipe the whole thing first */
+ if (clear_user(masks, ARM64_FEATURE_ID_SPACE_SIZE * sizeof(__u64)))
+ return -EFAULT;
+
+ for (int i = 0; i < ARRAY_SIZE(sys_reg_descs); i++) {
+ const struct sys_reg_desc *reg = &sys_reg_descs[i];
+ u32 encoding = reg_to_encoding(reg);
+ u64 val;
+
+ if (!is_feature_id_reg(encoding) || !reg->set_user)
+ continue;
+
+ /*
+ * For ID registers, we return the writable mask. Other feature
+ * registers return a full 64bit mask. That's not necessary
+ * compliant with a given revision of the architecture, but the
+ * RES0/RES1 definitions allow us to do that.
+ */
+ if (is_id_reg(encoding)) {
+ if (!reg->val ||
+ (is_aa32_id_reg(encoding) && !kvm_supports_32bit_el0()))
+ continue;
+ val = reg->val;
+ } else {
+ val = ~0UL;
+ }
+
+ if (put_user(val, (masks + ARM64_FEATURE_ID_SPACE_INDEX(encoding))))
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
int __init kvm_sys_reg_table_init(void)
{
struct sys_reg_params params;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 13065dd96132..2102df261f1b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1192,6 +1192,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_COUNTER_OFFSET 227
#define KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE 228
#define KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES 229
+#define KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES 230
#ifdef KVM_CAP_IRQ_ROUTING
@@ -1562,6 +1563,7 @@ struct kvm_s390_ucas_mapping {
#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags)
/* Available with KVM_CAP_COUNTER_OFFSET */
#define KVM_ARM_SET_COUNTER_OFFSET _IOW(KVMIO, 0xb5, struct kvm_arm_counter_offset)
+#define KVM_ARM_GET_REG_WRITABLE_MASKS _IOR(KVMIO, 0xb6, struct reg_mask_range)
/* ioctl for vm fd */
#define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
--
2.42.0.459.ge4e396fd5e-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS
2023-09-19 17:50 [PATCH v1 0/4] Get writable masks for feature ID from userspace Jing Zhang
2023-09-19 17:50 ` [PATCH v1 1/4] KVM: arm64: Allow userspace to get the writable masks for feature ID registers Jing Zhang
@ 2023-09-19 17:50 ` Jing Zhang
2024-02-13 13:59 ` Eric Auger
2023-09-19 17:50 ` [PATCH v1 3/4] KVM: arm64: Use guest ID register values for the sake of emulation Jing Zhang
2023-09-19 17:50 ` [PATCH v1 4/4] KVM: arm64: Reject attempts to set invalid debug arch version Jing Zhang
3 siblings, 1 reply; 9+ messages in thread
From: Jing Zhang @ 2023-09-19 17:50 UTC (permalink / raw)
To: KVM, KVMARM, ARMLinux, Marc Zyngier, Oliver Upton
Cc: Will Deacon, Paolo Bonzini, James Morse, Alexandru Elisei,
Suzuki K Poulose, Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck,
Shaoqin Huang, Jing Zhang
Add some basic documentation on how to get feature ID register writable
masks from userspace.
Signed-off-by: Jing Zhang <jingzhangos@google.com>
---
Documentation/virt/kvm/api.rst | 42 ++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 21a7578142a1..2defb5e198ce 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6070,6 +6070,48 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG
interface. No error will be returned, but the resulting offset will not be
applied.
+4.139 KVM_ARM_GET_REG_WRITABLE_MASKS
+-------------------------------------------
+
+:Capability: KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
+:Architectures: arm64
+:Type: vm ioctl
+:Parameters: struct reg_mask_range (in/out)
+:Returns: 0 on success, < 0 on error
+
+
+::
+
+ #define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8)
+ #define ARM64_FEATURE_ID_RANGE_IDREGS BIT(0)
+
+ struct reg_mask_range {
+ __u64 addr; /* Pointer to mask array */
+ __u32 range; /* Requested range */
+ __u32 reserved[13];
+ };
+
+This ioctl copies the writable masks for Feature ID registers to userspace.
+The Feature ID space is defined as the AArch64 System register space with
+op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
+
+The mask array pointed to by ``addr`` is indexed by the macro
+``ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2)``, allowing userspace
+to know what bits can be changed for the system register described by ``op0,
+op1, crn, crm, op2``.
+
+The ``range`` field describes the requested range of registers. The valid
+ranges can be retrieved by checking the return value of
+KVM_CAP_CHECK_EXTENSION_VM for the KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
+capability, which will return a bitmask of the supported ranges. Each bit
+set in the return value represents a possible value for the ``range``
+field. At the time of writing, only bit 0 is returned set by the
+capability, meaning that only the value ``ARM64_FEATURE_ID_RANGE_IDREGS``
+is valid for ``range``.
+
+The ``reserved[13]`` array is reserved for future use and should be 0, or
+KVM may return an error.
+
5. The kvm_run structure
========================
--
2.42.0.459.ge4e396fd5e-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS
2023-09-19 17:50 ` [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS Jing Zhang
@ 2024-02-13 13:59 ` Eric Auger
2024-02-13 14:53 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Eric Auger @ 2024-02-13 13:59 UTC (permalink / raw)
To: Jing Zhang, KVM, KVMARM, ARMLinux, Marc Zyngier, Oliver Upton
Cc: Will Deacon, Paolo Bonzini, James Morse, Alexandru Elisei,
Suzuki K Poulose, Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck,
Shaoqin Huang, Sebastian Ott
Hi,
On 9/19/23 19:50, Jing Zhang wrote:
> Add some basic documentation on how to get feature ID register writable
> masks from userspace.
>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
> Documentation/virt/kvm/api.rst | 42 ++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 21a7578142a1..2defb5e198ce 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6070,6 +6070,48 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG
> interface. No error will be returned, but the resulting offset will not be
> applied.
>
> +4.139 KVM_ARM_GET_REG_WRITABLE_MASKS
> +-------------------------------------------
> +
> +:Capability: KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
> +:Architectures: arm64
> +:Type: vm ioctl
> +:Parameters: struct reg_mask_range (in/out)
> +:Returns: 0 on success, < 0 on error
> +
> +
> +::
> +
> + #define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8)
> + #define ARM64_FEATURE_ID_RANGE_IDREGS BIT(0)
> +
> + struct reg_mask_range {
> + __u64 addr; /* Pointer to mask array */
> + __u32 range; /* Requested range */
> + __u32 reserved[13];
> + };
> +
> +This ioctl copies the writable masks for Feature ID registers to userspace.
> +The Feature ID space is defined as the AArch64 System register space with
> +op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
when attempting a migration between Ampere Altra and ThunderXv2 the
first hurdle is to handle a failure when writing ICC_CTLR_EL1
(3.0.12.12.4) on dest. This reg is outside of the scope of the above
single range (BIT(0)).
This may be questionable if we want to migrate between those types of
machines but the goal is to exercise different scenarios to have a
gloval view of the problems.
This reg exposes some RO capabilities such as ExtRange, A3V, SEIS,
IDBits, ...
So to get the migration going further I would need to tweek this on the
source - for instance I guess SEIS could be reset despite the host HW
cap - without making too much trouble.
What would you recommend, adding a new range? But I guess we need to
design ranges carefully otherwise we may be quickly limited by the
number of flag bits.
Any suggestion?
Thanks
Eric
> +
> +The mask array pointed to by ``addr`` is indexed by the macro
> +``ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2)``, allowing userspace
> +to know what bits can be changed for the system register described by ``op0,
> +op1, crn, crm, op2``.
> +
> +The ``range`` field describes the requested range of registers. The valid
> +ranges can be retrieved by checking the return value of
> +KVM_CAP_CHECK_EXTENSION_VM for the KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
> +capability, which will return a bitmask of the supported ranges. Each bit
> +set in the return value represents a possible value for the ``range``
> +field. At the time of writing, only bit 0 is returned set by the
> +capability, meaning that only the value ``ARM64_FEATURE_ID_RANGE_IDREGS``
> +is valid for ``range``.
> +
> +The ``reserved[13]`` array is reserved for future use and should be 0, or
> +KVM may return an error.
> +
> 5. The kvm_run structure
> ========================
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS
2024-02-13 13:59 ` Eric Auger
@ 2024-02-13 14:53 ` Marc Zyngier
2024-02-14 18:07 ` Eric Auger
0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2024-02-13 14:53 UTC (permalink / raw)
To: Eric Auger
Cc: Jing Zhang, KVM, KVMARM, ARMLinux, Oliver Upton, Will Deacon,
Paolo Bonzini, James Morse, Alexandru Elisei, Suzuki K Poulose,
Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck, Shaoqin Huang,
Sebastian Ott
Hey Eric,
On Tue, 13 Feb 2024 13:59:31 +0000,
Eric Auger <eauger@redhat.com> wrote:
>
> Hi,
>
> On 9/19/23 19:50, Jing Zhang wrote:
> > Add some basic documentation on how to get feature ID register writable
> > masks from userspace.
> >
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
> > ---
> > Documentation/virt/kvm/api.rst | 42 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 21a7578142a1..2defb5e198ce 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -6070,6 +6070,48 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG
> > interface. No error will be returned, but the resulting offset will not be
> > applied.
> >
> > +4.139 KVM_ARM_GET_REG_WRITABLE_MASKS
> > +-------------------------------------------
> > +
> > +:Capability: KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
> > +:Architectures: arm64
> > +:Type: vm ioctl
> > +:Parameters: struct reg_mask_range (in/out)
> > +:Returns: 0 on success, < 0 on error
> > +
> > +
> > +::
> > +
> > + #define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8)
> > + #define ARM64_FEATURE_ID_RANGE_IDREGS BIT(0)
> > +
> > + struct reg_mask_range {
> > + __u64 addr; /* Pointer to mask array */
> > + __u32 range; /* Requested range */
> > + __u32 reserved[13];
> > + };
> > +
> > +This ioctl copies the writable masks for Feature ID registers to userspace.
> > +The Feature ID space is defined as the AArch64 System register space with
> > +op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
> when attempting a migration between Ampere Altra and ThunderXv2 the
> first hurdle is to handle a failure when writing ICC_CTLR_EL1
> (3.0.12.12.4) on dest. This reg is outside of the scope of the above
> single range (BIT(0)).
Indeed. But more importantly, this isn't really an ID register. Plenty
of variable bits in there.
> This may be questionable if we want to migrate between those types of
> machines but the goal is to exercise different scenarios to have a
> gloval view of the problems.
I think this is a valuable experiment, and we should definitely
explore this sort of things (as I cannot see the diversity of ARM
system slowing down any time soon).
>
> This reg exposes some RO capabilities such as ExtRange, A3V, SEIS,
> IDBits, ...
> So to get the migration going further I would need to tweek this on the
> source - for instance I guess SEIS could be reset despite the host HW
> cap - without making too much trouble.
I'm not sure SEIS is such an easy one: if you promised the guest that
it would never get an SError doing the most stupid things (SEIS=0), it
really shouldn't get one after migration. If you advertised it on the
source HW (Altra), a migration to TX2 would be fine.
The other bits are possible to change depending on the requirements of
the VM (aff3, IDBits), and ExtRange should always be set to 0 (because
our GIC implementation doesn't support EPPI/ESPI).
The really ugly part here is that if you want to affect these bits,
you will need to trap and emulate the access. Not a big deal, but in
the absence of FGT, you will need to handle the full Common trap
group, which is going to slow things down (you will have to trap
ICV_PMR_EL1, for example).
> What would you recommend, adding a new range? But I guess we need to
> design ranges carefully otherwise we may be quickly limited by the
> number of flag bits.
I can see a need to adding a range that would cover non-ID registers
that have RO fields. But we also need to consider the case of EL2
registers that take part in this.
For example, ICV_CTLR_EL1 and ICH_VTR_EL2 and deeply linked, and share
some fields. Without NV, no need to expose HCR_VTR_EL2. With NV, this
register actually drives ICV_CTLR_EL1.
So careful planning is required here, and the impact of this measured.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS
2024-02-13 14:53 ` Marc Zyngier
@ 2024-02-14 18:07 ` Eric Auger
2024-02-14 20:16 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Eric Auger @ 2024-02-14 18:07 UTC (permalink / raw)
To: Marc Zyngier
Cc: Jing Zhang, KVM, KVMARM, ARMLinux, Oliver Upton, Will Deacon,
Paolo Bonzini, James Morse, Alexandru Elisei, Suzuki K Poulose,
Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck, Shaoqin Huang,
Sebastian Ott
Hi Marc,
On 2/13/24 15:53, Marc Zyngier wrote:
> Hey Eric,
>
> On Tue, 13 Feb 2024 13:59:31 +0000,
> Eric Auger <eauger@redhat.com> wrote:
>>
>> Hi,
>>
>> On 9/19/23 19:50, Jing Zhang wrote:
>>> Add some basic documentation on how to get feature ID register writable
>>> masks from userspace.
>>>
>>> Signed-off-by: Jing Zhang <jingzhangos@google.com>
>>> ---
>>> Documentation/virt/kvm/api.rst | 42 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>>
>>> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
>>> index 21a7578142a1..2defb5e198ce 100644
>>> --- a/Documentation/virt/kvm/api.rst
>>> +++ b/Documentation/virt/kvm/api.rst
>>> @@ -6070,6 +6070,48 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG
>>> interface. No error will be returned, but the resulting offset will not be
>>> applied.
>>>
>>> +4.139 KVM_ARM_GET_REG_WRITABLE_MASKS
>>> +-------------------------------------------
>>> +
>>> +:Capability: KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
>>> +:Architectures: arm64
>>> +:Type: vm ioctl
>>> +:Parameters: struct reg_mask_range (in/out)
>>> +:Returns: 0 on success, < 0 on error
>>> +
>>> +
>>> +::
>>> +
>>> + #define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8)
>>> + #define ARM64_FEATURE_ID_RANGE_IDREGS BIT(0)
>>> +
>>> + struct reg_mask_range {
>>> + __u64 addr; /* Pointer to mask array */
>>> + __u32 range; /* Requested range */
>>> + __u32 reserved[13];
>>> + };
>>> +
>>> +This ioctl copies the writable masks for Feature ID registers to userspace.
>>> +The Feature ID space is defined as the AArch64 System register space with
>>> +op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
>> when attempting a migration between Ampere Altra and ThunderXv2 the
>> first hurdle is to handle a failure when writing ICC_CTLR_EL1
>> (3.0.12.12.4) on dest. This reg is outside of the scope of the above
>> single range (BIT(0)).
>
> Indeed. But more importantly, this isn't really an ID register. Plenty
> of variable bits in there.
>
>> This may be questionable if we want to migrate between those types of
>> machines but the goal is to exercise different scenarios to have a
>> gloval view of the problems.
>
> I think this is a valuable experiment, and we should definitely
> explore this sort of things (as I cannot see the diversity of ARM
> system slowing down any time soon).
>
>>
>> This reg exposes some RO capabilities such as ExtRange, A3V, SEIS,hyp/nvhe/hyp-main.c
>> IDBits, ...
>> So to get the migration going further I would need to tweek this on the
>> source - for instance I guess SEIS could be reset despite the host HW
>> cap - without making too much trouble.
>
> I'm not sure SEIS is such an easy one: if you promised the guest that
> it would never get an SError doing the most stupid things (SEIS=0), it
> really shouldn't get one after migration. If you advertised it on the
> source HW (Altra), a migration to TX2 would be fine.
I see. Indeed for sure we must make sure KVM cannot inject SEIs in the
guest if SEIS is not advertised on guest side.
In this case SEIS is 0 on src Altra. On dst TX2 ich_vtr_el2 advertises
it and host seis =1 causing set_gic_ctlr to fail (vgic-sys-reg-v3.c).
>
> The other bits are possible to change depending on the requirements of
> the VM (aff3, IDBits), and ExtRange should always be set to 0 (because
> our GIC implementation doesn't support EPPI/ESPI).
>
> The really ugly part here is that if you want to affect these bits,
> you will need to trap and emulate the access. Not a big deal, but in
> the absence of FGT, you will need to handle the full Common trap
> group, which is going to slow things down (you will have to trap
> ICV_PMR_EL1, for example).
Would it be sensible to simplify things and only support the new range
if FGT is supported?
>
>> What would you recommend, adding a new range? But I guess we need to
>> design ranges carefully otherwise we may be quickly limited by the
>> number of flag bits.
>
> I can see a need to adding a range that would cover non-ID registers
> that have RO fields. But we also need to consider the case of EL2
> registers that take part in this.
Yes I need to better understand the consistency with ICH_VTR_EL2
>
> For example, ICV_CTLR_EL1 and ICH_VTR_EL2 and deeply linked, and share
> some fields. Without NV, no need to expose HCR_VTR_EL2. With NV, this
> register actually drives ICV_CTLR_EL1.
understood.
>
> So careful planning is required here, and the impact of this measured.
Understood. More time needed to study the code ;-)
Thanks!
Eric
>
> Thanks,
>
> M.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS
2024-02-14 18:07 ` Eric Auger
@ 2024-02-14 20:16 ` Marc Zyngier
0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2024-02-14 20:16 UTC (permalink / raw)
To: Eric Auger
Cc: Jing Zhang, KVM, KVMARM, ARMLinux, Oliver Upton, Will Deacon,
Paolo Bonzini, James Morse, Alexandru Elisei, Suzuki K Poulose,
Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck, Shaoqin Huang,
Sebastian Ott
Hi Eric,
On Wed, 14 Feb 2024 18:07:58 +0000,
Eric Auger <eauger@redhat.com> wrote:
>
> > I'm not sure SEIS is such an easy one: if you promised the guest that
> > it would never get an SError doing the most stupid things (SEIS=0), it
> > really shouldn't get one after migration. If you advertised it on the
> > source HW (Altra), a migration to TX2 would be fine.
> I see. Indeed for sure we must make sure KVM cannot inject SEIs in the
> guest if SEIS is not advertised on guest side.
The problem isn't KVM injecting an SError, but the HW doing it (that's
what SEIS indicates). On some HW, it only takes an old enough EFI
build to trigger those.
> In this case SEIS is 0 on src Altra. On dst TX2 ich_vtr_el2 advertises
> it and host seis =1 causing set_gic_ctlr to fail (vgic-sys-reg-v3.c).
>
> >
> > The other bits are possible to change depending on the requirements of
> > the VM (aff3, IDBits), and ExtRange should always be set to 0 (because
> > our GIC implementation doesn't support EPPI/ESPI).
> >
> > The really ugly part here is that if you want to affect these bits,
> > you will need to trap and emulate the access. Not a big deal, but in
> > the absence of FGT, you will need to handle the full Common trap
> > group, which is going to slow things down (you will have to trap
> > ICV_PMR_EL1, for example).
> Would it be sensible to simplify things and only support the new range
> if FGT is supported?
I'm not sure that helps. The only FGT that covers the GIC is for
ICC_IGRPENn_EL1, and we don't care much about that guy. So
ICH_VMCR_EL2.TC it is, and that sucks a bit. But if that's what you
want to show, you don't have a choice.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 3/4] KVM: arm64: Use guest ID register values for the sake of emulation
2023-09-19 17:50 [PATCH v1 0/4] Get writable masks for feature ID from userspace Jing Zhang
2023-09-19 17:50 ` [PATCH v1 1/4] KVM: arm64: Allow userspace to get the writable masks for feature ID registers Jing Zhang
2023-09-19 17:50 ` [PATCH v1 2/4] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS Jing Zhang
@ 2023-09-19 17:50 ` Jing Zhang
2023-09-19 17:50 ` [PATCH v1 4/4] KVM: arm64: Reject attempts to set invalid debug arch version Jing Zhang
3 siblings, 0 replies; 9+ messages in thread
From: Jing Zhang @ 2023-09-19 17:50 UTC (permalink / raw)
To: KVM, KVMARM, ARMLinux, Marc Zyngier, Oliver Upton
Cc: Will Deacon, Paolo Bonzini, James Morse, Alexandru Elisei,
Suzuki K Poulose, Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck,
Shaoqin Huang, Jing Zhang
Since KVM now supports per-VM ID registers, use per-VM ID register
values for the sake of emulation for DBGDIDR and LORegion.
Signed-off-by: Jing Zhang <jingzhangos@google.com>
---
arch/arm64/kvm/sys_regs.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index cdb9976d8091..4dcc9272fbb8 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -379,7 +379,7 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
const struct sys_reg_desc *r)
{
- u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
+ u64 val = IDREG(vcpu->kvm, SYS_ID_AA64MMFR1_EL1);
u32 sr = reg_to_encoding(r);
if (!(val & (0xfUL << ID_AA64MMFR1_EL1_LO_SHIFT))) {
@@ -2445,8 +2445,8 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
if (p->is_write) {
return ignore_write(vcpu, p);
} else {
- u64 dfr = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
- u64 pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+ u64 dfr = IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1);
+ u64 pfr = IDREG(vcpu->kvm, SYS_ID_AA64PFR0_EL1);
u32 el3 = !!cpuid_feature_extract_unsigned_field(pfr, ID_AA64PFR0_EL1_EL3_SHIFT);
p->regval = ((((dfr >> ID_AA64DFR0_EL1_WRPs_SHIFT) & 0xf) << 28) |
--
2.42.0.459.ge4e396fd5e-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v1 4/4] KVM: arm64: Reject attempts to set invalid debug arch version
2023-09-19 17:50 [PATCH v1 0/4] Get writable masks for feature ID from userspace Jing Zhang
` (2 preceding siblings ...)
2023-09-19 17:50 ` [PATCH v1 3/4] KVM: arm64: Use guest ID register values for the sake of emulation Jing Zhang
@ 2023-09-19 17:50 ` Jing Zhang
3 siblings, 0 replies; 9+ messages in thread
From: Jing Zhang @ 2023-09-19 17:50 UTC (permalink / raw)
To: KVM, KVMARM, ARMLinux, Marc Zyngier, Oliver Upton
Cc: Will Deacon, Paolo Bonzini, James Morse, Alexandru Elisei,
Suzuki K Poulose, Fuad Tabba, Suraj Jitindar Singh, Cornelia Huck,
Shaoqin Huang, Jing Zhang
From: Oliver Upton <oliver.upton@linux.dev>
The debug architecture is mandatory in ARMv8, so KVM should not allow
userspace to configure a vCPU with less than that. Of course, this isn't
handled elegantly by the generic ID register plumbing, as the respective
ID register fields have a nonzero starting value.
Add an explicit check for debug versions less than v8 of the
architecture.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Jing Zhang <jingzhangos@google.com>
---
arch/arm64/kvm/sys_regs.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 4dcc9272fbb8..fdebd9d042c3 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1216,8 +1216,14 @@ static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
/* Some features have different safe value type in KVM than host features */
switch (id) {
case SYS_ID_AA64DFR0_EL1:
- if (kvm_ftr.shift == ID_AA64DFR0_EL1_PMUVer_SHIFT)
+ switch (kvm_ftr.shift) {
+ case ID_AA64DFR0_EL1_PMUVer_SHIFT:
kvm_ftr.type = FTR_LOWER_SAFE;
+ break;
+ case ID_AA64DFR0_EL1_DebugVer_SHIFT:
+ kvm_ftr.type = FTR_LOWER_SAFE;
+ break;
+ }
break;
case SYS_ID_DFR0_EL1:
if (kvm_ftr.shift == ID_DFR0_EL1_PerfMon_SHIFT)
@@ -1476,14 +1482,22 @@ static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
return val;
}
+#define ID_REG_LIMIT_FIELD_ENUM(val, reg, field, limit) \
+({ \
+ u64 __f_val = FIELD_GET(reg##_##field##_MASK, val); \
+ (val) &= ~reg##_##field##_MASK; \
+ (val) |= FIELD_PREP(reg##_##field##_MASK, \
+ min(__f_val, (u64)reg##_##field##_##limit)); \
+ (val); \
+})
+
static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
const struct sys_reg_desc *rd)
{
u64 val = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
/* Limit debug to ARMv8.0 */
- val &= ~ID_AA64DFR0_EL1_DebugVer_MASK;
- val |= SYS_FIELD_PREP_ENUM(ID_AA64DFR0_EL1, DebugVer, IMP);
+ val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64DFR0_EL1, DebugVer, IMP);
/*
* Only initialize the PMU version if the vCPU was configured with one.
@@ -1503,6 +1517,7 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
const struct sys_reg_desc *rd,
u64 val)
{
+ u8 debugver = SYS_FIELD_GET(ID_AA64DFR0_EL1, DebugVer, val);
u8 pmuver = SYS_FIELD_GET(ID_AA64DFR0_EL1, PMUVer, val);
/*
@@ -1522,6 +1537,13 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
if (pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF)
val &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
+ /*
+ * ID_AA64DFR0_EL1.DebugVer is one of those awkward fields with a
+ * nonzero minimum safe value.
+ */
+ if (debugver < ID_AA64DFR0_EL1_DebugVer_IMP)
+ return -EINVAL;
+
return set_id_reg(vcpu, rd, val);
}
@@ -1543,6 +1565,7 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
u64 val)
{
u8 perfmon = SYS_FIELD_GET(ID_DFR0_EL1, PerfMon, val);
+ u8 copdbg = SYS_FIELD_GET(ID_DFR0_EL1, CopDbg, val);
if (perfmon == ID_DFR0_EL1_PerfMon_IMPDEF) {
val &= ~ID_DFR0_EL1_PerfMon_MASK;
@@ -1558,6 +1581,9 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
if (perfmon != 0 && perfmon < ID_DFR0_EL1_PerfMon_PMUv3)
return -EINVAL;
+ if (copdbg < ID_DFR0_EL1_CopDbg_Armv8)
+ return -EINVAL;
+
return set_id_reg(vcpu, rd, val);
}
--
2.42.0.459.ge4e396fd5e-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread