Skip to content

Commit

Permalink
KVM: arm64: Do not change the PMU event filter after a VCPU has run
Browse files Browse the repository at this point in the history
Userspace can specify which events a guest is allowed to use with the
KVM_ARM_VCPU_PMU_V3_FILTER attribute. The list of allowed events can be
identified by a guest from reading the PMCEID{0,1}_EL0 registers.

Changing the PMU event filter after a VCPU has run can cause reads of the
registers performed before the filter is changed to return different values
than reads performed with the new event filter in place. The architecture
defines the two registers as read-only, and this behaviour contradicts
that.

Keep track when the first VCPU has run and deny changes to the PMU event
filter to prevent this from happening.

Signed-off-by: Marc Zyngier <[email protected]>
[ Alexandru E: Added commit message, updated ioctl documentation ]
Signed-off-by: Alexandru Elisei <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
Marc Zyngier committed Feb 8, 2022
1 parent dfd42fa commit 5177fe9
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Documentation/virt/kvm/devices/vcpu.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ irqchip.
-ENODEV PMUv3 not supported or GIC not initialized
-ENXIO PMUv3 not properly configured or in-kernel irqchip not
configured as required prior to calling this attribute
-EBUSY PMUv3 already initialized
-EBUSY PMUv3 already initialized or a VCPU has already run
-EINVAL Invalid filter range
======= ======================================================

Expand Down
1 change: 1 addition & 0 deletions arch/arm64/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ struct kvm_arch {

/* Memory Tagging Extension enabled for the guest */
bool mte_enabled;
bool ran_once;
};

struct kvm_vcpu_fault_info {
Expand Down
4 changes: 4 additions & 0 deletions arch/arm64/kvm/arm.c
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,10 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
if (kvm_vm_is_protected(kvm))
kvm_call_hyp_nvhe(__pkvm_vcpu_init_traps, vcpu);

mutex_lock(&kvm->lock);
kvm->arch.ran_once = true;
mutex_unlock(&kvm->lock);

return ret;
}

Expand Down
33 changes: 20 additions & 13 deletions arch/arm64/kvm/pmu-emul.c
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,8 @@ static bool pmu_irq_is_valid(struct kvm *kvm, int irq)

int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
{
struct kvm *kvm = vcpu->kvm;

if (!kvm_vcpu_has_pmu(vcpu))
return -ENODEV;

Expand All @@ -941,7 +943,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
int __user *uaddr = (int __user *)(long)attr->addr;
int irq;

if (!irqchip_in_kernel(vcpu->kvm))
if (!irqchip_in_kernel(kvm))
return -EINVAL;

if (get_user(irq, uaddr))
Expand All @@ -951,7 +953,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
if (!(irq_is_ppi(irq) || irq_is_spi(irq)))
return -EINVAL;

if (!pmu_irq_is_valid(vcpu->kvm, irq))
if (!pmu_irq_is_valid(kvm, irq))
return -EINVAL;

if (kvm_arm_pmu_irq_initialized(vcpu))
Expand All @@ -966,7 +968,7 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
struct kvm_pmu_event_filter filter;
int nr_events;

nr_events = kvm_pmu_event_mask(vcpu->kvm) + 1;
nr_events = kvm_pmu_event_mask(kvm) + 1;

uaddr = (struct kvm_pmu_event_filter __user *)(long)attr->addr;

Expand All @@ -978,12 +980,17 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
filter.action != KVM_PMU_EVENT_DENY))
return -EINVAL;

mutex_lock(&vcpu->kvm->lock);
mutex_lock(&kvm->lock);

if (kvm->arch.ran_once) {
mutex_unlock(&kvm->lock);
return -EBUSY;
}

if (!vcpu->kvm->arch.pmu_filter) {
vcpu->kvm->arch.pmu_filter = bitmap_alloc(nr_events, GFP_KERNEL_ACCOUNT);
if (!vcpu->kvm->arch.pmu_filter) {
mutex_unlock(&vcpu->kvm->lock);
if (!kvm->arch.pmu_filter) {
kvm->arch.pmu_filter = bitmap_alloc(nr_events, GFP_KERNEL_ACCOUNT);
if (!kvm->arch.pmu_filter) {
mutex_unlock(&kvm->lock);
return -ENOMEM;
}

Expand All @@ -994,17 +1001,17 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
* events, the default is to allow.
*/
if (filter.action == KVM_PMU_EVENT_ALLOW)
bitmap_zero(vcpu->kvm->arch.pmu_filter, nr_events);
bitmap_zero(kvm->arch.pmu_filter, nr_events);
else
bitmap_fill(vcpu->kvm->arch.pmu_filter, nr_events);
bitmap_fill(kvm->arch.pmu_filter, nr_events);
}

if (filter.action == KVM_PMU_EVENT_ALLOW)
bitmap_set(vcpu->kvm->arch.pmu_filter, filter.base_event, filter.nevents);
bitmap_set(kvm->arch.pmu_filter, filter.base_event, filter.nevents);
else
bitmap_clear(vcpu->kvm->arch.pmu_filter, filter.base_event, filter.nevents);
bitmap_clear(kvm->arch.pmu_filter, filter.base_event, filter.nevents);

mutex_unlock(&vcpu->kvm->lock);
mutex_unlock(&kvm->lock);

return 0;
}
Expand Down

0 comments on commit 5177fe9

Please sign in to comment.