On Thu, Jun 13, 2024 at 10:19:56PM +0000, Oliver Upton wrote:
On Mon, Jun 03, 2024 at 03:05:04PM +0200, Sebastian Ott wrote:
+static int validate_cache_topology(struct kvm_vcpu *vcpu, u64 ctr_el0)
+{
+ const struct sys_reg_desc *clidr_el1;
+ unsigned int i;
+ int ret;
+
+ clidr_el1 = get_sys_reg_desc(SYS_CLIDR_EL1);
+ if (!clidr_el1)
+ return -ENOENT;
This doesn't actually matter if we agree on dropping the cross-checking,
but if this lookup fails it is 100% a KVM bug. Returning ENOENT isn't
exactly right here, since it gives userspace the impression that the
sysreg index it tried to access does not exist.
So in the future it'd be good to return EINVAL in places where the
kernel did something stupid, probably with a warning for good measure.
+static int set_ctr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ u64 val)
+{
+ u64 ctr, writable_mask = rd->val;
+ int ret = 0;
+
+ mutex_lock(&vcpu->kvm->arch.config_lock);
+ ctr = vcpu->kvm->arch.ctr_el0;
+ if (val == ctr)
+ goto out_unlock;
+
+ ret = -EBUSY;
+ if (kvm_vm_has_ran_once(vcpu->kvm))
+ goto out_unlock;
+
+ ret = -EINVAL;
+ if ((ctr & ~writable_mask) != (val & ~writable_mask))
+ goto out_unlock;
+
+ if (((ctr & CTR_EL0_DIC_MASK) < (val & CTR_EL0_DIC_MASK)) ||
+ ((ctr & CTR_EL0_IDC_MASK) < (val & CTR_EL0_IDC_MASK)) ||
+ ((ctr & CTR_EL0_DminLine_MASK) < (val & CTR_EL0_DminLine_MASK)) ||
+ ((ctr & CTR_EL0_IminLine_MASK) < (val & CTR_EL0_IminLine_MASK))) {
+ goto out_unlock;
I'd prefer if we addressed the issue w/ arm64_check_features() by making
CTR_EL0 behave like the other registers in the ID space instead of
open-coding these sorts of checks.
I believe that can be accomplished by using kvm_read_sanitised_id_reg()
as the ::reset() function in the descriptor and initializing
kvm->arch.ctr_el0 in kvm_reset_id_regs().
Durr, I got rid of kvm_reset_id_regs() in commit 44cbe80b7616 ("KVM: arm64:
Reset VM feature ID regs from kvm_reset_sys_regs()"), I should engage
brain before responding.
Adding a check for encoding == CTR_EL0 to is_vm_ftr_id_reg() seems to be
the best way out.