From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AB6A621106 for ; Mon, 28 Oct 2024 17:55:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730138152; cv=none; b=DoSQ0NCh4o1wSQt1Orxonfu2wywjOyfRfDNw4/H2Xo5sDF3z4yEJGhjU0fGJVdsd26cuPkPVOXUyC2cjvO0ZR76ZJWDsZxPUphuzCTMKpIHZ6GW7kHZzn51ME2//FebS38yTKQu2pmFZJn8n2yCnKeKF0YuKQxiz5b4vEdc8ldI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730138152; c=relaxed/simple; bh=rRpF3iwXmieYU7/4JfV8CgzmrhbxJaCee8QQYCWdaU4=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=JG+stG/pFOBGOju/dCvISPOfS3jrNJvNwTLbUjHd9pbIH1fo9J8RMal7Ia3lw5J6VUBBrmfdNTOaamKfCxPd2tOQmZEkdDc3Nn68r/n/put2WFZk4URtM4+17Ies4z9B8fyQrFx1O2qZ0LRsY3oTsmIbl179xCzjTpKtG1R83g8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=E68YPmt1; arc=none smtp.client-ip=209.85.219.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="E68YPmt1" Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-e1159159528so7875561276.1 for ; Mon, 28 Oct 2024 10:55:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730138150; x=1730742950; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=FrqO78IIQ3uzDC61CyWRYSNvlD33ikL6CxGnPLAkt6o=; b=E68YPmt1rkxkQAzY4fcpeSsqKZBCHWUIReGBJaT0/dKTC4qYY/emto5aqw/fEhp6ys q7oU5D/dzuaIWWLHUTMZVh1HnEJ0EeUIUG6jZ2Ln0LPGZP+RgNxm4HKRSv0faXj4N5qj gFtERnuhRNnFCpAc52MiYCJJi5jd94X5qruxYtf+NqWNFN5it1e2DCb76pPLFSx3cE7H IWWKLQkjqLycXgBIGL+ovi3hSNPBigihCBlIA08YUqR6jd0p3TU/2jHcpvDO5OGSBLnb eiGOc4vzZbzhNU6C3QqTPOoGks4hXc44SSrRVUU5iEJC3nlNgt1UskzeF45VZ50VwYTT RL6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730138150; x=1730742950; 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=FrqO78IIQ3uzDC61CyWRYSNvlD33ikL6CxGnPLAkt6o=; b=eW4Br4HIwQ7lcSc0i8DXJR6bFvAkYBXGQfxpLWlxGTo3xbolqRJucktgOzh8F+khgX fzAHFvMW0a4tO5DJViJU39oM4d7F8ebs6FtdU2M9SwI4bXijZ2HjMebCNAyArCd2eMRF qiyT5ztGPkQdZqFcd4/S7A5A7sbtShmD6g7nqS/Atsx7Z4IiJQgQbkD9IiCwq5XC5/Bg ACR6NJJ0uZiH1gAcf5p2X2AphNpZ/w80cch8AYqaGx+v7JGa88il3k6VW8l2AaFlHFSy d2+B0TA0H9GxuSnDSVulh4XvM1ekJ3KobxPJtA0rZQSn+1/UOpzm3nCC7Y83+uqun484 dzkA== X-Forwarded-Encrypted: i=1; AJvYcCWjWc3o6XLFtoltpGDVIm71QezvUx+8Z4eiSVFbxbGY0BIfdDSoH5XR0mfAmAuRnKo+YhSrHippYtR8Pfo=@vger.kernel.org X-Gm-Message-State: AOJu0YwP5Gpk0M0gLbUteKaFJDQgaUziGidV2c6aacvI6P/gZVA+eSeh s+hpBKTWZuQLWfVLkTGX1evOGRAcWz8I2wIkWon2TStH45tqzJW99OEsGFkd4Aj0q6DkUBeIWPQ Isw== X-Google-Smtp-Source: AGHT+IHIYGRKmxvk5oPu2A0suF66uGk8H4k5Ozqbh+vJ6wiElZtGw7G9xTqE/r5KkgQjLE8DDx5d6KFo/zU= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:9d:3983:ac13:c240]) (user=seanjc job=sendgmr) by 2002:a25:2b05:0:b0:e29:ad0:a326 with SMTP id 3f1490d57ef6-e30bc242b68mr6490276.0.1730138149734; Mon, 28 Oct 2024 10:55:49 -0700 (PDT) Date: Mon, 28 Oct 2024 10:55:48 -0700 In-Reply-To: <4984cba7-427a-4065-9fcc-97b9f67163ed@amd.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20240905124107.6954-1-pratikrajesh.sampat@amd.com> <20240905124107.6954-3-pratikrajesh.sampat@amd.com> <4984cba7-427a-4065-9fcc-97b9f67163ed@amd.com> Message-ID: Subject: Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test From: Sean Christopherson To: "Pratik R. Sampat" Cc: kvm@vger.kernel.org, pbonzini@redhat.com, pgonda@google.com, thomas.lendacky@amd.com, michael.roth@amd.com, shuah@kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Mon, Oct 21, 2024, Pratik R. Sampat wrote: > >> + test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES); > >> > >> test_sev_es_shutdown(); > >> > >> if (kvm_has_cap(KVM_CAP_XCRS) && > >> (xgetbv(0) & XFEATURE_MASK_X87_AVX) == XFEATURE_MASK_X87_AVX) { > >> - test_sync_vmsa(0); > >> - test_sync_vmsa(SEV_POLICY_NO_DBG); > >> + test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES); > >> + test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES | SEV_POLICY_NO_DBG); > >> + } > >> + } > >> + > >> + if (kvm_cpu_has(X86_FEATURE_SNP) && is_kvm_snp_supported()) { > > > > Why do we need both? KVM shouldn't advertise SNP if it's not supported. > > My rationale behind needing this was that the feature can be advertised > but it may not have the right API major or minor release which could be > updated post boot and could determine it's support during runtime. KVM will never determine support after KVM has been loaded. If *KVM* has a dependency on the API major.minor, then X86_FEATURE_SNP must be set if and only if the supported API version is available. If the API major.minor is purely a userspace thing, then is_kvm_snp_supported() is misnamed, because the check has nothing to do with KVM. E.g. something like is_snp_api_version_supported() would be more appropriate. > >> + unsigned long snp_policy = SNP_POLICY; > > > > u64, no? > > Yes, sorry for the oversight. Will change it to u64. > > > > >> + > >> + if (unlikely(!is_smt_active())) > >> + snp_policy &= ~SNP_POLICY_SMT; > > > > Why does SNP_POLICY assume SMT? And what is RSVD_MBO? E.g. why not this? > > > > u64 policy = is_smt_active() ? SNP_POLICY_SMT : SNP_POLICY; > > > > I think most systems support SMT so I enabled the bit in by default and > only unset it when there isn't any support. That's confusing though, because you're mixing architectural defines with semi- arbitrary selftests behavior. RSVD_MBO on the other is apparently tightly coupled with SNP, i.e. SNP can't exist without that bit, so it makes sense that RSVD_MBO needs to be part of SNP_POLICY If you want to have a *software*-defined default policy, then make it obvious that it's software defined. E.g. name the #define SNP_DEFAULT_POLICY, not simply SNP_POLICY, because the latter is too easily misconstrued as the base SNP policy, which it is not. That said, IIUC, SMT *must* match the host configuration, i.e. whether or not SMT is set is non-negotiable. In that case, there's zero value in defining SNP_DEFAULT_POLICY, because it can't be a sane default for all systems. Side topic, I assume one of SEV_POLICY_NO_DBG or SNP_POLICY_DBG *must* be specified, and that they are mutualy exclusive? E.g. what happens if the full policy is simply SNP_POLICY_RSVD_MBO?