linux-kernel - Re: [RFC Part2 PATCH v3 03/26] crypto: ccp: Add Secure Encrypted Virtualization (SEV) device support

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: <855be17c-382e-4e7d-7300-ca19cc50c625@amd.com>
Date:   Tue, 12 Sep 2017 10:32:13 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Borislav Petkov <bp@...e.de>
Cc:     brijesh.singh@....com, linux-kernel@...r.kernel.org,
        x86@...nel.org, kvm@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Joerg Roedel <joro@...tes.org>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        \"Radim Krčmář\" <rkrcmar@...hat.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Gary Hook <gary.hook@....com>, linux-crypto@...r.kernel.org
Subject: Re: [RFC Part2 PATCH v3 03/26] crypto: ccp: Add Secure Encrypted
 Virtualization (SEV) device support

Hi Boris,

I will address all your feedback in next rev.


On 09/12/2017 09:02 AM, Borislav Petkov wrote:
...


> 
> You could make that more tabular like this:
> 
>          case SEV_CMD_INIT:              return sizeof(struct sev_data_init);
>          case SEV_CMD_PLATFORM_STATUS:   return sizeof(struct sev_data_status);
>          case SEV_CMD_PEK_CSR:           return sizeof(struct sev_data_pek_csr);
> 	...
> 
> which should make it more readable.
> 
> But looking at this more, this is a static mapping between the commands
> and the corresponding struct sizes and you use it in
> 
>          print_hex_dump_debug("(in):  ", DUMP_PREFIX_OFFSET, 16, 2, data,
>                          sev_cmd_buffer_len(cmd), false);
> 
> But then, I don't see what that brings you because you're not dumping
> the actual @data length but the *expected* data length based on the
> command type.
> 
> And *that* you can look up in the manual and do not need it in code,
> enlarging the driver unnecessarily.
> 
> ...


The debug statement is very helpful during development, it gives me the full
view of what command we send to PSP, data dump of command buffer before and
after the request completion.  e.g when dyndbg is enabled the output looks like
this:

[392035.621308] ccp 0000:02:00.2: sev command id 0x4 buffer 0x000080146d232820
[392035.621312] (in):  00000000: 0000 0000 0000 0000 0000 0000
[392035.624725] (out): 00000000: 0e00 0000 0000 0b00 0000 0000

The first debug line prints command ID, second line prints memory dump of the command
structure and third line prints memory dump of command structure after PSP processed
the command.

The caller will use sev_issue_cmd() to issue PSP command. At this time we know
the command id and a opaque pointer (this points to command structure for command id).
Caller does not give us length of the command structure hence we need to derive it
from the command id using sev_cmd_buffer_len(). The command structure length is fixed
for a given command id.


Thanks
Brijesh