> Date: Wed, 10 Mar 2021 20:42:42 +0900 (JST)
> From: YASUOKA Masahiko <yasu...@openbsd.org>
>
> Sorry for making noise, let me update the diff.
>
> > + if (ed->blkio->Media->IoAlign > 1 &&
> > + ((UINTN)buf + i_lblks * DEV_BSIZE)
> > + % ed->blkio->Media->IoAlign == 0)
>
> first condition was reversed..
>
> On Wed, 10 Mar 2021 20:35:41 +0900 (JST)
> YASUOKA Masahiko <yasu...@openbsd.org> wrote:
> > efiboot cannot load the kernel properly on some machines if booted
> > from CD-ROM. In that case boot fails with a message like follow:
> >
> > booting cd0a:..... [359648read symbols: Unknown error: code 255
> >
> > As far as Asou and my test, this happens on hosts on VMware ESXi 6.7,
> > 7.0 and asou's physical machine.
> >
> > The problem happens because efiboot calls ReadBlocks function with an
> > unaligned pointer for medias which requires an aligned pointer. When
> > efiboot loads a kernel, the pointer becomes unaligned since there is
> > an ELF section located at unaligned place in CD-ROM. Previously our
> > kernel didn't have such a section but it does after switching lld as
> > the default linker.
> >
> > For test, let me show sample commands which creates a bootable cdrom
> > image for EFI:
> >
> > mkdir -p efiboot/EFI/BOOT
> > cp /usr/mdec/BOOTX64.EFI efiboot/EFI/BOOT
> > makefs -M 1m -m 1m -t msdos -o fat_type=12,sectors_per_cluster=1 \
> > efiboot.img efiboot
> > mkdir -p cd-dir/etc
> > cp bsd.rd cd-dir/
> > echo "set image bsd.rd" > cd-dir/etc/boot.conf
> > makefs -t cd9660 -o
> > 'rockridge,bootimage=i386;/usr/mdec/cdbr,no-emul-boot,allow-multidot,bootimage=efi;efiboot.img,no-emul-boot'
> > \
> > boot.iso cd-dir
> >
> > the diff is to fix the problem.
> >
> > ok?
Maybe it is better to always bounce through an aligned buffer? That
would make the code a little bit slower but a lot simpler. And the
overhead of doing the copy should be small compared to the actual I/O.
> Index: sys/arch/amd64/stand/efiboot/efidev.c
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/sys/arch/amd64/stand/efiboot/efidev.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 efidev.c
> --- sys/arch/amd64/stand/efiboot/efidev.c 9 Dec 2020 18:10:18 -0000
> 1.32
> +++ sys/arch/amd64/stand/efiboot/efidev.c 10 Mar 2021 11:41:39 -0000
> @@ -84,7 +84,7 @@ efid_init(struct diskinfo *dip, void *ha
> static EFI_STATUS
> efid_io(int rw, efi_diskinfo_t ed, u_int off, int nsect, void *buf)
> {
> - u_int blks, lba, i_lblks, i_tblks, i_nblks;
> + u_int i, blks, lba, i_lblks, i_tblks, i_nblks;
> EFI_STATUS status = EFI_SUCCESS;
> static u_char *iblk = NULL;
> static u_int iblksz = 0;
> @@ -127,10 +127,29 @@ efid_io(int rw, efi_diskinfo_t ed, u_int
> min(nsect, i_lblks) * DEV_BSIZE);
> }
> if (i_nblks > 0) {
> - status = EFI_CALL(ed->blkio->ReadBlocks,
> - ed->blkio, ed->mediaid, lba,
> - ed->blkio->Media->BlockSize * (i_nblks / blks),
> - buf + (i_lblks * DEV_BSIZE));
> + /*
> + * Pass the buffer directly to the EFI function only if
> + * the buffer is properly aligned as the media requires
> + */
> + if (ed->blkio->Media->IoAlign <= 1 ||
> + ((UINTN)buf + i_lblks * DEV_BSIZE)
> + % ed->blkio->Media->IoAlign == 0)
> + status = EFI_CALL(ed->blkio->ReadBlocks,
> + ed->blkio, ed->mediaid, lba,
> + ed->blkio->Media->BlockSize * (i_nblks /
> + blks), buf + i_lblks * DEV_BSIZE);
> + else {
> + for (i = 0; i < i_nblks; i += blks) {
> + status = EFI_CALL(ed->blkio->ReadBlocks,
> + ed->blkio, ed->mediaid,
> + lba + i / blks,
> + ed->blkio->Media->BlockSize, iblk);
> + if (EFI_ERROR(status))
> + break;
> + memcpy(buf + i * DEV_BSIZE, iblk,
> + ed->blkio->Media->BlockSize);
> + }
> + }
> if (EFI_ERROR(status))
> goto on_eio;
> }
> @@ -160,10 +179,30 @@ efid_io(int rw, efi_diskinfo_t ed, u_int
> ed->blkio->Media->BlockSize, iblk);
> }
> if (i_nblks > 0) {
> - status = EFI_CALL(ed->blkio->WriteBlocks,
> - ed->blkio, ed->mediaid, lba,
> - ed->blkio->Media->BlockSize * (i_nblks / blks),
> - buf + (i_lblks * DEV_BSIZE));
> + /*
> + * Pass the buffer directly to the EFI function only if
> + * the buffer is properly aligned as the media requires
> + */
> + if (ed->blkio->Media->IoAlign <= 1 ||
> + ((UINTN)buf + i_lblks * DEV_BSIZE)
> + % ed->blkio->Media->IoAlign == 0)
> + status = EFI_CALL(ed->blkio->WriteBlocks,
> + ed->blkio, ed->mediaid, lba,
> + ed->blkio->Media->BlockSize * (i_nblks /
> + blks), buf + i_lblks * DEV_BSIZE);
> + else {
> + for (i = 0; i < i_nblks; i += blks) {
> + memcpy(iblk, buf + i * DEV_BSIZE,
> + ed->blkio->Media->BlockSize);
> + status = EFI_CALL(
> + ed->blkio->WriteBlocks,
> + ed->blkio, ed->mediaid,
> + lba + i / blks,
> + ed->blkio->Media->BlockSize, iblk);
> + if (EFI_ERROR(status))
> + break;
> + }
> + }
> if (EFI_ERROR(status))
> goto on_eio;
> }
>
>
