| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9c216f6d-c885-41e7-ad66-656a09035e9b@arm.com>
Date: Mon, 10 Feb 2025 11:58:03 +0000
From: Steven Price <steven.price@....com>
To: Joey Gouly <joey.gouly@....com>
Cc: kvm@...r.kernel.org, kvmarm@...ts.linux.dev,
Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
Will Deacon <will@...nel.org>, James Morse <james.morse@....com>,
Oliver Upton <oliver.upton@...ux.dev>,
Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu
<yuzenghui@...wei.com>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, Alexandru Elisei <alexandru.elisei@....com>,
Christoffer Dall <christoffer.dall@....com>, Fuad Tabba <tabba@...gle.com>,
linux-coco@...ts.linux.dev,
Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>,
Gavin Shan <gshan@...hat.com>, Shanker Donthineni <sdonthineni@...dia.com>,
Alper Gun <alpergun@...gle.com>, "Aneesh Kumar K . V"
<aneesh.kumar@...nel.org>
Subject: Re: [PATCH v6 32/43] arm64: rme: Enable PMU support with a realm
guest
On 12/12/2024 16:54, Joey Gouly wrote:
> On Thu, Dec 12, 2024 at 03:55:57PM +0000, Steven Price wrote:
>> Use the PMU registers from the RmiRecExit structure to identify when an
>> overflow interrupt is due and inject it into the guest. Also hook up the
>> configuration option for enabling the PMU within the guest.
>>
>> When entering a realm guest with a PMU interrupt pending, it is
>> necessary to disable the physical interrupt. Otherwise when the RMM
>> restores the PMU state the physical interrupt will trigger causing an
>> immediate exit back to the host. The guest is expected to acknowledge
>> the interrupt causing a host exit (to update the GIC state) which gives
>> the opportunity to re-enable the physical interrupt before the next PMU
>> event.
>>
>> Number of PMU counters is configured by the VMM by writing to PMCR.N.
>>
>> Signed-off-by: Steven Price <steven.price@....com>
>> ---
>> Changes since v2:
>> * Add a macro kvm_pmu_get_irq_level() to avoid compile issues when PMU
>> support is disabled.
>> ---
>> arch/arm64/kvm/arm.c | 11 +++++++++++
>> arch/arm64/kvm/guest.c | 7 +++++++
>> arch/arm64/kvm/pmu-emul.c | 3 +++
>> arch/arm64/kvm/rme.c | 8 ++++++++
>> arch/arm64/kvm/sys_regs.c | 2 +-
>> include/kvm/arm_pmu.h | 4 ++++
>> 6 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index 6f7f96ab781d..ae3596a25272 100644
>> --- a/arch/arm64/kvm/arm.c
>> +++ b/arch/arm64/kvm/arm.c
>> @@ -15,6 +15,7 @@
>> #include <linux/vmalloc.h>
>> #include <linux/fs.h>
>> #include <linux/mman.h>
>> +#include <linux/perf/arm_pmu.h>
>> #include <linux/sched.h>
>> #include <linux/kvm.h>
>> #include <linux/kvm_irqfd.h>
>> @@ -1209,6 +1210,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> run->exit_reason = KVM_EXIT_UNKNOWN;
>> run->flags = 0;
>> while (ret > 0) {
>> + bool pmu_stopped = false;
>> +
>> /*
>> * Check conditions before entering the guest
>> */
>> @@ -1240,6 +1243,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>>
>> kvm_pmu_flush_hwstate(vcpu);
>>
>> + if (vcpu_is_rec(vcpu) && kvm_pmu_get_irq_level(vcpu)) {
>> + pmu_stopped = true;
>> + arm_pmu_set_phys_irq(false);
>> + }
>> +
>> local_irq_disable();
>>
>> kvm_vgic_flush_hwstate(vcpu);
>> @@ -1342,6 +1350,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>>
>> preempt_enable();
>>
>> + if (pmu_stopped)
>> + arm_pmu_set_phys_irq(true);
>> +
>> /*
>> * The ARMv8 architecture doesn't give the hypervisor
>> * a mechanism to prevent a guest from dropping to AArch32 EL0
>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>> index 920e0fd73698..b4e3839f6076 100644
>> --- a/arch/arm64/kvm/guest.c
>> +++ b/arch/arm64/kvm/guest.c
>> @@ -804,6 +804,8 @@ int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>> return kvm_arm_sys_reg_get_reg(vcpu, reg);
>> }
>>
>> +#define KVM_REG_ARM_PMCR_EL0 ARM64_SYS_REG(3, 3, 9, 12, 0)
>
> There's already SYS_PMCR_EL0 defined, so it seems this is not needed?
ARM64_SYS_REG() is slightly different from the sys_reg() macro used by
SYS_PMCR_EL0. The ARM64_SYS_REG() macro encodes extra bits about the
register size (KVM_REG_SIZE_U64) and that this is an arm64 system
register (KVM_REG_ARM64 | KVM_REG_ARM64_SYSREG).
There is a KVM_ARM64_SYS_REG() macro used by the selftests which would
allow using the SYS_PMCR_EL0 to avoid repeating the encoding, but I
can't find an equivalent outside the selftests.
To be honest I'm not sure quite what the best option here is. I was
tempted to put them in uapi/asm/kvm.h but other than the timer registers
(which have quirks due to a accident in the past) we don't define any
other register names as uABI - and there's no particular reason to start
for this one.
Thanks,
Steve
>> +
>> /*
>> * The RMI ABI only enables setting some GPRs and PC. The selection of GPRs
>> * that are available depends on the Realm state and the reason for the last
>> @@ -818,6 +820,11 @@ static bool validate_realm_set_reg(struct kvm_vcpu *vcpu,
>> u64 off = core_reg_offset_from_id(reg->id);
>>
>> return kvm_realm_validate_core_reg(off);
>> + } else {
>> + switch (reg->id) {
>> + case KVM_REG_ARM_PMCR_EL0:
>> + return true;
>> + }
>> }
>>
>> return false;
>> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
>> index 456102bc0b55..5628b573ca41 100644
>> --- a/arch/arm64/kvm/pmu-emul.c
>> +++ b/arch/arm64/kvm/pmu-emul.c
>> @@ -397,6 +397,9 @@ static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu)
>> {
>> u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0);
>>
>> + if (vcpu_is_rec(vcpu))
>> + return vcpu->arch.rec.run->exit.pmu_ovf_status;
>> +
>> reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1);
>>
>> /*
>> diff --git a/arch/arm64/kvm/rme.c b/arch/arm64/kvm/rme.c
>> index 27a479feb907..e562e77c1f94 100644
>> --- a/arch/arm64/kvm/rme.c
>> +++ b/arch/arm64/kvm/rme.c
>> @@ -303,6 +303,11 @@ static int realm_create_rd(struct kvm *kvm)
>> params->rtt_base = kvm->arch.mmu.pgd_phys;
>> params->vmid = realm->vmid;
>>
>> + if (kvm->arch.arm_pmu) {
>> + params->pmu_num_ctrs = kvm->arch.pmcr_n;
>> + params->flags |= RMI_REALM_PARAM_FLAG_PMU;
>> + }
>> +
>> params_phys = virt_to_phys(params);
>>
>> if (rmi_realm_create(rd_phys, params_phys)) {
>> @@ -1370,6 +1375,9 @@ int kvm_create_rec(struct kvm_vcpu *vcpu)
>> if (!vcpu_has_feature(vcpu, KVM_ARM_VCPU_PSCI_0_2))
>> return -EINVAL;
>>
>> + if (vcpu->kvm->arch.arm_pmu && !kvm_vcpu_has_pmu(vcpu))
>> + return -EINVAL;
>> +
>> BUILD_BUG_ON(sizeof(*params) > PAGE_SIZE);
>> BUILD_BUG_ON(sizeof(*rec->run) > PAGE_SIZE);
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 83c6b4a07ef5..a4713609e230 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1324,7 +1324,7 @@ static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
>> * implements. Ignore this error to maintain compatibility
>> * with the existing KVM behavior.
>> */
>> - if (!kvm_vm_has_ran_once(kvm) &&
>> + if (!kvm_vm_has_ran_once(kvm) && !kvm_realm_is_created(kvm) &&
>> new_n <= kvm_arm_pmu_get_max_counters(kvm))
>> kvm->arch.pmcr_n = new_n;
>>
>> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
>> index e61dd7dd2286..30625a6fd143 100644
>> --- a/include/kvm/arm_pmu.h
>> +++ b/include/kvm/arm_pmu.h
>> @@ -77,6 +77,8 @@ void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
>> void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>> void kvm_vcpu_pmu_resync_el0(void);
>>
>> +#define kvm_pmu_get_irq_level(vcpu) ((vcpu)->arch.pmu.irq_level)
>> +
>> #define kvm_vcpu_has_pmu(vcpu) \
>> (vcpu_has_feature(vcpu, KVM_ARM_VCPU_PMU_V3))
>>
>> @@ -164,6 +166,8 @@ static inline u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>> return 0;
>> }
>>
>> +#define kvm_pmu_get_irq_level(vcpu) (false)
>> +
>> #define kvm_vcpu_has_pmu(vcpu) ({ false; })
>> static inline void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu) {}
>> static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
>
> Thanks,
> Joey
Powered by blists - more mailing lists