All of lore.kernel.org
 help / color / mirror / Atom feed
From: "André Przywara" <andre@andrep.de>
To: Stefan Bader <stefan.bader@canonical.com>
Cc: Borislav Petkov <bp@alien8.de>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Matthew Garrett <mjg@redhat.com>
Subject: Re: [Xen-devel] kernel 3.7+ cpufreq regression on AMD system running as dom0
Date: Mon, 14 Jan 2013 18:40:37 +0100	[thread overview]
Message-ID: <20130114184037.21c49b8c@hydra> (raw)
In-Reply-To: <50F43B9D.8030300@canonical.com>

On Mon, 14 Jan 2013 18:08:45 +0100
Stefan Bader <stefan.bader@canonical.com> wrote:

> On 14.01.2013 17:34, Borislav Petkov wrote:
> > On Mon, Jan 14, 2013 at 04:58:54PM +0100, Stefan Bader wrote:
> >> Starting with kernel v3.7 the following commit added a quirk
> >> to obtain the real frequencies of certain AMD systems:
> >>
> >> commit f594065faf4f9067c2283a34619fc0714e79a98d
> >> Author: Matthew Garrett <mjg@redhat.com>
> >> Date:   Tue Sep 4 08:28:06 2012 +0000
> >>
> >>     ACPI: Add fixups for AMD P-state figures
> >>
> >> When running bare-metal, on my Opteron 6128 test box results
> >> in the frequencies remaining effectively unchanged:
> >> [    5.475735] P0: MSR(hi,lo): 8000015c-50004004
> >> [    5.479049] P0: fid=0x4, did=0x0, freq: 2000 -> 2000
> >> [    5.484001] P1: MSR(hi,lo): 8000014c-50004a4e
> >> [    5.487314] P1: fid=0xe, did=0x1, freq: 1500 -> 1500
> >> [    5.492272] P2: MSR(hi,lo): 80000141-50005048
> >> [    5.495584] P2: fid=0x8, did=0x1, freq: 1200 -> 1200
> >> [    5.500540] P3: MSR(hi,lo): 80000138-50005844
> >> [    5.503853] P3: fid=0x4, did=0x1, freq: 1000 -> 1000
> >> [    5.508812] P4: MSR(hi,lo): 80000131-50005c40
> >> [    5.512125] P4: fid=0x0, did=0x1, freq: 800 -> 800
> >>
> >> However running as dom0 under Xen 4.2, reading this MSR returns
> >> null:
> >> [   11.613068] P0: MSR(hi,lo): 00000000-00000000
> >> [   11.613074] P0: fid=0x0, did=0x0, freq: 2000 -> 1600
> >> [   11.613078] P1: MSR(hi,lo): 00000000-00000000
> >> [   11.613081] P1: fid=0x0, did=0x0, freq: 1500 -> 1600
> >> [   11.613085] P2: MSR(hi,lo): 00000000-00000000
> >> [   11.613088] P2: fid=0x0, did=0x0, freq: 1200 -> 1600
> >> [   11.613091] P3: MSR(hi,lo): 00000000-00000000
> >> [   11.613094] P3: fid=0x0, did=0x0, freq: 1000 -> 1600
> >> [   11.613098] P4: MSR(hi,lo): 00000000-00000000
> >> [   11.613101] P4: fid=0x0, did=0x0, freq: 800 -> 1600
> >>
> >> And this results in Xen failing to change the governor:
> >>   "(XEN) Fail change to ondemand governor"
> >>
> >> I suppose this ultimately requires some support in the hypervisor
> >> to pass through the real values. But since this is at least on my
> >> combination of Xen 4.2 + kernel v3.7+ and AMD family 0x10 CPU a
> >> regression compared to older kernels, I wonder whether the
> >> following change might be something that should go into mainline:
> >>
> >> --- a/drivers/acpi/processor_perflib.c
> >> +++ b/drivers/acpi/processor_perflib.c
> >> @@ -340,6 +340,9 @@ static void amd_fixup_frequency(struct
> >> acpi_processor_px *px if ((boot_cpu_data.x86 == 0x10 &&
> >> boot_cpu_data.x86_model < 10) || boot_cpu_data.x86 == 0x11) {
> >>                 rdmsr(MSR_AMD_PSTATE_DEF_BASE + index, lo, hi);
> >> +               /* Bit 63 indicates whether contents are valid */
> >> +               if (!(hi & 0x8000000))
> >> +                       return;
> > 
> > I don't think that's the right change - this is fixing baremetal so
> > that it works on xen. And besides, this code was in powernow-k8
> > before so I'm wondering why did it work then.
> 
> This actually only started to work when the xen-processor module got
> introduced to provide acpi information to the hypervisor. If I
> remember correctly powernow-k8 did fail.
> For the way I did the fix: the AMD BIOS docs seemed to indicate that
> even for bare metal bit 63 would say whether the values are valid. So
> I thought this is a nice coincidence that under Xen with all 0 this
> matches that special case... ;)

>From a first glance I think this fix is a valid approach. There are
BIOSes which disable P-states via this bit, so we have to observe this
for bare-metal, too. Let me think a bit more about this, however, and
see whether there is a better solution to do the right thing (tm) under
Xen. Getting back to you then.

Thanks,
Andre.


WARNING: multiple messages have this Message-ID (diff)
From: "André Przywara" <andre@andrep.de>
To: Stefan Bader <stefan.bader@canonical.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>, Borislav Petkov <bp@alien8.de>,
	Matthew Garrett <mjg@redhat.com>
Subject: Re: kernel 3.7+ cpufreq regression on AMD system running as dom0
Date: Mon, 14 Jan 2013 18:40:37 +0100	[thread overview]
Message-ID: <20130114184037.21c49b8c@hydra> (raw)
In-Reply-To: <50F43B9D.8030300@canonical.com>

On Mon, 14 Jan 2013 18:08:45 +0100
Stefan Bader <stefan.bader@canonical.com> wrote:

> On 14.01.2013 17:34, Borislav Petkov wrote:
> > On Mon, Jan 14, 2013 at 04:58:54PM +0100, Stefan Bader wrote:
> >> Starting with kernel v3.7 the following commit added a quirk
> >> to obtain the real frequencies of certain AMD systems:
> >>
> >> commit f594065faf4f9067c2283a34619fc0714e79a98d
> >> Author: Matthew Garrett <mjg@redhat.com>
> >> Date:   Tue Sep 4 08:28:06 2012 +0000
> >>
> >>     ACPI: Add fixups for AMD P-state figures
> >>
> >> When running bare-metal, on my Opteron 6128 test box results
> >> in the frequencies remaining effectively unchanged:
> >> [    5.475735] P0: MSR(hi,lo): 8000015c-50004004
> >> [    5.479049] P0: fid=0x4, did=0x0, freq: 2000 -> 2000
> >> [    5.484001] P1: MSR(hi,lo): 8000014c-50004a4e
> >> [    5.487314] P1: fid=0xe, did=0x1, freq: 1500 -> 1500
> >> [    5.492272] P2: MSR(hi,lo): 80000141-50005048
> >> [    5.495584] P2: fid=0x8, did=0x1, freq: 1200 -> 1200
> >> [    5.500540] P3: MSR(hi,lo): 80000138-50005844
> >> [    5.503853] P3: fid=0x4, did=0x1, freq: 1000 -> 1000
> >> [    5.508812] P4: MSR(hi,lo): 80000131-50005c40
> >> [    5.512125] P4: fid=0x0, did=0x1, freq: 800 -> 800
> >>
> >> However running as dom0 under Xen 4.2, reading this MSR returns
> >> null:
> >> [   11.613068] P0: MSR(hi,lo): 00000000-00000000
> >> [   11.613074] P0: fid=0x0, did=0x0, freq: 2000 -> 1600
> >> [   11.613078] P1: MSR(hi,lo): 00000000-00000000
> >> [   11.613081] P1: fid=0x0, did=0x0, freq: 1500 -> 1600
> >> [   11.613085] P2: MSR(hi,lo): 00000000-00000000
> >> [   11.613088] P2: fid=0x0, did=0x0, freq: 1200 -> 1600
> >> [   11.613091] P3: MSR(hi,lo): 00000000-00000000
> >> [   11.613094] P3: fid=0x0, did=0x0, freq: 1000 -> 1600
> >> [   11.613098] P4: MSR(hi,lo): 00000000-00000000
> >> [   11.613101] P4: fid=0x0, did=0x0, freq: 800 -> 1600
> >>
> >> And this results in Xen failing to change the governor:
> >>   "(XEN) Fail change to ondemand governor"
> >>
> >> I suppose this ultimately requires some support in the hypervisor
> >> to pass through the real values. But since this is at least on my
> >> combination of Xen 4.2 + kernel v3.7+ and AMD family 0x10 CPU a
> >> regression compared to older kernels, I wonder whether the
> >> following change might be something that should go into mainline:
> >>
> >> --- a/drivers/acpi/processor_perflib.c
> >> +++ b/drivers/acpi/processor_perflib.c
> >> @@ -340,6 +340,9 @@ static void amd_fixup_frequency(struct
> >> acpi_processor_px *px if ((boot_cpu_data.x86 == 0x10 &&
> >> boot_cpu_data.x86_model < 10) || boot_cpu_data.x86 == 0x11) {
> >>                 rdmsr(MSR_AMD_PSTATE_DEF_BASE + index, lo, hi);
> >> +               /* Bit 63 indicates whether contents are valid */
> >> +               if (!(hi & 0x8000000))
> >> +                       return;
> > 
> > I don't think that's the right change - this is fixing baremetal so
> > that it works on xen. And besides, this code was in powernow-k8
> > before so I'm wondering why did it work then.
> 
> This actually only started to work when the xen-processor module got
> introduced to provide acpi information to the hypervisor. If I
> remember correctly powernow-k8 did fail.
> For the way I did the fix: the AMD BIOS docs seemed to indicate that
> even for bare metal bit 63 would say whether the values are valid. So
> I thought this is a nice coincidence that under Xen with all 0 this
> matches that special case... ;)

>From a first glance I think this fix is a valid approach. There are
BIOSes which disable P-states via this bit, so we have to observe this
for bare-metal, too. Let me think a bit more about this, however, and
see whether there is a better solution to do the right thing (tm) under
Xen. Getting back to you then.

Thanks,
Andre.

  reply	other threads:[~2013-01-14 17:47 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-14 15:58 kernel 3.7+ cpufreq regression on AMD system running as dom0 Stefan Bader
2013-01-14 16:34 ` Borislav Petkov
2013-01-14 16:55   ` [Xen-devel] " Jan Beulich
2013-01-14 17:08   ` Stefan Bader
2013-01-14 17:40     ` André Przywara [this message]
2013-01-14 17:40       ` André Przywara
2013-01-15 17:53   ` Konrad Rzeszutek Wilk
2013-01-15 18:18     ` Borislav Petkov
2013-01-18 19:00       ` Konrad Rzeszutek Wilk
2013-01-18 19:38         ` [Xen-devel] " Boris Ostrovsky
2013-01-18 19:44           ` Andrew Cooper
2013-01-18 20:03         ` Borislav Petkov
2013-01-18 22:00           ` Konrad Rzeszutek Wilk
2013-01-21 12:22           ` Stefan Bader
2013-01-21 12:42             ` Borislav Petkov
2013-01-21 12:53               ` Rafael J. Wysocki
2013-01-21 13:08                 ` Borislav Petkov
2013-01-21 13:11               ` Stefan Bader
2013-01-21 15:03               ` Stefan Bader
2013-01-21 15:31                 ` Borislav Petkov
2013-01-22 13:54                   ` Rafael J. Wysocki
2013-01-22  0:01         ` [Xen-devel] " Boris Ostrovsky
2013-01-16 10:26     ` Jan Beulich
2013-01-16 14:34       ` Stefan Bader
2013-01-16 14:34       ` [Xen-devel] " Stefan Bader
2013-01-16 10:26     ` Jan Beulich
2013-01-15 13:04 ` Matt Wilson
2013-01-15 17:59   ` [Xen-devel] " Matt Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130114184037.21c49b8c@hydra \
    --to=andre@andrep.de \
    --cc=bp@alien8.de \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg@redhat.com \
    --cc=rjw@sisk.pl \
    --cc=stefan.bader@canonical.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.