linux-kernel - Re: [PATCH RFC v7 37/64] KVM: SVM: Add KVM_SNP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y7BSXT5ddtwsnomq@kernel.org>
Date:   Sat, 31 Dec 2022 15:16:45 +0000
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Michael Roth <michael.roth@....com>
Cc:     kvm@...r.kernel.org, linux-coco@...ts.linux.dev,
        linux-mm@...ck.org, linux-crypto@...r.kernel.org, x86@...nel.org,
        linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
        jroedel@...e.de, thomas.lendacky@....com, hpa@...or.com,
        ardb@...nel.org, pbonzini@...hat.com, seanjc@...gle.com,
        vkuznets@...hat.com, wanpengli@...cent.com, jmattson@...gle.com,
        luto@...nel.org, dave.hansen@...ux.intel.com, slp@...hat.com,
        pgonda@...gle.com, peterz@...radead.org,
        srinivas.pandruvada@...ux.intel.com, rientjes@...gle.com,
        dovmurik@...ux.ibm.com, tobin@....com, bp@...en8.de,
        vbabka@...e.cz, kirill@...temov.name, ak@...ux.intel.com,
        tony.luck@...el.com, marcorr@...gle.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com, alpergun@...gle.com,
        dgilbert@...hat.com, ashish.kalra@....com, harald@...fian.com,
        Brijesh Singh <brijesh.singh@....com>,
        Pavan Kumar Paluri <papaluri@....com>
Subject: Re: [PATCH RFC v7 37/64] KVM: SVM: Add KVM_SNP_INIT command

On Sat, Dec 31, 2022 at 02:47:29PM +0000, Jarkko Sakkinen wrote:
> A couple of fixups.
> 
> On Sat, Dec 31, 2022 at 02:27:57PM +0000, Jarkko Sakkinen wrote:
> > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> > index 6c4fdcaed72b..462c9aaa2e7e 100644
> > --- a/drivers/crypto/ccp/sev-dev.c
> > +++ b/drivers/crypto/ccp/sev-dev.c
> > @@ -1381,6 +1381,12 @@ static int __sev_snp_init_locked(int *error)
> >  	if (sev->snp_initialized)
> >  		return 0;
> >  
> > +	if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR)) {
> > +		dev_dbg(sev->dev, "SEV-SNP support requires firmware version >= %d:%d\n",
> > +			SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
> > +		return -ENODEV;
> 
>         return 0;
> 
> It is not a failure case anyway.
> 
> > +	}
> > +
> >  	/*
> >  	 * The SNP_INIT requires the MSR_VM_HSAVE_PA must be set to 0h
> >  	 * across all cores.
> > @@ -2313,25 +2319,19 @@ void sev_pci_init(void)
> >  		}
> >  	}
> >  
> > +	rc = sev_snp_init(&error, true);
> > +	if (rc != -ENODEV)
> 
> 
> if (rc)
> 
> Because other wise there would need to be nasty "if (rc && rc != ENODEV)"
> so that this does not happen:
> 
> [    9.321588] ccp 0000:49:00.1: SEV-SNP: failed to INIT error 0x0
> 
> BR, Jarkko

This patch (not dependent on the series) is kind of related to my
feedback. Since platform init can span from quite many locations
it would be useful to get errors reported from all locations:

https://www.lkml.org/lkml/2022/12/31/175

Would be IMHO good to have this in the baseline when testing SNP init
functionality.

BR, Jarkko