From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B36CC43461 for ; Wed, 9 Sep 2020 09:55:04 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id C680921D79 for ; Wed, 9 Sep 2020 09:55:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="LDTmZ78r" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C680921D79 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 254394B373; Wed, 9 Sep 2020 05:55:03 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@kernel.org Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id K-ORRjqItZUt; Wed, 9 Sep 2020 05:55:01 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id DF1954B359; Wed, 9 Sep 2020 05:55:01 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 5B2B64B33E for ; Wed, 9 Sep 2020 05:55:00 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id C9QybY6hf5mi for ; Wed, 9 Sep 2020 05:54:59 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 189CD4B2F7 for ; Wed, 9 Sep 2020 05:54:59 -0400 (EDT) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BE4D521D79; Wed, 9 Sep 2020 09:54:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599645297; bh=yb9q5YlJL0pBVdVI2oIUvHjVDi83Gtgj3rffl2bbaxA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=LDTmZ78rZ1yG/Y8OwIpmSRb94pFseP9TOpBYvzNIyscYtMpC6ScR8LrCy2joHzDV+ IGBS15W3OSXXICYGDaCzXQitUNEtIzUXUyCL0oly8jHtxPd8NSMwYZ7J//sYfyKSWi ZJpwQjn/6XU86VJTVzlP95Yge8EhPbyURhNUBNkk= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1kFwoa-00AKzZ-1L; Wed, 09 Sep 2020 10:54:56 +0100 MIME-Version: 1.0 Date: Wed, 09 Sep 2020 10:54:55 +0100 From: Marc Zyngier To: Auger Eric Subject: Re: [PATCH v3 2/5] KVM: arm64: Use event mask matching architecture revision In-Reply-To: <2e1257aa-9e8c-89ba-c09b-3cfee38c8486@redhat.com> References: <20200908075830.1161921-1-maz@kernel.org> <20200908075830.1161921-3-maz@kernel.org> <2e1257aa-9e8c-89ba-c09b-3cfee38c8486@redhat.com> User-Agent: Roundcube Webmail/1.4.8 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: eric.auger@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, robin.murphy@arm.com, mark.rutland@arm.com, graf@amazon.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: kvm@vger.kernel.org, kernel-team@android.com, linux-arm-kernel@lists.infradead.org, graf@amazon.com, Robin Murphy , kvmarm@lists.cs.columbia.edu X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Hi Eric, On 2020-09-09 10:38, Auger Eric wrote: > Hi Marc, > > On 9/8/20 9:58 AM, Marc Zyngier wrote: >> The PMU code suffers from a small defect where we assume that the >> event >> number provided by the guest is always 16 bit wide, even if the CPU >> only >> implements the ARMv8.0 architecture. This isn't really problematic in >> the sense that the event number ends up in a system register, cropping >> it to the right width, but still this needs fixing. >> >> In order to make it work, let's probe the version of the PMU that the >> guest is going to use. This is done by temporarily creating a kernel >> event and looking at the PMUVer field that has been saved at probe >> time >> in the associated arm_pmu structure. This in turn gets saved in the >> kvm >> structure, and subsequently used to compute the event mask that gets >> used throughout the PMU code. >> >> Signed-off-by: Marc Zyngier >> --- >> arch/arm64/include/asm/kvm_host.h | 2 + >> arch/arm64/kvm/pmu-emul.c | 81 >> +++++++++++++++++++++++++++++-- >> 2 files changed, 78 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_host.h >> b/arch/arm64/include/asm/kvm_host.h >> index 65568b23868a..6cd60be69c28 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -110,6 +110,8 @@ struct kvm_arch { >> * supported. >> */ >> bool return_nisv_io_abort_to_user; >> + >> + unsigned int pmuver; >> }; >> >> struct kvm_vcpu_fault_info { >> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c >> index 93d797df42c6..8a5f65763814 100644 >> --- a/arch/arm64/kvm/pmu-emul.c >> +++ b/arch/arm64/kvm/pmu-emul.c >> @@ -20,6 +20,20 @@ static void kvm_pmu_stop_counter(struct kvm_vcpu >> *vcpu, struct kvm_pmc *pmc); >> >> #define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1 >> >> +static u32 kvm_pmu_event_mask(struct kvm *kvm) >> +{ >> + switch (kvm->arch.pmuver) { >> + case 1: /* ARMv8.0 */ >> + return GENMASK(9, 0); >> + case 4: /* ARMv8.1 */ >> + case 5: /* ARMv8.4 */ >> + case 6: /* ARMv8.5 */ >> + return GENMASK(15, 0); >> + default: /* Shouldn't be there, just for sanity */ >> + return 0; >> + } >> +} >> + >> /** >> * kvm_pmu_idx_is_64bit - determine if select_idx is a 64bit counter >> * @vcpu: The vcpu pointer >> @@ -100,7 +114,7 @@ static bool kvm_pmu_idx_has_chain_evtype(struct >> kvm_vcpu *vcpu, u64 select_idx) >> return false; >> >> reg = PMEVTYPER0_EL0 + select_idx; >> - eventsel = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_EVENT; >> + eventsel = __vcpu_sys_reg(vcpu, reg) & >> kvm_pmu_event_mask(vcpu->kvm); >> >> return eventsel == ARMV8_PMUV3_PERFCTR_CHAIN; >> } >> @@ -495,7 +509,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu >> *vcpu, u64 val) >> >> /* PMSWINC only applies to ... SW_INC! */ >> type = __vcpu_sys_reg(vcpu, PMEVTYPER0_EL0 + i); >> - type &= ARMV8_PMU_EVTYPE_EVENT; >> + type &= kvm_pmu_event_mask(vcpu->kvm); >> if (type != ARMV8_PMUV3_PERFCTR_SW_INCR) >> continue; >> >> @@ -578,7 +592,7 @@ static void kvm_pmu_create_perf_event(struct >> kvm_vcpu *vcpu, u64 select_idx) >> data = __vcpu_sys_reg(vcpu, reg); >> >> kvm_pmu_stop_counter(vcpu, pmc); >> - eventsel = data & ARMV8_PMU_EVTYPE_EVENT; >> + eventsel = data & kvm_pmu_event_mask(vcpu->kvm);; >> >> /* Software increment event does't need to be backed by a perf event >> */ >> if (eventsel == ARMV8_PMUV3_PERFCTR_SW_INCR && >> @@ -679,17 +693,68 @@ static void kvm_pmu_update_pmc_chained(struct >> kvm_vcpu *vcpu, u64 select_idx) >> void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data, >> u64 select_idx) >> { >> - u64 reg, event_type = data & ARMV8_PMU_EVTYPE_MASK; >> + u64 reg, mask; >> + >> + mask = ARMV8_PMU_EVTYPE_MASK; >> + mask &= ~ARMV8_PMU_EVTYPE_EVENT; >> + mask |= kvm_pmu_event_mask(vcpu->kvm); >> >> reg = (select_idx == ARMV8_PMU_CYCLE_IDX) >> ? PMCCFILTR_EL0 : PMEVTYPER0_EL0 + select_idx; >> >> - __vcpu_sys_reg(vcpu, reg) = event_type; >> + __vcpu_sys_reg(vcpu, reg) = data & mask; >> >> kvm_pmu_update_pmc_chained(vcpu, select_idx); >> kvm_pmu_create_perf_event(vcpu, select_idx); >> } >> >> +static int kvm_pmu_probe_pmuver(void) >> +{ >> + struct perf_event_attr attr = { }; >> + struct perf_event *event; >> + struct arm_pmu *pmu; >> + int pmuver = 0xf; >> + >> + /* >> + * Create a dummy event that only counts user cycles. As we'll never >> + * leave thing function with the event being live, it will never >> + * count anything. But it allows us to probe some of the PMU >> + * details. Yes, this is terrible. > I fail to understand why we can't directly read ID_DFR0_EL1.PerfMon? Because you're missing the big-little nightmare. What you read on the current CPU is irrelevant. You want to read the PMU version on the PMU that is going to actually be used (and that's the whatever perf decides to use at this point). That's also the reason why PMU in guests only work in BL systems on one class of CPUs only. Yes, all CPUs should have the same PMU version. Unfortunately, that's not always the case... M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D3C7C433E2 for ; Wed, 9 Sep 2020 09:56:27 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2C220207DE for ; Wed, 9 Sep 2020 09:56:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="hm8/m07s"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="LDTmZ78r" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2C220207DE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=j9CDzJI64uqQpogjK8QQGxHFbknFszEZRXqsrkXRKLc=; b=hm8/m07sc+EFPus4qRjujXWv4 nglC0bIMseK1IFOV9VZxKvxFOmkqrDC0Abd1d/I9P5JwWw0v2UNuZuGRxPgXoqEUUVEWza9TWgN/G WPBzWnHkEQOIJO7Ic6nO4o5pxTNIn+x699RInzoTHQzJO8rhEVyd0mHBM5BUofWqLhVhiUB55USM1 tc/fEVVXR6MrCUKaG1vPAYROGfBqjzGcz0eLhpy3+I6CUqj+tcMw6eRtg6e9LtIPa55mu6JecKAlG QqLIi3UeRP9zeh0mZ64yNgo6PzpqDzfPZ9ujza01S7qLUU5x4zRZIxSFOYMSfhkGSLV+VGOsSyLWZ WinRoQHOA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFwof-0003Vj-CK; Wed, 09 Sep 2020 09:55:01 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFwoc-0003UW-OU for linux-arm-kernel@lists.infradead.org; Wed, 09 Sep 2020 09:54:59 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BE4D521D79; Wed, 9 Sep 2020 09:54:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599645297; bh=yb9q5YlJL0pBVdVI2oIUvHjVDi83Gtgj3rffl2bbaxA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=LDTmZ78rZ1yG/Y8OwIpmSRb94pFseP9TOpBYvzNIyscYtMpC6ScR8LrCy2joHzDV+ IGBS15W3OSXXICYGDaCzXQitUNEtIzUXUyCL0oly8jHtxPd8NSMwYZ7J//sYfyKSWi ZJpwQjn/6XU86VJTVzlP95Yge8EhPbyURhNUBNkk= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1kFwoa-00AKzZ-1L; Wed, 09 Sep 2020 10:54:56 +0100 MIME-Version: 1.0 Date: Wed, 09 Sep 2020 10:54:55 +0100 From: Marc Zyngier To: Auger Eric Subject: Re: [PATCH v3 2/5] KVM: arm64: Use event mask matching architecture revision In-Reply-To: <2e1257aa-9e8c-89ba-c09b-3cfee38c8486@redhat.com> References: <20200908075830.1161921-1-maz@kernel.org> <20200908075830.1161921-3-maz@kernel.org> <2e1257aa-9e8c-89ba-c09b-3cfee38c8486@redhat.com> User-Agent: Roundcube Webmail/1.4.8 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: eric.auger@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, robin.murphy@arm.com, mark.rutland@arm.com, graf@amazon.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200909_055458_913831_CA63B592 X-CRM114-Status: GOOD ( 26.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , kvm@vger.kernel.org, Suzuki K Poulose , kernel-team@android.com, James Morse , linux-arm-kernel@lists.infradead.org, graf@amazon.com, Robin Murphy , kvmarm@lists.cs.columbia.edu, Julien Thierry Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Eric, On 2020-09-09 10:38, Auger Eric wrote: > Hi Marc, > > On 9/8/20 9:58 AM, Marc Zyngier wrote: >> The PMU code suffers from a small defect where we assume that the >> event >> number provided by the guest is always 16 bit wide, even if the CPU >> only >> implements the ARMv8.0 architecture. This isn't really problematic in >> the sense that the event number ends up in a system register, cropping >> it to the right width, but still this needs fixing. >> >> In order to make it work, let's probe the version of the PMU that the >> guest is going to use. This is done by temporarily creating a kernel >> event and looking at the PMUVer field that has been saved at probe >> time >> in the associated arm_pmu structure. This in turn gets saved in the >> kvm >> structure, and subsequently used to compute the event mask that gets >> used throughout the PMU code. >> >> Signed-off-by: Marc Zyngier >> --- >> arch/arm64/include/asm/kvm_host.h | 2 + >> arch/arm64/kvm/pmu-emul.c | 81 >> +++++++++++++++++++++++++++++-- >> 2 files changed, 78 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_host.h >> b/arch/arm64/include/asm/kvm_host.h >> index 65568b23868a..6cd60be69c28 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -110,6 +110,8 @@ struct kvm_arch { >> * supported. >> */ >> bool return_nisv_io_abort_to_user; >> + >> + unsigned int pmuver; >> }; >> >> struct kvm_vcpu_fault_info { >> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c >> index 93d797df42c6..8a5f65763814 100644 >> --- a/arch/arm64/kvm/pmu-emul.c >> +++ b/arch/arm64/kvm/pmu-emul.c >> @@ -20,6 +20,20 @@ static void kvm_pmu_stop_counter(struct kvm_vcpu >> *vcpu, struct kvm_pmc *pmc); >> >> #define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1 >> >> +static u32 kvm_pmu_event_mask(struct kvm *kvm) >> +{ >> + switch (kvm->arch.pmuver) { >> + case 1: /* ARMv8.0 */ >> + return GENMASK(9, 0); >> + case 4: /* ARMv8.1 */ >> + case 5: /* ARMv8.4 */ >> + case 6: /* ARMv8.5 */ >> + return GENMASK(15, 0); >> + default: /* Shouldn't be there, just for sanity */ >> + return 0; >> + } >> +} >> + >> /** >> * kvm_pmu_idx_is_64bit - determine if select_idx is a 64bit counter >> * @vcpu: The vcpu pointer >> @@ -100,7 +114,7 @@ static bool kvm_pmu_idx_has_chain_evtype(struct >> kvm_vcpu *vcpu, u64 select_idx) >> return false; >> >> reg = PMEVTYPER0_EL0 + select_idx; >> - eventsel = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_EVENT; >> + eventsel = __vcpu_sys_reg(vcpu, reg) & >> kvm_pmu_event_mask(vcpu->kvm); >> >> return eventsel == ARMV8_PMUV3_PERFCTR_CHAIN; >> } >> @@ -495,7 +509,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu >> *vcpu, u64 val) >> >> /* PMSWINC only applies to ... SW_INC! */ >> type = __vcpu_sys_reg(vcpu, PMEVTYPER0_EL0 + i); >> - type &= ARMV8_PMU_EVTYPE_EVENT; >> + type &= kvm_pmu_event_mask(vcpu->kvm); >> if (type != ARMV8_PMUV3_PERFCTR_SW_INCR) >> continue; >> >> @@ -578,7 +592,7 @@ static void kvm_pmu_create_perf_event(struct >> kvm_vcpu *vcpu, u64 select_idx) >> data = __vcpu_sys_reg(vcpu, reg); >> >> kvm_pmu_stop_counter(vcpu, pmc); >> - eventsel = data & ARMV8_PMU_EVTYPE_EVENT; >> + eventsel = data & kvm_pmu_event_mask(vcpu->kvm);; >> >> /* Software increment event does't need to be backed by a perf event >> */ >> if (eventsel == ARMV8_PMUV3_PERFCTR_SW_INCR && >> @@ -679,17 +693,68 @@ static void kvm_pmu_update_pmc_chained(struct >> kvm_vcpu *vcpu, u64 select_idx) >> void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data, >> u64 select_idx) >> { >> - u64 reg, event_type = data & ARMV8_PMU_EVTYPE_MASK; >> + u64 reg, mask; >> + >> + mask = ARMV8_PMU_EVTYPE_MASK; >> + mask &= ~ARMV8_PMU_EVTYPE_EVENT; >> + mask |= kvm_pmu_event_mask(vcpu->kvm); >> >> reg = (select_idx == ARMV8_PMU_CYCLE_IDX) >> ? PMCCFILTR_EL0 : PMEVTYPER0_EL0 + select_idx; >> >> - __vcpu_sys_reg(vcpu, reg) = event_type; >> + __vcpu_sys_reg(vcpu, reg) = data & mask; >> >> kvm_pmu_update_pmc_chained(vcpu, select_idx); >> kvm_pmu_create_perf_event(vcpu, select_idx); >> } >> >> +static int kvm_pmu_probe_pmuver(void) >> +{ >> + struct perf_event_attr attr = { }; >> + struct perf_event *event; >> + struct arm_pmu *pmu; >> + int pmuver = 0xf; >> + >> + /* >> + * Create a dummy event that only counts user cycles. As we'll never >> + * leave thing function with the event being live, it will never >> + * count anything. But it allows us to probe some of the PMU >> + * details. Yes, this is terrible. > I fail to understand why we can't directly read ID_DFR0_EL1.PerfMon? Because you're missing the big-little nightmare. What you read on the current CPU is irrelevant. You want to read the PMU version on the PMU that is going to actually be used (and that's the whatever perf decides to use at this point). That's also the reason why PMU in guests only work in BL systems on one class of CPUs only. Yes, all CPUs should have the same PMU version. Unfortunately, that's not always the case... M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CC3AAC433E2 for ; Wed, 9 Sep 2020 09:55:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8E66E21D79 for ; Wed, 9 Sep 2020 09:55:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599645301; bh=yb9q5YlJL0pBVdVI2oIUvHjVDi83Gtgj3rffl2bbaxA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=sjGDtwlrTnKPCYf950+1hdQ6NfiPy6YSFvgidCHuVbmTRw+or5lojkb6pk9Dq8f+P y5QSRK5fn2rze/+kvl6kWb3p64Xs9RefjXVIedEe9YVSvCdfI+U3jYToDrnyPXE0pl zIz1faLWmIahxFVUVlInBWkcfll9brGqx0go9bDc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728197AbgIIJzA (ORCPT ); Wed, 9 Sep 2020 05:55:00 -0400 Received: from mail.kernel.org ([198.145.29.99]:59468 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726293AbgIIJy6 (ORCPT ); Wed, 9 Sep 2020 05:54:58 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BE4D521D79; Wed, 9 Sep 2020 09:54:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1599645297; bh=yb9q5YlJL0pBVdVI2oIUvHjVDi83Gtgj3rffl2bbaxA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=LDTmZ78rZ1yG/Y8OwIpmSRb94pFseP9TOpBYvzNIyscYtMpC6ScR8LrCy2joHzDV+ IGBS15W3OSXXICYGDaCzXQitUNEtIzUXUyCL0oly8jHtxPd8NSMwYZ7J//sYfyKSWi ZJpwQjn/6XU86VJTVzlP95Yge8EhPbyURhNUBNkk= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1kFwoa-00AKzZ-1L; Wed, 09 Sep 2020 10:54:56 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 09 Sep 2020 10:54:55 +0100 From: Marc Zyngier To: Auger Eric Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, James Morse , Julien Thierry , Suzuki K Poulose , Robin Murphy , Mark Rutland , graf@amazon.com, kernel-team@android.com Subject: Re: [PATCH v3 2/5] KVM: arm64: Use event mask matching architecture revision In-Reply-To: <2e1257aa-9e8c-89ba-c09b-3cfee38c8486@redhat.com> References: <20200908075830.1161921-1-maz@kernel.org> <20200908075830.1161921-3-maz@kernel.org> <2e1257aa-9e8c-89ba-c09b-3cfee38c8486@redhat.com> User-Agent: Roundcube Webmail/1.4.8 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: eric.auger@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, robin.murphy@arm.com, mark.rutland@arm.com, graf@amazon.com, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Eric, On 2020-09-09 10:38, Auger Eric wrote: > Hi Marc, > > On 9/8/20 9:58 AM, Marc Zyngier wrote: >> The PMU code suffers from a small defect where we assume that the >> event >> number provided by the guest is always 16 bit wide, even if the CPU >> only >> implements the ARMv8.0 architecture. This isn't really problematic in >> the sense that the event number ends up in a system register, cropping >> it to the right width, but still this needs fixing. >> >> In order to make it work, let's probe the version of the PMU that the >> guest is going to use. This is done by temporarily creating a kernel >> event and looking at the PMUVer field that has been saved at probe >> time >> in the associated arm_pmu structure. This in turn gets saved in the >> kvm >> structure, and subsequently used to compute the event mask that gets >> used throughout the PMU code. >> >> Signed-off-by: Marc Zyngier >> --- >> arch/arm64/include/asm/kvm_host.h | 2 + >> arch/arm64/kvm/pmu-emul.c | 81 >> +++++++++++++++++++++++++++++-- >> 2 files changed, 78 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_host.h >> b/arch/arm64/include/asm/kvm_host.h >> index 65568b23868a..6cd60be69c28 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -110,6 +110,8 @@ struct kvm_arch { >> * supported. >> */ >> bool return_nisv_io_abort_to_user; >> + >> + unsigned int pmuver; >> }; >> >> struct kvm_vcpu_fault_info { >> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c >> index 93d797df42c6..8a5f65763814 100644 >> --- a/arch/arm64/kvm/pmu-emul.c >> +++ b/arch/arm64/kvm/pmu-emul.c >> @@ -20,6 +20,20 @@ static void kvm_pmu_stop_counter(struct kvm_vcpu >> *vcpu, struct kvm_pmc *pmc); >> >> #define PERF_ATTR_CFG1_KVM_PMU_CHAINED 0x1 >> >> +static u32 kvm_pmu_event_mask(struct kvm *kvm) >> +{ >> + switch (kvm->arch.pmuver) { >> + case 1: /* ARMv8.0 */ >> + return GENMASK(9, 0); >> + case 4: /* ARMv8.1 */ >> + case 5: /* ARMv8.4 */ >> + case 6: /* ARMv8.5 */ >> + return GENMASK(15, 0); >> + default: /* Shouldn't be there, just for sanity */ >> + return 0; >> + } >> +} >> + >> /** >> * kvm_pmu_idx_is_64bit - determine if select_idx is a 64bit counter >> * @vcpu: The vcpu pointer >> @@ -100,7 +114,7 @@ static bool kvm_pmu_idx_has_chain_evtype(struct >> kvm_vcpu *vcpu, u64 select_idx) >> return false; >> >> reg = PMEVTYPER0_EL0 + select_idx; >> - eventsel = __vcpu_sys_reg(vcpu, reg) & ARMV8_PMU_EVTYPE_EVENT; >> + eventsel = __vcpu_sys_reg(vcpu, reg) & >> kvm_pmu_event_mask(vcpu->kvm); >> >> return eventsel == ARMV8_PMUV3_PERFCTR_CHAIN; >> } >> @@ -495,7 +509,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu >> *vcpu, u64 val) >> >> /* PMSWINC only applies to ... SW_INC! */ >> type = __vcpu_sys_reg(vcpu, PMEVTYPER0_EL0 + i); >> - type &= ARMV8_PMU_EVTYPE_EVENT; >> + type &= kvm_pmu_event_mask(vcpu->kvm); >> if (type != ARMV8_PMUV3_PERFCTR_SW_INCR) >> continue; >> >> @@ -578,7 +592,7 @@ static void kvm_pmu_create_perf_event(struct >> kvm_vcpu *vcpu, u64 select_idx) >> data = __vcpu_sys_reg(vcpu, reg); >> >> kvm_pmu_stop_counter(vcpu, pmc); >> - eventsel = data & ARMV8_PMU_EVTYPE_EVENT; >> + eventsel = data & kvm_pmu_event_mask(vcpu->kvm);; >> >> /* Software increment event does't need to be backed by a perf event >> */ >> if (eventsel == ARMV8_PMUV3_PERFCTR_SW_INCR && >> @@ -679,17 +693,68 @@ static void kvm_pmu_update_pmc_chained(struct >> kvm_vcpu *vcpu, u64 select_idx) >> void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data, >> u64 select_idx) >> { >> - u64 reg, event_type = data & ARMV8_PMU_EVTYPE_MASK; >> + u64 reg, mask; >> + >> + mask = ARMV8_PMU_EVTYPE_MASK; >> + mask &= ~ARMV8_PMU_EVTYPE_EVENT; >> + mask |= kvm_pmu_event_mask(vcpu->kvm); >> >> reg = (select_idx == ARMV8_PMU_CYCLE_IDX) >> ? PMCCFILTR_EL0 : PMEVTYPER0_EL0 + select_idx; >> >> - __vcpu_sys_reg(vcpu, reg) = event_type; >> + __vcpu_sys_reg(vcpu, reg) = data & mask; >> >> kvm_pmu_update_pmc_chained(vcpu, select_idx); >> kvm_pmu_create_perf_event(vcpu, select_idx); >> } >> >> +static int kvm_pmu_probe_pmuver(void) >> +{ >> + struct perf_event_attr attr = { }; >> + struct perf_event *event; >> + struct arm_pmu *pmu; >> + int pmuver = 0xf; >> + >> + /* >> + * Create a dummy event that only counts user cycles. As we'll never >> + * leave thing function with the event being live, it will never >> + * count anything. But it allows us to probe some of the PMU >> + * details. Yes, this is terrible. > I fail to understand why we can't directly read ID_DFR0_EL1.PerfMon? Because you're missing the big-little nightmare. What you read on the current CPU is irrelevant. You want to read the PMU version on the PMU that is going to actually be used (and that's the whatever perf decides to use at this point). That's also the reason why PMU in guests only work in BL systems on one class of CPUs only. Yes, all CPUs should have the same PMU version. Unfortunately, that's not always the case... M. -- Jazz is not dead. It just smells funny...