qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c


From: Denis V. Lunev
Subject: Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c
Date: Sun, 6 Nov 2011 20:56:45 +0400
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1

On 11/6/11 8:47 PM, Takashi Iwai wrote:
At Sun, 06 Nov 2011 18:31:42 +0200,
Avi Kivity wrote:
On 11/06/2011 06:15 PM, Denis V. Lunev wrote:
On 11/6/11 6:51 PM, Avi Kivity wrote:
The recently merged 228cf79376f1 ("ALSA: intel8x0: Improve performance
in virtual environment") is hacky and somewhat wrong.

First, the detection code

+       if (inside_vm<   0) {
+               /* detect KVM and Parallels virtual environments */
+               inside_vm = kvm_para_available();
+#if defined(__i386__) || defined(__x86_64__)
+               inside_vm = inside_vm ||
boot_cpu_has(X86_FEATURE_HYPERVISOR);
+#endif
+       }
+

is incorrect.  It detects that you're running in a guest, but that
doesn't imply that the device you're accessing is emulated.  It may be a
host device assigned to the guest; presumably the optimization you apply
doesn't work for real devices.

Second, the optimization itself looks fishy:

          spin_lock(&chip->reg_lock);
          do {
                  civ = igetbyte(chip, ichdev->reg_offset +
ICH_REG_OFF_CIV);
                  ptr1 = igetword(chip, ichdev->reg_offset +
ichdev->roff_picb);
                  position = ichdev->position;
                  if (ptr1 == 0) {
                          udelay(10);
                          continue;
                  }
-               if (civ == igetbyte(chip, ichdev->reg_offset +
ICH_REG_OFF_CIV)&&
-                   ptr1 == igetword(chip, ichdev->reg_offset +
ichdev->roff_picb))
+               if (civ != igetbyte(chip, ichdev->reg_offset +
ICH_REG_OFF_CIV))
+                       continue;
+               if (chip->inside_vm)
+                       break;
+               if (ptr1 == igetword(chip, ichdev->reg_offset +
ichdev->roff_picb))
                          break;
          } while (timeout--);


Why is the emulated device timing out?  Can't the emulation be fixed to
behave like real hardware?

Last, please copy address@hidden on such issues.

The problem is that emulation can not be fixed.

How this is working for real hardware? You get data from real sound
card register.
The scheduling is off at the moment thus you can not be re-scheduled.

In the virtual environment the situation is different. Any IO
emulation is expensive.
The processor is switched from guest to hypervisor and further to
emulation process
takes a lot of time.  This time is enough to obtain different value on
next register read.
That's why this code is really timed out. Please also note that host
scheduler also
plays his games and could schedule out VCPU thread.

Note on kvm this is rare, since the guest thread and the emulator thread
are the same.

The problem could be potentially fixed reducing precision of PICB
emulation,
but this results in lower sound quality.

This kludge has been written this way in order not to break legacy
card for which we
do not have an access. The code reading PICB/CIV registers second time
was added
to fix issues on unknown for now platform and it looks not possible
how to find/test
against this platform now. We have checked Windows drivers written by
Intel/AMD
(32/64 bit) and MacOS ones. There is no second reading of CIV/PICB
inside. We
hope that this is relay needed only for some rare hadware devices.

Ok, so if I understand correctly, this loop is a hack for broken
hardware, and this patch basically unhacks it back, assuming that the
emulated (or assigned) hardware is not broken.
Exactly.  The loop itself is still needed, but the check can be
less restrictive.

The only thing we can is to improve detection code. Suggestions are
welcome.
I think it's fine to assume that you're not assigning a 2004 era sound
card to a guest.  So I think the code is fine as it is, and can only
suggest to add a comment explaining the mess.
True.  Denis, care to submit a patch?
sure! I'll do that tomorrow



reply via email to

[Prev in Thread] Current Thread [Next in Thread]