On Sun, Mar 04, 2018 at 05:02:45PM +0100, Stefan Sperling wrote: > On my T5220 LDOM guests cannot boot from softraid because ofwboot > crashes with a "Fast Data Access MMU Miss" while loading the kernel: > > >> OpenBSD BOOT 1.9 > ERROR: /iscsi-hba: No iscsi-network-bootpath property > sr0* > Passphrase: > Booting sr0:a/bsd > 8480888@0x1000000 > ERROR: Last Trap: Fast Data Access MMU Miss > > I've tracked down the failure to a crash in OF_open() called in sr_strategy(). > There is no missing OF_close() call. So it seems a memory/resource leak > of some kind is happening in firmware during OF_open()/OF_close(). > > Affected firmware version info: > > SP firmware 3.0.12.8.a > SP firmware build number: 108523 > SP firmware date: Fri Mar 11 07:19:16 PST 2016 > SP filesystem version: 0.1.22 > > hypervisor_version = Hypervisor 1.10.7.h 2016/03/11 07:13 > obp_version = OpenBoot 4.33.6.g 2016/03/11 06:05 > post_version = POST 4.33.6.g 2016/03/11 06:15 > status = OpenBSD running > sysfw_version = Sun System Firmware 7.4.10.a 2016/03/11 07:45 > > The diff below allows a guest to boot from softraid on this machine.
I haven't had any feedback on the previous diff. However, I felt it was a bit of a hack, so I tried to come up with a cleaner solution. The diff below opens the softraid boot chunk early on and keeps the handle open until a kernel has been loaded from the softraid volume. This means sr_strategy() does not have to worry about obtaining a handle from the firmware anymore. Also consolidate some repeated lines of code in sr_strategy(). Tested on a T5220 ldom guest (CRYPTO) and a v240 machine (RAID1). Index: boot.c =================================================================== RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/boot.c,v retrieving revision 1.27 diff -u -p -r1.27 boot.c --- boot.c 11 Sep 2016 17:53:26 -0000 1.27 +++ boot.c 12 Mar 2018 10:26:23 -0000 @@ -248,6 +248,8 @@ loadfile(int fd, char *args) close(fd); #ifdef SOFTRAID + if (bootdev_dip) + OF_close(bootdev_dip->sr_handle); sr_clear_keys(); #endif chain(entry, args, ssym, esym); @@ -283,12 +285,11 @@ fail: } #ifdef SOFTRAID -/* Set bootdev_dip to the software boot volume, if specified. */ +/* Set bootdev_dip to the softraid boot volume, if specified. */ static int srbootdev(const char *bootline) { struct sr_boot_volume *bv; - struct diskinfo *dip; int unit; bootdev_dip = NULL; @@ -320,9 +321,23 @@ srbootdev(const char *bootline) return EPERM; if (bv->sbv_diskinfo == NULL) { + struct sr_boot_chunk *bc; + struct diskinfo *dip, *bc_dip; + int sr_handle; + + /* All reads will come from the boot chunk. */ + bc = sr_vol_boot_chunk(bv); + if (bc == NULL) + return ENXIO; + bc_dip = (struct diskinfo *)bc->sbc_diskinfo; + sr_handle = OF_open(bc_dip->path); + if (sr_handle == -1) + return EIO; + dip = alloc(sizeof(struct diskinfo)); bzero(dip, sizeof(*dip)); dip->sr_vol = bv; + dip->sr_handle = sr_handle; bv->sbv_diskinfo = dip; } @@ -331,7 +346,8 @@ srbootdev(const char *bootline) /* Attempt to read disklabel. */ bv->sbv_part = 'c'; - if (sr_getdisklabel(bv, &dip->disklabel)) { + if (sr_getdisklabel(bv, &bootdev_dip->disklabel)) { + OF_close(bootdev_dip->sr_handle); free(bv->sbv_diskinfo, sizeof(struct diskinfo)); bv->sbv_diskinfo = NULL; bootdev_dip = NULL; Index: disk.h =================================================================== RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/disk.h,v retrieving revision 1.1 diff -u -p -r1.1 disk.h --- disk.h 26 Nov 2014 20:30:41 -0000 1.1 +++ disk.h 12 Mar 2018 10:26:23 -0000 @@ -36,7 +36,10 @@ struct diskinfo { char path[256]; struct disklabel disklabel; + + /* Used during softraid boot. */ struct sr_boot_volume *sr_vol; + int sr_handle; /* open handle for reading from boot chunk */ TAILQ_ENTRY(diskinfo) list; }; Index: ofdev.c =================================================================== RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/ofdev.c,v retrieving revision 1.25 diff -u -p -r1.25 ofdev.c --- ofdev.c 1 Oct 2015 16:08:20 -0000 1.25 +++ ofdev.c 12 Mar 2018 10:26:23 -0000 @@ -125,8 +125,8 @@ strategy(void *devdata, int rw, daddr32_ #ifdef SOFTRAID /* Intercept strategy for softraid volumes. */ if (dev->type == OFDEV_SOFTRAID) - return sr_strategy(bootdev_dip->sr_vol, rw, - blk, size, buf, rsize); + return sr_strategy(bootdev_dip->sr_vol, bootdev_dip->sr_handle, + rw, blk, size, buf, rsize); #endif if (dev->type != OFDEV_DISK) panic("strategy"); Index: softraid_sparc64.c =================================================================== RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/softraid_sparc64.c,v retrieving revision 1.2 diff -u -p -r1.2 softraid_sparc64.c --- softraid_sparc64.c 11 Sep 2016 17:53:26 -0000 1.2 +++ softraid_sparc64.c 12 Mar 2018 10:26:23 -0000 @@ -306,9 +306,25 @@ srprobe(void) free(md, SR_META_SIZE * DEV_BSIZE); } +struct sr_boot_chunk * +sr_vol_boot_chunk(struct sr_boot_volume *bv) +{ + struct sr_boot_chunk *bc = NULL; + + if (bv->sbv_level == 1 || bv->sbv_level == 'C' ) { /* RAID1 or CRYPTO */ + /* Select first online chunk. */ + SLIST_FOREACH(bc, &bv->sbv_chunks, sbc_link) + if (bc->sbc_state == BIOC_SDONLINE) + break; + } + + return bc; +} + + int -sr_strategy(struct sr_boot_volume *bv, int rw, daddr32_t blk, size_t size, - void *buf, size_t *rsize) +sr_strategy(struct sr_boot_volume *bv, int sr_handle, int rw, daddr32_t blk, + size_t size, void *buf, size_t *rsize) { struct diskinfo *sr_dip, *dip; struct partition *pp; @@ -320,7 +336,6 @@ sr_strategy(struct sr_boot_volume *bv, i u_char iv[8]; u_char *bp; int err; - int ihandle; /* We only support read-only softraid. */ if (rw != F_READ) @@ -331,55 +346,28 @@ sr_strategy(struct sr_boot_volume *bv, i blk += DL_GETPOFFSET(&sr_dip->disklabel.d_partitions[bv->sbv_part - 'a']); + bc = sr_vol_boot_chunk(bv); + if (bc == NULL) + return ENXIO; + + dip = (struct diskinfo *)bc->sbc_diskinfo; + pp = &dip->disklabel.d_partitions[bc->sbc_part - 'a']; + bzero(&ofdev, sizeof(ofdev)); + ofdev.handle = sr_handle; + ofdev.type = OFDEV_DISK; + ofdev.bsize = DEV_BSIZE; + ofdev.partoff = DL_GETPOFFSET(pp); + if (bv->sbv_level == 0) { return ENOTSUP; } else if (bv->sbv_level == 1) { - - /* Select first online chunk. */ - SLIST_FOREACH(bc, &bv->sbv_chunks, sbc_link) - if (bc->sbc_state == BIOC_SDONLINE) - break; - if (bc == NULL) - return EIO; - - dip = (struct diskinfo *)bc->sbc_diskinfo; - pp = &dip->disklabel.d_partitions[bc->sbc_part - 'a']; blk += bv->sbv_data_blkno; /* XXX - If I/O failed we should try another chunk... */ - ihandle = OF_open(dip->path); - if (ihandle == -1) - return EIO; - bzero(&ofdev, sizeof(ofdev)); - ofdev.handle = ihandle; - ofdev.type = OFDEV_DISK; - ofdev.bsize = DEV_BSIZE; - ofdev.partoff = DL_GETPOFFSET(pp); err = strategy(&ofdev, rw, blk, size, buf, rsize); - OF_close(ihandle); return err; } else if (bv->sbv_level == 'C') { - - /* Select first online chunk. */ - SLIST_FOREACH(bc, &bv->sbv_chunks, sbc_link) - if (bc->sbc_state == BIOC_SDONLINE) - break; - if (bc == NULL) - return EIO; - - dip = (struct diskinfo *)bc->sbc_diskinfo; - pp = &dip->disklabel.d_partitions[bc->sbc_part - 'a']; - - ihandle = OF_open(dip->path); - if (ihandle == -1) - return EIO; - bzero(&ofdev, sizeof(ofdev)); - ofdev.handle = ihandle; - ofdev.type = OFDEV_DISK; - ofdev.bsize = DEV_BSIZE; - ofdev.partoff = DL_GETPOFFSET(pp); - /* XXX - select correct key. */ aes_xts_setkey(&ctx, (u_char *)bv->sbv_keys, 64); @@ -395,7 +383,6 @@ sr_strategy(struct sr_boot_volume *bv, i printf("Read from crypto volume failed " "(read %d bytes): %s\n", *rsize, strerror(err)); - OF_close(ihandle); return err; } bcopy(&blkno, iv, sizeof(blkno)); @@ -404,7 +391,6 @@ sr_strategy(struct sr_boot_volume *bv, i aes_xts_decrypt(&ctx, bp + j); } *rsize = nsect * DEV_BSIZE; - OF_close(ihandle); return err; } else Index: softraid_sparc64.h =================================================================== RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/softraid_sparc64.h,v retrieving revision 1.3 diff -u -p -r1.3 softraid_sparc64.h --- softraid_sparc64.h 11 Sep 2016 17:53:26 -0000 1.3 +++ softraid_sparc64.h 12 Mar 2018 10:26:23 -0000 @@ -21,8 +21,9 @@ void srprobe(void); +struct sr_boot_chunk *sr_vol_boot_chunk(struct sr_boot_volume *); const char *sr_getdisklabel(struct sr_boot_volume *, struct disklabel *); -int sr_strategy(struct sr_boot_volume *, int, daddr32_t, size_t, +int sr_strategy(struct sr_boot_volume *, int, int, daddr32_t, size_t, void *, size_t *); #endif /* _SOFTRAID_SPARC64_H */ Index: vers.c =================================================================== RCS file: /cvs/src/sys/arch/sparc64/stand/ofwboot/vers.c,v retrieving revision 1.10 diff -u -p -r1.10 vers.c --- vers.c 18 Sep 2016 16:36:09 -0000 1.10 +++ vers.c 12 Mar 2018 10:26:23 -0000 @@ -1 +1 @@ -const char version[] = "1.9"; +const char version[] = "1.10";