runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

So, the yocto folks moved from 4.1 to 4.4 and one of their automated
qemu x86-32 boot tests started failing.  None of the yocto details seem
to matter since I offered to help and I've repropduced it using 100%
mainline kernels and a generic distro toolchain as well.

The test case is slightly complicated, in that it relies on uvesafb
being modular, and so one has to juggle modules within an ext4 image
that qemu boots from.  We tried making uvesafb builtin, but that made
the issue magically vanish.  Given PAT, this isn't too surprising.

Richard did the preliminary investigation and analysis, and from that I
did a bisect, and found the commit in $SUBJECT to be the root cause, as
per the discussion here:

http://lists.openembedded.org/pipermail/openembedded-core/2016-March/118397.html

I'd mentioned the above to bpetkov on IRC and after confirming it was
still an issue on 4.5-rc6, he'd asked if I had a portable reproducer.  

Not sure how complicated that would be, I set out to make one from my
build.   With a little LD_PRELOAD type magic and ensuring all the qemu
components are in ./  I have one that runs on an otherwise qemu-free
x86-64 box. 

The stand alone reproducer is here; launched in 00-runme:

http://openlinux.wrs.com/pat-splat/reproducer.tar.bz2  

It is nothing fancy, just a generic yocto build of "sato" (gfx enabled
rootfs).  When it "works" it boots to a UI touchscreen interface.  When
it fails, you get a black screen with a blinking cursor (as seen in
"vncviewer localhost:0").

Upon failure, you can do <Ctrl>-<Alt>-<2> to get to a passwd-less root
login ; there you can run dmesg and see the splat.  The image is
currently using 4.5-rc6 ; but any kernel can be inserted; "make
modules_install INSTALL_MOD_PATH=here" and then populating those modules
from "here" into /lib/modules of the loopback mounted image. And of
course updating the bzImage on the qemu cmdline.    Currently it
contains a bzImage and modules for 4.5-rc6 as I last tested that.

Also note that vncviewer will disconnect when it goes from early boot
80x25 to a higer res gfx mode; just reconnect and continue observing the
target.

I've ruled out yocto kernel changes, and yocto toolchain -- but maybe it
is a qemu issue this commit triggers ; who knows at this point.

Since I've NFI what component(s) cause this, I wanted to have the qemu
binary, all libraries etc as part of the reproducer and nothing left to
chance, and I've tested the reproducer on an ancient dual core w/o vmx
and w/o any qemu binaries installed.  Bruce also tested it on a slightly
more modern dual socket xeon with vmx and confirmed it failed there..

Inside there is a 00-runme ; mostly a copy of qemu args the yocto
automated tests were using.  There is also everything the qemu binaries
need to run ; toplevel dir is noisy since qemu only looks in ./ it
seems.  There is also an ext4.img ; as mentioned earlier, this only
happens when uvesafb.ko is a module, so one has to loopback mount that
image and repopulate /lib/modules/  for each boot test/bisect step.

I've also included 00-bisect.txt as the output of git bisect log.  And
there is also 00-configs/ dir that has the ".config" kernel file for
each build (dir names are "git describe" in here for easy correlation)
done for the bisect (plus the latest mainline build).  The failing commit
in the subject is v4.1-rc5-22-g9cd25aac1f44 .

My contribution here is largely a bisect that can be relied on and
providing a portable reproducer of the regression; I am by no means a
PAT expert ; Richard invested more time into actually understanding the
problem than I did, so I'm going to totally throw him under the bus on
this when it comes to considering the ultimate root cause and possible
fixes.  :)

Paul.
--

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 03/03/2016 (Thu 15:59) Paul Gortmaker wrote:

> So, the yocto folks moved from 4.1 to 4.4 and one of their automated
> qemu x86-32 boot tests started failing.  None of the yocto details seem
> to matter since I offered to help and I've repropduced it using 100%
> mainline kernels and a generic distro toolchain as well.
> 
> The test case is slightly complicated, in that it relies on uvesafb
> being modular, and so one has to juggle modules within an ext4 image
> that qemu boots from.  We tried making uvesafb builtin, but that made
> the issue magically vanish.  Given PAT, this isn't too surprising.
> 
> Richard did the preliminary investigation and analysis, and from that I
> did a bisect, and found the commit in $SUBJECT to be the root cause, as
> per the discussion here:
> 
> http://lists.openembedded.org/pipermail/openembedded-core/2016-March/118397.html
> 
> I'd mentioned the above to bpetkov on IRC and after confirming it was
> still an issue on 4.5-rc6, he'd asked if I had a portable reproducer.  
> 
> Not sure how complicated that would be, I set out to make one from my
> build.   With a little LD_PRELOAD type magic and ensuring all the qemu
> components are in ./  I have one that runs on an otherwise qemu-free
> x86-64 box. 
> 
> The stand alone reproducer is here; launched in 00-runme:
> 
> http://openlinux.wrs.com/pat-splat/reproducer.tar.bz2  

Apologies, I'd used an internal DNS abbreviation here that isn't global.
Replace the wrs with windriver and everything should be good.

P.
--

> 
> It is nothing fancy, just a generic yocto build of "sato" (gfx enabled
> rootfs).  When it "works" it boots to a UI touchscreen interface.  When
> it fails, you get a black screen with a blinking cursor (as seen in
> "vncviewer localhost:0").
> 
> Upon failure, you can do <Ctrl>-<Alt>-<2> to get to a passwd-less root
> login ; there you can run dmesg and see the splat.  The image is
> currently using 4.5-rc6 ; but any kernel can be inserted; "make
> modules_install INSTALL_MOD_PATH=here" and then populating those modules
> from "here" into /lib/modules of the loopback mounted image. And of
> course updating the bzImage on the qemu cmdline.    Currently it
> contains a bzImage and modules for 4.5-rc6 as I last tested that.
> 
> Also note that vncviewer will disconnect when it goes from early boot
> 80x25 to a higer res gfx mode; just reconnect and continue observing the
> target.
> 
> I've ruled out yocto kernel changes, and yocto toolchain -- but maybe it
> is a qemu issue this commit triggers ; who knows at this point.
> 
> Since I've NFI what component(s) cause this, I wanted to have the qemu
> binary, all libraries etc as part of the reproducer and nothing left to
> chance, and I've tested the reproducer on an ancient dual core w/o vmx
> and w/o any qemu binaries installed.  Bruce also tested it on a slightly
> more modern dual socket xeon with vmx and confirmed it failed there..
> 
> Inside there is a 00-runme ; mostly a copy of qemu args the yocto
> automated tests were using.  There is also everything the qemu binaries
> need to run ; toplevel dir is noisy since qemu only looks in ./ it
> seems.  There is also an ext4.img ; as mentioned earlier, this only
> happens when uvesafb.ko is a module, so one has to loopback mount that
> image and repopulate /lib/modules/  for each boot test/bisect step.
> 
> I've also included 00-bisect.txt as the output of git bisect log.  And
> there is also 00-configs/ dir that has the ".config" kernel file for
> each build (dir names are "git describe" in here for easy correlation)
> done for the bisect (plus the latest mainline build).  The failing commit
> in the subject is v4.1-rc5-22-g9cd25aac1f44 .
> 
> My contribution here is largely a bisect that can be relied on and
> providing a portable reproducer of the regression; I am by no means a
> PAT expert ; Richard invested more time into actually understanding the
> problem than I did, so I'm going to totally throw him under the bus on
> this when it comes to considering the ultimate root cause and possible
> fixes.  :)
> 
> Paul.
> --

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Thu, 2016-03-03 at 15:59 -0500, Paul Gortmaker wrote:
> So, the yocto folks moved from 4.1 to 4.4 and one of their automated
> qemu x86-32 boot tests started failing.  None of the yocto details seem
> to matter since I offered to help and I've repropduced it using 100%
> mainline kernels and a generic distro toolchain as well.
> 
> The test case is slightly complicated, in that it relies on uvesafb
> being modular, and so one has to juggle modules within an ext4 image
> that qemu boots from.  We tried making uvesafb builtin, but that made
> the issue magically vanish.  Given PAT, this isn't too surprising.
> 
> Richard did the preliminary investigation and analysis, and from that I
> did a bisect, and found the commit in $SUBJECT to be the root cause, as
> per the discussion here:
> 
> http://lists.openembedded.org/pipermail/openembedded-core/2016-March/1183
> 97.html
> 
> I'd mentioned the above to bpetkov on IRC and after confirming it was
> still an issue on 4.5-rc6, he'd asked if I had a portable reproducer.  
> 
> Not sure how complicated that would be, I set out to make one from my
> build.   With a little LD_PRELOAD type magic and ensuring all the qemu
> components are in ./  I have one that runs on an otherwise qemu-free
> x86-64 box. 
> 
> The stand alone reproducer is here; launched in 00-runme:
> 
> http://openlinux.wrs.com/pat-splat/reproducer.tar.bz2  
> 
> It is nothing fancy, just a generic yocto build of "sato" (gfx enabled
> rootfs).  When it "works" it boots to a UI touchscreen interface.  When
> it fails, you get a black screen with a blinking cursor (as seen in
> "vncviewer localhost:0").

Thanks for tracking down, and packaging the reproducer.  I simply untar'd
and ran 00-runme, but was not able to connect with localhost:0.  I am not
familiar with qemu, so I have not looked into why, though...

Anyway, with regarding the error message:
  "x86/PAT: Xorg:705 map pfn expected mapping type uncached-minus for [mem
0xfd000000-0xfdffffff], got write-combining"

Did it came from the following path during fork()?
 copy_process
  copy_mm
   dup_mm
    dup_mmap
     copy_page_range
      track_pfn_copy
       reserve_pfn_range

If so, track_pfn_copy() obtained pgprot from a PTE, and called
reserve_pfn_range() with it.  So, the error message indicates that previous
ioremap_wc() (i.e. pcm WC) resulted in creating UC- map (i.e. pgprot UC-).
 pcm is a logical cache type and pgprot is a HW cache type.  They can be
different when CPU does not have support for a given logical type.  This WC
to UC- conversion happens when CPU does not support PAT.

Richard's change, which compares with pgprot values in reserve_pfn_range()
is a good one, but I do not understand how we get into this mess.  We do
not have this check when PAT is disabled, and WC is supported when PAT is
enabled.

Commit 9cd25aac1 changed the initial values of the pcm<->pgrot conversion
tables.  The tables should be initialized with the same values after
pat_init() is called.  Is there any possibility that ioremap_wc() was
called before pat_init()..?

Also, can you send me a whole dmesg output?  I'd like to check how PAT is
initialized.

Thanks!
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 03/03/2016 (Thu 22:02) Toshi Kani wrote:

> On Thu, 2016-03-03 at 15:59 -0500, Paul Gortmaker wrote:
> > So, the yocto folks moved from 4.1 to 4.4 and one of their automated
> > qemu x86-32 boot tests started failing.  None of the yocto details seem
> > to matter since I offered to help and I've repropduced it using 100%
> > mainline kernels and a generic distro toolchain as well.
> > 
> > The test case is slightly complicated, in that it relies on uvesafb
> > being modular, and so one has to juggle modules within an ext4 image
> > that qemu boots from.  We tried making uvesafb builtin, but that made
> > the issue magically vanish.  Given PAT, this isn't too surprising.
> > 
> > Richard did the preliminary investigation and analysis, and from that I
> > did a bisect, and found the commit in $SUBJECT to be the root cause, as
> > per the discussion here:
> > 
> > http://lists.openembedded.org/pipermail/openembedded-core/2016-March/1183
> > 97.html
> > 
> > I'd mentioned the above to bpetkov on IRC and after confirming it was
> > still an issue on 4.5-rc6, he'd asked if I had a portable reproducer.  
> > 
> > Not sure how complicated that would be, I set out to make one from my
> > build.   With a little LD_PRELOAD type magic and ensuring all the qemu
> > components are in ./  I have one that runs on an otherwise qemu-free
> > x86-64 box. 
> > 
> > The stand alone reproducer is here; launched in 00-runme:
> > 
> > http://openlinux.wrs.com/pat-splat/reproducer.tar.bz2  
> > 
> > It is nothing fancy, just a generic yocto build of "sato" (gfx enabled
> > rootfs).  When it "works" it boots to a UI touchscreen interface.  When
> > it fails, you get a black screen with a blinking cursor (as seen in
> > "vncviewer localhost:0").
> 
> Thanks for tracking down, and packaging the reproducer.  I simply untar'd
> and ran 00-runme, but was not able to connect with localhost:0.  I am not
> familiar with qemu, so I have not looked into why, though...

Maybe it was localhost:1 in your case?  The qemu should have indicated
what vncserver sessions it started.  Can you paste in the output from
the 00-runme?   I tested the reproducer on a machine that was physically
distinct from the build, and that was a generic ubuntu install, but with
no qemu support installed at all and it worked there.  Plus I got Bruce
to test it worked on his machine, so I'm rather surprised it did not
work for you.

> 
> Anyway, with regarding the error message:
>   "x86/PAT: Xorg:705 map pfn expected mapping type uncached-minus for [mem
> 0xfd000000-0xfdffffff], got write-combining"
> 
> Did it came from the following path during fork()?
>  copy_process
>   copy_mm
>    dup_mm
>     dup_mmap
>      copy_page_range
>       track_pfn_copy
>        reserve_pfn_range

The trace is consistent, and was already captured by Richard, as per:

http://lists.openembedded.org/pipermail/openembedded-core/2016-March/118397.html

which is the link given earlier.  When I say consistent, I mean that I
get essentially the same thing when booting 4.5-rc6:

[   30.098100] x86/PAT: Xorg:509 map pfn expected mapping type uncached-minus for [mem 0xfd000000-0xfdffffff], got write-combining
[   30.106782] ------------[ cut here ]------------
[   30.107093] WARNING: CPU: 0 PID: 509 at /home/paul/poky/build/tmp-glibc/work-shared/qemux86/kernel-source/arch/x86/mm/pat.c:986 untrack_pfn+0x9f/0xb0()
[   30.112553] Modules linked in: 8021q parport_pc parport floppy uvesafb
[   30.113766] CPU: 0 PID: 509 Comm: Xorg Not tainted 4.5.0-rc6-yocto-standard #1
[   30.113806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
[   30.114078]  00000000 00003286 c0149d78 c13a6c7f 00000000 00000000 c0149dac c1052bcb
[   30.114214]  c1ac7544 00000000 000001fd c1ac1ea4 000003da c104cbdf 000003da c104cbdf
[   30.114214]  00000000 cdcf0528 00000000 c0149dbc c1052ca2 00000009 00000000 c0149de0
[   30.114214] Call Trace:
[   30.114214]  [<c13a6c7f>] dump_stack+0x58/0x79
[   30.114214]  [<c1052bcb>] warn_slowpath_common+0x8b/0xc0
[   30.114214]  [<c104cbdf>] ? untrack_pfn+0x9f/0xb0
[   30.114214]  [<c104cbdf>] ? untrack_pfn+0x9f/0xb0
[   30.114214]  [<c1052ca2>] warn_slowpath_null+0x22/0x30
[   30.114214]  [<c104cbdf>] untrack_pfn+0x9f/0xb0
[   30.114214]  [<c104ecf4>] ? __kunmap_atomic+0x54/0x110
[   30.114214]  [<c114f1cf>] unmap_single_vma+0x56f/0x580
[   30.114214]  [<c11321d0>] ? pagevec_move_tail_fn+0xa0/0xa0
[   30.114214]  [<c1150123>] unmap_vmas+0x43/0x60
[   30.114214]  [<c1154d5f>] exit_mmap+0x5f/0xf0
[   30.114214]  [<c10507bd>] mmput+0x2d/0xa0
[   30.114214]  [<c1051c19>] copy_process.part.47+0x1229/0x1430
[   30.114214]  [<c1051fb4>] _do_fork+0xb4/0x3b0
[   30.114214]  [<c105239c>] SyS_clone+0x2c/0x30
[   30.114214]  [<c1001a04>] do_syscall_32_irqs_on+0x54/0xb0
[   30.114214]  [<c18b06ca>] entry_INT80_32+0x2a/0x2a
[   30.124383] ---[ end trace f7c8a5d94542f94e ]---

> 
> If so, track_pfn_copy() obtained pgprot from a PTE, and called
> reserve_pfn_range() with it.  So, the error message indicates that previous
> ioremap_wc() (i.e. pcm WC) resulted in creating UC- map (i.e. pgprot UC-).
>  pcm is a logical cache type and pgprot is a HW cache type.  They can be
> different when CPU does not have support for a given logical type.  This WC
> to UC- conversion happens when CPU does not support PAT.
> 
> Richard's change, which compares with pgprot values in reserve_pfn_range()
> is a good one, but I do not understand how we get into this mess.  We do
> not have this check when PAT is disabled, and WC is supported when PAT is
> enabled.
> 
> Commit 9cd25aac1 changed the initial values of the pcm<->pgrot conversion
> tables.  The tables should be initialized with the same values after
> pat_init() is called.  Is there any possibility that ioremap_wc() was
> called before pat_init()..?

I don't think it is an initcall ordering thing; recall that I said the
problem seems to go away when built-in vs uvesafb as module.  So given
that, I think it is more related to where the code lands.

> 
> Also, can you send me a whole dmesg output?  I'd like to check how PAT is
> initialized.

I'll send the full file off list vs. spamming everyone with it.  I'm open
to booting the pre-fail commit with PAT specific bootargs and the post-fail
with the same and diffing the two dmesg if there are bootargs you'd like
me to test.  I'd also like to ensure you have a working reproducer locally
so maybe we should look at how that failed 1st.

Thanks,
Paul.
--

> 
> Thanks!
> -Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Fri, 2016-03-04 at 13:37 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 03/03/2016 (Thu 22:02) Toshi Kani wrote:
> 
> > On Thu, 2016-03-03 at 15:59 -0500, Paul Gortmaker wrote:
 :
> > > 
> > > The stand alone reproducer is here; launched in 00-runme:
> > > 
> > > http://openlinux.wrs.com/pat-splat/reproducer.tar.bz2  
> > > 
> > > It is nothing fancy, just a generic yocto build of "sato" (gfx
> > > enabled rootfs).  When it "works" it boots to a UI touchscreen
> > > interface.  When
> > > it fails, you get a black screen with a blinking cursor (as seen in
> > > "vncviewer localhost:0").
> > 
> > Thanks for tracking down, and packaging the reproducer.  I simply
> > untar'd and ran 00-runme, but was not able to connect with localhost:0.
> >  I am not familiar with qemu, so I have not looked into why, though...
> 
> Maybe it was localhost:1 in your case?  The qemu should have indicated
> what vncserver sessions it started.  Can you paste in the output from
> the 00-runme?   I tested the reproducer on a machine that was physically
> distinct from the build, and that was a generic ubuntu install, but with
> no qemu support installed at all and it worked there.  Plus I got Bruce
> to test it worked on his machine, so I'm rather surprised it did not
> work for you.

I am not really sure what I am doing is correct.

On one window:
# ./00-runme
Warning: vlan 0 is not connected to host network
VNC server running on '::1:5900'

And another window on the same system:
# vncviewer localhost:1

TigerVNC Viewer 64-bit v1.6.0
Built on: 2016-01-04 15:09
Copyright (C) 1999-2015 TigerVNC Team and many others (see README.txt)
See http://www.tigervnc.org for information on TigerVNC.
Can't open display: 

> > Anyway, with regarding the error message:
> >   "x86/PAT: Xorg:705 map pfn expected mapping type uncached-minus for
> > [mem
> > 0xfd000000-0xfdffffff], got write-combining"
> > 
> > Did it came from the following path during fork()?
> >  copy_process
> >   copy_mm
> >    dup_mm
> >     dup_mmap
> >      copy_page_range
> >       track_pfn_copy
> >        reserve_pfn_range
> 
> The trace is consistent, and was already captured by Richard, as per:
> 
> http://lists.openembedded.org/pipermail/openembedded-core/2016-March/1183
> 97.html
> 
> which is the link given earlier.  When I say consistent, I mean that I
> get essentially the same thing when booting 4.5-rc6:

There are two issues here.
 1. copy_process() failed in the check in reserve_pfn_range()
 2. error path in copy_process() then hit WARN_ON_ONCE in untrack_pfn().

We are currently looking into #1, which Richard referred as:
| It appears that Xorg mmaps this from userspace, then later does a
| fork() to execute a utility. At this point, when creating the vmas for
| the new process, the pat code says "eeek!" as the protection mode for
| the new vmas don't match the old one, returns -EINVAL, the process dies
| and X goes with it.

Since it seemed that Richard had some analysis on #1, I wanted to confirm
that the failure path of #1 was the above stack trace.

The stack trace below shows the failure path of #2.

> [   30.098100] x86/PAT: Xorg:509 map pfn expected mapping type uncached-
> minus for [mem 0xfd000000-0xfdffffff], got write-combining
> [   30.106782] ------------[ cut here ]------------
> [   30.107093] WARNING: CPU: 0 PID: 509 at /home/paul/poky/build/tmp-
> glibc/work-shared/qemux86/kernel-source/arch/x86/mm/pat.c:986
> untrack_pfn+0x9f/0xb0()
> [   30.112553] Modules linked in: 8021q parport_pc parport floppy uvesafb
> [   30.113766] CPU: 0 PID: 509 Comm: Xorg Not tainted 4.5.0-rc6-yocto-
> standard #1
> [   30.113806] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> [   30.114078]  00000000 00003286 c0149d78 c13a6c7f 00000000 00000000
> c0149dac c1052bcb
> [   30.114214]  c1ac7544 00000000 000001fd c1ac1ea4 000003da c104cbdf
> 000003da c104cbdf
> [   30.114214]  00000000 cdcf0528 00000000 c0149dbc c1052ca2 00000009
> 00000000 c0149de0
> [   30.114214] Call Trace:
> [   30.114214]  [<c13a6c7f>] dump_stack+0x58/0x79
> [   30.114214]  [<c1052bcb>] warn_slowpath_common+0x8b/0xc0
> [   30.114214]  [<c104cbdf>] ? untrack_pfn+0x9f/0xb0
> [   30.114214]  [<c104cbdf>] ? untrack_pfn+0x9f/0xb0
> [   30.114214]  [<c1052ca2>] warn_slowpath_null+0x22/0x30
> [   30.114214]  [<c104cbdf>] untrack_pfn+0x9f/0xb0
> [   30.114214]  [<c104ecf4>] ? __kunmap_atomic+0x54/0x110
> [   30.114214]  [<c114f1cf>] unmap_single_vma+0x56f/0x580
> [   30.114214]  [<c11321d0>] ? pagevec_move_tail_fn+0xa0/0xa0
> [   30.114214]  [<c1150123>] unmap_vmas+0x43/0x60
> [   30.114214]  [<c1154d5f>] exit_mmap+0x5f/0xf0
> [   30.114214]  [<c10507bd>] mmput+0x2d/0xa0
> [   30.114214]  [<c1051c19>] copy_process.part.47+0x1229/0x1430
> [   30.114214]  [<c1051fb4>] _do_fork+0xb4/0x3b0
> [   30.114214]  [<c105239c>] SyS_clone+0x2c/0x30
> [   30.114214]  [<c1001a04>] do_syscall_32_irqs_on+0x54/0xb0
> [   30.114214]  [<c18b06ca>] entry_INT80_32+0x2a/0x2a
> [   30.124383] ---[ end trace f7c8a5d94542f94e ]---
> 
> > 
> > If so, track_pfn_copy() obtained pgprot from a PTE, and called
> > reserve_pfn_range() with it.  So, the error message indicates that
> > previous ioremap_wc() (i.e. pcm WC) resulted in creating UC- map (i.e.
> > pgprot UC-).  pcm is a logical cache type and pgprot is a HW cache
> > type.  They can be different when CPU does not have support for a given
> > logical type.  This WC to UC- conversion happens when CPU does not
> > support PAT.
> > 
> > Richard's change, which compares with pgprot values in
> > reserve_pfn_range() is a good one, but I do not understand how we get
> > into this mess.  We do not have this check when PAT is disabled, and WC
> > is supported when PAT is enabled.
> > 
> > Commit 9cd25aac1 changed the initial values of the pcm<->pgrot
> > conversion tables.  The tables should be initialized with the same
> > values after pat_init() is called.  Is there any possibility that
> > ioremap_wc() was called before pat_init()..?
> 
> I don't think it is an initcall ordering thing; recall that I said the
> problem seems to go away when built-in vs uvesafb as module.  So given
> that, I think it is more related to where the code lands.

>From the failure, it looks as if pat_init() was not called at all, but I
cannot explain how built-in vs module can make such difference. 

> > Also, can you send me a whole dmesg output?  I'd like to check how PAT
> > is initialized.
> 
> I'll send the full file off list vs. spamming everyone with it.  I'm open
> to booting the pre-fail commit with PAT specific bootargs and the post-
> fail with the same and diffing the two dmesg if there are bootargs you'd
> like me to test.  I'd also like to ensure you have a working reproducer
> locally so maybe we should look at how that failed 1st.

Great.  Yes, two dmesg will be really helpful.

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 04/03/2016 (Fri 15:12) Toshi Kani wrote:

> On Fri, 2016-03-04 at 13:37 -0500, Paul Gortmaker wrote:
> > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > disabled"] On 03/03/2016 (Thu 22:02) Toshi Kani wrote:
> > 
> > > On Thu, 2016-03-03 at 15:59 -0500, Paul Gortmaker wrote:
>  :
> > > > 
> > > > The stand alone reproducer is here; launched in 00-runme:
> > > > 
> > > > http://openlinux.wrs.com/pat-splat/reproducer.tar.bz2  
> > > > 
> > > > It is nothing fancy, just a generic yocto build of "sato" (gfx
> > > > enabled rootfs).  When it "works" it boots to a UI touchscreen
> > > > interface.  When
> > > > it fails, you get a black screen with a blinking cursor (as seen in
> > > > "vncviewer localhost:0").
> > > 
> > > Thanks for tracking down, and packaging the reproducer.  I simply
> > > untar'd and ran 00-runme, but was not able to connect with localhost:0.
> > >  I am not familiar with qemu, so I have not looked into why, though...
> > 
> > Maybe it was localhost:1 in your case?  The qemu should have indicated
> > what vncserver sessions it started.  Can you paste in the output from
> > the 00-runme?   I tested the reproducer on a machine that was physically
> > distinct from the build, and that was a generic ubuntu install, but with
> > no qemu support installed at all and it worked there.  Plus I got Bruce
> > to test it worked on his machine, so I'm rather surprised it did not
> > work for you.
> 
> I am not really sure what I am doing is correct.
> 
> On one window:
> # ./00-runme
> Warning: vlan 0 is not connected to host network
> VNC server running on '::1:5900'

Ah, it seems for some reason your system confuses qemu from using the
"normal"  IPv4 default.   Try editing ./00-runme and add an explicit
vnc option    "-vnc 127.0.0.1:5" in front of "-show-cursor" and then:

> 
> And another window on the same system:
> # vncviewer localhost:1

...connect to localhost:5  here.  I chose 5 just to not bump into
anything your system might have on :0 or :1 already.

> 
> TigerVNC Viewer 64-bit v1.6.0
> Built on: 2016-01-04 15:09
> Copyright (C) 1999-2015 TigerVNC Team and many others (see README.txt)
> See http://www.tigervnc.org for information on TigerVNC.
> Can't open display: 
> 

[snip analysis, leaving that for Richard to comment on.]

> 
> > > Also, can you send me a whole dmesg output?  I'd like to check how PAT
> > > is initialized.
> > 
> > I'll send the full file off list vs. spamming everyone with it.  I'm open
> > to booting the pre-fail commit with PAT specific bootargs and the post-
> > fail with the same and diffing the two dmesg if there are bootargs you'd
> > like me to test.  I'd also like to ensure you have a working reproducer
> > locally so maybe we should look at how that failed 1st.
> 
> Great.  Yes, two dmesg will be really helpful.

So I booted both with "debugpat" on the bootline; both being the last
working commit [v4.1-rc5-21-g9dac62909451]  and then the 1st failing
commit [v4.1-rc5-22-g9cd25aac1f44].  I captured the dmesg of each, then
stripped the timestamps and diffed them.


--- works.txt	2016-03-06 19:13:28.245836555 -0500
+++ fail.txt	2016-03-06 19:13:17.321836308 -0500
@@ -454,15 +454,38 @@
  8021q: 802.1Q VLAN Support v1.8
  8021q: adding VLAN 0 to HW filter on device eth0
  x86/PAT: Overlap at 0xfd000000-0xfe000000
- x86/PAT: reserve_memtype added [mem 0xfd000000-0xfdffffff], track write-combining, req write-combining, ret write-combining
- x86/PAT: Overlap at 0xfe000000-0xfe010000
- x86/PAT: reserve_memtype added [mem 0xfe000000-0xfe00ffff], track uncached-minus, req uncached-minus, ret uncached-minus
+ x86/PAT: reserve_memtype added [mem 0xfd000000-0xfdffffff], track write-combining, req uncached-minus, ret write-combining
  x86/PAT: free_memtype request [mem 0xfd000000-0xfdffffff]
- x86/PAT: free_memtype request [mem 0xfe000000-0xfe00ffff]
+ x86/PAT: Xorg:475 map pfn expected mapping type uncached-minus for [mem 0xfd000000-0xfdffffff], got write-combining
+ ------------[ cut here ]------------
+ WARNING: CPU: 0 PID: 475 at /home/paul/poky/build/tmp-glibc/work-shared/qemux86/kernel-source/arch/x86/mm/pat.c:938 untrack_pfn+0x9f/0xb0()
+ Modules linked in: 8021q parport_pc parport floppy uvesafb
+ CPU: 0 PID: 475 Comm: Xorg Not tainted 4.1.0-rc5-yocto-standard #1
+ Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
+  00000000 00000000 cdf75db0 c1837711 00000000 cdf75de4 c104f0cb c1a39d10
+  00000000 000001db c1a38f54 000003aa c1049ddf 000003aa c1049ddf 00000000
+  c0108840 00000000 cdf75df4 c104f1a2 00000009 00000000 cdf75e18 c1049ddf
+ Call Trace:
+  [<c1837711>] dump_stack+0x4b/0x75
+  [<c104f0cb>] warn_slowpath_common+0x8b/0xc0
+  [<c1049ddf>] ? untrack_pfn+0x9f/0xb0
+  [<c1049ddf>] ? untrack_pfn+0x9f/0xb0
+  [<c104f1a2>] warn_slowpath_null+0x22/0x30
+  [<c1049ddf>] untrack_pfn+0x9f/0xb0
+  [<c104b9d3>] ? __kunmap_atomic+0x33/0xc0
+  [<c113c07f>] unmap_single_vma+0x49f/0x4b0
+  [<c113cdc3>] unmap_vmas+0x43/0x60
+  [<c11428ff>] exit_mmap+0x5f/0xf0
+  [<c1072d39>] ? get_parent_ip+0x9/0x40
+  [<c104cccd>] mmput+0x2d/0xa0
+  [<c104dfdd>] copy_process.part.45+0x10dd/0x14a0
+  [<c104e541>] do_fork+0xc1/0x390
+  [<c104e8d5>] SyS_clone+0x25/0x30
+  [<c183ef46>] syscall_call+0x7/0x7
+ ---[ end trace dbbf5a6b2dea64ff ]---
  x86/PAT: Overlap at 0xfd000000-0xfe000000
- x86/PAT: reserve_memtype added [mem 0xfd000000-0xfdffffff], track write-combining, req write-combining, ret write-combining
- x86/PAT: Overlap at 0xfe000000-0xfe010000
- x86/PAT: reserve_memtype added [mem 0xfe000000-0xfe00ffff], track uncached-minus, req uncached-minus, ret uncached-minus
+ x86/PAT: reserve_memtype added [mem 0xfd000000-0xfdffffff], track write-combining, req uncached-minus, ret write-combining
  x86/PAT: free_memtype request [mem 0xfd000000-0xfdffffff]
+ x86/PAT: Xorg:475 map pfn expected mapping type uncached-minus for [mem 0xfd000000-0xfdffffff], got write-combining
  x86/PAT: free_memtype request [mem 0xfe000000-0xfe00ffff]
- hrtimer: interrupt took 5057254 ns
+ x86/PAT: free_memtype request [mem 0xfd000000-0xfdffffff]

One obvious difference is the number of overlaps:

~$cat fail.txt |grep Overlap | wc -l
7
~$cat works.txt |grep Overlap | wc -l
9
~$

I'm assuming everyone here is used to reading diffs, but if someone
wants the full dmesg files let me know and I'll send them off-list.

Paul.

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Sun, 2016-03-06 at 19:35 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 04/03/2016 (Fri 15:12) Toshi Kani wrote:
> 
> > On Fri, 2016-03-04 at 13:37 -0500, Paul Gortmaker wrote:
> > > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > > disabled"] On 03/03/2016 (Thu 22:02) Toshi Kani wrote:
> > > 
:
> > I am not really sure what I am doing is correct.
> > 
> > On one window:
> > # ./00-runme
> > Warning: vlan 0 is not connected to host network
> > VNC server running on '::1:5900'
> 
> Ah, it seems for some reason your system confuses qemu from using the
> "normal"  IPv4 default.   Try editing ./00-runme and add an explicit
> vnc option    "-vnc 127.0.0.1:5" in front of "-show-cursor" and then:
> 
> > 
> > And another window on the same system:
> > # vncviewer localhost:1
> 
> ...connect to localhost:5  here.  I chose 5 just to not bump into
> anything your system might have on :0 or :1 already.

Still no luck.  Perhaps, vlan needs to connect?

# ./00-runme
Warning: vlan 0 is not connected to host network

# vncviewer localhost:5

TigerVNC Viewer 64-bit v1.6.0
Built on: 2016-01-04 15:09
Copyright (C) 1999-2015 TigerVNC Team and many others (see README.txt)
See http://www.tigervnc.org for information on TigerVNC.
Can't open display: 

> I'm assuming everyone here is used to reading diffs, but if someone
> wants the full dmesg files let me know and I'll send them off-list.

Yes, please send me full dmesg files.  Since I do not know your original
state, the diff does not give me the whole picture.

Thanks!
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Mon, 2016-03-07 at 16:08 -0500, Paul Gortmaker wrote:
> [dropping oe list and lkml since attaching dmesg files.]
> 
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 07/03/2016 (Mon 09:03) Toshi Kani wrote:
> 
> > On Sun, 2016-03-06 at 19:35 -0500, Paul Gortmaker wrote:
> > > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > > disabled"] On 04/03/2016 (Fri 15:12) Toshi Kani wrote:
> > > 
> > > > On Fri, 2016-03-04 at 13:37 -0500, Paul Gortmaker wrote:
> > > > > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > > > > disabled"] On 03/03/2016 (Thu 22:02) Toshi Kani wrote:
> > > > > 
> > :
> > > > I am not really sure what I am doing is correct.
> > > > 
> > > > On one window:
> > > > # ./00-runme
> > > > Warning: vlan 0 is not connected to host network
> > > > VNC server running on '::1:5900'
> > > 
> > > Ah, it seems for some reason your system confuses qemu from using the
> > > "normal"  IPv4 default.   Try editing ./00-runme and add an explicit
> > > vnc option    "-vnc 127.0.0.1:5" in front of "-show-cursor" and then:
> > > 
> > > > 
> > > > And another window on the same system:
> > > > # vncviewer localhost:1
> > > 
> > > ...connect to localhost:5  here.  I chose 5 just to not bump into
> > > anything your system might have on :0 or :1 already.
> > 
> > Still no luck.  Perhaps, vlan needs to connect?
> > 
> > # ./00-runme
> > Warning: vlan 0 is not connected to host network
> 
> Nope, I get that too, it isn't critical.
> > 
> > # vncviewer localhost:5
> > 
> > TigerVNC Viewer 64-bit v1.6.0
> > Built on: 2016-01-04 15:09
> > Copyright (C) 1999-2015 TigerVNC Team and many others (see README.txt)
> > See http://www.tigervnc.org for information on TigerVNC.
> > Can't open display: 
> > 
> > > I'm assuming everyone here is used to reading diffs, but if someone
> > > wants the full dmesg files let me know and I'll send them off-list.
> > 
> > Yes, please send me full dmesg files.  Since I do not know your
> > original state, the diff does not give me the whole picture.
> 
> Attached.

Thanks for the dmesg files!  As I suspected, there is no message from
pat_init() in both cases.  That is, you are missing the following message,
which shows how PAT is configured to support cache attributes.

# dmesg | grep PAT
[0.000000] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WC  UC- WT  

It may have seemed working before, but you did not have WC configured to
PAT without calling pat_init().  There was not a proper check in place to
detect this error before.  Can you please check your code to see what
caused this skip of pat_init()?  If you have a git tree, I can take a look
as well. 

-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 07/03/2016 (Mon 16:38) Toshi Kani wrote:

> On Mon, 2016-03-07 at 16:08 -0500, Paul Gortmaker wrote:
> > [dropping oe list and lkml since attaching dmesg files.]
> > 

[...]

> > > Yes, please send me full dmesg files.  Since I do not know your
> > > original state, the diff does not give me the whole picture.
> > 
> > Attached.
> 
> Thanks for the dmesg files!  As I suspected, there is no message from
> pat_init() in both cases.  That is, you are missing the following message,
> which shows how PAT is configured to support cache attributes.
> 
> # dmesg | grep PAT
> [0.000000] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WC  UC- WT  

Interesting...

> 
> It may have seemed working before, but you did not have WC configured to
> PAT without calling pat_init().  There was not a proper check in place to
> detect this error before.  Can you please check your code to see what
> caused this skip of pat_init()?  If you have a git tree, I can take a look
> as well. 

You already have git copies of what I'm running, since it is vanilla
mainline commits.  No code changes at this end whatsoever.  I did the
bisect on vanilla mainline.  All I took from yocto was their ".config"

To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-g9cd25aac1f44
fails, and v4.5-rc6 also fails.  If pat_init() isn't called then this
is a bug in current mainline.  I'll have a look later myself and see
if I can trace out how we expect to get to pat_init() and how that
might be skipped inadvertently unless someone beats me to it.

Paul.
--

> 
> -Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Mon, 2016-03-07 at 18:53 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 07/03/2016 (Mon 16:38) Toshi Kani wrote:
> 
> > On Mon, 2016-03-07 at 16:08 -0500, Paul Gortmaker wrote:
> > > [dropping oe list and lkml since attaching dmesg files.]
> > > 
> 
> [...]
> 
> > > > Yes, please send me full dmesg files.  Since I do not know your
> > > > original state, the diff does not give me the whole picture.
> > > 
> > > Attached.
> > 
> > Thanks for the dmesg files!  As I suspected, there is no message from
> > pat_init() in both cases.  That is, you are missing the following
> > message,
> > which shows how PAT is configured to support cache attributes.
> > 
> > # dmesg | grep PAT
> > [0.000000] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WC  UC-
> > WT  
> 
> Interesting...
> 
> > 
> > It may have seemed working before, but you did not have WC configured
> > to PAT without calling pat_init().  There was not a proper check in
> > place to detect this error before.  Can you please check your code to
> > see what caused this skip of pat_init()?  If you have a git tree, I can
> > take a look as well. 
> 
> You already have git copies of what I'm running, since it is vanilla
> mainline commits.  No code changes at this end whatsoever.  I did the
> bisect on vanilla mainline.  All I took from yocto was their ".config"
> 
> To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-g9cd25aac1f44
> fails, and v4.5-rc6 also fails.  If pat_init() isn't called then this
> is a bug in current mainline.  I'll have a look later myself and see
> if I can trace out how we expect to get to pat_init() and how that
> might be skipped inadvertently unless someone beats me to it.

Oh, I see.  Can you send me the ".config" file?

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Mon, 2016-03-07 at 17:56 -0700, Toshi Kani wrote:
> On Mon, 2016-03-07 at 18:53 -0500, Paul Gortmaker wrote:
> > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > disabled"] On 07/03/2016 (Mon 16:38) Toshi Kani wrote:
> > 
> > > On Mon, 2016-03-07 at 16:08 -0500, Paul Gortmaker wrote:
> > > > [dropping oe list and lkml since attaching dmesg files.]
> > > > 
> > 
> > [...]
> > 
> > > > > Yes, please send me full dmesg files.  Since I do not know your
> > > > > original state, the diff does not give me the whole picture.
> > > > 
> > > > Attached.
> > > 
> > > Thanks for the dmesg files!  As I suspected, there is no message from
> > > pat_init() in both cases.  That is, you are missing the following
> > > message,
> > > which shows how PAT is configured to support cache attributes.
> > > 
> > > # dmesg | grep PAT
> > > [0.000000] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WC  UC-
> > > WT  
> > 
> > Interesting...
> > 
> > > 
> > > It may have seemed working before, but you did not have WC configured
> > > to PAT without calling pat_init().  There was not a proper check in
> > > place to detect this error before.  Can you please check your code to
> > > see what caused this skip of pat_init()?  If you have a git tree, I
> > > can
> > > take a look as well. 
> > 
> > You already have git copies of what I'm running, since it is vanilla
> > mainline commits.  No code changes at this end whatsoever.  I did the
> > bisect on vanilla mainline.  All I took from yocto was their ".config"
> > 
> > To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-g9cd25aac1f44
> > fails, and v4.5-rc6 also fails.  If pat_init() isn't called then this
> > is a bug in current mainline.  I'll have a look later myself and see
> > if I can trace out how we expect to get to pat_init() and how that
> > might be skipped inadvertently unless someone beats me to it.
> 
> Oh, I see.  Can you send me the ".config" file?

And also an output of /proc/cpuinfo, please?

I think I know what's going on.  I noticed that you have the following
message in your dmesg files.

 [    0.000000] MTRR: Disabled

MTRR is set to disabled when your CPU is Intel but does not support MTRR.
 Perhaps, QEMU does not emulate MTRR?

pat_init() is not called when MTRR is disabled.  I think this dependency is
wrong, and it needs to be fixed.

This issue has been there for a long time, and you have been running
essentially as PAT disabled in the past.  The commit in question simply
detected this issue.

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 07/03/2016 (Mon 17:56) Toshi Kani wrote:

[..]


> > > It may have seemed working before, but you did not have WC configured
> > > to PAT without calling pat_init().  There was not a proper check in
> > > place to detect this error before.  Can you please check your code to
> > > see what caused this skip of pat_init()?  If you have a git tree, I can
> > > take a look as well. 
> > 
> > You already have git copies of what I'm running, since it is vanilla
> > mainline commits.  No code changes at this end whatsoever.  I did the
> > bisect on vanilla mainline.  All I took from yocto was their ".config"
> > 
> > To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-g9cd25aac1f44
> > fails, and v4.5-rc6 also fails.  If pat_init() isn't called then this
> > is a bug in current mainline.  I'll have a look later myself and see
> > if I can trace out how we expect to get to pat_init() and how that
> > might be skipped inadvertently unless someone beats me to it.
> 
> Oh, I see.  Can you send me the ".config" file?

I packaged the .config file for every bisect point in the original
reproducer tarball.  Let me know if you can't find them.  It should
look like this:

paul@dell760-paul:~/qemu-fail$ ls -al 00-configs/
total 92
drwxrwxr-x 23 paul paul 4096 Mar  3 10:59 .
drwxrwxr-x  5 paul paul 4096 Mar  3 11:01 ..
-rw-rw-r--  1 paul paul    0 Mar  3 10:59 00-all-dot-configs-are-in-here
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-1281-g43224b96af31
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-1625-g44d21c3f3a2e
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-3251-g4e241557fc1c
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-6547-g4570a37169d4
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-912-ge75c73ad6447
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc4-85-gd4688bdc6335
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-21-g9dac62909451
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-22-g9cd25aac1f44
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-23-g7202fdb1b329
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-27-gd838270e2516
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-35-g7ea402d01cb6
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc6-256-g9dda1658a9bd
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc6-296-g7ef3d7d58d9d
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.2
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.2-rc1
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.3
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.4
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.4.1
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.4.1-348-g0194c7658611
drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.5-rc6
paul@dell760-paul:~/qemu-fail$ 

Please check if they are present in the reproducer you downloaded.
Maybe somehow I uploaded a version without the config files.

Thanks,
Paul.
--

> 
> Thanks,
> -Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 07/03/2016 (Mon 18:35) Toshi Kani wrote:

> On Mon, 2016-03-07 at 17:56 -0700, Toshi Kani wrote:
> > On Mon, 2016-03-07 at 18:53 -0500, Paul Gortmaker wrote:
> > > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > > disabled"] On 07/03/2016 (Mon 16:38) Toshi Kani wrote:
> > > 
> > > > On Mon, 2016-03-07 at 16:08 -0500, Paul Gortmaker wrote:
> > > > > [dropping oe list and lkml since attaching dmesg files.]
> > > > > 
> > > 
> > > [...]
> > > 
> > > > > > Yes, please send me full dmesg files.  Since I do not know your
> > > > > > original state, the diff does not give me the whole picture.
> > > > > 
> > > > > Attached.
> > > > 
> > > > Thanks for the dmesg files!  As I suspected, there is no message from
> > > > pat_init() in both cases.  That is, you are missing the following
> > > > message,
> > > > which shows how PAT is configured to support cache attributes.
> > > > 
> > > > # dmesg | grep PAT
> > > > [0.000000] x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WC  UC-
> > > > WT  
> > > 
> > > Interesting...
> > > 
> > > > 
> > > > It may have seemed working before, but you did not have WC configured
> > > > to PAT without calling pat_init().  There was not a proper check in
> > > > place to detect this error before.  Can you please check your code to
> > > > see what caused this skip of pat_init()?  If you have a git tree, I
> > > > can
> > > > take a look as well. 
> > > 
> > > You already have git copies of what I'm running, since it is vanilla
> > > mainline commits.  No code changes at this end whatsoever.  I did the
> > > bisect on vanilla mainline.  All I took from yocto was their ".config"
> > > 
> > > To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-g9cd25aac1f44
> > > fails, and v4.5-rc6 also fails.  If pat_init() isn't called then this
> > > is a bug in current mainline.  I'll have a look later myself and see
> > > if I can trace out how we expect to get to pat_init() and how that
> > > might be skipped inadvertently unless someone beats me to it.
> > 
> > Oh, I see.  Can you send me the ".config" file?
> 
> And also an output of /proc/cpuinfo, please?

Host?  Guest?  Both?

> 
> I think I know what's going on.  I noticed that you have the following
> message in your dmesg files.
> 
>  [    0.000000] MTRR: Disabled
> 
> MTRR is set to disabled when your CPU is Intel but does not support MTRR.

I've run the test on a modern expensive xeon, a 4-5 year old xeon, and
on an old pentium dual core (the cheaper dumbed down core2-duo that doesn't
support virtualization) from around 2007.  In all cases the result was
the same.  Perhaps that is because the qemu launch script appears to set
the CPU type regardless?  (it uses "-cpu qemu32" but I confess that I do
not know exactly what silicon that tries to emulate).

>  Perhaps, QEMU does not emulate MTRR?

I will be the 1st to admit that I am not a seasoned qemu user, so I've
no idea if the above is true.  I still prefer testing on real hardware,
even if that comes across as "old school".  :)

> 
> pat_init() is not called when MTRR is disabled.  I think this dependency is
> wrong, and it needs to be fixed.
> 
> This issue has been there for a long time, and you have been running
> essentially as PAT disabled in the past.  The commit in question simply
> detected this issue.

OK, that sounds good -- in that it seems we are finally getting to the
bottom of what happened here.  Any thoughts on why built-in vs. modular
somehow managed to mask the issue?

Paul.
--

> 
> Thanks,
> -Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 08/03/2016 (Tue 09:13) Toshi Kani wrote:

[...]

> 
> Yes, I have these directories, but I do not see anything under them...  Now
> that I do not think there is anything unique in your .config file, but it'd
> be still good to check.  Please send me .config file for v4.5-rc6 (if you
> do not have it now, .config for other ver is OK as well).
> 
> $ ls -Rl

Try "ls -aRl" -- they are _dot_ config files.  I did not rename them.

Paul.
--

> .:
> total 84
> -rw-rw-r--. 1 toshi toshi    0 Mar  3 08:59 00-all-dot-configs-are-in-here
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-1281-g43224b96af31
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-1625-g44d21c3f3a2e
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-3251-g4e241557fc1c
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-6547-g4570a37169d4
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-912-ge75c73ad6447
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc4-85-gd4688bdc6335
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-21-g9dac62909451
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-22-g9cd25aac1f44
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-23-g7202fdb1b329
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-27-gd838270e2516
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-35-g7ea402d01cb6
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc6-256-g9dda1658a9bd
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc6-296-g7ef3d7d58d9d
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.2
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.2-rc1
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.3
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.4
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.4.1
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.4.1-348-g0194c7658611
> drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.5-rc6
> 
> ./v4.1:
> total 0
> 
> ./v4.1-1281-g43224b96af31:
> total 0
> 
> ./v4.1-1625-g44d21c3f3a2e:
> total 0
> 
> ./v4.1-3251-g4e241557fc1c:
> total 0
> ./v4.1-6547-g4570a37169d4:
> total 0
> 
> ./v4.1-912-ge75c73ad6447:
> total 0
> 
> ./v4.1-rc4-85-gd4688bdc6335:
> total 0
> 
> ./v4.1-rc5-21-g9dac62909451:
> total 0
> 
> ./v4.1-rc5-22-g9cd25aac1f44:
> total 0
> 
> ./v4.1-rc5-23-g7202fdb1b329:
> total 0
> 
> ./v4.1-rc5-27-gd838270e2516:
> total 0
> ./v4.1-rc5-35-g7ea402d01cb6:
> total 0
> 
> ./v4.1-rc6-256-g9dda1658a9bd:
> total 0
> 
> ./v4.1-rc6-296-g7ef3d7d58d9d:
> total 0
> 
> ./v4.2:
> total 0
> 
> ./v4.2-rc1:
> total 0
> 
> ./v4.3:
> total 0
> 
> ./v4.4:
> total 0
> ./v4.4.1:
> total 0
> 
> ./v4.4.1-348-g0194c7658611:
> total 0
> 
> ./v4.5-rc6:
> total 0
> 
> Thanks,
> -Toshi
> 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Mon, 2016-03-07 at 22:16 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 07/03/2016 (Mon 17:56) Toshi Kani wrote:
> 
> [..]
> 
> 
> > > > It may have seemed working before, but you did not have WC
> > > > configured
> > > > to PAT without calling pat_init().  There was not a proper check in
> > > > place to detect this error before.  Can you please check your code
> > > > to
> > > > see what caused this skip of pat_init()?  If you have a git tree, I
> > > > can
> > > > take a look as well. 
> > > 
> > > You already have git copies of what I'm running, since it is vanilla
> > > mainline commits.  No code changes at this end whatsoever.  I did the
> > > bisect on vanilla mainline.  All I took from yocto was their
> > > ".config"
> > > 
> > > To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-g9cd25aac1f44
> > > fails, and v4.5-rc6 also fails.  If pat_init() isn't called then this
> > > is a bug in current mainline.  I'll have a look later myself and see
> > > if I can trace out how we expect to get to pat_init() and how that
> > > might be skipped inadvertently unless someone beats me to it.
> > 
> > Oh, I see.  Can you send me the ".config" file?
> 
> I packaged the .config file for every bisect point in the original
> reproducer tarball.  Let me know if you can't find them.  It should
> look like this:
> 
> paul@dell760-paul:~/qemu-fail$ ls -al 00-configs/
> total 92
> drwxrwxr-x 23 paul paul 4096 Mar  3 10:59 .
> drwxrwxr-x  5 paul paul 4096 Mar  3 11:01 ..
> -rw-rw-r--  1 paul paul    0 Mar  3 10:59 00-all-dot-configs-are-in-here
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-1281-g43224b96af31
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-1625-g44d21c3f3a2e
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-3251-g4e241557fc1c
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-6547-g4570a37169d4
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-912-ge75c73ad6447
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc4-85-gd4688bdc6335
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-21-g9dac62909451
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-22-g9cd25aac1f44
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-23-g7202fdb1b329
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-27-gd838270e2516
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc5-35-g7ea402d01cb6
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc6-256-g9dda1658a9bd
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.1-rc6-296-g7ef3d7d58d9d
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.2
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.2-rc1
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.3
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.4
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.4.1
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.4.1-348-g0194c7658611
> drwxrwxr-x  2 paul paul 4096 Mar  3 10:59 v4.5-rc6
> paul@dell760-paul:~/qemu-fail$ 
> 
> Please check if they are present in the reproducer you downloaded.
> Maybe somehow I uploaded a version without the config files.

Yes, I have these directories, but I do not see anything under them...  Now
that I do not think there is anything unique in your .config file, but it'd
be still good to check.  Please send me .config file for v4.5-rc6 (if you
do not have it now, .config for other ver is OK as well).

$ ls -Rl
.:
total 84
-rw-rw-r--. 1 toshi toshi    0 Mar  3 08:59 00-all-dot-configs-are-in-here
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-1281-g43224b96af31
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-1625-g44d21c3f3a2e
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-3251-g4e241557fc1c
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-6547-g4570a37169d4
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-912-ge75c73ad6447
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc4-85-gd4688bdc6335
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-21-g9dac62909451
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-22-g9cd25aac1f44
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-23-g7202fdb1b329
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-27-gd838270e2516
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc5-35-g7ea402d01cb6
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc6-256-g9dda1658a9bd
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.1-rc6-296-g7ef3d7d58d9d
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.2
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.2-rc1
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.3
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.4
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.4.1
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.4.1-348-g0194c7658611
drwxrwxr-x. 2 toshi toshi 4096 Mar  3 08:59 v4.5-rc6

./v4.1:
total 0

./v4.1-1281-g43224b96af31:
total 0

./v4.1-1625-g44d21c3f3a2e:
total 0

./v4.1-3251-g4e241557fc1c:
total 0
./v4.1-6547-g4570a37169d4:
total 0

./v4.1-912-ge75c73ad6447:
total 0

./v4.1-rc4-85-gd4688bdc6335:
total 0

./v4.1-rc5-21-g9dac62909451:
total 0

./v4.1-rc5-22-g9cd25aac1f44:
total 0

./v4.1-rc5-23-g7202fdb1b329:
total 0

./v4.1-rc5-27-gd838270e2516:
total 0
./v4.1-rc5-35-g7ea402d01cb6:
total 0

./v4.1-rc6-256-g9dda1658a9bd:
total 0

./v4.1-rc6-296-g7ef3d7d58d9d:
total 0

./v4.2:
total 0

./v4.2-rc1:
total 0

./v4.3:
total 0

./v4.4:
total 0
./v4.4.1:
total 0

./v4.4.1-348-g0194c7658611:
total 0

./v4.5-rc6:
total 0

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Mon, 2016-03-07 at 22:28 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 07/03/2016 (Mon 18:35) Toshi Kani wrote:
> 
> > On Mon, 2016-03-07 at 17:56 -0700, Toshi Kani wrote:
> > > On Mon, 2016-03-07 at 18:53 -0500, Paul Gortmaker wrote:
> > > > [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> > > > disabled"] On 07/03/2016 (Mon 16:38) Toshi Kani wrote:
> > > > 
> > > > > On Mon, 2016-03-07 at 16:08 -0500, Paul Gortmaker wrote:
> > > > > > [dropping oe list and lkml since attaching dmesg files.]
> > > > > > 
> > > > 
> > > > [...]
> > > > 
> > > > > > > Yes, please send me full dmesg files.  Since I do not know
> > > > > > > your original state, the diff does not give me the whole
> > > > > > > picture.
> > > > > > 
> > > > > > Attached.
> > > > > 
> > > > > Thanks for the dmesg files!  As I suspected, there is no message
> > > > > from pat_init() in both cases.  That is, you are missing the
> > > > > following message, which shows how PAT is configured to support
> > > > > cache attributes.
> > > > > 
> > > > > # dmesg | grep PAT
> > > > > [0.000000] x86/PAT: Configuration [0-7]:
> > > > > WB  WC  UC- UC  WB  WC  UC- WT  
> > > > 
> > > > Interesting...
> > > > 
> > > > > 
> > > > > It may have seemed working before, but you did not have WC
> > > > > configured to PAT without calling pat_init().  There was not a
> > > > > proper check in place to detect this error before.  Can you
> > > > > please check your code to see what caused this skip of
> > > > > pat_init()?  If you have a git tree, I can take a look as well. 
> > > > 
> > > > You already have git copies of what I'm running, since it is
> > > > vanilla mainline commits.  No code changes at this end
> > > > whatsoever.  I did the bisect on vanilla mainline.  All I took from
> > > > yocto was their ".config"
> > > > 
> > > > To recap, v4.1-rc5-21-g9dac62909451 works,  v4.1-rc5-22-
> > > > g9cd25aac1f44 fails, and v4.5-rc6 also fails.  If pat_init() isn't
> > > > called then this is a bug in current mainline.  I'll have a look
> > > > later myself and see if I can trace out how we expect to get to
> > > > pat_init() and how that might be skipped inadvertently unless
> > > > someone beats me to it.
> > > 
> > > Oh, I see.  Can you send me the ".config" file?
> > 
> > And also an output of /proc/cpuinfo, please?
> 
> Host?  Guest?  Both?

Guest.


> > I think I know what's going on.  I noticed that you have the following
> > message in your dmesg files.
> > 
> >  [    0.000000] MTRR: Disabled
> > 
> > MTRR is set to disabled when your CPU is Intel but does not support
> > MTRR.
> 
> I've run the test on a modern expensive xeon, a 4-5 year old xeon, and
> on an old pentium dual core (the cheaper dumbed down core2-duo that
> doesn't support virtualization) from around 2007.  In all cases the
> result was the same.  Perhaps that is because the qemu launch script
> appears to set the CPU type regardless?  (it uses "-cpu qemu32" but I
> confess that I do not know exactly what silicon that tries to emulate).

There is a matter of how qemu emulates CPU features.  There is no such
Intel CPU that supports PAT w/o MTRR.  This is why the current code assumes
this dependency.

> >  Perhaps, QEMU does not emulate MTRR?
> 
> I will be the 1st to admit that I am not a seasoned qemu user, so I've
> no idea if the above is true.  I still prefer testing on real hardware,
> even if that comes across as "old school".  :)

We can check it with /proc/cpuinfo on a guest. 

> > pat_init() is not called when MTRR is disabled.  I think this
> > dependency is wrong, and it needs to be fixed.
> > 
> > This issue has been there for a long time, and you have been running
> > essentially as PAT disabled in the past.  The commit in question simply
> > detected this issue.
> 
> OK, that sounds good -- in that it seems we are finally getting to the
> bottom of what happened here.  Any thoughts on why built-in vs. modular
> somehow managed to mask the issue?

No idea.  I do not think uvesafb being built-in vs. module has anything to
do with it.  But we need to verify your /proc/cpuinfo to be sure.  I will
work on the PAT fix once this issue is confirmed.

For the time being, please use "nopat" boot option to workaround this
issue.  This keeps the PAT state consistent as disabled in your env.

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Tue, 2016-03-08 at 11:03 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 08/03/2016 (Tue 09:13) Toshi Kani wrote:
> 
> [...]
> 
> > 
> > Yes, I have these directories, but I do not see anything under them...
> >  Now
> > that I do not think there is anything unique in your .config file, but
> > it'd
> > be still good to check.  Please send me .config file for v4.5-rc6 (if
> > you
> > do not have it now, .config for other ver is OK as well).
> > 
> > $ ls -Rl
> 
> Try "ls -aRl" -- they are _dot_ config files.  I did not rename them.

Ah! Thanks, I see them now. :-)
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 07/03/2016 (Mon 18:35) Toshi Kani wrote:

> On Mon, 2016-03-07 at 17:56 -0700, Toshi Kani wrote:

[...]

> And also an output of /proc/cpuinfo, please?

Here is the output of /proc/cpuinfo in the guest session, while
running on pentium dual core as host (no vmx):


processor	: 0
vendor_id	: GenuineIntel
cpu family	: 6
model		: 6
model name	: QEMU Virtual CPU version 2.5+
stepping	: 3
cpu MHz		: 2593.449
cache size	: 4096 KB
physical id	: 0
siblings	: 1
core id		: 0
cpu cores	: 1
apicid		: 0
initial apicid	: 0
fdiv_bug	: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 4
wp		: yes
flags		: fpu de pse tsc msr pae mce cx8 apic sep pge cmov mmx fxsr sse sse2 pni hypervisor
bugs		:
bogomips	: 5186.89
clflush size	: 32
cache_alignment	: 32
address sizes	: 36 bits physical, 32 bits virtual
power management:

Paul.
--

> 
> I think I know what's going on.  I noticed that you have the following
> message in your dmesg files.
> 
>  [    0.000000] MTRR: Disabled
> 
> MTRR is set to disabled when your CPU is Intel but does not support MTRR.
>  Perhaps, QEMU does not emulate MTRR?
> 
> pat_init() is not called when MTRR is disabled.  I think this dependency is
> wrong, and it needs to be fixed.
> 
> This issue has been there for a long time, and you have been running
> essentially as PAT disabled in the past.  The commit in question simply
> detected this issue.
> 
> Thanks,
> -Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Thu, 2016-03-10 at 09:42 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 07/03/2016 (Mon 18:35) Toshi Kani wrote:
> 
> > On Mon, 2016-03-07 at 17:56 -0700, Toshi Kani wrote:
> 
> [...]
> 
> > And also an output of /proc/cpuinfo, please?
> 
> Here is the output of /proc/cpuinfo in the guest session, while
> running on pentium dual core as host (no vmx):
> 
> 
> processor	: 0
> vendor_id	: GenuineIntel
> cpu family	: 6
> model		: 6
> model name	: QEMU Virtual CPU version 2.5+
> stepping	: 3
> cpu MHz		: 2593.449
> cache size	: 4096 KB
> physical id	: 0
> siblings	: 1
> core id		: 0
> cpu cores	: 1
> apicid		: 0
> initial apicid	: 0
> fdiv_bug	: no
> f00f_bug	: no
> coma_bug	: no
> fpu		: yes
> fpu_exception	: yes
> cpuid level	: 4
> wp		: yes
> flags		: fpu de pse tsc msr pae mce cx8 apic sep pge cmov
> mmx fxsr sse sse2 pni hypervisor
> bugs		:
> bogomips	: 5186.89
> clflush size	: 32
> cache_alignment	: 32
> address sizes	: 36 bits physical, 32 bits virtual
> power management:

This confirms the issue - QEMU's virtual Intel CPU does not support MTRR. 

When MTRR is disabled, the kernel does not call pat_init().  pat_enabled()
is still set to true when CONFIG_X86_PAT is set.  CONFIG_X86_PAT depends on
CONFIG_MTRR, and assumes that MTRR is enabled. 

Thanks,
-Toshi

> 
> Paul.
> --
> 
> > 
> > I think I know what's going on.  I noticed that you have the following
> > message in your dmesg files.
> > 
> >  [    0.000000] MTRR: Disabled
> > 
> > MTRR is set to disabled when your CPU is Intel but does not support
> > MTRR.
> >  Perhaps, QEMU does not emulate MTRR?
> > 
> > pat_init() is not called when MTRR is disabled.  I think this
> > dependency is
> > wrong, and it needs to be fixed.
> > 
> > This issue has been there for a long time, and you have been running
> > essentially as PAT disabled in the past.  The commit in question simply
> > detected this issue.
> > 
> > Thanks,
> > -Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Thu, Mar 10, 2016 at 09:49:51AM -0700, Toshi Kani wrote:
> This confirms the issue - QEMU's virtual Intel CPU does not support MTRR. 
> 
> When MTRR is disabled, the kernel does not call pat_init().  pat_enabled()
> is still set to true when CONFIG_X86_PAT is set.  CONFIG_X86_PAT depends on
> CONFIG_MTRR, and assumes that MTRR is enabled. 

Aha, so "qemu32" model doesn't support MTRRs but "kvm32" does, for
example. And so do the majority of the other CPU types.

Paul, can you guys run with something else besides "qemu32"? You can
even take a 64-bit one and run a 32-bit guest on it.

:-)

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paul Gortmaker]

[Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"] On 10/03/2016 (Thu 18:20) Borislav Petkov wrote:

> On Thu, Mar 10, 2016 at 09:49:51AM -0700, Toshi Kani wrote:
> > This confirms the issue - QEMU's virtual Intel CPU does not support MTRR. 
> > 
> > When MTRR is disabled, the kernel does not call pat_init().  pat_enabled()
> > is still set to true when CONFIG_X86_PAT is set.  CONFIG_X86_PAT depends on
> > CONFIG_MTRR, and assumes that MTRR is enabled. 
> 
> Aha, so "qemu32" model doesn't support MTRRs but "kvm32" does, for
> example. And so do the majority of the other CPU types.

So, I guess that is a qemu bug?  If there is no real silicon out there
that has no MTRR but does claim PAT, then qemu32 is a flawed CPU type?

> 
> Paul, can you guys run with something else besides "qemu32"? You can
> even take a 64-bit one and run a 32-bit guest on it.

That is probably more of an RP question.  In principle I guess other CPU
types are on the table, and most likely qemu32 is just there from
historical reasons.  We do know that we don't want "-cpu host" though,
since that will introduce variability into the automated testing.

Paul.
--

> 
> :-)
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> -- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Thu, Mar 10, 2016 at 02:04:29PM -0500, Paul Gortmaker wrote:
> So, I guess that is a qemu bug?  If there is no real silicon out there
> that has no MTRR but does claim PAT, then qemu32 is a flawed CPU type?

Well, AFAICT, "qemu32" is emulating something PPRO-like:

#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
          CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
	  ^^^^^^^^^

and that one advertizes PAT but not MTRRs.

I need to go dig into history to find out what PPRO actually supported.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Thu, Mar 10, 2016 at 01:04:21PM -0700, Toshi Kani wrote:
> I will send a patch that sets PAT disabled when MTRR is disabled.  This
> will solve the Paul's issue.  His qemu32 model does not support PAT,
> either.

It does, see my other mail. We need to figure out first why is
pat_init() being called as part of MTRR setup.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Thu, 2016-03-10 at 18:20 +0100, Borislav Petkov wrote:
> On Thu, Mar 10, 2016 at 09:49:51AM -0700, Toshi Kani wrote:
> > This confirms the issue - QEMU's virtual Intel CPU does not support
> > MTRR. 
> > 
> > When MTRR is disabled, the kernel does not call pat_init().
> >  pat_enabled() is still set to true when CONFIG_X86_PAT is set.
> >  CONFIG_X86_PAT depends on CONFIG_MTRR, and assumes that MTRR is
> > enabled. 
> 
> Aha, so "qemu32" model doesn't support MTRRs but "kvm32" does, for
> example. And so do the majority of the other CPU types.
> 
> Paul, can you guys run with something else besides "qemu32"? You can
> even take a 64-bit one and run a 32-bit guest on it.
> 
> :-)
> 

I will send a patch that sets PAT disabled when MTRR is disabled.  This
will solve the Paul's issue.  His qemu32 model does not support PAT,
either.

Ideally, PAT and MTRR features should be managed independently, but this
will require much more effort and will not be easily applied to stable.

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Thu, 2016-03-10 at 14:04 -0500, Paul Gortmaker wrote:
> [Re: runtime regression with "x86/mm/pat: Emulate PAT when it is
> disabled"] On 10/03/2016 (Thu 18:20) Borislav Petkov wrote:
> 
> > On Thu, Mar 10, 2016 at 09:49:51AM -0700, Toshi Kani wrote:
> > > This confirms the issue - QEMU's virtual Intel CPU does not support
> > > MTRR. 
> > > 
> > > When MTRR is disabled, the kernel does not call pat_init().
> > >  pat_enabled() is still set to true when CONFIG_X86_PAT is set.
> > >  CONFIG_X86_PAT depends on CONFIG_MTRR, and assumes that MTRR is
> > > enabled. 
> > 
> > Aha, so "qemu32" model doesn't support MTRRs but "kvm32" does, for
> > example. And so do the majority of the other CPU types.
> 
> So, I guess that is a qemu bug?  If there is no real silicon out there
> that has no MTRR but does claim PAT, then qemu32 is a flawed CPU type?

It turns out that your qemu's virtual CPU does not support PAT, either. :-) 
 So, it is consistent on this regard.  I will send patches to address this
issue.

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

On Thu, 2016-03-10 at 20:20 +0100, Borislav Petkov wrote:
> On Thu, Mar 10, 2016 at 01:04:21PM -0700, Toshi Kani wrote:
> > I will send a patch that sets PAT disabled when MTRR is disabled.  This
> > will solve the Paul's issue.  His qemu32 model does not support PAT,
> > either.
> 
> It does, see my other mail. We need to figure out first why is
> pat_init() being called as part of MTRR setup.

I am not familiar with PPRO_FEATURES, but shouldn't 'flags' in
/proc/cpuinfo show "pat" when X86_FEATURE_PAT is set?

pat_init() is being called as part of MTRR setup because PAT initialization
requires the same CPU rendezvous operation implemented in the MTRR code.

Thanks,
-Toshi

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Thu, Mar 10, 2016 at 01:24:11PM -0700, Toshi Kani wrote:
> I am not familiar with PPRO_FEATURES,

That's the feature bits of the "qemu32" model, and others, in qemu.

> but shouldn't 'flags' in /proc/cpuinfo show "pat" when X86_FEATURE_PAT is set?

static void early_init_intel(struct cpuinfo_x86 *c)
...

        /*
         * There is a known erratum on Pentium III and Core Solo
         * and Core Duo CPUs.
         * " Page with PAT set to WC while associated MTRR is UC
         *   may consolidate to UC "
         * Because of this erratum, it is better to stick with
         * setting WC in MTRR rather than using PAT on these CPUs.
         *
         * Enable PAT WC only on P4, Core 2 or later CPUs.
         */
        if (c->x86 == 6 && c->x86_model < 15)
                clear_cpu_cap(c, X86_FEATURE_PAT);
---

which also gives a hint as to how we should fix this: pat_enabled()
needs to look at that feature bit too:

---
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index faec01e7a17d..359c30d9a78c 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -56,7 +56,7 @@ early_param("nopat", nopat);
 
 bool pat_enabled(void)
 {
-	return !!__pat_enabled;
+	return !!__pat_enabled && static_cpu_has(X86_FEATURE_PAT);
 }
 EXPORT_SYMBOL_GPL(pat_enabled);
---

Makes sense?

> pat_init() is being called as part of MTRR setup because PAT
> initialization requires the same CPU rendezvous operation implemented
> in the MTRR code.

... which means, PAT depends on MTRR being present.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Toshi Kani]

[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]

On Thu, 2016-03-10 at 22:07 +0100, Borislav Petkov wrote:
> On Thu, Mar 10, 2016 at 01:24:11PM -0700, Toshi Kani wrote:
> > I am not familiar with PPRO_FEATURES,
> 
> That's the feature bits of the "qemu32" model, and others, in qemu.
> 
> > but shouldn't 'flags' in /proc/cpuinfo show "pat" when X86_FEATURE_PAT
> > is set?
> 
> static void early_init_intel(struct cpuinfo_x86 *c)
> ...
> 
>         /*
>          * There is a known erratum on Pentium III and Core Solo
>          * and Core Duo CPUs.
>          * " Page with PAT set to WC while associated MTRR is UC
>          *   may consolidate to UC "
>          * Because of this erratum, it is better to stick with
>          * setting WC in MTRR rather than using PAT on these CPUs.
>          *
>          * Enable PAT WC only on P4, Core 2 or later CPUs.
>          */
>         if (c->x86 == 6 && c->x86_model < 15)
>                 clear_cpu_cap(c, X86_FEATURE_PAT);
> ---
> 
> which also gives a hint as to how we should fix this: pat_enabled()
> needs to look at that feature bit too:

I see.  I will take a look.

> ---
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index faec01e7a17d..359c30d9a78c 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -56,7 +56,7 @@ early_param("nopat", nopat);
>  
>  bool pat_enabled(void)
>  {
> -	return !!__pat_enabled;
> +	return !!__pat_enabled && static_cpu_has(X86_FEATURE_PAT);
>  }
>  EXPORT_SYMBOL_GPL(pat_enabled);
> ---
> 
> Makes sense?

Yes, I agree that pat_enable() needs to check the PAT feature bit.  In some
reason, static_cpu_has(X86_FEATURE_PAT) returns 0 while cpu_has_pat returns
1 in my testing...  I need to check this.

> > pat_init() is being called as part of MTRR setup because PAT
> > initialization requires the same CPU rendezvous operation implemented
> > in the MTRR code.
> 
> ... which means, PAT depends on MTRR being present.

Yes, and we need more changes to handle this dependency since MTRR does not
call pat_init() when it is disabled.

Attached is the changes I am working now.  I will include your changes, and
send them out once I finished testing.  Let me know if you have any
suggestion.

Thanks,
-Toshi

[-- Attachment #2: 01-pat-disable --]
[-- Type: message/rfc822, Size: 4444 bytes --]

From: Toshi Kani <toshi.kani@hpe.com>
Subject: No Subject
Date: Thu, 10 Mar 2016 16:04:11 -0700
Message-ID: <1457651051.15454.573.camel@hpe.com>

---
 arch/x86/include/asm/pat.h |    1 +
 arch/x86/mm/pat.c          |   84 +++++++++++++++++++++++++++-----------------
 2 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/arch/x86/include/asm/pat.h b/arch/x86/include/asm/pat.h
index ca6c228..016142b 100644
--- a/arch/x86/include/asm/pat.h
+++ b/arch/x86/include/asm/pat.h
@@ -5,6 +5,7 @@
 #include <asm/pgtable_types.h>
 
 bool pat_enabled(void);
+void pat_disable(const char *reason);
 extern void pat_init(void);
 void pat_init_cache_modes(u64);
 
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index f4ae536..af2b3d8 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -40,11 +40,19 @@
 static bool boot_cpu_done;
 
 static int __read_mostly __pat_enabled = IS_ENABLED(CONFIG_X86_PAT);
+static void pat_disable_init(void);
 
-static inline void pat_disable(const char *reason)
+void pat_disable(const char *reason)
 {
+	if (boot_cpu_done) {
+		pr_info("x86/PAT: PAT cannot be disabled after initialized\n");
+		return;
+	}
+
 	__pat_enabled = 0;
 	pr_info("x86/PAT: %s\n", reason);
+
+	pat_disable_init();
 }
 
 static int __init nopat(char *str)
@@ -207,9 +215,6 @@ static void pat_bsp_init(u64 pat)
 		return;
 	}
 
-	if (!pat_enabled())
-		goto done;
-
 	rdmsrl(MSR_IA32_CR_PAT, tmp_pat);
 	if (!tmp_pat) {
 		pat_disable("PAT MSR is 0, disabled.");
@@ -218,15 +223,11 @@ static void pat_bsp_init(u64 pat)
 
 	wrmsrl(MSR_IA32_CR_PAT, pat);
 
-done:
 	pat_init_cache_modes(pat);
 }
 
 static void pat_ap_init(u64 pat)
 {
-	if (!pat_enabled())
-		return;
-
 	if (!cpu_has_pat) {
 		/*
 		 * If this happens we are on a secondary CPU, but switched to
@@ -238,38 +239,55 @@ static void pat_ap_init(u64 pat)
 	wrmsrl(MSR_IA32_CR_PAT, pat);
 }
 
+static void pat_disable_init(void)
+{
+	u64 pat;
+	static int disable_init_done = 0;
+
+	if (disable_init_done)
+		return;
+
+	/*
+	 * No PAT. Emulate the PAT table that corresponds to the two
+	 * cache bits, PWT (Write Through) and PCD (Cache Disable). This
+	 * setup is the same as the BIOS default setup when the system
+	 * has PAT but the "nopat" boot option has been specified. This
+	 * emulated PAT table is used when MSR_IA32_CR_PAT returns 0.
+	 *
+	 * PTE encoding:
+	 *
+	 *       PCD
+	 *       |PWT  PAT
+	 *       ||    slot
+	 *       00    0    WB : _PAGE_CACHE_MODE_WB
+	 *       01    1    WT : _PAGE_CACHE_MODE_WT
+	 *       10    2    UC-: _PAGE_CACHE_MODE_UC_MINUS
+	 *       11    3    UC : _PAGE_CACHE_MODE_UC
+	 *
+	 * NOTE: When WC or WP is used, it is redirected to UC- per
+	 * the default setup in __cachemode2pte_tbl[].
+	 */
+	pat = PAT(0, WB) | PAT(1, WT) | PAT(2, UC_MINUS) | PAT(3, UC) |
+	      PAT(4, WB) | PAT(5, WT) | PAT(6, UC_MINUS) | PAT(7, UC);
+
+	pat_init_cache_modes(pat);
+
+	disable_init_done = 1;
+}
+
 void pat_init(void)
 {
 	u64 pat;
 	struct cpuinfo_x86 *c = &boot_cpu_data;
 
 	if (!pat_enabled()) {
-		/*
-		 * No PAT. Emulate the PAT table that corresponds to the two
-		 * cache bits, PWT (Write Through) and PCD (Cache Disable). This
-		 * setup is the same as the BIOS default setup when the system
-		 * has PAT but the "nopat" boot option has been specified. This
-		 * emulated PAT table is used when MSR_IA32_CR_PAT returns 0.
-		 *
-		 * PTE encoding:
-		 *
-		 *       PCD
-		 *       |PWT  PAT
-		 *       ||    slot
-		 *       00    0    WB : _PAGE_CACHE_MODE_WB
-		 *       01    1    WT : _PAGE_CACHE_MODE_WT
-		 *       10    2    UC-: _PAGE_CACHE_MODE_UC_MINUS
-		 *       11    3    UC : _PAGE_CACHE_MODE_UC
-		 *
-		 * NOTE: When WC or WP is used, it is redirected to UC- per
-		 * the default setup in __cachemode2pte_tbl[].
-		 */
-		pat = PAT(0, WB) | PAT(1, WT) | PAT(2, UC_MINUS) | PAT(3, UC) |
-		      PAT(4, WB) | PAT(5, WT) | PAT(6, UC_MINUS) | PAT(7, UC);
+		pat_disable_init();
+		return;
+	}
 
-	} else if ((c->x86_vendor == X86_VENDOR_INTEL) &&
-		   (((c->x86 == 0x6) && (c->x86_model <= 0xd)) ||
-		    ((c->x86 == 0xf) && (c->x86_model <= 0x6)))) {
+	if ((c->x86_vendor == X86_VENDOR_INTEL) &&
+	    (((c->x86 == 0x6) && (c->x86_model <= 0xd)) ||
+	     ((c->x86 == 0xf) && (c->x86_model <= 0x6)))) {
 		/*
 		 * PAT support with the lower four entries. Intel Pentium 2,
 		 * 3, M, and 4 are affected by PAT errata, which makes the

[-- Attachment #3: 02-mtrr --]
[-- Type: message/rfc822, Size: 2805 bytes --]

From: Toshi Kani <toshi.kani@hpe.com>
Subject: No Subject
Date: Thu, 10 Mar 2016 16:04:11 -0700
Message-ID: <1457651051.15454.574.camel@hpe.com>

---
 arch/x86/kernel/cpu/mtrr/generic.c |   24 ++++++++++++++----------
 arch/x86/kernel/cpu/mtrr/main.c    |   13 ++++++++++++-
 arch/x86/kernel/cpu/mtrr/mtrr.h    |    1 +
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index c870af1..136ae86 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -444,11 +444,24 @@ static void __init print_mtrr_state(void)
 		pr_debug("TOM2: %016llx aka %lldM\n", mtrr_tom2, mtrr_tom2>>20);
 }
 
+/* PAT setup for BP. We need to go through sync steps here */
+void __init mtrr_bp_pat_init(void)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	prepare_set();
+
+	pat_init();
+
+	post_set();
+	local_irq_restore(flags);
+}
+
 /* Grab all of the MTRR state for this CPU into *state */
 bool __init get_mtrr_state(void)
 {
 	struct mtrr_var_range *vrs;
-	unsigned long flags;
 	unsigned lo, dummy;
 	unsigned int i;
 
@@ -481,15 +494,6 @@ bool __init get_mtrr_state(void)
 
 	mtrr_state_set = 1;
 
-	/* PAT setup for BP. We need to go through sync steps here */
-	local_irq_save(flags);
-	prepare_set();
-
-	pat_init();
-
-	post_set();
-	local_irq_restore(flags);
-
 	return !!(mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED);
 }
 
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 5c3d149..d9e91f1 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -752,6 +752,9 @@ void __init mtrr_bp_init(void)
 			/* BIOS may override */
 			__mtrr_enabled = get_mtrr_state();
 
+			if (mtrr_enabled())
+				mtrr_bp_pat_init();
+
 			if (mtrr_cleanup(phys_addr)) {
 				changed_by_mtrr_cleanup = 1;
 				mtrr_if->set_all();
@@ -759,8 +762,16 @@ void __init mtrr_bp_init(void)
 		}
 	}
 
-	if (!mtrr_enabled())
+	if (!mtrr_enabled()) {
 		pr_info("MTRR: Disabled\n");
+
+		/*
+		 * PAT initialization relies on MTRR's rendezvous handler.
+		 * Disable PAT until the handler can initialize both features
+		 * independently.
+		 */
+		pat_disable("PAT disabled by MTRR");
+	}
 }
 
 void mtrr_ap_init(void)
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h
index 951884d..6c7ced0 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.h
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
@@ -52,6 +52,7 @@ void set_mtrr_prepare_save(struct set_mtrr_context *ctxt);
 void fill_mtrr_var_range(unsigned int index,
 		u32 base_lo, u32 base_hi, u32 mask_lo, u32 mask_hi);
 bool get_mtrr_state(void);
+void mtrr_bp_pat_init(void);
 
 extern void set_mtrr_ops(const struct mtrr_ops *ops);
 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[One Thousand Gnomes]

On Thu, 10 Mar 2016 20:19:33 +0100
Borislav Petkov <bp@suse.de> wrote:

> On Thu, Mar 10, 2016 at 02:04:29PM -0500, Paul Gortmaker wrote:
> > So, I guess that is a qemu bug?  If there is no real silicon out there
> > that has no MTRR but does claim PAT, then qemu32 is a flawed CPU type?  
> 
> Well, AFAICT, "qemu32" is emulating something PPRO-like:
> 
> #define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
>           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>           CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
> 	  ^^^^^^^^^
> 
> and that one advertizes PAT but not MTRRs.
> 
> I need to go dig into history to find out what PPRO actually supported.

Pentium Pro has MTRR, PAT came later.

I believe the qemu32 CPU isn't a "real" CPU type therefore.

Alan

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Fri, Mar 11, 2016 at 01:23:56PM +0000, One Thousand Gnomes wrote:
> Pentium Pro has MTRR, PAT came later.

Yep, this page says so too:

http://www.cpu-world.com/CPUs/Pentium-II/Intel-Pentium%20Pro%20200%201%20MB%20-%20GJ80521EX200%201M%20%28BP80521200%201M%29.html

> I believe the qemu32 CPU isn't a "real" CPU type therefore.

And why is that so? I suspect it has again something to do with
migration and fun. Let me add the kvm ML.

Guys, does anyone have an idea why

#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
          CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
          CPUID_PAE | CPUID_SEP | CPUID_APIC)

has CPUID_PAT *instead* of CPUID_MTRR?

If that define is really supposed to be a bitfield of Pentium Pro
features, then it should be:

#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
          CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
          CPUID_MTRR | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
          CPUID_PAE | CPUID_SEP | CPUID_APIC)

Hmmm?

Btw,

#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
          CPUID_PSE36 | CPUID_FXSR)

looks correct in that it has MTRR but it has PAT too.

And that looks wrong to me too since, according to Wikipedia, Pentium
III was the first to sport PAT:

https://en.wikipedia.org/wiki/Page_attribute_table

More hmmmm...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Paolo Bonzini]

On 11/03/2016 14:40, Borislav Petkov wrote:
> On Fri, Mar 11, 2016 at 01:23:56PM +0000, One Thousand Gnomes wrote:
>> > Pentium Pro has MTRR, PAT came later.
> Yep, this page says so too:
> 
> http://www.cpu-world.com/CPUs/Pentium-II/Intel-Pentium%20Pro%20200%201%20MB%20-%20GJ80521EX200%201M%20%28BP80521200%201M%29.html
> 
>> > I believe the qemu32 CPU isn't a "real" CPU type therefore.
> And why is that so? I suspect it has again something to do with
> migration and fun. Let me add the kvm ML.
> 
> Guys, does anyone have an idea why
> 
> #define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
>           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
>           CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>           CPUID_PAE | CPUID_SEP | CPUID_APIC)
> 
> has CPUID_PAT *instead* of CPUID_MTRR?

Somebody got it wrong 10-ish years ago, and nobody has ever checked since.

But, don't use qemu32 or qemu64.  Use kvm32 and kvm64, or better
something like the host you run on ("-cpu Nehalem", "-cpu SandyBridge",
"-cpu Haswell-noTSX" etc.).

I really, really should fix those defaults...

Paolo

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Fri, Mar 11, 2016 at 08:18:23PM +0100, Paolo Bonzini wrote:
> Somebody got it wrong 10-ish years ago, and nobody has ever checked since.
> 
> But, don't use qemu32 or qemu64.  Use kvm32 and kvm64, or better
> something like the host you run on ("-cpu Nehalem", "-cpu SandyBridge",
> "-cpu Haswell-noTSX" etc.).

Paul, Richard, how about it?

> I really, really should fix those defaults...

Here's a start, while I have everything fresh in my head.

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 11 Mar 2016 23:11:05 +0100
Subject: [PATCH] target-i386/cpu: Correct MTRR and PAT feature bits

Pentium Pro had MTRRs but not PAT, PAT support appeared in Pentium III.
Fix all defines.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 target-i386/cpu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0f38d1eae317..fa7ea4a8c229 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -308,12 +308,12 @@ static const char *cpuid_6_feature_name[] = {
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
 #define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
-          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | \
           CPUID_PSE36 | CPUID_FXSR)
-#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE)
+#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE | CPUID_PAT)
 #define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
-          CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
+          CPUID_MTRR | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
           CPUID_PAE | CPUID_SEP | CPUID_APIC)
 
 #define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
-- 
2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Bruce Ashfield]

On 2016-03-11 5:16 PM, Borislav Petkov wrote:
> On Fri, Mar 11, 2016 at 08:18:23PM +0100, Paolo Bonzini wrote:
>> Somebody got it wrong 10-ish years ago, and nobody has ever checked since.
>>
>> But, don't use qemu32 or qemu64.  Use kvm32 and kvm64, or better
>> something like the host you run on ("-cpu Nehalem", "-cpu SandyBridge",
>> "-cpu Haswell-noTSX" etc.).
>
> Paul, Richard, how about it?

I'm not Paul/Richard, but I can answer :)

We want a more generic cpu for these qemu references. Something that
doesn't vary, since it runs on any number of hosts. There are some
horrible issues we've had to solve with -cpu host in the past.

Switching to the kvm cpu type should work, plus we can add cpu
extensions on the fly as necessary.

That's definitely a valid outcome from this discussion .. a cpu
type that doesn't shoot through the middle of the expected
capabilities .. and a bonus if the kernel or qemu can be tweaked
to survive a similar mix up in the flags in the future.

Cheers,

Bruce

>
>> I really, really should fix those defaults...
>
> Here's a start, while I have everything fresh in my head.
>
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Fri, 11 Mar 2016 23:11:05 +0100
> Subject: [PATCH] target-i386/cpu: Correct MTRR and PAT feature bits
>
> Pentium Pro had MTRRs but not PAT, PAT support appeared in Pentium III.
> Fix all defines.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>   target-i386/cpu.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 0f38d1eae317..fa7ea4a8c229 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -308,12 +308,12 @@ static const char *cpuid_6_feature_name[] = {
>   #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
>             CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
>   #define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
> -          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
> +          CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | \
>             CPUID_PSE36 | CPUID_FXSR)
> -#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE)
> +#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE | CPUID_PAT)
>   #define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
>             CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
> -          CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
> +          CPUID_MTRR | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
>             CPUID_PAE | CPUID_SEP | CPUID_APIC)
>
>   #define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
>

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Richard Purdie]

On Fri, 2016-03-11 at 17:28 -0500, Bruce Ashfield wrote:
> On 2016-03-11 5:16 PM, Borislav Petkov wrote:
> > On Fri, Mar 11, 2016 at 08:18:23PM +0100, Paolo Bonzini wrote:
> > > Somebody got it wrong 10-ish years ago, and nobody has ever
> > > checked since.
> > > 
> > > But, don't use qemu32 or qemu64.  Use kvm32 and kvm64, or better
> > > something like the host you run on ("-cpu Nehalem", "-cpu
> > > SandyBridge",
> > > "-cpu Haswell-noTSX" etc.).
> > 
> > Paul, Richard, how about it?
> 
> I'm not Paul/Richard, but I can answer :)
> 
> We want a more generic cpu for these qemu references. Something that
> doesn't vary, since it runs on any number of hosts. There are some
> horrible issues we've had to solve with -cpu host in the past.
> 
> Switching to the kvm cpu type should work, plus we can add cpu
> extensions on the fly as necessary.
> 
> That's definitely a valid outcome from this discussion .. a cpu
> type that doesn't shoot through the middle of the expected
> capabilities .. and a bonus if the kernel or qemu can be tweaked
> to survive a similar mix up in the flags in the future.

We'll have to go and trawl our commit logs to figure out how we ended
up with choosing qemuXX, I do remember everything else we tried
previously broke in some way. We do need something which works on quite
a wide variety of systems and if I remember rightly, the CPU features
the kvm* options provide vary quite widely and wasn't consistent. The
qemu* ones at least consistently broke everywhere! :)

Tweaking the kernel to only enable PAT with MTRR sounds like a good
fix, as does fixing the qemu cpu feature bits and I'd like to get those
patches into our builds once they're heading into the upstreams.
Meanwhile we'll see if we can find a better cpu option as well.

Cheers,

Richard

Re: runtime regression with "x86/mm/pat: Emulate PAT when it is disabled"

[Borislav Petkov]

On Fri, Mar 11, 2016 at 11:29:54PM +0000, Richard Purdie wrote:
> ... and if I remember rightly, the CPU features the kvm* options
> provide vary quite widely and wasn't consistent.

How so? Please elaborate so that we can fix those.

Btw, your reproducer works fine with -cpu kvm32 - only vncviewer's
window gets killed after a "Rect too large: 640x480 at (0, 0)" but
when I reconnect again right after it I see an X window asking me to
calibrate my touch screen and I'm at a loss as to where my touch screen
is... :-P

> Tweaking the kernel to only enable PAT with MTRR sounds like a good
> fix,

Yeah, the idea is to basically switch to uncacheable memtype as when
MTRRs are disabled.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--