Received: by 2002:a9a:4c47:0:b029:116:c383:538 with SMTP id u7csp903626lko; Tue, 13 Jul 2021 12:36:20 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzeDX6bSntNT41/hmewz1gxx/TAsA7SNvTumn9RvP0iPAeLoSHnYtjaw+86gIweiaeq8eJ0 X-Received: by 2002:a05:6602:2204:: with SMTP id n4mr4403920ion.181.1626204980211; Tue, 13 Jul 2021 12:36:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1626204980; cv=none; d=google.com; s=arc-20160816; b=bNLXuqfzhH8Ri53gwQX4J9vEr4bw2aMM0pPA0N0H+CvqHeHfIVmIyNcwOJOWBeY4zh b4i3fNggt62EiUfEBUyWE+KAen81wSZ3cNiBXGtslUGFryuhtroJJSPEewFEzs8ZYlpU 2irT2MgXZG3xxyeMNl/LHcpYZ/PDar2x1cQQKOYhJmjYVcmvxm1/Dt/oV82qoae96dHZ WrzO8BvYLn9+4wu3Ami/TY+bdTXgsi3FAN7d5xyoF2sZlIael02lExjIRWSUsvtCl1Ek r/UZJBf4XmppCV/ab/auHJ7NVVnowF2d2/vrzq/A7vpVX5ynUKCegqbAAMDb4myO5JIy 34ZQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=N0sTfKKyE0+JAut7zXsvgnlkS/bdeOAiaAnE0HjBo3Q=; b=m+L+FK3RZkDM0K7/uruQ+8b4/fMhqVqbSKzy/yBs+7gXmPcDSRI80Veltk/dRl78sz 4vzOImTHXikkcsdGqep4qvb8tvuhkH6Fph6FH2Xpuf81QpEYSKAw1yfi1DYcRENv+uCt a5z9K3DqYFZtasWgy4jpAvaZaXx5unLxQMne6PLawRGfDjYNKgztpiE4Yu9JTyRdFFTe 4+qp6Jq/I8bZm166xLMv8sbSKAVWEU4zV5Z3eQ/M6kW9K+o0ukR/ywhgiO1E5qiAcrkR 0fYTdgFmH/K3g4Gncjm4q+QD/sBx2oN5M3PtJiG4ByPOBDKjcuH/DgB5I5Hxw4UtkgJG wZKA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=ZrqS9e5z; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id d6si17766034jak.41.2021.07.13.12.36.08; Tue, 13 Jul 2021 12:36:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=ZrqS9e5z; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234766AbhGMTgM (ORCPT + 99 others); Tue, 13 Jul 2021 15:36:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57580 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234411AbhGMTgM (ORCPT ); Tue, 13 Jul 2021 15:36:12 -0400 Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E8019C0613E9 for ; Tue, 13 Jul 2021 12:33:21 -0700 (PDT) Received: by mail-pg1-x52e.google.com with SMTP id s18so22475558pgg.8 for ; Tue, 13 Jul 2021 12:33:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=N0sTfKKyE0+JAut7zXsvgnlkS/bdeOAiaAnE0HjBo3Q=; b=ZrqS9e5z8WA9MdXrss28wbIf+rlilE8R81QX10ZcTKLpeNys78DSMzebQTZX/DM7Xh lfiSWubTeELe9rZW8xKA2MrjfQ0x0wu7FfSgJ4BRj1Die1VytFGSJom20Za/69qEU0Fv EVWyK56AgIja8WE0ruQLKGjKZRgMAfNv1UOer6xr6lpqN6cpZ1qCZt//+EinVl0HHcUW dS7PiZOMZZa09FjJHbtss3N8QV09hDgMAoaOJotOax+wK4Fzu4muecJFhisRmfTLnbQN /+FiYlIbYf5Ev1TrLau5/uwy9UvDAP3PRSzOzj3prZSFenvGLZr61lylf1p0wcyLEY13 Cr2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=N0sTfKKyE0+JAut7zXsvgnlkS/bdeOAiaAnE0HjBo3Q=; b=iljnSZyJhUGK/UHtBUb7Y43qwu8qQxQC8dOLAkMMkFIM9yu/aZp8Q8+wNL2bL7SylR aFffe/9hr5QSXp69dlBI8mT+1BVF+GyA5MSQ+GlsavB2O+59VpZVPofdAMS2yW/xku5L JN0mDg9h4C5jdHcfZeDD5SGjPreydIAeSQn8m9kt/FYDbMfp9u3eSN59pkj2hmCruTgv AVz9iWEkgeayGnLePiX9Em91/SiusfiKhCKbRP+beOupUx05f32E70Of6g+vblgAOlPE I5h20Kc3jSB0t0VvhggQ9RdaR5ZFWpPmDtjOUpqhhW/Zsr7iy04P2NAQe84xQngH5WyF ywBA== X-Gm-Message-State: AOAM531R0DwbwlLrGizhsJdVvY6KQyswzoIJWTL6vWO5YUTQcXYj9us9 BofHlJYnCT1wwmK7zRwoY604OA== X-Received: by 2002:a63:5f11:: with SMTP id t17mr5807364pgb.37.1626204801148; Tue, 13 Jul 2021 12:33:21 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id r10sm20278553pff.7.2021.07.13.12.33.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Jul 2021 12:33:20 -0700 (PDT) Date: Tue, 13 Jul 2021 19:33:16 +0000 From: Sean Christopherson To: isaku.yamahata@intel.com Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , erdemaktas@google.com, Connor Kuehl , x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, isaku.yamahata@gmail.com Subject: Re: [RFC PATCH v2 08/69] KVM: TDX: add trace point before/after TDX SEAMCALLs Message-ID: References: <28a0ae6b767260fcb410c6ddff7de84f4e13062c.1625186503.git.isaku.yamahata@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28a0ae6b767260fcb410c6ddff7de84f4e13062c.1625186503.git.isaku.yamahata@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 02, 2021, isaku.yamahata@intel.com wrote: > From: Isaku Yamahata > > Signed-off-by: Isaku Yamahata > --- > arch/x86/kvm/trace.h | 80 ++++++++++++++++++++++++++++++ > arch/x86/kvm/vmx/seamcall.h | 22 ++++++++- > arch/x86/kvm/vmx/tdx_arch.h | 47 ++++++++++++++++++ > arch/x86/kvm/vmx/tdx_errno.h | 96 ++++++++++++++++++++++++++++++++++++ > arch/x86/kvm/x86.c | 2 + > 5 files changed, 246 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > index 4f839148948b..c3398d0de9a7 100644 > --- a/arch/x86/kvm/trace.h > +++ b/arch/x86/kvm/trace.h > @@ -8,6 +8,9 @@ > #include > #include > > +#include "vmx/tdx_arch.h" > +#include "vmx/tdx_errno.h" > + > #undef TRACE_SYSTEM > #define TRACE_SYSTEM kvm > > @@ -659,6 +662,83 @@ TRACE_EVENT(kvm_nested_vmexit_inject, > __entry->exit_int_info, __entry->exit_int_info_err) > ); > > +/* > + * Tracepoint for the start of TDX SEAMCALLs. > + */ > +TRACE_EVENT(kvm_tdx_seamcall_enter, To avoid confusion, I think it makes sense to avoid "enter" and "exit". E.g. my first reaction was that the tracepoint was specific to TDENTER. And under the hood, SEAMCALL is technically an exit :-) What about kvm_tdx_seamcall and kvm_tdx_seamret? If the seamret usage is too much of a stretch, kvm_tdx_seamcall_begin/end? > + TP_PROTO(int cpuid, __u64 op, __u64 rcx, __u64 rdx, __u64 r8, > + __u64 r9, __u64 r10), > + TP_ARGS(cpuid, op, rcx, rdx, r8, r9, r10), "cpuid" is potentially confusing without looking at the caller. pcpu or pcpu_id would be preferable. > diff --git a/arch/x86/kvm/vmx/seamcall.h b/arch/x86/kvm/vmx/seamcall.h > index a318940f62ed..2c83ab46eeac 100644 > --- a/arch/x86/kvm/vmx/seamcall.h > +++ b/arch/x86/kvm/vmx/seamcall.h > @@ -9,12 +9,32 @@ > #else > > #ifndef seamcall > +#include "trace.h" > + > struct tdx_ex_ret; > asmlinkage u64 __seamcall(u64 op, u64 rcx, u64 rdx, u64 r8, u64 r9, u64 r10, > struct tdx_ex_ret *ex); > > +static inline u64 _seamcall(u64 op, u64 rcx, u64 rdx, u64 r8, u64 r9, u64 r10, > + struct tdx_ex_ret *ex) > +{ > + u64 err; > + > + trace_kvm_tdx_seamcall_enter(smp_processor_id(), op, > + rcx, rdx, r8, r9, r10); > + err = __seamcall(op, rcx, rdx, r8, r9, r10, ex); What was the motivation behind switching from the macro magic[*] to a dedicated asm subroutine? The macros are gross, but IMO they yielded much more readable code for the upper level helpers, which is what people will look at the vast majority of time. E.g. static inline u64 tdh_sys_lp_shutdown(void) { return seamcall(TDH_SYS_LP_SHUTDOWN, 0, 0, 0, 0, 0, NULL); } static inline u64 tdh_mem_track(hpa_t tdr) { return seamcall(TDH_MEM_TRACK, tdr, 0, 0, 0, 0, NULL); } versus static inline u64 tdsysshutdownlp(void) { seamcall_0(TDSYSSHUTDOWNLP); } static inline u64 tdtrack(hpa_t tdr) { seamcall_1(TDTRACK, tdr); } The new approach also generates very suboptimal code due to the need to shuffle registers everywhere, e.g. gcc doesn't inline _seamcall because it's a whopping 200+ bytes. [*] https://patchwork.kernel.org/project/kvm/patch/25f0d2c2f73c20309a1b578cc5fc15f4fd6b9a13.1605232743.git.isaku.yamahata@intel.com/ > + if (ex) > + trace_kvm_tdx_seamcall_exit(smp_processor_id(), op, err, ex->rcx, smp_processor_id() is not stable since this code runs with IRQs and preemption enabled, e.g. if the task is preempted between the tracepoint and the actual SEAMCALL then the tracepoint may be wrong. There could also be weirdly "nested" tracepoints since migrating the task will generate TDH_VP_FLUSH. > + ex->rdx, ex->r8, ex->r9, ex->r10, > + ex->r11); > + else > + trace_kvm_tdx_seamcall_exit(smp_processor_id(), op, err, > + 0, 0, 0, 0, 0, 0); > + return err; > +} > + > #define seamcall(op, rcx, rdx, r8, r9, r10, ex) \ > - __seamcall(SEAMCALL_##op, (rcx), (rdx), (r8), (r9), (r10), (ex)) > + _seamcall(SEAMCALL_##op, (rcx), (rdx), (r8), (r9), (r10), (ex)) > #endif