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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2828C77B61 for ; Fri, 7 Apr 2023 14:45:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232199AbjDGOpC (ORCPT ); Fri, 7 Apr 2023 10:45:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55384 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230135AbjDGOpB (ORCPT ); Fri, 7 Apr 2023 10:45:01 -0400 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CB5F2E53 for ; Fri, 7 Apr 2023 07:44:59 -0700 (PDT) Received: by mail-pl1-x64a.google.com with SMTP id w14-20020a170902e88e00b001a238a5946cso21864032plg.11 for ; Fri, 07 Apr 2023 07:44:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680878699; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=gv6eZEPRoHwC1V+eZxjkjvEEaDAR56CuZbvvIwjon/E=; b=c8GStGHENBEjP8bjbhams8fpLgZjY6HZfrxyFX8rt4+dlyOm7p5rb+XBePUnNCEStr w8+WfuSn5dO+NiRbo+ChOabDyviGYrIB5i1ispRBIWO0yNrnrdal5Je2h+cMO++AAqEf EgZITFFY1nHa31im5YEUwTJvnSrsvN0dwuPe6EHBJGzJW69Bnat8FjN9lwwjZG4CAeSp LkVksEgvek5ysiT8+Vcf4VSn5duRN9qbl3UPTbsX9t/35sy4ytjfMtH3aodrpkRFH705 Fhp9U6jAWnB7m6FdbuKWZIF9AJRIfvWtw+Z93qCgQfZogC3ZcwO+S6mbzmWdtHT/4xtw 6g/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680878699; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gv6eZEPRoHwC1V+eZxjkjvEEaDAR56CuZbvvIwjon/E=; b=r3yosc3KSUa+XZV0+q5m0QWxUvKCYlwiA47ZJRr1WDFZZ9/njDY6ym5Qu82eT1pwjJ KhuQmToLf3Rtgc042TMxd9Wb8Tf2h+ZaV6M06mjSQGJL3X4IFmXJeBFE1JOarDlTpGUU KeTdxE3W552kD9HBuBmG9xuTXmXHArBWKA81swkE7Sjs/mI3X/8WWSSGDRJolTyzSkrR /anWH4FEqznrRl2aatY5WOYNdX5FxZJwQ9Wp6OW6ncrUP9tQ+on2YCo5FLVDFACKN9d2 UwKPeQpbX/nbeHexhSelpAe74oe9oG4dZzUI4V2yADjtLVMUX406uh6L6K49lTe1Os2y AibQ== X-Gm-Message-State: AAQBX9fT3pc9JdrWpvXQ2tlalaINAVXZCL4IF38IIGdLNljUnuYR/bze 3D0jtn667HgwMWeFXu3uuOZnrMJTWZ0= X-Google-Smtp-Source: AKy350YSwRq3OADo5G/ym7vokRAHIB96wcOcT3eqFqDZlY6eakqG7MNhVkZRGOQPI2dgFHK63yGf+NomaYs= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:da8a:b0:1a2:8fa0:fbd7 with SMTP id j10-20020a170902da8a00b001a28fa0fbd7mr3954205plx.2.1680878699355; Fri, 07 Apr 2023 07:44:59 -0700 (PDT) Date: Fri, 7 Apr 2023 07:44:57 -0700 In-Reply-To: Mime-Version: 1.0 References: <20230214050757.9623-1-likexu@tencent.com> <20230214050757.9623-12-likexu@tencent.com> Message-ID: Subject: Re: [PATCH v4 11/12] KVM: x86/svm/pmu: Add AMD PerfMonV2 support From: Sean Christopherson To: Like Xu Cc: Paolo Bonzini , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Sandipan Das Content-Type: text/plain; charset="us-ascii" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 07, 2023, Like Xu wrote: > On 7/4/2023 9:35 am, Sean Christopherson wrote: > > On Tue, Feb 14, 2023, Like Xu wrote: > > > + case MSR_AMD64_PERF_CNTR_GLOBAL_STATUS: > > > + if (!msr_info->host_initiated) > > > + return 0; /* Writes are ignored */ > > > > Where is the "writes ignored" behavior documented? I can't find anything in the > > APM that defines write behavior. > > KVM would follow the real hardware behavior once specifications stay silent > on details or secret. So is that a "this isn't actually documented anywhere" answer? It's not your responsibility to get AMD to document their CPUs, but I want to clearly document when KVM's behavior is based solely off of observed hardware behavior, versus an actual specification. > How about this: > > /* > * Note, AMD ignores writes to reserved bits and read-only PMU MSRs, > * whereas Intel generates #GP on attempts to write reserved/RO MSRs. > */ Looks good. > > > + pmu->nr_arch_gp_counters = min_t(unsigned int, > > > + ebx.split.num_core_pmc, > > > + kvm_pmu_cap.num_counters_gp); > > > + } else if (guest_cpuid_has(vcpu, X86_FEATURE_PERFCTR_CORE)) { > > > pmu->nr_arch_gp_counters = AMD64_NUM_COUNTERS_CORE; > > > > This needs to be sanitized, no? E.g. if KVM only has access to 4 counters, but > > userspace sets X86_FEATURE_PERFCTR_CORE anyways. Hrm, unless I'm missing something, > > that's a pre-existing bug. > > Now your point is that if a user space more capbility than KVM can support, > KVM should constrain it. > Your previous preference was that the user space can set capbilities that > evene if KVM doesn't support as long as it doesn't break KVM and host and the > guest will eat its own. Letting userspace define a "bad" configuration is perfectly ok, but KVM needs to be careful not to endanger itself by consuming the bad state. A good example is the handling of nested SVM features in svm_vcpu_after_set_cpuid(). KVM lets userspace define anything and everything, but KVM only actually tries to utilize a feature if the feature is actually supported in hardware. In this case, it's not clear to me that putting a bogus value into "nr_arch_gp_counters" is safe (for KVM). And AIUI, the guest can't actually use more than kvm_pmu_cap.num_counters_gp counters, i.e. KVM isn't arbitrarily restricting the setup.