Page MenuHomeFreeBSD

HD Audio Emulation For Bhyve (Based on D7840)
AbandonedPublic

Authored by araujo on Sep 20 2017, 7:48 AM.

Details

Reviewers
mav
rgrimes
Group Reviewers
bhyve
Summary
  • Just fixed some style(9).
  • Add BSD License and a proper copyright for the author.

Note: I did open another review because I don't want to command the review D7840.

Test Plan
  • Tested with HEAD r323795.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 11776
Build 12120: arc lint + arc unit

Event Timeline

There are a bunch of minor style(9) issues.

I was able to apply the diff and build cleanly. However, with an 11.1/amd64 guest and using mpg123 to play an MP3 file, I had an unimplemented instruction exit - I'll look more into this.

usr.sbin/bhyve/audio.c
63

Not strict style(9) - variables declarations don't have initializers.

69

style(9) "Values in return statements should be enclosed in parentheses"

usr.sbin/bhyve/hda_codec.c
143

See style(9) on sorting struct elements.

869

No need to cast a void *

920

No error return to assert on here.

940

Not in style(9), but the FreeBSD convention for function pointers is to make it explicit with a dereference e.g. use (*actx->do_setup)(actx->priv)

usr.sbin/bhyve/hdac_reg.h
269

style(9), should be '#endif /* _HDAC_REG_H_ */

usr.sbin/bhyve/pci_hda.c
296

No need for return statements in void routines.

  • Address grehan@ request about some minor style(9) issues all over the place.
  • Some structs were sorted by use and some by size and in alphabetical order.
  • Remove the return from void functions.
usr.sbin/bhyve/audio.c
63

There is nothing wrong to initialize the variable, we can do it explicit or let the compiler does it for us.

usr.sbin/bhyve/hda_codec.c
920

Actually that assert should not be there according with pthread_set_name_np(3).

I disabled capsicum by commenting out the cap_enter() call, and with the above struct revert, was able to cat a .WAV file and also use mpg123 to play a long mp3 file on a FreeBSD 11.1/amd64 guest, with a cheap USB audio adapter for output. Volume control seemed to work fine with mpg123. There was a very small glitch/skip when the audio was first played (or on resume after pause in mpg123).

The instruction-emlation patch for the OR instruction is at https://people.freebsd.org/~grehan/patches/vmm_instruction_emul.c.OR.diff

usr.sbin/bhyve/pci_hda.c
79

I should have added that struct element ordering only applies for internal-use structs, and not ones that are used to represent in-memory device-emulation data structures (in this case, the HDA BDL struct).

This one has to go back to the original ordering. You may want to have a look at any others that were re-ordered in case they were used for HDA device data structures.

The OR emulation has been checked in with r325261

I've tested with Rhythmbox and browsers on Ubuntu 17.10, Edge and Skype (full audio call, audio in/out via HDA) on Win10.

Just need to add some Capsicum-ization and this will be good to go.

The OR emulation has been checked in with r325261

I've tested with Rhythmbox and browsers on Ubuntu 17.10, Edge and Skype (full audio call, audio in/out via HDA) on Win10.

Just need to add some Capsicum-ization and this will be good to go.

Needs capsicum patches or it won't work (testing was done with Capsicum disabled).

Oooops! I completely forgot this patch, I will add capsicum this week.
My bad!

Any update on this? I've tried this diff on a recent current but it seems like it doesn't work anymore?

Any update on this? I've tried this diff on a recent current but it seems like it doesn't work anymore?

I will come back to this patch soon. I'm waiting some capsicum patches do land. I didn't have enough time to check if those were committed or not.

Just a hands up, this patch needs the following patches:

Committed already: https://reviews.freebsd.org/D14557
Pending: https://reviews.freebsd.org/D14407

Hi @araujo, are there plans to move forward with this now that https://reviews.freebsd.org/rS340373 (libcasper: introduce cap_fileargs service) was commited?

Hi @araujo, are there plans to move forward with this now that https://reviews.freebsd.org/rS340373 (libcasper: introduce cap_fileargs service) was commited?

Yes, definitely! I'm working on it, I got some issues with cap_fileargs and I have asked Mariuz support to help me debug the issue. But yeah, I'm on it right now!

Hi @araujo, are there plans to move forward with this now that https://reviews.freebsd.org/rS340373 (libcasper: introduce cap_fileargs service) was commited?

Yes, definitely! I'm working on it, I got some issues with cap_fileargs and I have asked Mariuz support to help me debug the issue. But yeah, I'm on it right now!

I have implemented cap_fileargs(3) and now the driver is in capabilities mode and works relatively fine.
Still there are some adjustments that I need to make and I hope till next week I will update this review with the new code.

If anybody else is interested on this patch, ping me and I can share my latest changes. However I'm not interested on it anymore.

If anybody else is interested on this patch, ping me and I can share my latest changes. However I'm not interested on it anymore.

I am interested seeing this patch hit bhyve main.

If anybody else is interested on this patch, ping me and I can share my latest changes. However I'm not interested on it anymore.

I am interested seeing this patch hit bhyve main.

Yes! Please do share your latest changes!

If anybody else is interested on this patch, ping me and I can share my latest changes. However I'm not interested on it anymore.

I am interested seeing this patch hit bhyve main.

Yes! Please do share your latest changes!

For anybody interested in this: This patch correctly applies to 12.0-Release-Stable.

I would like to see more development on this as it provides a native bandaid to the lack of WideVine DRM/EME support.

Is anybody still working on this?

As far as I can see it's ready to get committed. So let's get this into bhyve after all these years...

Just tested with TruOS/Trident from a May 2019 kernel, and it's getting ECAPMODE. Maybe the problems with Capsicum aren't worked out yet. I'll look some more at it.

Just tested with TruOS/Trident from a May 2019 kernel, and it's getting ECAPMODE. Maybe the problems with Capsicum aren't worked out yet. I'll look some more at it.

Please read the whole review, Peter Grehan points out several times that it has capsicum issues, and that it has only been shown to work with Capsicum disabled.