acidanthera / bugtracker

Acidanthera Bugtracker
385 stars 44 forks source link

UsbOwnership bugs #1291

Open nms42 opened 3 years ago

nms42 commented 3 years ago

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 commented 3 years ago

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

vit9696 commented 3 years ago

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

nms42 commented 3 years ago

OK. I'll do it. Stay tuned!

vit9696 commented 3 years ago

@nms42 happy holidays, any progress on this?

nms42 commented 3 years ago

Yes. Sir! (-;

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

Need review.

vit9696 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Elaborate, please.

vit9696 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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