Page MenuHomeFreeBSD

libefi don't use TERM_EMU on arm
ClosedPublic

Authored by manu on Jun 9 2016, 12:30 AM.
Tags
None
Referenced Files
F106181637: D6783.diff
Thu, Dec 26, 6:00 PM
Unknown Object (File)
Wed, Dec 25, 7:37 AM
Unknown Object (File)
Nov 23 2024, 1:31 AM
Unknown Object (File)
Nov 16 2024, 6:32 AM
Unknown Object (File)
Oct 19 2024, 2:18 AM
Unknown Object (File)
Oct 12 2024, 8:47 PM
Unknown Object (File)
Oct 1 2024, 12:22 PM
Unknown Object (File)
Oct 1 2024, 6:20 AM
Subscribers

Details

Summary

Don't use TERM_EMU on arm as it does not work with u-boot efi implementation.

Test Plan

Tested on beaglebone black with u-boot efi implementation.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

manu retitled this revision from to libefi don't use TERM_EMU on arm.
manu updated this object.
manu edited the test plan for this revision. (Show Details)
manu added a reviewer: andrew.
manu set the repository for this revision to rS FreeBSD src repository - subversion.

Can you also do this on aarch64.

manu edited edge metadata.

Add aarch64.

The commit message should say it's disabled on arm/arm64 as it's more likely we will use a serial console there where the terminal emulation doesn't work as well. Come to think of it this should also be in a comment above the .if.

The commit message should say it's disabled on arm/arm64 as it's more likely we will use a serial console there where the terminal emulation doesn't work as well. Come to think of it this should also be in a comment above the .if.

I'll update the diff soon to include this in the comment.
Is this something that you want commited for 11.0 ?

I don't see how this patch is supposed to work when efi_console.c:CD() requires curx/cury, but is not protected by #ifdef TERM_EMU.

Never mind, I applied the patch against a slightly older version of head.

Don't use TERM_EMU on arm as it does not work with u-boot efi implementation.

Is it that u-boot EFI just does not implement SetAttribute, ClearScreen etc.?

The commit message should say it's disabled on arm/arm64 as it's more likely we will use a serial console there where the terminal emulation doesn't work as well.

I don't think disabling TERM_EMU in loader.efi is the right way to handle this case; the loader 4th bits shouldn't be using the full screen menus etc. in serial console mode. The TERM_EMU code path should be disabled only if it's not actually implemented.

Anyhow, I think the change is reasonable for now, particularly for ARM.

I don't think disabling TERM_EMU in loader.efi is the right way to handle this case; the loader 4th bits shouldn't be using the full screen menus etc. in serial console mode. The TERM_EMU code path should be disabled only if it's not actually implemented.

It's not an issue with the boot menu. The command line also has issues with TERM_EMU.

emaste edited edge metadata.

It's not an issue with the boot menu. The command line also has issues with TERM_EMU.

Ok, we'll need to fix that as well.

Anyway, we should go ahead with this for now, and revisit after 11.0 is out.
@manu, how does it fail on u-boot EFI? Can we check for NULL SetAttribute etc.?

This revision is now accepted and ready to land.Jun 17 2016, 2:42 PM

It's not an issue with the boot menu. The command line also has issues with TERM_EMU.

Ok, we'll need to fix that as well.

Anyway, we should go ahead with this for now, and revisit after 11.0 is out.
@manu, how does it fail on u-boot EFI? Can we check for NULL SetAttribute etc.?

Sorry I've been delayed by other stuff :)
So with U-Boot the output is just garbage.
We could refactor the code to not have TERM_EMU at all and just handle things at runtime by checking SetAttribute return value I guess.
I U-Boot SetAttribute exist, it just returns EFI_NOT_SUPPORTED.
Now that the 11 switch is almost over and that I have every bits needed to make u-boot->boot1.efi->loader.efi->kernel working, I'm not in a hurry to make loader.efi works with U-Boot so probably close this revision soon after I have everything in better shape.

andrew edited edge metadata.

You should commit this

This revision was automatically updated to reflect the committed changes.