On 8/4/22 08:37, Harald Hoyer wrote:
Am 04.08.22 um 15:13 schrieb Tom Lendacky:
On 8/3/22 20:02, Jarkko Sakkinen wrote:
From: Jarkko Sakkinen <jarkko@xxxxxxxxxxx>
SEV-SNP does not initialize to a legit state, unless the firmware is
loaded twice, when SEP API version < 1.43, and the firmware is updated
to a later version. Because of this user space needs to work around
this with "rmmod && modprobe" combo. Fix this by implementing the
workaround to the driver.
The SNP hypervisor patches are placing a minimum supported version
requirement for the SEV firmware that exceeds the specified version
above [1] (for the reason above, as well as some others), so this patch
is not needed, NAK.
As described in the "Milan Release Notes.txt" of the AMD firmware update
package amd_sev_fam19h_model0xh_1.33.03.zip.
"If upgrading to 1.33.01 or later from something older (picking up
CSF-1201), it is required that two Download Firmware commands be run to
fix the "Committed Version" across the firmware. CSF-1201 fixed a bug
where the committed version in the attestation report was incorrect.
Performing a single Download Firmware will upgrade the firmware, but
performing a second one will correct the committed version. This is a
one-time upgrade issue.
"
Note that `1.33.01` is not the same version number as "1.51" in [1]. One
is the firmware version, the other is the SEV-SNP API version.
It is the same and are meant to correlate, the 33 is hex => 51.
I am definitely seeing a wrong TCB version, if the firmware is only
updated once to `1.33.01` aka "1.51".
Reloading the `ccp` module, which triggers another firmware load, cures
the problem.
The patch might be wrong, as it might not do the right thing, but the
problem and the solution exist.
What is your suggestion then to fix the wrong committed TCB version?
Hmmm... ok, I see what you're saying. We don't want to have to make
everyone update their BIOS/firmware to get to a starting level above 1.43
to begin with.
Ok, let me review/comment on the patch.
Thanks,
Tom
[1]
https://lore.kernel.org/lkml/87a0481526e66ddd5f6192cbb43a50708aee2883.1655761627.git.ashish.kalra@xxxxxxx/
Thanks,
Tom
Reported-by: Harald Hoyer <harald@xxxxxxxxxxx>
Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxxx>
---
drivers/crypto/ccp/sev-dev.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 799b476fc3e8..f2abb7439dde 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -76,6 +76,9 @@ static void *sev_es_tmr;
#define NV_LENGTH (32 * 1024)
static void *sev_init_ex_buffer;
+/*
+ * SEV API version >= maj.min?
+ */
static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
{
struct sev_device *sev = psp_master->sev_data;
@@ -89,6 +92,14 @@ static inline bool sev_version_greater_or_equal(u8
maj, u8 min)
return false;
}
+/*
+ * SEV API version < maj.min?
+ */
+static inline bool sev_version_less(u8 maj, u8 min)
+{
+ return !sev_version_greater_or_equal(maj, min);
+}
+
static void sev_irq_handler(int irq, void *data, unsigned int status)
{
struct sev_device *sev = data;
@@ -1274,6 +1285,7 @@ void sev_pci_init(void)
{
struct sev_device *sev = psp_master->sev_data;
int error, rc;
+ int i;
if (!sev)
return;
@@ -1283,9 +1295,13 @@ void sev_pci_init(void)
if (sev_get_api_version())
goto err;
- if (sev_version_greater_or_equal(0, 15) &&
- sev_update_firmware(sev->dev) == 0)
- sev_get_api_version();
+ /*
+ * SEV-SNP does not work properly before loading the FW twice in
the API
+ * versions older than SEV 1.43.
+ */
+ for (i = 0; i < sev_version_greater_or_equal(0, 15) +
sev_version_less(1, 43); i++)
+ if (sev_update_firmware(sev->dev) == 0)
+ sev_get_api_version();
/* If an init_ex_path is provided rely on INIT_EX for PSP
initialization
* instead of INIT.
