linux-kernel - Re: [PATCH v5 07/14] x86/sev: Move and reorganize sev guest request api

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: <d38bf900-98b0-479f-bcd5-7366f339938b@amd.com>
Date:   Thu, 2 Nov 2023 09:17:14 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     nikunj@....com, linux-kernel@...r.kernel.org, x86@...nel.org,
        kvm@...r.kernel.org
Cc:     bp@...en8.de, mingo@...hat.com, tglx@...utronix.de,
        dave.hansen@...ux.intel.com, dionnaglaze@...gle.com,
        pgonda@...gle.com, seanjc@...gle.com, pbonzini@...hat.com
Subject: Re: [PATCH v5 07/14] x86/sev: Move and reorganize sev guest request
 api

On 11/1/23 23:28, Nikunj A. Dadhania wrote:
> On 10/31/2023 12:46 AM, Tom Lendacky wrote:
>> On 10/30/23 01:36, Nikunj A Dadhania wrote:
>>> For enabling Secure TSC, SEV-SNP guests need to communicate with the
>>> AMD Security Processor early during boot. Many of the required
>>> functions are implemented in the sev-guest driver and therefore not
>>> available at early boot. Move the required functions and provide an
>>> API to the driver to assign key and send guest request.
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@....com>
>>> ---

>>> +static void snp_inc_msg_seqno(struct snp_guest_dev *snp_dev)
>>> +{
>>> +    u32 *os_area_msg_seqno = snp_get_os_area_msg_seqno(snp_dev->vmpck_id);
>>> +
>>> +    if (!os_area_msg_seqno) {
>>> +        pr_err("SNP unable to get message sequence counter\n");
>>> +        return;
>>> +    }
>>
>> I probably missed this in the other patch or even when the driver was first created, but shouldn't we have a lockdep_assert_held() here, too, before updating the count?
> 
> As per the current code flow, snp_get_msg_seqno() is always called before snp_inc_msg_seqno(), maybe because of that the check wasnt there. It still makes sense to have a lockdep_assert_held() in snp_inc_msg_seqno().
> 
> Should I add this change as a separate fix ?

It can be sent as a separate patch (I don't think it is a fix) either 
before or after this series.

Thanks,
Tom

> 
>>