Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UsbOwnership bugs #1291

Open
nms42 opened this issue Nov 14, 2020 · 17 comments
Open

UsbOwnership bugs #1291

nms42 opened this issue Nov 14, 2020 · 17 comments

Comments

@nms42
Copy link

nms42 commented Nov 14, 2020

There are two implementations for ownership switch: (a) in Library/OcMiscLib/ReleaseUsbOwnership.c and (b) in Library/DuetBdsLib/BdsPlatform.c.

(1) In EHCI controllers the ExtendedCaps register is not on fixed position. That happen when debug register present. The same search like in XHCI case must be performed.

(2) Dangling interrupts can preclude proper ownership switch. Some relevant register reads maybe?

(3) The version (a) is more developed so far. Should it be used for OpenDuet too? After fixing bugs.

@nms42
Copy link
Author

nms42 commented Nov 14, 2020

OHCI case should be handled too. At least documented somehow.

@vit9696
Copy link
Contributor

vit9696 commented Nov 15, 2020

I am fine to make Duet use the new ownership release code. Also can update it. Please submit patches.

@nms42
Copy link
Author

nms42 commented Nov 15, 2020

OK. I'll do it. Stay tuned!

@vit9696
Copy link
Contributor

vit9696 commented Dec 25, 2020

@nms42 happy holidays, any progress on this?

@nms42
Copy link
Author

nms42 commented Dec 25, 2020

Yes. Sir! (-;

Already made branch where all the business of turning off legacy emulation delegated to edk2 (audk) machinery. Quirk removed.

Need review.

@vit9696
Copy link
Contributor

vit9696 commented Dec 25, 2020

If you mean https://github.com/acidanthera/OpenCorePkg/tree/OffUsbLegacyEmulation, then it does not make sense. PcdTurnOffUsbLegacySupport is implemented in EhciDxe and UhciDxe. None of these drivers are used when OpenCore is used on a firmware different from OpenDuet (and they actually cannot be used).

Therefore I get it that your solution may work fine for OpenDuet, and the removal of DisableUsbLegacySupport in OpenDuet BDS does make food sense. However, the removal of the quirk, which is intended exclusively for non-duet systems does not make sense. Please recover.

@nms42
Copy link
Author

nms42 commented Dec 25, 2020

Just for the record, edk2 XHCI driver has this feature (LegacyEmulationOff) too.

Let me refactor this stuff after our consent on this points

(1) In OpenDuet switch from custom made emulation handling to edk2/audk one. As the result of switch emulation will be turned off at the start of every usb driver (uhci|ehci|xhci) used.

(2) In OpenCore custom handling of emulation should stay, as a protection with odd EFI bioses.

(3) In OpenCore emulation should be turned off always. At the end of OpenCore run? At the beginning?

(4) Quirk must be removed.

@vit9696
Copy link
Contributor

vit9696 commented Dec 25, 2020

I agree on point (1) and (2). I do not agree on points (3) and (4). BIOS should normally turn off emulation on its own at ExitBootServives. Sometimes it does not, and thus the quirk. I disagree that we should unconditionally use this code therefore.

So I suggest that (4) does not apply, and (3) is read as "OpenCore emulation is turned off before ExitBootServices when enabled through the quirk. The quirk may be renamed if you insist."

@nms42
Copy link
Author

nms42 commented Dec 25, 2020

BIOS should normally turn off emulation on its own at ExitBootServives. Sometimes it does not, and thus the quirk. I disagree that we should unconditionally use this code therefore.

Why turning off can not be made twice?

"OpenCore emulation is turned off before ExitBootServices ..."

Why before ExitBootServices? Is it correct to run OpenCore (on usb devices) concurrently with BIOS (Intel ME), when emulation on?

@vit9696
Copy link
Contributor

vit9696 commented Dec 25, 2020

Why turning off can not be made twice?

Because we do not do stuff we can do without permission. This is not Clover.

Why before ExitBootServices? Is it correct to run OpenCore (on usb devices) concurrently with BIOS (Intel ME), when emulation on?

Yes, I think so.

@nms42
Copy link
Author

nms42 commented Dec 25, 2020

Because we do not do stuff we can do without permission.

Elaborate, please.

@vit9696
Copy link
Contributor

vit9696 commented Dec 25, 2020

Turning off USB emulation is a hack, because under normal conditions the firmware should handle this on its own. We do not make hacks mandatory even if they are unlikely to cause harm, this is a project design decision.

@nms42
Copy link
Author

nms42 commented Dec 25, 2020

OK, lets reiterate

we can do without permission

What kind of permission? From whom|what? In my understanding control of usb controller can be shifted by any software at any time as long as it has proper access to device registers.

Looks like our dictionaries disagree on terms.

Mine -- usb emulation = "pseudo PS2" devices plus ability to work with usb data media as hard disk drives (in BIOS sense). Ability to work with usb data media irrelevant as long as software not using (BIOS INT 0x13).

Yours?

@nms42
Copy link
Author

nms42 commented Dec 25, 2020

I propose to solve "usb legacy emulation" by little baby steps.

I would do points (1) & (2), fix the relevant code in OpenCore without functionality change. The rest would be decided after thesauruses alignment, discussion in another issue (may be), ...

???

@vit9696
Copy link
Contributor

vit9696 commented Dec 25, 2020

@nms42 I do not quite care what the code does (i.e. which particular registers it updates), but I do care about this code being optional and opt-in.

However, I believe it is a good idea to address points (1) and (2) first since we agree on them.

@vit9696
Copy link
Contributor

vit9696 commented Jan 1, 2021

@nms42 @roddy20 seems that the EDK II code is entirely broken with ASUS P5E3 Premium, could you try to understand why? It does not look too different to me.

@nms42
Copy link
Author

nms42 commented Jan 1, 2021

@roddy20, could You provide details, logs, ... please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants