From: Takashi Iwai <tiwai@suse.de>
To: George Dunlap <george.dunlap@citrix.com>
Cc: ALSA development <alsa-devel@alsa-project.org>,
Konstantin Ozerkov <kozerkov@parallels.com>,
Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
George.Dunlap@eu.citrix.com,
Andrew Cooper <andrew.cooper3@citrix.com>,
Dario Faggioli <dario.faggioli@citrix.com>,
Jim Gettys <jg@freedesktop.org>,
Steven Rostedt <rostedt@goodmis.org>,
Andy Lutomirski <luto@amacapital.net>,
Stephen Hemminger <stephen@networkplumber.org>,
Matt Fleming <matt@codeblueprint.co.uk>,
"Luis R. Rodriguez" <mcgrof@kernel.org>,
Borislav Petkov <bp@alien8.de>, Mel Gorman <mgorman@suse.de>,
xen-devel <xen-devel@lists.xenproject.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Joerg Roedel <joro@8bytes.org>
Subject: Re: Getting rid of inside_vm in intel8x0
Date: Mon, 04 Apr 2016 11:15:14 +0200 [thread overview]
Message-ID: <s5hr3elag65.wl-tiwai__36482.8574275181$1459768607$gmane$org@suse.de> (raw)
In-Reply-To: <57022E67.7050907@citrix.com>
On Mon, 04 Apr 2016 11:05:43 +0200,
George Dunlap wrote:
>
> On 02/04/16 13:57, Andy Lutomirski wrote:
> > On Fri, Apr 1, 2016 at 10:33 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> On Sat, 02 Apr 2016 00:28:31 +0200,
> >> Luis R. Rodriguez wrote:
> >>> If the former, could a we somehow detect an emulated device other than through
> >>> this type of check ? Or could we *add* a capability of some sort to detect it
> >>> on the driver ? This would not address the removal, but it could mean finding a
> >>> way to address emulation issues.
> >>>
> >>> If its an IO issue -- exactly what is the causing the delays in IO ?
> >>
> >> Luis, there is no problem about emulation itself. It's rather an
> >> optimization to lighten the host side load, as I/O access on a VM is
> >> much heavier.
> >>
> >>>>>> This is satisfied mostly only on VM, and can't
> >>>>>> be measured easily unlike the IO read speed.
> >>>>>
> >>>>> Interesting, note the original patch claimed it was for KVM and
> >>>>> Parallels hypervisor only, but since the code uses:
> >>>>>
> >>>>> +#if defined(__i386__) || defined(__x86_64__)
> >>>>> + inside_vm = inside_vm || boot_cpu_has(X86_FEATURE_HYPERVISOR);
> >>>>> +#endif
> >>>>>
> >>>>> This makes it apply also to Xen as well, this makes this hack more
> >>>>> broad, but does is it only applicable when an emulated device is
> >>>>> used ? What about if a hypervisor is used and PCI passthrough is
> >>>>> used ?
> >>>>
> >>>> A good question. Xen was added there at the time from positive
> >>>> results by quick tests, but it might show an issue if it's running on
> >>>> a very old chip with PCI passthrough. But I'm not sure whether PCI
> >>>> passthrough would work on such old chipsets at all.
> >>>
> >>> If it did have an issue then that would have to be special cased, that
> >>> is the module parameter would not need to be enabled for such type of
> >>> systems, and heuristics would be needed. As you note, fortunately this
> >>> may not be common though...
> >>
> >> Actually this *is* module parametered. If set to a boolean value, it
> >> can be applied / skipped forcibly. So, if there has been a problem on
> >> Xen, this should have been reported. That's why I wrote it's no
> >> common case. This comes from the real experience.
> >>
> >>> but if this type of work around may be
> >>> taken as a precedent to enable other types of hacks in other drivers
> >>> I'm very fearful of more hacks later needing these considerations as
> >>> well.
> >>>
> >>>>>>> There are a pile of nonsensical "are we in a VM" checks of various
> >>>>>>> sorts scattered throughout the kernel, they're all a mess to maintain
> >>>>>>> (there are lots of kinds of VMs in the world, and Linux may not even
> >>>>>>> know it's a guest), and, in most cases, it appears that the correct
> >>>>>>> solution is to delete the checks. I just removed a nasty one in the
> >>>>>>> x86_32 entry asm, and this one is written in C so it should be a piece
> >>>>>>> of cake :)
> >>>>>>
> >>>>>> This cake looks sweet, but a worm is hidden behind the cream.
> >>>>>> The loop in the code itself is already a kludge for the buggy hardware
> >>>>>> where the inconsistent read happens not so often (only at the boundary
> >>>>>> and in a racy way). It would be nice if we can have a more reliably
> >>>>>> way to know the hardware buggyness, but it's difficult,
> >>>>>> unsurprisingly.
> >>>>>
> >>>>> The concern here is setting precedents for VM cases sprinkled in the kernel.
> >>>>> The assumption here is such special cases are really paper'ing over another
> >>>>> type of issue, so its best to ultimately try to root cause the issue in
> >>>>> a more generalized fashion.
> >>>>
> >>>> Well, it's rather bare metal that shows the buggy behavior, thus we
> >>>> need to paper over it. In that sense, it's other way round; we don't
> >>>> tune for VM. The VM check we're discussing is rather for skipping the
> >>>> strange workaround.
> >>>
> >>> What is it exactly about a VM that enables this work around to be skipped?
> >>> I don't quite get it yet.
> >>
> >> VM -- at least the full one with the sound hardware emulation --
> >> doesn't have the hardware bug. So, the check isn't needed.
> >
> > Here's the issue, though: asking "am I in a VM" is not a good way to
> > learn properties of hardware. Just off the top of my head, here are
> > some types of VM and what they might imply about hardware:
> >
> > Intel Kernel Guard: your sound card is passed through from real hardware.
> >
> > Xen: could go either way. In dom0, it's likely passed through. In
> > domU, it could be passed through or emulated, and I believe this is
> > the case for all of the Xen variants.
> >
> > KVM: Probably emulated, but could be passed through.
>
> I'm not sure exactly why I was CC'd into this thread, but this is an
> important point -- even if you're running in a VM, you may actually have
> direct un-emulated IO access to a real (buggy) piece of hardware; in
> which case it sounds like you still need the work-around. So
> boot_cpu_has(X86_FEATURE_HYPERVISOR) is probably not the right check.
The VM check is kept there only to show a kernel message; in case
where a similar issue is seen on another VM, user may notice more
easily by that. The VM check itself doesn't change any kernel
behavior any longer.
Takashi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
prev parent reply other threads:[~2016-04-04 9:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CALCETrWnsn4CoRXcKDXpMTjxCYj8==mZQHkZRuXfZQyvERB3Gw@mail.gmail.com>
[not found] ` <s5h60w48po7.wl-tiwai@suse.de>
2016-03-31 22:26 ` Getting rid of inside_vm in intel8x0 Luis R. Rodriguez
[not found] ` <20160331222618.GE1990@wotan.suse.de>
2016-04-01 5:34 ` Takashi Iwai
[not found] ` <s5hvb4151v1.wl-tiwai@suse.de>
2016-04-01 22:28 ` Luis R. Rodriguez
[not found] ` <20160401222831.GX1990@wotan.suse.de>
2016-04-02 5:33 ` Takashi Iwai
[not found] ` <s5hwpog377v.wl-tiwai@suse.de>
2016-04-02 12:57 ` Andy Lutomirski
[not found] ` <CALCETrW6Og2_cNnmgMWkGhfFNdLyJjtojqn6_s+BfFCsdzzczg@mail.gmail.com>
2016-04-02 16:07 ` Takashi Iwai
[not found] ` <s5hfuv42dve.wl-tiwai@suse.de>
2016-04-02 18:05 ` Andy Lutomirski
[not found] ` <CALCETrW8jBXwCqXNMra667qk-CALOTvAXnQqYQuPBC_sWoJxHg@mail.gmail.com>
2016-04-02 20:22 ` Takashi Iwai
[not found] ` <s5h8u0v3gmj.wl-tiwai@suse.de>
2016-04-04 18:31 ` Luis R. Rodriguez
2016-04-04 9:05 ` George Dunlap
[not found] ` <57022E67.7050907@citrix.com>
2016-04-04 9:15 ` Takashi Iwai [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='s5hr3elag65.wl-tiwai__36482.8574275181$1459768607$gmane$org@suse.de' \
--to=tiwai@suse.de \
--cc=George.Dunlap@eu.citrix.com \
--cc=alsa-devel@alsa-project.org \
--cc=andrew.cooper3@citrix.com \
--cc=bp@alien8.de \
--cc=dario.faggioli@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=jg@freedesktop.org \
--cc=joro@8bytes.org \
--cc=kozerkov@parallels.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=matt@codeblueprint.co.uk \
--cc=mcgrof@kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=rostedt@goodmis.org \
--cc=stefano.stabellini@eu.citrix.com \
--cc=stephen@networkplumber.org \
--cc=tglx@linutronix.de \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).