On 2/18/25 6:54 PM, Sean Christopherson wrote: > On Fri, Feb 14, 2025, Pratik Rajesh Sampat wrote: >> >> >> On 2/11/25 8:31 PM, Sean Christopherson wrote: >>> On Mon, Feb 03, 2025, Pratik R. Sampat wrote: >>>> @@ -217,5 +244,20 @@ int main(int argc, char *argv[]) >>>> } >>>> } >>>> >>>> + if (kvm_cpu_has(X86_FEATURE_SEV_SNP)) { >>>> + uint64_t snp_policy = snp_default_policy(); >>>> + >>>> + test_snp(snp_policy); >>>> + /* Test minimum firmware level */ >>>> + test_snp(snp_policy | SNP_FW_VER_MAJOR(SNP_MIN_API_MAJOR) | >>>> + SNP_FW_VER_MINOR(SNP_MIN_API_MINOR)); >>> >>> Ah, this is where the firmware policy stuff is used. Refresh me, can userspace >>> request _any_ major/minor as the min, and expect failure if the version isn't >>> supported? If so, the test should iterate over the major/minor combinations that >>> are guaranteed to fail. And if userspace can query the supported minor/major, >>> the test should iterate over all the happy versions too. >>> >> >> Yes, any policy greater than the min policy (defined in sev-dev.c) >> should be supported. The sad path tests were intended to be added in the >> upcoming negative test patch series so that we could have the proper >> infrastructure to handle and report failures. >> >>> Unless there's nothing interesting to test, I would move the major/minor stuff to >>> a separate patch. >> >> Would you rather prefer I do the happy tests here (something like - >> min_policy and min_policy + 1?) and defer the failure tests for the >> next patchset? Or, I can remove policy testing from here entirely and >> introduce it only when the sad path testing infrastructure is ready, so >> that we can test this completely at once? > > Let's do the latter. For the initial series, do the bare minimum so that we can > get that merged, and then focus on the min API version stuff in a separate series. > The version testing shouldn't be terribly complex, but it doesn't seem like it's > entirely trivial either, and I don't want it to block the base SNP support. Sure thing, will do. Thanks!