Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 17 Nov 2022 13:53:24 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     "Nikunj A. Dadhania" <nikunj@amd.com>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org, kvm@vger.kernel.org,
        mingo@redhat.com, tglx@linutronix.de, dave.hansen@linux.intel.com,
        seanjc@google.com, pbonzini@redhat.com, thomas.lendacky@amd.com,
        michael.roth@amd.com, stable@kernel.org
Subject: Re: [PATCH] x86/sev: Add SEV-SNP guest feature negotiation support
Message-ID: <Y3YuxGO8Kycymxg3@zn.tnic>
References: <20221117044433.244656-1-nikunj@amd.com>
 <Y3YP5FQ6OHzVFKbp@zn.tnic>
 <7e224da6-6b02-2972-1334-bc8e7894cdab@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <7e224da6-6b02-2972-1334-bc8e7894cdab@amd.com>
Precedence: bulk

On Thu, Nov 17, 2022 at 05:50:34PM +0530, Nikunj A. Dadhania wrote:
> Purpose of this patch is older guests kernel that have SNP enabled
> (5.19 onward), when a particular SNP feature is enabled by the
> hypervisor that needs enlightened guest, older kernel wont be able to
> support the feature. There is no mechanism that the hypervisor can
> find out what feature is supported by the SNP guest before hand.
>
> For example PREVENT_HOST_IBS needs changes on hypervisor and no
> changes in the guest kernel. In this any guest kernel having SNP
> support should work.
>
> While for SECURE_TSC, hypervisor and guest kernel changes are
> required. And older guest kernel will not work if hypervisor enables
> Secure TSC. When secure tsc feature is enabled following define should
> be changed:

This all is still veiled in mist to me. What are you trying to do here?

- Make sure older SNP guests boot on newer hypervisors?

- Newer guests boot on older hypervisors?

So, first, pls explain in detail what the goal here is.

I'm reading the above in multiple ways so you need to spell out first
what you wanna do.

PREVENT_HOST_IBS doesn't need any enablement. So why is it in the mask?

SECURE_TSC needs enablement on both. Why aren't you checking only this
one.

IOW, I would expect to check *only* for features which the guest needs
for the hypervisor to support before it boots. But not check everything
wholesale.

IOW, I see it this way: guest boots, sees what the hypervisor has
enabled as SEV_STATUS cannot be intercepted and acts accordingly.

Now, the question how *old* guests should act here is a whole different
story as it depends on whether this gets backported to old guests -
which doesn't make them old anymore as the checking will happen - or to
really old guests without the checking. There it doesn't matter.

And come to think of it, this whole deal is no different than having
feature bits in CPUID and the kernel implementing them.

If the kernel finds a feature bit set in CPUID, it enables the
corresponding code. If it doesn't know about it, then it doesn't do
anything.

Pretty much the same here: if a SNP guest finds a feature flag in
SEV_STATUS, then it enables the code corresponding to it. If it doesn't
find it but it needs it due to enablement, then it stops booting.

So let's define the problem first.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette