Re: [RFC Part2 PATCH 05/30] x86: define RMP violation #PF error code

2021-04-20 Thread Borislav Petkov
On Wed, Mar 24, 2021 at 12:04:11PM -0500, Brijesh Singh wrote: Btw, for all your patches where the subject prefix is only "x86:": The tip tree preferred format for patch subject prefixes is 'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:', 'genirq/core:'. Please do not use fi

Re: [RFC Part2 PATCH 04/30] x86/mm: split the physmap when adding the page in RMP table

2021-04-20 Thread Borislav Petkov
On Mon, Apr 19, 2021 at 11:33:08AM -0700, Dave Hansen wrote: > My point was just that you can't _easily_ do the 2M->4k kernel mapping > demotion in a page fault handler, like I think Borislav was suggesting. Yeah, see my reply to Brijesh. Not in the #PF handler but when the guest does update the R

Re: [RFC Part2 PATCH 04/30] x86/mm: split the physmap when adding the page in RMP table

2021-04-20 Thread Borislav Petkov
On Mon, Apr 19, 2021 at 12:46:53PM -0500, Brijesh Singh wrote: > - KVM calls  alloc_page() to allocate a VMSA page. The allocator returns > 0xc820 (PFN 0x200, page-level=2M). The VMSA page is private > page so KVM will call RMPUPDATE to add the page as a private page in the > RMP table.

Re: [RFC Part2 PATCH 04/30] x86/mm: split the physmap when adding the page in RMP table

2021-04-19 Thread Borislav Petkov
On Mon, Apr 19, 2021 at 10:25:01AM -0500, Brijesh Singh wrote: > To my understanding, we don't group 512 4K entries into a 2M for the > kernel address range. We do this for the userspace address through > khugepage daemon. If page tables get out of sync then it will cause an > RMP violation, the Pa

Re: [RFC Part2 PATCH 04/30] x86/mm: split the physmap when adding the page in RMP table

2021-04-19 Thread Borislav Petkov
On Wed, Mar 24, 2021 at 12:04:10PM -0500, Brijesh Singh wrote: > A write from the hypervisor goes through the RMP checks. When the > hypervisor writes to pages, hardware checks to ensures that the assigned > bit in the RMP is zero (i.e page is shared). If the page table entry that > gives the sPA i

Re: [RFC Part2 PATCH 02/30] x86/sev-snp: add RMP entry lookup helpers

2021-04-15 Thread Borislav Petkov
On Thu, Apr 15, 2021 at 01:08:09PM -0500, Brijesh Singh wrote: > This is from Family 19h Model 01h Rev B01. The processor which > introduces the SNP feature. Yes, I have already upload the PPR on the BZ. > > The PPR is also available at AMD: https://www.amd.com/en/support/tech-docs Please add the

Re: [RFC Part2 PATCH 03/30] x86: add helper functions for RMPUPDATE and PSMASH instruction

2021-04-15 Thread Borislav Petkov
On Wed, Mar 24, 2021 at 12:04:09PM -0500, Brijesh Singh wrote: > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c > index 06394b6d56b2..7a0138cb3e17 100644 > --- a/arch/x86/mm/mem_encrypt.c > +++ b/arch/x86/mm/mem_encrypt.c > @@ -644,3 +644,44 @@ rmpentry_t *lookup_page_in_rmptabl

Re: [RFC Part2 PATCH 02/30] x86/sev-snp: add RMP entry lookup helpers

2021-04-15 Thread Borislav Petkov
On Wed, Mar 24, 2021 at 12:04:08PM -0500, Brijesh Singh wrote: > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c Also, why is all this SNP stuff landing in this file instead of in sev.c or so which is AMD-specific? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/n

Re: [RFC Part2 PATCH 02/30] x86/sev-snp: add RMP entry lookup helpers

2021-04-15 Thread Borislav Petkov
On Wed, Mar 24, 2021 at 12:04:08PM -0500, Brijesh Singh wrote: > The lookup_page_in_rmptable() can be used by the host to read the RMP > entry for a given page. The RMP entry format is documented in PPR > section 2.1.5.2. I see Table 15-36. Fields of an RMP Entry in the APM. Which PPR do you me

Re: [RFC Part2 PATCH 01/30] x86: Add the host SEV-SNP initialization support

2021-04-14 Thread Borislav Petkov
On Wed, Mar 24, 2021 at 12:04:07PM -0500, Brijesh Singh wrote: > @@ -538,6 +540,10 @@ > #define MSR_K8_SYSCFG0xc0010010 > #define MSR_K8_SYSCFG_MEM_ENCRYPT_BIT23 > #define MSR_K8_SYSCFG_MEM_ENCRYPTBIT_ULL(MSR_K8_SYSCFG_MEM_ENCRYPT_BIT) > +#define MSR_K8_SYSCFG

Re: [PATCH v2 8/8] KVM: SVM: Allocate SEV command structures on local stack

2021-04-07 Thread Borislav Petkov
On Wed, Apr 07, 2021 at 05:05:07PM +, Sean Christopherson wrote: > I used memset() to defer initialization until after the various sanity > checks, I'd actually vote for that too - I don't like doing stuff which is not going to be used. I.e., don't change what you have. -- Regards/Gruss,

Re: [PATCH v2 8/8] KVM: SVM: Allocate SEV command structures on local stack

2021-04-07 Thread Borislav Petkov
First of all, I'd strongly suggest you trim your emails when you reply - that would be much appreciated. On Wed, Apr 07, 2021 at 07:24:54AM +0200, Christophe Leroy wrote: > > @@ -258,7 +240,7 @@ static int sev_issue_cmd(struct kvm *kvm, int id, void > > *data, int *error) > > static int sev_lau

Re: [PATCH v1 1/3] x86/cpufeatures: Add low performance CRC32C instruction CPU feature

2021-01-11 Thread Borislav Petkov
On Mon, Jan 11, 2021 at 06:51:59PM +0800, Tony W Wang-oc wrote: > This issue will be enhanced by hardware and patch submit will be pending. I have no clue what that has to do with your current patch... you might need to explain more verbosely. -- Regards/Gruss, Boris. https://people.kernel.

Re: [PATCH v1 1/3] x86/cpufeatures: Add low performance CRC32C instruction CPU feature

2021-01-06 Thread Borislav Petkov
On Thu, Jan 07, 2021 at 02:19:06PM +0800, Tony W Wang-oc wrote: > SSE4.2 on Zhaoxin CPUs are compatible with Intel. The presence of > CRC32C instruction is enumerated by CPUID.01H:ECX.SSE4_2[bit 20] = 1. > Some Zhaoxin CPUs declare support SSE4.2 instruction sets but their > CRC32C instruction are

Re: [RFC PATCH 7/8] crypto: x86/aes-kl - Support AES algorithm using Key Locker instructions

2020-12-18 Thread Borislav Petkov
On Fri, Dec 18, 2020 at 10:34:28AM +, Bae, Chang Seok wrote: > I’m open to drop the macros if there is any better way to define them > without binutils support. Yap, make the driver build depend on the binutils version which supports them. -- Regards/Gruss, Boris. SUSE Software Solution

Re: [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value

2020-08-02 Thread Borislav Petkov
On Sun, Aug 02, 2020 at 02:14:06PM -0500, Bjorn Helgaas wrote: > Wait, I'm not convinced yet. I know that if a PCI read fails, you > normally get ~0 data because the host bridge fabricates it to complete > the CPU load. > > But what guarantees that a PCI config register cannot contain ~0? Well,

Re: [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value

2020-08-02 Thread Borislav Petkov
On Sun, Aug 02, 2020 at 07:28:00PM +0200, Saheed Bolarinwa wrote: > Because the value ~0 has a meaning to some drivers and only No, ~0 means that the PCI read failed. For *every* PCI device I know. Here's me reading from 0xf0 offset of my hostbridge: # setpci -s 00:00.0 0xf0.l 0100 That dev

Re: [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value

2020-08-01 Thread Borislav Petkov
On Sat, Aug 01, 2020 at 01:24:29PM +0200, Saheed O. Bolarinwa wrote: > The return value of pci_read_config_*() may not indicate a device error. > However, the value read by these functions is more likely to indicate > this kind of error. This presents two overlapping ways of reporting > errors and

Re: [PATCH] crypto: ccp - Fix sparse warnings in sev-dev

2020-06-04 Thread Borislav Petkov
+ Tom. On Thu, Jun 04, 2020 at 06:09:41PM +1000, Herbert Xu wrote: > This patch fixes a bunch of sparse warnings in sev-dev where the > __user marking is incorrectly handled. > > Reported-by: kbuild test robot > Fixes: 7360e4b14350 ("crypto: ccp: Implement SEV_PEK_CERT_IMPORT...") > Fixes: e7990

Re: [PATCH v9 24/28] x86_64/asm: Change all ENTRY+ENDPROC to SYM_FUNC_*

2019-10-16 Thread Borislav Petkov
64, given these were > the last users. > > Signed-off-by: Jiri Slaby > Reviewed-by: Rafael J. Wysocki [hibernate] > Reviewed-by: Boris Ostrovsky [xen bits] > Cc: "H. Peter Anvin" > Cc: Borislav Petkov > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: x..

Re: [PATCH v8 06/28] x86/asm/crypto: annotate local functions

2019-08-17 Thread Borislav Petkov
On Thu, Aug 08, 2019 at 12:38:32PM +0200, Jiri Slaby wrote: > Use the newly added SYM_FUNC_START_LOCAL to annotate starts of all > functions which do not have ".globl" annotation, but their ends are > annotated by ENDPROC. This is needed to balance ENDPROC for tools that > generate debuginfo. > >

Re: [PATCH v9 00/11] x86: PIE support to extend KASLR randomization

2019-08-06 Thread Borislav Petkov
On Tue, Jul 30, 2019 at 12:12:44PM -0700, Thomas Garnier wrote: > These patches make some of the changes necessary to build the kernel as > Position Independent Executable (PIE) on x86_64. Another patchset will > add the PIE option and larger architecture changes. Yeah, about this: do we have a lo

Re: [PATCH v9 01/11] x86/crypto: Adapt assembly for PIE support

2019-08-05 Thread Borislav Petkov
On Mon, Aug 05, 2019 at 09:54:44AM -0700, Kees Cook wrote: > I think there was some long-ago feedback from someone (Ingo?) about > giving context for the patch so looking at one individually would let > someone know that it was part of a larger series. Strange. But then we'd have to "mark" all pat

Re: [PATCH v9 01/11] x86/crypto: Adapt assembly for PIE support

2019-08-05 Thread Borislav Petkov
On Tue, Jul 30, 2019 at 12:12:45PM -0700, Thomas Garnier wrote: > Change the assembly code to use only relative references of symbols for the > kernel to be PIE compatible. > > Position Independent Executable (PIE) support will allow to extend the > KASLR randomization range below 0x80

Re: [PATCH v7 07/28] x86/asm/crypto: annotate local functions

2019-02-25 Thread Borislav Petkov
On Wed, Jan 30, 2019 at 01:46:50PM +0100, Jiri Slaby wrote: > Use the newly added SYM_FUNC_START_LOCAL to annotate starts of all > functions which do not have ".globl" annotation, but their ends are > annotated by ENDPROC. This is needed to balance ENDPROC for tools that > generate debuginfo. > >

Re: [PATCH v6 01/27] x86/crypto: Adapt assembly for PIE support

2019-02-07 Thread Borislav Petkov
On Thu, Jan 31, 2019 at 11:24:08AM -0800, Thomas Garnier wrote: > Change the assembly code to use only relative references of symbols for the > kernel to be PIE compatible. > > Position Independent Executable (PIE) support will allow to extend the > KASLR randomization range below 0x80

Re: [PATCH net-next v6 04/23] zinc: ChaCha20 x86_64 implementation

2018-09-29 Thread Borislav Petkov
On Sat, Sep 29, 2018 at 10:00:29AM +0200, Ard Biesheuvel wrote: > Note that this is the author of the *patch* not necessarily the author > of the code. > > Anyone is free to submit patches adding code authored by others as > long as the author has made it available under a suitable license, and >

Re: [PATCH net-next v6 04/23] zinc: ChaCha20 x86_64 implementation

2018-09-29 Thread Borislav Petkov
On Sat, Sep 29, 2018 at 04:01:53AM +0200, Jason A. Donenfeld wrote: > I was wondering about the ordering of these, actually. I've seen s-o-b > on top and s-o-b on bottom of the cc list in lots of commits and > haven't yet divined the One True Position. Documentation/process/submitting-patches.rst

Re: [PATCH crypto-2.6] crypto: ccp: add timeout support in the SEV command

2018-09-11 Thread Borislav Petkov
On Mon, Sep 10, 2018 at 02:06:57PM -0500, Brijesh Singh wrote: > Nothing prevent user from supplying a bogus number. The main question > is, clamp with what number ? So you definitely want to forbid too large timeouts - that wouldn't make any sense anyway. And too small either, because a too small

Re: [PATCH crypto-2.6] crypto: ccp: add timeout support in the SEV command

2018-09-04 Thread Borislav Petkov
On Wed, Aug 15, 2018 at 04:11:25PM -0500, Brijesh Singh wrote: > Currently, the CCP driver assumes that the SEV command issued to the PSP > will always return (i.e. it will never hang). But recently, firmware bugs > have shown that a command can hang. Since of the SEV commands are used > in probe

Re: [PATCH] x86/crypto: Add missing RETs

2018-06-24 Thread Borislav Petkov
On Sun, Jun 24, 2018 at 09:12:35AM +0200, Thomas Gleixner wrote: > We should really have something like that exactly to catch cases like this. Sounds like a good use case for the snake language. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.

[PATCH] x86/crypto: Add missing RETs

2018-06-23 Thread Borislav Petkov
Lemme send a proper patch now... --- From: Borislav Petkov Date: Sun, 17 Jun 2018 13:57:42 +0200 Subject: [PATCH] x86/crypto: Add missing RETs Add explicit RETs to the tail calls of AEGIS and MORUS crypto algorithms otherwise they run into INT3 padding due to 51bad67ffbce ("x86/asm

Re: [PATCH RESEND 1/2] Add DOWNLOAD_FIRMWARE SEV command

2018-05-10 Thread Borislav Petkov
Use a prefix for the subject pls: Subject: [PATCH RESEND 1/2] crypto: ccp: Add DOWNLOAD_FIRMWARE SEV command or Subject: [PATCH RESEND 1/2] crypto/ccp: Add DOWNLOAD_FIRMWARE SEV command or so. On Wed, May 09, 2018 at 11:18:27AM -0500, Janakarajan Natarajan wrote: > The DOWNLOAD_FIRMWARE comma

Re: [Part2 PATCH v7 20/38] crypto: ccp: Implement SEV_PDH_CERT_EXPORT ioctl command

2017-11-05 Thread Borislav Petkov
On Wed, Nov 01, 2017 at 04:16:05PM -0500, Brijesh Singh wrote: > The SEV_PDH_CERT_EXPORT command can be used to export the PDH and its > certificate chain. The command is defined in SEV spec section 5.10. ... > --- > drivers/crypto/ccp/psp-dev.c | 98 > ++

Re: [Part2 PATCH v7 19/38] crypto: ccp: Implement SEV_PEK_CERT_IMPORT ioctl command

2017-11-04 Thread Borislav Petkov
On Wed, Nov 01, 2017 at 04:16:04PM -0500, Brijesh Singh wrote: > The SEV_PEK_CERT_IMPORT command can be used to import the signed PEK > certificate. The command is defined in SEV spec section 5.8. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v7 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-11-03 Thread Borislav Petkov
On Wed, Nov 01, 2017 at 04:16:03PM -0500, Brijesh Singh wrote: > The SEV_PEK_CSR command can be used to generate a PEK certificate > signing request. The command is defined in SEV spec section 5.7. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v7 14/38] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-11-03 Thread Borislav Petkov
On Wed, Nov 01, 2017 at 04:15:59PM -0500, Brijesh Singh wrote: > The SEV_FACTORY_RESET command can be used by the platform owner to > reset the non-volatile SEV related data. The command is defined in > SEV spec section 5.4 > > Cc: Paolo Bonzini > Cc: "Radim Krčmář"

Re: [Part2 PATCH v7 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-11-02 Thread Borislav Petkov
gt; - an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borisla

Re: [Part2 PATCH v6.1 16/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-31 Thread Borislav Petkov
On Mon, Oct 30, 2017 at 08:29:25PM -0500, Brijesh Singh wrote: > Okay, Just tried static global with CONFIG_VMAP_STACK=y and I am getting > wrong physical address with __pa. PSP command fails with error code > "INVALID_ADDRESS". The same thing works fine with kmalloc() buffer. Ah, right, module sp

Re: [Part2 PATCH v6.1 16/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-30 Thread Borislav Petkov
On Mon, Oct 30, 2017 at 12:49:14PM -0500, Brijesh Singh wrote: > If the buffer is allocated on the stack then there is no guarantee that static global is not allocated on the stack. > I can certainly move the allocation outside, but then it may increase the > code size in other functions. If its

Re: [Part2 PATCH v6.1 16/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-30 Thread Borislav Petkov
On Sun, Oct 29, 2017 at 03:48:25PM -0500, Brijesh Singh wrote: > AMD's new Secure Encrypted Virtualization (SEV) feature allows the > memory contents of virtual machines to be transparently encrypted with a > key unique to the VM. The programming and management of the encryption > keys are handled

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 05:59:23PM -0500, Brijesh Singh wrote: > Yes it is typo. PEK_GEN wants FW to be in INIT state hence someone need > to transition from UNINIT -> INIT. Which, once you've done it once on driver init, is there. > That's what I am doing except FACTORY_RESET. Well, not really.

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 04:28:31PM -0500, Brijesh Singh wrote: > This will fail because PEK_GEN require the platform in INIT state and > nobody has done the state transition from INIT -> UINIT. Huh, FW is in INIT state and PEK_GEN wants it to be in INIT state. Typo? Aaanyway, I don't like this wh

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 03:25:24PM -0500, Brijesh Singh wrote: > Yep, we are doing state transition only when we really need to. At least > so far I have tried to avoid making any unnecessary state transitions. So change all those which do INIT -> CMD -> SHUTDOWN to do only the command as the INIT

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-27 Thread Borislav Petkov
On Fri, Oct 27, 2017 at 06:28:38AM -0500, Brijesh Singh wrote: > ... User can retry the command sometime later when nobody else is > using the PSP. That still doesn't prevent you from doing two things: * make that fw_init_count a proper kref instead of your homegrown thing * do not preemptively

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-27 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 03:59:32PM -0500, Brijesh Singh wrote: > we can workaround #1 by adding some hooks in sp_pci_init() to invoke the PSP > initialization routines after pci_register_driver() is done but #2 can get > painful because it will require us calling the SHUTDOWN outside the > sp_pci_e

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-26 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 02:26:15PM -0500, Brijesh Singh wrote: > SHUTDOWN command unconditionally transitions a platform to uninitialized > state. The command does not care how many processes are actively using the > PSP. We don't want to shutdown the firmware while other process is still > using i

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-26 Thread Borislav Petkov
On Thu, Oct 26, 2017 at 11:56:57AM -0500, Brijesh Singh wrote: > The variable is used as ref counter. ... and it can't be converted to a boolean because...? > In your previous reply you comments on global semaphore (fw_init_mutex) and > in response I tried to highlight why we need the global sema

Re: [Part2 PATCH v6.1 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-26 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 04:55:19PM -0500, Brijesh Singh wrote: > The SEV_PEK_GEN command is used to generate a new Platform Endorsement > Key (PEK). The command is defined in SEV spec section 5.6. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-26 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 02:57:04PM -0500, Brijesh Singh wrote: > Calling PLATFORM_GET_STATUS is not required, we can manage the state through > a simple ref count variable. Issuing PSP commands will always be much more > expensive compare to accessing a protected global variable. What does "protec

Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-23 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 08:32:57AM -0500, Brijesh Singh wrote: > If both the command fails then we return status from the last command. > IIRC, in my previous patches I was returning status from sev_do_cmd() > instead of sev_platform_shutdown() but based on our previous > communication I thought yo

Re: [Part2 PATCH v6 18/38] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:53PM -0500, Brijesh Singh wrote: > The SEV_PEK_CSR command can be used to generate a PEK certificate > signing request. The command is defined in SEV spec section 5.7. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v6 17/38] crypto: ccp: Implement SEV_PDH_GEN ioctl command

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:52PM -0500, Brijesh Singh wrote: > The SEV_PDH_GEN command is used to re-generate the Platform > Diffie-Hellman (PDH) key. The command is defined in SEV spec section > 5.6. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav

Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-23 Thread Borislav Petkov
On Mon, Oct 23, 2017 at 07:15:30AM -0500, Brijesh Singh wrote: > I am not sure if I am able to understand your feedback. The > sev_platform_shutdown() is called unconditionally. How's that: If sev_do_cmd() fails and sev_do_cmd(SEV_CMD_SHUTDOWN, ...) in sev_platform_shutdown() fails, then the firs

Re: [Part2 PATCH v6 16/38] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:51PM -0500, Brijesh Singh wrote: > The SEV_PEK_GEN command is used to generate a new Platform Endorsement > Key (PEK). The command is defined in SEV spec section 5.6. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > +static int __sev_platform_init(struct sev_data_init *data, int *error) > +{ > + int rc = 0; > + > + mutex_lock(&fw_init_mutex); > + > + if (!fw_init_count) { I still don't like global semaphores. Can you get the status a

Re: [Part2 PATCH v6 15/38] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:50PM -0500, Brijesh Singh wrote: > The SEV_PLATFORM_STATUS command can be used by the platform owner to > get the current status of the platform. The command is defined in > SEV spec section 5.5. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář&

Re: [Part2 PATCH v6 14/38] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:49PM -0500, Brijesh Singh wrote: > The SEV_FACTORY_RESET command can be used by the platform owner to > reset the non-volatile SEV related data. The command is defined in > SEV spec section 5.4 > > Cc: Paolo Bonzini > Cc: "Radim Krčmář"

Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-23 Thread Borislav Petkov
On Thu, Oct 19, 2017 at 09:33:48PM -0500, Brijesh Singh wrote: > AMD's new Secure Encrypted Virtualization (SEV) feature allows the > memory contents of virtual machines to be transparently encrypted with a > key unique to the VM. The programming and management of the encryption > keys are handled

Re: [Part2 PATCH v5.1 12.9/31] crypto: ccp: Implement SEV_PDH_CERT_EXPORT ioctl command

2017-10-13 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:07PM -0500, Brijesh Singh wrote: > The SEV_PDH_CERT_EXPORT command can be used to export the PDH and its > certificate chain. The command is defined in SEV spec section 5.10. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Pe

Re: [Part2 PATCH v5.1 12.8/31] crypto: ccp: Implement SEV_PEK_CERT_IMPORT ioctl command

2017-10-13 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:06PM -0500, Brijesh Singh wrote: > The SEV_PEK_CERT_IMPORT command can be used to import the signed PEK > certificate. The command is defined in SEV spec section 5.8. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-13 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 11:13:44PM -0500, Brijesh Singh wrote: > As per the spec, its perfectly legal to pass input.address = 0x0 and > input.length = 0x0. If userspace wants to query CSR length then it will > fill all the fields with. In response, FW will return error > (LENGTH_TO_SMALL) and input

Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-13 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 09:24:01PM -0500, Brijesh Singh wrote: > I assume you mean performing the SEV state check before allocating the > memory for the CSR blob, right ? I mean, do those first: if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) return -

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 04:52:32PM -0500, Brijesh Singh wrote: > See my above comment, I think the simplest solution is remove psp->sev_misc Ok, so far so good. But now you still need to track which is the last psp device and to call misc_deregister() only when the last device exits. Because if y

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 04:11:18PM -0500, Brijesh Singh wrote: > The sev_exit() will be called for all the psp_device instance. we need > to set psp_misc_dev = NULL after deregistering the device. > > if (psp_misc_dev) { >   misc_deregister(psp_misc_dev); >    psp_misc_dev = NULL; Right, except w

Re: [Part2 PATCH v5.1 12.6/31] crypto: ccp: Implement SEV_PDH_GEN ioctl command

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 03:21:04PM -0500, Brijesh Singh wrote: > We need to follow the platform state machine logic defined in SEV spec > section 5.1.2. The PEK_GEN can not be issued when platform is in WORKING > state because the command actually re-generate the identity of the > platform itself (

Re: [Part2 PATCH v5.1 12.5/31] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-12 Thread Borislav Petkov
On Thu, Oct 12, 2017 at 03:11:07PM -0500, Brijesh Singh wrote: > Lets  consider this scenario > 1- platform is in uninit state, we transition it to INIT > 2- PEK_GEN command failed > 3- since we have transitioned the platform in INIT state hence we must > call the shutdown otherwise we will leave t

Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR ioctl command

2017-10-12 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:05PM -0500, Brijesh Singh wrote: > The SEV_PEK_CSR command can be used to generate a PEK certificate > signing request. The command is defined in SEV spec section 5.7. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v5.1 12.6/31] crypto: ccp: Implement SEV_PDH_GEN ioctl command

2017-10-12 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:04PM -0500, Brijesh Singh wrote: > The SEV_PDH_GEN command is used to re-generate the Platform > Diffie-Hellman (PDH) key. The command is defined in SEV spec section > 5.9. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav

Re: [Part2 PATCH v5.1 12.5/31] crypto: ccp: Implement SEV_PEK_GEN ioctl command

2017-10-12 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:03PM -0500, Brijesh Singh wrote: > The SEV_PEK_GEN command is used to generate a new Platform Endorsement > Key (PEK). The command is defined in SEV spec section 5.6. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 11:50:30AM -0500, Brijesh Singh wrote: > +static int sev_do_cmd(int cmd, void *data, int *psp_ret) > +{ > + unsigned int phys_lsb, phys_msb; > + struct psp_device *psp; > + unsigned int reg, ret; > + struct sp_device *sp; > + > + sp = sp_get_psp_master_de

Re: [Part2 PATCH v5.2 12.3/31] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-10-12 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 11:55:21AM -0500, Brijesh Singh wrote: > The SEV_FACTORY_RESET command can be used by the platform owner to > reset the non-volatile SEV related data. The command is defined in > SEV spec section 5.4 > > Cc: Paolo Bonzini > Cc: "Radim Krčmář"

Re: [Part2 PATCH v5.2 12.2/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-12 Thread Borislav Petkov
gt; - an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borisla

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 03:45:00PM -0500, Brijesh Singh wrote: > OK, if userspace is going to pick bits apart then how about this: > > struct sev_data_status { > u8 api_major; /* Out */ > u8 api_minor; /* Out */ > u8 s

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 03:10:49PM -0500, Brijesh Singh wrote: > The current 'struct sev_data_status' matches with the firmware names and the > bit fields. Only thing I did was the fields with no name is called as > "reservedX" Ok, I see it. So what you actually wanna do is: struct sev_data_statu

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 10:04:44PM +0200, Borislav Petkov wrote: > Then it is easy: > >&= GENMASK(23, 1); [1:23] range cleared, of course: &= ~GENMASK(23, 1); -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norto

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Wed, Oct 11, 2017 at 02:49:55PM -0500, Brijesh Singh wrote: > This is OK for now. But in future if FW steals another bit from reserved1 > field to expose a new flag then 'owner' name will no longer be valid. If you > don't to use bit field then we have to use some generic name instead of > namin

Re: [Part2 PATCH v5.1 12.4/31] crypto: ccp: Implement SEV_PLATFORM_STATUS ioctl command

2017-10-11 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:02PM -0500, Brijesh Singh wrote: > The SEV_PLATFORM_STATUS command can be used by the platform owner to > get the current status of the platform. The command is defined in > SEV spec section 5.5. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář&

Re: [Part2 PATCH v5.1 12.3/31] crypto: ccp: Implement SEV_FACTORY_RESET ioctl command

2017-10-11 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:01PM -0500, Brijesh Singh wrote: > The SEV_FACTORY_RESET command can be used by the platform owner to > reset the non-volatile SEV related data. The command is defined in > SEV spec section 5.4 > > Cc: Paolo Bonzini > Cc: "Radim Krčmář"

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-11 Thread Borislav Petkov
On Sun, Oct 08, 2017 at 08:30:47AM -0500, Brijesh Singh wrote: > Basically we need some variable which is outside the per-device > structure so that we don't end up creating multiple /dev/sev nodes. If > needed, I think we can remove 'has_sev_fops' variable from struct > psp_device if we decide to

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-10 Thread Borislav Petkov
On Tue, Oct 10, 2017 at 01:43:22PM -0500, Tom Lendacky wrote: > Maybe for the very first implementation we could do that and that was what > was originally done for the CCP. But as you can see the CCP does not have > a set register offset between various iterations of the device and it can > be ex

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-09 Thread Borislav Petkov
On Sun, Oct 08, 2017 at 07:11:04PM -0500, Brijesh Singh wrote: > There is a single security processor driver (ccp) which provides the > complete functionality including PSP.  But the driver should be able to > work with multiple devices. e.g In my 2P EPYC configuration, security > processor driver

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-08 Thread Borislav Petkov
On Sun, Oct 08, 2017 at 08:30:47AM -0500, Brijesh Singh wrote: > During the device probe, sev_ops_init() will be called for every device > instance which claims to support the SEV.  One of the device will be > 'master' but we don't the master until we probe all the instances. Hence > the probe for

Re: [Part2 PATCH v5.1 12.1/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-07 Thread Borislav Petkov
gt; - an in-kernel API to communicate with the SEV firmware. The API can be >used by the hypervisor to create encryption context for a SEV guest. > > - a userspace IOCTL to manage the platform certificates. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borisla

Re: [Part2 PATCH v5.1 12.2/31] crypto: ccp: Define SEV userspace ioctl and command id

2017-10-07 Thread Borislav Petkov
On Fri, Oct 06, 2017 at 08:06:00PM -0500, Brijesh Singh wrote: > Add a include file which defines the ioctl and command id used for > issuing SEV platform management specific commands. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov > Cc: Herbert

Re: [Part2 PATCH v5 12/31] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-06 Thread Borislav Petkov
userspace IOCTL to manage the platform certificates." > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov > Cc: Herbert Xu > Cc: Gary Hook > Cc: Tom Lendacky > Cc: linux-crypto@vger.kernel.org > Cc: k...@vger.kernel.org > Cc: linux-k

Re: [Part2 PATCH v5 11/31] crypto: ccp: Define SEV key management command id

2017-10-05 Thread Borislav Petkov
ecification.pdf > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov > Cc: Herbert Xu > Cc: Gary Hook > Cc: Tom Lendacky > Cc: linux-crypto@vger.kernel.org > Cc: k...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Signed-off

Re: [Part2 PATCH v5 10/31] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-10-05 Thread Borislav Petkov
The PSP is a dedicated processor that provides support for key management commands in Secure Encrypted Virtualization (SEV) mode, along with software-based Trusted Execution Environment (TEE) to - enable the third-party trusted applications. +enable third-party trusted applica

Re: [Part2 PATCH v5 09/31] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 08:13:50AM -0500, Brijesh Singh wrote: > From: Borislav Petkov > > This is AMD-specific hardware so present it in Kconfig only when AMD > CPU support is enabled or on ARM64 where it is also used. > > Signed-off-by: > Signed-off-by: Borislav Petkov

Re: [Part2 PATCH v4.1 07/29] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 04:12:37PM +0530, P J P wrote: > Quick glance would work if it is readable. Currently it is not if > one is viewing it in 80 cols screen/window. They do that. Writing > return on the same line does not add specific value IMO. Then you'll have to scroll to the right like y

Re: [Part2 PATCH v4.1 07/29] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 03:24:36PM +0530, P J P wrote: > It appears to cross 80 columns limit, checkpatch.pl throws warnings. Adding > new line would be consistent with coding style. The 80 cols rule is not a hard one and checkpatch should not override common sense. This is a function which maps

Re: [Part2 PATCH v4.1 07/29] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 12:26:11PM +0530, P J P wrote: > Each return above needs to be on its own line. ... because? -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

Re: [Part2 Patch v4.2] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-10-04 Thread Borislav Petkov
On Wed, Oct 04, 2017 at 12:06:42PM +0530, P J P wrote: > Needs to kfree(sp->psp_data) before setting to NULL. Not if it is allocated with devm_kzalloc(). -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-10-03 Thread Borislav Petkov
hing. Your patch should concentrate only on adding the PSP and its dependencies. Thx. --- From: Borislav Petkov Date: Sat, 30 Sep 2017 10:06:27 +0200 Subject: [PATCH] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support This is AMD-specific hardware so present it in Kc

Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-30 Thread Borislav Petkov
On Sat, Sep 30, 2017 at 10:55:25AM -0500, Brijesh Singh wrote: > CRYPTO_DEV_CCP_DD is supported on aarch64 and x86. Whereas the PSP > interface I am adding is available on x86 only hence its safe to add add > depend on CPU_SUP_AMD for CRYPTO_DEV_SP_PSP. I think just from having CRYPTO_DEV_CCP_DD d

Re: [PATCH] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support

2017-09-30 Thread Borislav Petkov
On Sat, Sep 30, 2017 at 09:06:26AM -0500, Brijesh Singh wrote: > > diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig > > index 627f3e61dcac..f58a6521270b 100644 > > --- a/drivers/crypto/ccp/Kconfig > > +++ b/drivers/crypto/ccp/Kconfig > > @@ -1,5 +1,6 @@ > > config CRYPTO_DEV_CC

[PATCH] crypto: ccp: Build the AMD secure processor driver only with AMD CPU support

2017-09-30 Thread Borislav Petkov
Hi, just a small Kconfig correction. Feel free to add it to your patchset. Thx. --- From: Borislav Petkov This is AMD-specific hardware so present it in Kconfig only when AMD CPU support is enabled. Signed-off-by: Borislav Petkov Cc: Brijesh Singh Cc: Tom Lendacky Cc: Gary Hook Cc

Re: [Part2 PATCH v4 05/29] crypto: ccp: Add Platform Security Processor (PSP) device support

2017-09-29 Thread Borislav Petkov
ffon", 3 * &spell, 3) in my .vimrc And I'm pretty sure you can do a similar thing with other editors. > software-based Trusted Executation Environment (TEE) to enable the > third-party trusted applications. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář"

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

2017-09-13 Thread Borislav Petkov
On Mon, Jul 24, 2017 at 03:02:40PM -0500, Brijesh Singh wrote: > AMDs new Secure Encrypted Virtualization (SEV) feature allows the memory > contents of a virtual machine to be transparently encrypted with a key > unique to the guest VM. The programming and management of the encryption > keys are ha

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

2017-09-12 Thread Borislav Petkov
On Tue, Sep 12, 2017 at 10:32:13AM -0500, Brijesh Singh wrote: > 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 >

  1   2   >