Created attachment 205930 [details] Patch that fixes the problem for me Hello. I am running FreeBSD 12.0-RELEASE and want to try hda (audio) support, which was added to CURRENT recently (base r349335). So I have cherry-picked that commit (I use git mirror on github) and rebuilt bhyve. When I add HDA device, bhyve aborts on assertion: root@vonbraun:~ # bhyve -c 1 -m 2G -w -H -s 0,hostbridge -s 4,ahci-cd,/home/vasily/Fedora-Workstation-Live-x86_64-30-1.2.iso -s 6,hda,play=/dev/dsp -s 29,fbuf,tcp=127.0.0.1:5900,w=1920,h=1080 -s 30,xhci,tablet -s 31,lpc -l com1,stdio -l bootrom,BHYVE_UEFI.fd fedora (Fedora is booting. Some messages skipped.) Assertion failed: (!err), function hda_set_sdctl, file /usr/src/usr.sbin/bhyve/pci_hda.c, line 1072. Here are the last lines is bhyve_hda.log: hda_set_sdctl-1063: stream_ind: 0x4 old: 0x50001c value: 0x50001e hda_stream_start-677: stream: 0x4 bdl_cnt: 0x3 bdl_paddr: 0x773c5000 hda_stream_start-706: Fail to get the guest virtual address So I have examined pci_hda.c and discovered that bdle pointer in hda_stream_start function is shifted by 4 bytes from actual position of that structure in memory. I cannot explain it, but the following patch fixes the problem. Maybe it's because I did not rebuild the whole system, but maybe it's a bug in bhyve. Please tell me is HDA support work on your machine.
Unfortunately, this quick&dirty fix works only for Fedora as a guest. And let me clarify myself a bit about that 4 byte shift. I've added some additional debug code into hda_stream_start diff --git a/usr.sbin/bhyve/pci_hda.c b/usr.sbin/bhyve/pci_hda.c index 99f8aec31c6..9d19e2ec504 100644 --- a/usr.sbin/bhyve/pci_hda.c +++ b/usr.sbin/bhyve/pci_hda.c @@ -678,6 +678,20 @@ hda_stream_start(struct hda_softc *sc, uint8_t stream_ind) st->bdl_cnt = bdl_cnt; + /* bdl_vaddr -= 4; */ + + bdle = (struct hda_bdle *)bdl_vaddr; + for (i = 0; i < bdl_cnt; i++, bdle++) { + bdle_sz = bdle->len; + assert(!(bdle_sz % HDA_DMA_ACCESS_LEN)); + + bdle_addrl = bdle->addrl; + bdle_addrh = bdle->addrh; + + bdle_paddr = bdle_addrl | (bdle_addrh << 32); + DPRINTF("paddr: 0x%lx, len: 0x%x\n", bdle_paddr, bdle_sz); + } + bdle = (struct hda_bdle *)bdl_vaddr; for (i = 0; i < bdl_cnt; i++, bdle++) { bdle_sz = bdle->len; This is what I get in bhyve_hda.log when booting Fedora. hda_stream_start-677: stream: 0x4 bdl_cnt: 0x2 bdl_paddr: 0x75a09000 hda_stream_start-692: paddr: 0x79f0000000000000, len: 0x0 hda_stream_start-692: paddr: 0x79f2b11000000000, len: 0x0 hda_stream_start-706: Fail to get the guest virtual address I have gussed that bdl_vaddr is incorrect and bdle->addrh actualy contains bdle->addrl, and bdle->ioc is actually bdle->len. So I subtracted 4 from bdl_vaddr and got sound worked. But, unfortunatelly, it works only with Fedora guest. Other guests give no sound output or noise. Waiting reply from users of CURRENT. Do you have the same trouble?
Created attachment 205973 [details] New dirty fix
I've managed to get audio working for all guests (tested with Win 10, Debian, Fedora). First, it's important to apply "New dirty fix" in attachements. Then, run bhyve with guest memory <(strictly less than) 4Gb: bhyve -c 2 -m 3G -w -H -s 0,hostbridge -s 4,ahci-hd,/path/to/bootimg -s 5,virtio-net,tap0 -s 6,hda,play=/dev/dsp -s 29,fbuf,tcp=127.0.0.1:5900,w=1920,h=1080 -s 30,xhci,tablet -s 31,lpc -l com1,stdio -l bootrom,BHYVE_UEFI.fd vm1 Sound will work then. I still cannot understand why any manipulations with source code in this ugly way are required.
Forgot to mention: host and all guests are x86-64. CPU is AMD Ryzen 1600x
According to https://github.com/freebsd/freebsd/blob/1872e290af2d/sys/dev/sound/pci/hda/hdac_private.h#L146-L151 , it seems the definition of `struct hda_bdle` is wrong. Something like this is needed: diff --git c/usr.sbin/bhyve/pci_hda.c i/usr.sbin/bhyve/pci_hda.c index ace88274ac9..e0324f46a95 100644 --- c/usr.sbin/bhyve/pci_hda.c +++ i/usr.sbin/bhyve/pci_hda.c @@ -80,10 +80,10 @@ typedef void (*hda_set_reg_handler)(struct hda_softc *sc, uint32_t offset, uint32_t old); struct hda_bdle { - uint32_t addrh; uint32_t addrl; + uint32_t addrh; + uint32_t len; uint32_t ioc; - uint32_t len; } __packed; struct hda_bdle_desc {
comment #5 You are correct! My bad, I should have the definition checked to be in agreement with the driver earlier ;) I confirm, the sound works with this patch.
Created attachment 205994 [details] Patch proposed by Tiwei Bie that fixes definition of struct hda_bdle and the problem
Section 3.6.3 of the High Definition Audio 1.0a spec is pretty clear on the format of the BDL, and that matches was was suggested here and what's in the `hdac_private.h` file as noted. I'll test and commit this. I'm really curious how this could have ever previously worked.
A commit references this bug: Author: scottl Date: Tue Jul 23 18:40:07 UTC 2019 New revision: 350255 URL: https://svnweb.freebsd.org/changeset/base/350255 Log: Fix the register layout for the Buffer Descript List Entry. It got jumbled around during some other cleanups and was causing audio failures on some guests. PR: 239341 Reported by: shamaz.mazum@gmail.com Changes: head/usr.sbin/bhyve/pci_hda.c
Scott, will you MFC this?
hda support does not exist in stable/12