Re: [PATCH] x86: add SEV hashing to fw_cfg for kernel/initrd/cmdline

From: Thomas Huth <thuth@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Dov Murik" <dovmurik@linux.ibm.com>,
	"James Bottomley" <jejb@linux.ibm.com>,
	"Connor Kuehl" <ckuehl@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>,
	Ashish Kalra <ashish.kalra@amd.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Tobin Feldman-Fitzthum <tobin@ibm.com>,
	qemu-devel@nongnu.org, Hubertus Franke <frankeh@us.ibm.com>,
	Jim Cadden <jcadden@ibm.com>,
	Tobin Feldman-Fitzthum <tobin@linux.ibm.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: Re: [PATCH] x86: add SEV hashing to fw_cfg for kernel/initrd/cmdline
Date: Mon, 21 Jun 2021 10:44:57 +0200	[thread overview]
Message-ID: <29a0541d-64f2-1c55-ccf2-776d41f09c95@redhat.com> (raw)
In-Reply-To: <649abfe3-e799-3535-c56a-ad55ca5df947@redhat.com>

On 17/06/2021 17.48, Philippe Mathieu-Daudé wrote:
[...]
> This works, but I'd rather use:
> 
>    if (sev_enabled()) {
>        sev_kernel_loader_calc_cmdline_hash(&sev_loader_context,
>                                            kernel_cmdline);
>    }
> 
> And have sev_enabled() defined as:
> 
> #ifdef CONFIG_SEV
> bool sev_enabled(void);
> #else
> #define sev_enabled() false
> #endif
> 
> So the compiler could elide the statement if SEV is disabled,
> and stub is not necessary.
> 
> But that means we'd need to add "#include CONFIG_DEVICES" in
> a sysemu/ header, which looks like an anti-pattern.
> 
> Thomas / Paolo, what do you think?

I'd only do that if you are very, very sure that the header file is  only 
included from target-specific files. Otherwise this will of course cause 
more trouble than benefit.

  Thomas