Currently CONFIG_HAS_CC_PLATFORM is a prereq for building anything in
arch/x86/coco, but that is generally only applicable for guest support.

For SEV-SNP, helpers related purely to host support will also live in
arch/x86/coco. To allow for CoCo-related host support code in
arch/x86/coco, move that check down into the Makefile and check for it
specifically when needed.

Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 arch/x86/Kbuild        | 2 +-
 arch/x86/coco/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
index 5a83da703e87..1889cef48b58 100644
--- a/arch/x86/Kbuild
+++ b/arch/x86/Kbuild
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += coco/
+obj-y += coco/
 
 obj-y += entry/
 
diff --git a/arch/x86/coco/Makefile b/arch/x86/coco/Makefile
index c816acf78b6a..6aa52e719bf5 100644
--- a/arch/x86/coco/Makefile
+++ b/arch/x86/coco/Makefile
@@ -3,6 +3,6 @@ CFLAGS_REMOVE_core.o	= -pg
 KASAN_SANITIZE_core.o	:= n
 CFLAGS_core.o		+= -fno-stack-protector
 
-obj-y += core.o
+obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += core.o
 
 obj-$(CONFIG_INTEL_TDX_GUEST)	+= tdx/
-- 
2.25.1

Hi,

On 6/11/23 9:25 PM, Michael Roth wrote:
> Currently CONFIG_HAS_CC_PLATFORM is a prereq for building anything in
> arch/x86/coco, but that is generally only applicable for guest support.> 
> For SEV-SNP, helpers related purely to host support will also live in
> arch/x86/coco. To allow for CoCo-related host support code in
> arch/x86/coco, move that check down into the Makefile and check for it
> specifically when needed.


I think CONFIG_HAS_CC_PLATFORM is not meant to be guest specific (otherwise,
we could have named it CONFIG_HAS_CC_GUEST). Will it create any issue if
we enable it in host?

> 
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>  arch/x86/Kbuild        | 2 +-
>  arch/x86/coco/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
> index 5a83da703e87..1889cef48b58 100644
> --- a/arch/x86/Kbuild
> +++ b/arch/x86/Kbuild
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += coco/
> +obj-y += coco/
>  
>  obj-y += entry/
>  
> diff --git a/arch/x86/coco/Makefile b/arch/x86/coco/Makefile
> index c816acf78b6a..6aa52e719bf5 100644
> --- a/arch/x86/coco/Makefile
> +++ b/arch/x86/coco/Makefile
> @@ -3,6 +3,6 @@ CFLAGS_REMOVE_core.o	= -pg
>  KASAN_SANITIZE_core.o	:= n
>  CFLAGS_core.o		+= -fno-stack-protector
>  
> -obj-y += core.o
> +obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += core.o
>  
>  obj-$(CONFIG_INTEL_TDX_GUEST)	+= tdx/

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

On 7/9/23 22:05, Sathyanarayanan Kuppuswamy wrote:
> Hi,
> 
> On 6/11/23 9:25 PM, Michael Roth wrote:
>> Currently CONFIG_HAS_CC_PLATFORM is a prereq for building anything in
>> arch/x86/coco, but that is generally only applicable for guest support.>
>> For SEV-SNP, helpers related purely to host support will also live in
>> arch/x86/coco. To allow for CoCo-related host support code in
>> arch/x86/coco, move that check down into the Makefile and check for it
>> specifically when needed.
> 
> 
> I think CONFIG_HAS_CC_PLATFORM is not meant to be guest specific (otherwise,

Correct, it is used in bare-metal for SME support, so that needs to 
continue to work.

Thanks,
Tom

> we could have named it CONFIG_HAS_CC_GUEST). Will it create any issue if
> we enable it in host?
> 
>>
>> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>> ---
>>   arch/x86/Kbuild        | 2 +-
>>   arch/x86/coco/Makefile | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
>> index 5a83da703e87..1889cef48b58 100644
>> --- a/arch/x86/Kbuild
>> +++ b/arch/x86/Kbuild
>> @@ -1,5 +1,5 @@
>>   # SPDX-License-Identifier: GPL-2.0
>> -obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += coco/
>> +obj-y += coco/
>>   
>>   obj-y += entry/
>>   
>> diff --git a/arch/x86/coco/Makefile b/arch/x86/coco/Makefile
>> index c816acf78b6a..6aa52e719bf5 100644
>> --- a/arch/x86/coco/Makefile
>> +++ b/arch/x86/coco/Makefile
>> @@ -3,6 +3,6 @@ CFLAGS_REMOVE_core.o	= -pg
>>   KASAN_SANITIZE_core.o	:= n
>>   CFLAGS_core.o		+= -fno-stack-protector
>>   
>> -obj-y += core.o
>> +obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += core.o
>>   
>>   obj-$(CONFIG_INTEL_TDX_GUEST)	+= tdx/
>

On Sun, Jun 11, 2023 at 11:25:13PM -0500, Michael Roth wrote:
> Currently CONFIG_HAS_CC_PLATFORM is a prereq for building anything in
					^^^^^^

Use proper english words pls.

> arch/x86/coco, but that is generally only applicable for guest support.
> 
> For SEV-SNP, helpers related purely to host support will also live in
> arch/x86/coco. To allow for CoCo-related host support code in
> arch/x86/coco, move that check down into the Makefile and check for it
> specifically when needed.

I have no clue what that means. Example?

The last time we talked about paths, we ended up agreeing on:

https://lore.kernel.org/all/Yg5nh1RknPRwIrb8@zn.tnic/

So your "helpers related purely to host support" should go to

arch/x86/virt/svm/sev*.c

And just to keep it simple, that should be

arch/x86/virt/svm/sev.c

and if there's real need to split that, we can do that later.

Thx.

-- 
Regards/Gruss,
    Boris.

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

On Tue, Jun 20, 2023 at 02:09:20PM +0200, Borislav Petkov wrote:
> On Sun, Jun 11, 2023 at 11:25:13PM -0500, Michael Roth wrote:
> > Currently CONFIG_HAS_CC_PLATFORM is a prereq for building anything in
> 					^^^^^^
> 
> Use proper english words pls.
> 
> > arch/x86/coco, but that is generally only applicable for guest support.
> > 
> > For SEV-SNP, helpers related purely to host support will also live in
> > arch/x86/coco. To allow for CoCo-related host support code in
> > arch/x86/coco, move that check down into the Makefile and check for it
> > specifically when needed.
> 
> I have no clue what that means. Example?

Basically, arch/x86/coco/Makefile is never processed if arch/x86/Kbuild
indicates that CONFIG_HAS_CC_PLATFORM is not set. So if we want to have
stuff in arch/x86/coco/Makefile that build for !CONFIG_HAS_CC_PLATFORM,
like SNP host support, which does not rely on CONFIG_HAS_CC_PLATFORM
being set, that check needs to be moved down into arch/x86/coco/Makefile.

> 
> The last time we talked about paths, we ended up agreeing on:
> 
> https://lore.kernel.org/all/Yg5nh1RknPRwIrb8@zn.tnic/
> 
> So your "helpers related purely to host support" should go to
> 
> arch/x86/virt/svm/sev*.c
> 
> And just to keep it simple, that should be
> 
> arch/x86/virt/svm/sev.c
> 
> and if there's real need to split that, we can do that later.

Ok, makes sense.

Thanks,

Mike

> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

On Tue, Jun 20, 2023 at 03:43:15PM -0500, Michael Roth wrote:
> Basically, arch/x86/coco/Makefile is never processed if arch/x86/Kbuild
> indicates that CONFIG_HAS_CC_PLATFORM is not set. So if we want to have
> stuff in arch/x86/coco/Makefile that build for !CONFIG_HAS_CC_PLATFORM,
> like SNP host support, which does not rely on CONFIG_HAS_CC_PLATFORM
> being set, that check needs to be moved down into arch/x86/coco/Makefile.

Ok, so if you put SNP host support into arch/x86/virt/svm/sev.c, that
should work too and won't have any relation to CONFIG_HAS_CC_PLATFORM,
right?

The CC_PLATFORM thing is a way to check for confidential computing guest
features by abstracting the capabilities so that you don't have to check
*each* and *every* conf guest type in the conditionals and thus go nuts.

Thx.

-- 
Regards/Gruss,
    Boris.

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

On Wed, Jun 21, 2023 at 10:54:00AM +0200, Borislav Petkov wrote:
> On Tue, Jun 20, 2023 at 03:43:15PM -0500, Michael Roth wrote:
> > Basically, arch/x86/coco/Makefile is never processed if arch/x86/Kbuild
> > indicates that CONFIG_HAS_CC_PLATFORM is not set. So if we want to have
> > stuff in arch/x86/coco/Makefile that build for !CONFIG_HAS_CC_PLATFORM,
> > like SNP host support, which does not rely on CONFIG_HAS_CC_PLATFORM
> > being set, that check needs to be moved down into arch/x86/coco/Makefile.
> 
> Ok, so if you put SNP host support into arch/x86/virt/svm/sev.c, that
> should work too and won't have any relation to CONFIG_HAS_CC_PLATFORM,
> right?

Right, that works out just as well, and ends up being a bit more
straightforward. I have it implemented here:

  https://github.com/mdroth/linux/commits/snp-host-latest-v9b

  https://github.com/mdroth/linux/commit/a889a2dd64b62d9c3bf74cf02e7d8d71c7061667

and dropped the patch that reworks arch/x86/coco/Makefile.

Thanks,

Mike

> 
> The CC_PLATFORM thing is a way to check for confidential computing guest
> features by abstracting the capabilities so that you don't have to check
> *each* and *every* conf guest type in the conditionals and thus go nuts.
> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

On Sun, Jun 11, 2023 at 11:25:13PM -0500, Michael Roth wrote:
> Currently CONFIG_HAS_CC_PLATFORM is a prereq for building anything in
> arch/x86/coco, but that is generally only applicable for guest support.
> 
> For SEV-SNP, helpers related purely to host support will also live in
> arch/x86/coco. To allow for CoCo-related host support code in
> arch/x86/coco, move that check down into the Makefile and check for it
> specifically when needed.

Hm. TDX host support uses arch/x86/virt/vmx/tdx/. I think we need to be
consistent here.

IIRC, Borislav proposed the scheme that TDX uses.


-- 
  Kiryl Shutsemau / Kirill A. Shutemov