Messages in this thread | | | Date | Thu, 4 Aug 2022 08:51:02 -0500 | Subject | Re: [PATCH] crypto: ccp: Load the firmware twice when SEV API version < 1.43 | From | Tom Lendacky <> |
| |
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@profian.com> >>> >>> 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@amd.com/ >> >> >> Thanks, >> Tom >> >>> >>> Reported-by: Harald Hoyer <harald@profian.com> >>> Signed-off-by: Jarkko Sakkinen <jarkko@profian.com> >>> --- >>> 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. >
| |