On Wed, Dec 13, 2023 at 06:35:35PM +0100, Paolo Bonzini wrote: > 1) patch 4 should have unconditionally cleared the feature (until the > initialization code comes around in patch 6); and it should have mentioned > in the commit message that we don't want X86_FEATURE_SEV_SNP to be set, > unless SNP can be enabled via MSR_AMD64_SYSCFG. I guess. > 2) possibly, the commit message of patch 5 could have said something like > "at this point in the kernel SNP is never enabled". Sure. > 3) Patch 23 should have been placed before the SNP initialization, because > as things stand the patches (mildly) break bisectability. Ok, I still haven't reached that one. > Understood now. With the patch ordering and commit message edits I > suggested above, indeed I would not have picked up patch 4. In the future, please simply refrain from picking up x86 patches if you haven't gotten an explicit ACK. We could have conflicting changes in tip, we could be reworking that part of the code and the thing the patch touches could be completely gone, and so on and so on... Also, we want to have a full picture of what goes in. Exactly the same as how you'd like to have a full picture of what goes into kvm and how we don't apply kvm patches unless there's some extraordinary circumstance or we have received an explicit ACK. > But with your explanation, I would even say that "4/50 needs to go > together with the rest" *for correctness*, not just to mean something. Yes, but for ease of integration it would be easier if they go in two groups - kvm and x86 bits. Not: some x86 bits first, then kvm bits through your tree and then some more x86 bits. That would be a logistical nightmare. And even if you bisect and land at 4/50 and you disable AIBRS even without SNP being really enabled, that is not a big deal - you're only bisecting and not really using that kernel and it's not like it breaks builds so... Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette