| Message ID | 20190605023616.26893-1-npiggin@gmail.com |
|---|---|
| Headers | show |
| Series | WIP VMM for OPAL boot | expand |
Nicholas Piggin <npiggin@gmail.com> writes: > This is my current work in progress, I've gradually been adding bits > and fixing bugs so time for another rebase. It's been booting pretty > reliably on my P9, I've next been trying to bring VMM mode back up > when we fast-reboot which is still a bit buggy but almost works. Cool. I'm chasing the secure boot problems, which has now become "get all the genuine secure "rom" stuff running in Mambo". This is the most obivous "Something is going to go wrong without it" bit I have, but that doesn't magically fix things, nor explain why everything stops suddenly. --- a/libstb/cvc.c +++ b/libstb/cvc.c @@ -167,6 +167,7 @@ static int cvc_reserved_mem_init(struct dt_node *parent) { return -1; } addr = dt_get_address(cvc_resv_mem, 0, &size); + vm_map_global_text("STB-CVC", addr, size); cvc_register(addr, addr + size-1); /* I'll update you on my random progress in seeing if I can get it all going in Mambo at least.
Stewart Smith <stewart@linux.ibm.com> writes: > Nicholas Piggin <npiggin@gmail.com> writes: >> This is my current work in progress, I've gradually been adding bits >> and fixing bugs so time for another rebase. It's been booting pretty >> reliably on my P9, I've next been trying to bring VMM mode back up >> when we fast-reboot which is still a bit buggy but almost works. > > Cool. I'm chasing the secure boot problems, which has now become "get > all the genuine secure "rom" stuff running in Mambo". > > This is the most obivous "Something is going to go wrong without it" bit > I have, but that doesn't magically fix things, nor explain why > everything stops suddenly. As mentioned on slack, I found the problem was that we had a "li %r2,0" in the mix. The Container Verification Code is a blob that we get passed in from hostboot (well, all the way from the hostboot bootloader actually) that has two entry points: one for validating a secureboot header and the other for computing sha512 checksum of a blob. In skiboot, the libstb/cvc.c code finds it, which on P9 is from a reserved memory region, and on P8 it's by copying it from an actual ROM that's on chip that needs to be accessed with cache inhibited reads. The asm/cvc_entry.S wrapper, as part of calling the CVC function, sets %r2 to zero. Thus, when we take the ISI for the page of text, we end up in vm_isi() without %r2 set to anything sane, and we end up hitting the assert(!_dw0) in htab_install because we're poking around where we're not meant to. Naturally, all the (soon multiple levels) of exception_entry() and abort() without a sane %r2 goes about as well as you'd expect and we eventually xstop. For at least a quick hack (I haven't thought about it beyond "hey, we're not using that register") I stashed the skiboot TOC in HSPRG0 and pulled it in in _exception. So, the below patch is on top of this series that (at least in Mambo, I haven't tested on hardware yet), will run the CVC code and boot a kernel. I'm not sure what's up with my mambo tcl to ensure I get the references/size right for the CVC code in the device tree, so have quickly hacked around it with a if (size==0) size = 64*1024 in cvc.c (maybe the Masters of Mambo can enlighten me). I'm almost interested to try and pull out the CVC code and use that rather than the "softrom" hack I put in a couple of years ago. diff --git a/asm/head.S b/asm/head.S index 18a9c12cfb06..ae9a73b1fd7b 100644 --- a/asm/head.S +++ b/asm/head.S @@ -238,6 +238,7 @@ _exception: mr %r3,%r1 LOAD_IMM64(%r4, SKIBOOT_BASE) LOAD_IMM32(%r5, exception_entry_foo - __head) + mfspr %r2, SPR_HSPRG0 add %r4,%r4,%r5 mtctr %r4 bctr @@ -367,6 +368,7 @@ boot_entry: LOAD_IMM32(%r2,__toc_start - __head) LOAD_IMM64(%r29, SKIBOOT_BASE) add %r2,%r2,%r29 + mtspr SPR_HSPRG0, %r2 /* Fixup our MSR (remove TA) */ LOAD_IMM64(%r3, (MSR_HV | MSR_SF)) diff --git a/core/flash.c b/core/flash.c index 420ae3244ae9..5555856e681a 100644 --- a/core/flash.c +++ b/core/flash.c @@ -762,11 +762,10 @@ done_reading: * Verify and measure the retrieved PNOR partition as part of the * secure boot and trusted boot requirements */ -#if 0 -// XXX: this chekstops + secureboot_verify(id, buf, *len); trustedboot_measure(id, buf, *len); -#endif + /* Find subpartition */ if (subid != RESOURCE_SUBID_NONE) { memmove(buf, bufp, content_size); diff --git a/core/init.c b/core/init.c index 0fad02f67550..2f620c17b6c9 100644 --- a/core/init.c +++ b/core/init.c @@ -81,7 +81,7 @@ struct debug_descriptor debug_descriptor = { #ifdef DEBUG .console_log_levels = (PR_TRACE << 4) | PR_DEBUG, #else - .console_log_levels = (PR_DEBUG << 4) | PR_NOTICE, + .console_log_levels = (PR_DEBUG << 4) | PR_DEBUG, #endif }; diff --git a/core/mem_region.c b/core/mem_region.c index fe89cedb6e79..d33093c61e97 100644 --- a/core/mem_region.c +++ b/core/mem_region.c @@ -154,7 +154,7 @@ static struct alloc_hdr *next_hdr(const struct mem_region *region, } #if POISON_MEM_REGION == 1 -static void mem_poison(struct free_hdr *f) +static void mem_poison(struct free_hdr *f __unused) { } #endif diff --git a/core/vm.c b/core/vm.c index 1bf5e4bd81a8..c741c3e55a8f 100644 --- a/core/vm.c +++ b/core/vm.c @@ -360,7 +360,7 @@ void vm_map_global(const char *name, unsigned long addr, unsigned long len, bool __vm_map(name, addr, len, addr, true, rw, false, ci, false); } -static void vm_map_global_text(const char *name, unsigned long addr, unsigned long len) +void vm_map_global_text(const char *name, unsigned long addr, unsigned long len) { __vm_map(name, addr, len, addr, true, false, true, false, false); } @@ -611,15 +611,26 @@ bool __nomcount vm_isi(uint64_t nia) { struct cpu_thread *c = this_cpu(); bool vm_setup = c->vm_setup; + struct vm_map *vmm; assert(vm_setup); - if (nia < (unsigned long)_stext) - return false; - if (nia >= (unsigned long)_etext) - return false; + lock(&vm_maps_lock); + list_for_each(&vm_maps, vmm, list) { + assert(vmm->pa == vmm->address); + if (nia >= vmm->address && nia < vmm->address + vmm->length) { + if (!vmm->executable) + printf("Page fault at NIA:0x%016llx NX mapping!\n", nia); + goto found; + } + } - c->vm_setup = false; + prerror("Page fault, no mapping for NIA:0x%016llx !\n", nia); + +found: + unlock(&vm_maps_lock); + c->vm_setup = false; + printf("Page fault, mapping NIA:0x%016llx !\n", nia); htab_install(nia, nia, 0, 1, 0, false); c->vm_setup = true; diff --git a/external/mambo/skiboot.tcl b/external/mambo/skiboot.tcl index 9f424dd9af23..f7014aed1658 100644 --- a/external/mambo/skiboot.tcl +++ b/external/mambo/skiboot.tcl @@ -306,6 +306,19 @@ if { [info exists env(SKIBOOT_NVRAM)] } { mysim mcm 0 memory fread $fake_nvram_start $fake_nvram_size $fake_nvram_file } +set cvc_code_start [expr $fake_nvram_start + $fake_nvram_size] +set cvc_code_end $cvc_code_start +set cvc_code_size 0 +if { [info exists env(SKIBOOT_CVC_CODE)] } { + + set cvc_file $env(SKIBOOT_CVC_CODE) + + set cvc_code_size [file size $cvc_file] + mysim mcm 0 memory fread $cvc_code_start $cvc_code_size $cvc_file + set cvc_code_end [expr $cvc_code_start + $cvc_code_size] +} + + # Add device tree entry for NVRAM set reserved_memory [mysim of addchild $root_node "reserved-memory" ""] mysim of addprop $reserved_memory int "#size-cells" 2 @@ -322,6 +335,18 @@ set reg [list $fake_nvram_start $fake_nvram_size ] mysim of addprop $fake_nvram_node array64 "reg" reg mysim of addprop $fake_nvram_node empty "name" "ibm,fake-nvram" +set hb [mysim of addchild $root_node "ibm,hostboot" ""] +set hb_reserved_memory [mysim of addchild $hb "reserved-memory" ""] +set hb_cvc_code_node [mysim of addchild $hb_reserved_memory "ibm,secure-crypt-algo-code" [format %x $cvc_code_start]] +set reg [list $cvc_code_start $cvc_code_size] +mysim of addprop $hb_cvc_code_node array64 "reg" reg +mysim of addprop $hb_cvc_code_node empty "name" "ibm,secure-crypt-algo-code" + +set cvc_code_node [mysim of addchild $reserved_memory "ibm,secure-crypt-algo-code" [format %x $cvc_code_start]] +set reg [list $cvc_code_start $cvc_code_size] +mysim of addprop $cvc_code_node array64 "reg" reg +mysim of addprop $cvc_code_node empty "name" "ibm,secure-crypt-algo-code" + set opal_node [mysim of addchild $root_node "ibm,opal" ""] # Allow P9 to use all idle states @@ -546,10 +571,12 @@ mconfig enable_stb SKIBOOT_ENABLE_MAMBO_STB 0 if { [info exists env(SKIBOOT_ENABLE_MAMBO_STB)] } { set stb_node [ mysim of addchild $root_node "ibm,secureboot" "" ] - mysim of addprop $stb_node string "compatible" "ibm,secureboot-v1-softrom" +# mysim of addprop $stb_node string "compatible" "ibm,secureboot-v1-softrom" + mysim of addprop $stb_node string "compatible" "ibm,secureboot-v2" # mysim of addprop $stb_node string "secure-enabled" "" mysim of addprop $stb_node string "trusted-enabled" "" mysim of addprop $stb_node string "hash-algo" "sha512" + mysim of addprop $stb_node int "hw-key-hash-size" 64 set hw_key_hash {} lappend hw_key_hash 0x40d487ff lappend hw_key_hash 0x7380ed6a @@ -568,6 +595,22 @@ if { [info exists env(SKIBOOT_ENABLE_MAMBO_STB)] } { lappend hw_key_hash 0xfb708535 lappend hw_key_hash 0x1d01d6d1 mysim of addprop $stb_node array "hw-key-hash" hw_key_hash + + set cvc_node [ mysim of addchild $stb_node "ibm,cvc" "" ] + mysim of addprop $cvc_node string "compatible" "ibm,container-verification-code" + mysim of addprop $cvc_node int "memory-region" $hb_cvc_code_node + + set sha_node [ mysim of addchild $cvc_node "ibm,cvc-service" [format %x 0x40]] + mysim of addprop $sha_node string "name" "ibm,cvc-service" + mysim of addprop $sha_node string "compatible" "ibm,cvc-sha512" + mysim of addprop $sha_node int "reg" 0x40 + mysim of addprop $sha_node int "version" 1 + + set verify_node [ mysim of addchild $cvc_node "ibm,cvc-service" [format %x 0x50]] + mysim of addprop $verify_node string "name" "ibm,cvc-service" + mysim of addprop $verify_node string "compatible" "ibm,cvc-verify" + mysim of addprop $verify_node int "reg" 0x50 + mysim of addprop $verify_node int "version" 1 } # Kernel command line args, appended to any from the device tree diff --git a/include/skiboot.h b/include/skiboot.h index 98a69ef1d68c..32f6307a2f45 100644 --- a/include/skiboot.h +++ b/include/skiboot.h @@ -352,6 +352,7 @@ extern int fake_nvram_write(uint32_t offset, void *src, uint32_t size); bool vm_realmode(void); void vm_map_global(const char *name, unsigned long addr, unsigned long len, bool rw, bool ci); +void vm_map_global_text(const char *name, unsigned long addr, unsigned long len); void vm_unmap_global(unsigned long addr, unsigned long len); void *vm_map(unsigned long addr, unsigned long len, bool rw); void vm_unmap(unsigned long addr, unsigned long len); diff --git a/libstb/cvc.c b/libstb/cvc.c index d46617b29070..4a3a08f5f4f5 100644 --- a/libstb/cvc.c +++ b/libstb/cvc.c @@ -167,6 +167,9 @@ static int cvc_reserved_mem_init(struct dt_node *parent) { return -1; } addr = dt_get_address(cvc_resv_mem, 0, &size); + if (size == 0) // MAMBO HACK + size = 64*1024; + vm_map_global_text("STB-CVC", addr, size); cvc_register(addr, addr + size-1); /* diff --git a/libstb/secureboot.c b/libstb/secureboot.c index 1578f52ecdfc..2f0f3298e5bc 100644 --- a/libstb/secureboot.c +++ b/libstb/secureboot.c @@ -176,6 +176,7 @@ void secureboot_init(void) int secureboot_verify(enum resource_id id, void *buf, size_t len) { const char *name; + void *vbuf; uint64_t log; int rc = -1; @@ -194,7 +195,9 @@ int secureboot_verify(enum resource_id id, void *buf, size_t len) return -1; } - rc = call_cvc_verify(buf, len, hw_key_hash, hw_key_hash_size, &log); + vbuf = vm_map((unsigned long)buf, len, false); + rc = call_cvc_verify(vbuf, len, hw_key_hash, hw_key_hash_size, &log); + vm_unmap((unsigned long)buf, len); if (rc == OPAL_SUCCESS) { prlog(PR_NOTICE, "%s verified\n", name); diff --git a/libstb/trustedboot.c b/libstb/trustedboot.c index f431e338acd4..ba922bb67862 100644 --- a/libstb/trustedboot.c +++ b/libstb/trustedboot.c @@ -174,7 +174,7 @@ out_free: int trustedboot_measure(enum resource_id id, void *buf, size_t len) { uint8_t digest[SHA512_DIGEST_LENGTH]; - void *buf_aux; + void *buf_aux, *vbuf; size_t len_aux; const char *name; TPM_Pcr pcr; @@ -232,7 +232,9 @@ int trustedboot_measure(enum resource_id id, void *buf, size_t len) len_aux = len; } - rc = call_cvc_sha512(buf_aux, len_aux, digest, SHA512_DIGEST_LENGTH); + vbuf = vm_map((unsigned long)buf_aux, len_aux, false); + rc = call_cvc_sha512(vbuf, len_aux, digest, SHA512_DIGEST_LENGTH); + vm_unmap((unsigned long)buf_aux, len_aux); if (rc == OPAL_SUCCESS) { prlog(PR_NOTICE, "%s hash calculated\n", name);
Stewart Smith's on June 7, 2019 7:56 am: > Stewart Smith <stewart@linux.ibm.com> writes: >> Nicholas Piggin <npiggin@gmail.com> writes: >>> This is my current work in progress, I've gradually been adding bits >>> and fixing bugs so time for another rebase. It's been booting pretty >>> reliably on my P9, I've next been trying to bring VMM mode back up >>> when we fast-reboot which is still a bit buggy but almost works. >> >> Cool. I'm chasing the secure boot problems, which has now become "get >> all the genuine secure "rom" stuff running in Mambo". >> >> This is the most obivous "Something is going to go wrong without it" bit >> I have, but that doesn't magically fix things, nor explain why >> everything stops suddenly. > > As mentioned on slack, I found the problem was that we had a "li %r2,0" > in the mix. > > The Container Verification Code is a blob that we get passed in from > hostboot (well, all the way from the hostboot bootloader actually) that > has two entry points: one for validating a secureboot header and the > other for computing sha512 checksum of a blob. > > In skiboot, the libstb/cvc.c code finds it, which on P9 is from a > reserved memory region, and on P8 it's by copying it from an actual ROM > that's on chip that needs to be accessed with cache inhibited reads. > > The asm/cvc_entry.S wrapper, as part of calling the CVC function, sets > %r2 to zero. Thus, when we take the ISI for the page of text, we end up > in vm_isi() without %r2 set to anything sane, and we end up hitting the > assert(!_dw0) in htab_install because we're poking around where we're > not meant to. Naturally, all the (soon multiple levels) of > exception_entry() and abort() without a sane %r2 goes about as well as > you'd expect and we eventually xstop. > > For at least a quick hack (I haven't thought about it beyond "hey, we're > not using that register") I stashed the skiboot TOC in HSPRG0 and pulled > it in in _exception. > > So, the below patch is on top of this series that (at least in Mambo, I > haven't tested on hardware yet), will run the CVC code and boot a > kernel. Awesome, that r2 clobber was a good catch. I think we can make a separate patch that always loads up the skiboot r2 in the exception path (preserving the original), which will also prevent bad crashes anywhere else that r2 is modified on purpose or by mistake. > > I'm not sure what's up with my mambo tcl to ensure I get the > references/size right for the CVC code in the device tree, so have > quickly hacked around it with a if (size==0) size = 64*1024 in cvc.c > (maybe the Masters of Mambo can enlighten me). What are you doing there, is that creating a CVC blob that actually runs when you boot mambo? > > I'm almost interested to try and pull out the CVC code and use that > rather than the "softrom" hack I put in a couple of years ago. >
Nicholas Piggin <npiggin@gmail.com> writes: > Stewart Smith's on June 7, 2019 7:56 am: >> Stewart Smith <stewart@linux.ibm.com> writes: >>> Nicholas Piggin <npiggin@gmail.com> writes: >>>> This is my current work in progress, I've gradually been adding bits >>>> and fixing bugs so time for another rebase. It's been booting pretty >>>> reliably on my P9, I've next been trying to bring VMM mode back up >>>> when we fast-reboot which is still a bit buggy but almost works. >>> >>> Cool. I'm chasing the secure boot problems, which has now become "get >>> all the genuine secure "rom" stuff running in Mambo". >>> >>> This is the most obivous "Something is going to go wrong without it" bit >>> I have, but that doesn't magically fix things, nor explain why >>> everything stops suddenly. >> >> As mentioned on slack, I found the problem was that we had a "li %r2,0" >> in the mix. >> >> The Container Verification Code is a blob that we get passed in from >> hostboot (well, all the way from the hostboot bootloader actually) that >> has two entry points: one for validating a secureboot header and the >> other for computing sha512 checksum of a blob. >> >> In skiboot, the libstb/cvc.c code finds it, which on P9 is from a >> reserved memory region, and on P8 it's by copying it from an actual ROM >> that's on chip that needs to be accessed with cache inhibited reads. >> >> The asm/cvc_entry.S wrapper, as part of calling the CVC function, sets >> %r2 to zero. Thus, when we take the ISI for the page of text, we end up >> in vm_isi() without %r2 set to anything sane, and we end up hitting the >> assert(!_dw0) in htab_install because we're poking around where we're >> not meant to. Naturally, all the (soon multiple levels) of >> exception_entry() and abort() without a sane %r2 goes about as well as >> you'd expect and we eventually xstop. >> >> For at least a quick hack (I haven't thought about it beyond "hey, we're >> not using that register") I stashed the skiboot TOC in HSPRG0 and pulled >> it in in _exception. >> >> So, the below patch is on top of this series that (at least in Mambo, I >> haven't tested on hardware yet), will run the CVC code and boot a >> kernel. > > Awesome, that r2 clobber was a good catch. > > I think we can make a separate patch that always loads up > the skiboot r2 in the exception path (preserving the original), > which will also prevent bad crashes anywhere else that r2 is > modified on purpose or by mistake. Yeah, I hit a bad r2 again in the fast reboot path with my hack, as of course linux used HSPRG0 >> I'm not sure what's up with my mambo tcl to ensure I get the >> references/size right for the CVC code in the device tree, so have >> quickly hacked around it with a if (size==0) size = 64*1024 in cvc.c >> (maybe the Masters of Mambo can enlighten me). > > What are you doing there, is that creating a CVC blob that actually > runs when you boot mambo? Yeah, I grabbed one from a running machine (just dumped out the memory range of it) and loaded it up in Mambo. We could also compile our own I believe, the securerom code in Hostboot doesn't need a complex makefile.