On 06/01/2012 07:52 AM, Borislav Petkov wrote:
> From: Andre Przywara <[email protected]>
>
> f7f286a910221 ("x86/amd: Re-enable CPU topology extensions in case BIOS
> has disabled it") wrongfully added code which used the AMD-specific
> {rd,wr}msr variants for no real reason.
>
> This caused boot panics on xen which wasn't initializing the
> {rd,wr}msr_safe_regs pv_ops members properly.
>
> This, in turn, caused a heated discussion leading to us reviewing all
> uses of the AMD-specific variants and removing them where unneeded
> (almost everywhere except an obscure K8 BIOS fix, see 6b0f43ddfa358).
>
> Finally, this patch switches to the standard {rd,wr}msr*_safe* variants
> which should've been used in the first place anyway and avoided unneeded
> excitation with xen.
>
> Signed-off-by: Andre Przywara <[email protected]>
> Cc: Andreas Herrmann <[email protected]>
> Cc: [email protected] # 3.4+
> Link: <http://lkml.kernel.org/r/[email protected]>
> [Boris: correct and expand commit message]
> Signed-off-by: Borislav Petkov <[email protected]>
Why -stable? I though we had agreed that we didn't have an active
problem (unclean hack, yes, but not an active problem) in 3.4/3.5 as it
currently sits?
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Wed, Jun 06, 2012 at 03:00:14PM -0700, H. Peter Anvin wrote:
> On 06/01/2012 07:52 AM, Borislav Petkov wrote:
> > From: Andre Przywara <[email protected]>
> >
> > f7f286a910221 ("x86/amd: Re-enable CPU topology extensions in case BIOS
> > has disabled it") wrongfully added code which used the AMD-specific
> > {rd,wr}msr variants for no real reason.
> >
> > This caused boot panics on xen which wasn't initializing the
> > {rd,wr}msr_safe_regs pv_ops members properly.
> >
> > This, in turn, caused a heated discussion leading to us reviewing all
> > uses of the AMD-specific variants and removing them where unneeded
> > (almost everywhere except an obscure K8 BIOS fix, see 6b0f43ddfa358).
> >
> > Finally, this patch switches to the standard {rd,wr}msr*_safe* variants
> > which should've been used in the first place anyway and avoided unneeded
> > excitation with xen.
> >
> > Signed-off-by: Andre Przywara <[email protected]>
> > Cc: Andreas Herrmann <[email protected]>
> > Cc: [email protected] # 3.4+
> > Link: <http://lkml.kernel.org/r/[email protected]>
> > [Boris: correct and expand commit message]
> > Signed-off-by: Borislav Petkov <[email protected]>
>
> Why -stable? I though we had agreed that we didn't have an active
> problem (unclean hack, yes, but not an active problem) in 3.4/3.5 as it
> currently sits?
Yes, AFAICT, we need at least one fix for 3.4 where the original patch
f7f286a910221 broke xen.
So either this one or 1ab46fd319bc should be backported to stable, if
I'm not mistaken. If the second, I'll drop the stable tag from this one
and resend.
Konrad, what do you want wrt xen paravirt nullptr breakage for
3.4-stable?
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
On 06/07/2012 09:21 AM, Borislav Petkov wrote:
> On Wed, Jun 06, 2012 at 03:00:14PM -0700, H. Peter Anvin wrote:
>> On 06/01/2012 07:52 AM, Borislav Petkov wrote:
>>> From: Andre Przywara<[email protected]>
>>>
>>> f7f286a910221 ("x86/amd: Re-enable CPU topology extensions in case BIOS
>>> has disabled it") wrongfully added code which used the AMD-specific
>>> {rd,wr}msr variants for no real reason.
>>>
>>> This caused boot panics on xen which wasn't initializing the
>>> {rd,wr}msr_safe_regs pv_ops members properly.
>>>
>>> This, in turn, caused a heated discussion leading to us reviewing all
>>> uses of the AMD-specific variants and removing them where unneeded
>>> (almost everywhere except an obscure K8 BIOS fix, see 6b0f43ddfa358).
>>>
>>> Finally, this patch switches to the standard {rd,wr}msr*_safe* variants
>>> which should've been used in the first place anyway and avoided unneeded
>>> excitation with xen.
>>>
>>> Signed-off-by: Andre Przywara<[email protected]>
>>> Cc: Andreas Herrmann<[email protected]>
>>> Cc: [email protected] # 3.4+
>>> Link:<http://lkml.kernel.org/r/[email protected]>
>>> [Boris: correct and expand commit message]
>>> Signed-off-by: Borislav Petkov<[email protected]>
>>
>> Why -stable? I though we had agreed that we didn't have an active
>> problem (unclean hack, yes, but not an active problem) in 3.4/3.5 as it
>> currently sits?
This is probably a leftover from the original patch, before Konrad's
patch got committed.
> Yes, AFAICT, we need at least one fix for 3.4 where the original patch
> f7f286a910221 broke xen.
>
> So either this one or 1ab46fd319bc should be backported to stable, if
> I'm not mistaken. If the second, I'll drop the stable tag from this one
> and resend.
>
> Konrad, what do you want wrt xen paravirt nullptr breakage for
> 3.4-stable?
Greg just sent out the review mail for 1ab46fd319bc, so we can drop the
stable tag from this one.
Regards,
Andre.
-------- Original Message --------
Subject: [ 21/82] x86, amd, xen: Avoid NULL pointer paravirt references
Date: Thu, 7 Jun 2012 13:03:57 +0900
From: Greg KH <[email protected]>
To: <[email protected]>, <[email protected]>
CC: <[email protected]>, <[email protected]>,
<[email protected]>, Andre Przywara <[email protected]>, "H.
Peter Anvin" <[email protected]>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Konrad Rzeszutek Wilk <[email protected]>
commit 1ab46fd319bcf1fcd9fb6311727d532b580e4eba upstream.
Stub out MSR methods that aren't actually needed. This fixes a crash
as Xen Dom0 on AMD Trinity systems. A bigger patch should be added to
remove the paravirt machinery completely for the methods which
apparently have no users!
.....
--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712
On Thu, Jun 07, 2012 at 09:49:01AM +0200, Andre Przywara wrote:
> On 06/07/2012 09:21 AM, Borislav Petkov wrote:
> >On Wed, Jun 06, 2012 at 03:00:14PM -0700, H. Peter Anvin wrote:
> >>On 06/01/2012 07:52 AM, Borislav Petkov wrote:
> >>>From: Andre Przywara<[email protected]>
> >>>
> >>>f7f286a910221 ("x86/amd: Re-enable CPU topology extensions in case BIOS
> >>>has disabled it") wrongfully added code which used the AMD-specific
> >>>{rd,wr}msr variants for no real reason.
> >>>
> >>>This caused boot panics on xen which wasn't initializing the
> >>>{rd,wr}msr_safe_regs pv_ops members properly.
> >>>
> >>>This, in turn, caused a heated discussion leading to us reviewing all
> >>>uses of the AMD-specific variants and removing them where unneeded
> >>>(almost everywhere except an obscure K8 BIOS fix, see 6b0f43ddfa358).
> >>>
> >>>Finally, this patch switches to the standard {rd,wr}msr*_safe* variants
> >>>which should've been used in the first place anyway and avoided unneeded
> >>>excitation with xen.
> >>>
> >>>Signed-off-by: Andre Przywara<[email protected]>
> >>>Cc: Andreas Herrmann<[email protected]>
> >>>Cc: [email protected] # 3.4+
> >>>Link:<http://lkml.kernel.org/r/[email protected]>
> >>>[Boris: correct and expand commit message]
> >>>Signed-off-by: Borislav Petkov<[email protected]>
> >>
> >>Why -stable? I though we had agreed that we didn't have an active
> >>problem (unclean hack, yes, but not an active problem) in 3.4/3.5 as it
> >>currently sits?
>
> This is probably a leftover from the original patch, before Konrad's
> patch got committed.
>
> >Yes, AFAICT, we need at least one fix for 3.4 where the original patch
> >f7f286a910221 broke xen.
> >
> >So either this one or 1ab46fd319bc should be backported to stable, if
> >I'm not mistaken. If the second, I'll drop the stable tag from this one
> >and resend.
> >
> >Konrad, what do you want wrt xen paravirt nullptr breakage for
> >3.4-stable?
>
> Greg just sent out the review mail for 1ab46fd319bc, so we can drop
> the stable tag from this one.
>
> Regards,
> Andre.
What? So I don't need to do anything?
Totally confused,
greg k-h
On 06/07/2012 10:08 AM, Greg KH wrote:
> On Thu, Jun 07, 2012 at 09:49:01AM +0200, Andre Przywara wrote:
>> On 06/07/2012 09:21 AM, Borislav Petkov wrote:
>>> On Wed, Jun 06, 2012 at 03:00:14PM -0700, H. Peter Anvin wrote:
>>>> On 06/01/2012 07:52 AM, Borislav Petkov wrote:
>>>>> From: Andre Przywara<[email protected]>
>>>>>
>>>>> f7f286a910221 ("x86/amd: Re-enable CPU topology extensions in case BIOS
>>>>> has disabled it") wrongfully added code which used the AMD-specific
>>>>> {rd,wr}msr variants for no real reason.
>>>>>
>>>>> This caused boot panics on xen which wasn't initializing the
>>>>> {rd,wr}msr_safe_regs pv_ops members properly.
>>>>>
>>>>> This, in turn, caused a heated discussion leading to us reviewing all
>>>>> uses of the AMD-specific variants and removing them where unneeded
>>>>> (almost everywhere except an obscure K8 BIOS fix, see 6b0f43ddfa358).
>>>>>
>>>>> Finally, this patch switches to the standard {rd,wr}msr*_safe* variants
>>>>> which should've been used in the first place anyway and avoided unneeded
>>>>> excitation with xen.
>>>>>
>>>>> Signed-off-by: Andre Przywara<[email protected]>
>>>>> Cc: Andreas Herrmann<[email protected]>
>>>>> Cc: [email protected] # 3.4+
>>>>> Link:<http://lkml.kernel.org/r/[email protected]>
>>>>> [Boris: correct and expand commit message]
>>>>> Signed-off-by: Borislav Petkov<[email protected]>
>>>>
>>>> Why -stable? I though we had agreed that we didn't have an active
>>>> problem (unclean hack, yes, but not an active problem) in 3.4/3.5 as it
>>>> currently sits?
>>
>> This is probably a leftover from the original patch, before Konrad's
>> patch got committed.
>>
>>> Yes, AFAICT, we need at least one fix for 3.4 where the original patch
>>> f7f286a910221 broke xen.
>>>
>>> So either this one or 1ab46fd319bc should be backported to stable, if
>>> I'm not mistaken. If the second, I'll drop the stable tag from this one
>>> and resend.
>>>
>>> Konrad, what do you want wrt xen paravirt nullptr breakage for
>>> 3.4-stable?
>>
>> Greg just sent out the review mail for 1ab46fd319bc, so we can drop
>> the stable tag from this one.
>>
>> Regards,
>> Andre.
>
> What? So I don't need to do anything?
>
> Totally confused,
Sorry for that.
To fix the issue, we need only one of those patches.
Mine ("Fix crash as Xen Dom0 on AMD Trinity systems", removing "_amd"
from the rd/wrmsr calls) was sent out earlier, so I added the stable tag.
A bit later Konrad sent his patch (1ab46fd319bc, initializing the
forgotten PVOPS members), which hpa took (dropping mine). So this fix is
in 3.5-rc1 and should also be in -stable.
Now for making things smoother Boris sent out mine again - for 3.6 - and
more or less accidentally kept the stable tag in it.
Long story short: everything is fine, just apply the one you sent the
review message for.
HTH,
Andre.
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Hi Peter,
so here are the final versions of the patches, as discussed. 3/4 has
lost the stable tag and all have received Konrad's Acked-by. Other than
that, 1ab46fd319bc takes care of the -stable issue for xen and Greg is
picking that one up. So all those should be queued for 3.6.
Please pull, thanks.
The following changes since commit f8f5701bdaf9134b1f90e5044a82c66324d2073f:
Linux 3.5-rc1 (2012-06-02 18:29:26 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git tags/amd-rdmsr-cleanups-for-3.6
for you to fetch changes up to c5214c0192813ebaa5784be36d2ae142034f84db:
x86, CPU, AMD: Deprecate AMD-specific MSR variants (2012-06-07 12:52:58 +0200)
----------------------------------------------------------------
x86, CPU, AMD: Cleanup AMD-specific MSR-rw users
This patchset takes care of all the {rd,wr}msrl_amd_safe headaches we
had wrt xen. After it is applied, the AMD-specific variants become
private to amd.c and issue a warning when used on anything else beside
K8 because they're supposed to be used only on K8.
This also contains the two patches from Andre which cleanup the PV-side
of things.
----------------------------------------------------------------
Andre Przywara (2):
x86, pvops: Remove hooks for {rd,wr}msr_safe_regs
x86, AMD: Fix crash as Xen Dom0 on AMD Trinity systems
Borislav Petkov (2):
x86, CPU: Fix show_msr MSR accessing function
x86, CPU, AMD: Deprecate AMD-specific MSR variants
arch/x86/include/asm/msr.h | 42 ++---------------------------------
arch/x86/include/asm/paravirt.h | 39 --------------------------------
arch/x86/include/asm/paravirt_types.h | 2 --
arch/x86/kernel/cpu/amd.c | 37 ++++++++++++++++++++++++++++--
arch/x86/kernel/cpu/common.c | 2 +-
arch/x86/kernel/paravirt.c | 2 --
arch/x86/lib/msr-reg-export.c | 4 ++--
arch/x86/lib/msr-reg.S | 10 ++++-----
arch/x86/xen/enlighten.c | 2 --
9 files changed, 45 insertions(+), 95 deletions(-)
--
Regards/Gruss,
Boris.
[email protected] - where all your Linux questions get answered.
Operating Systems Research Center
Advanced Micro Devices, Inc.
On 06/07/2012 01:08 AM, Greg KH wrote:
>
> What? So I don't need to do anything?
>
Correct, you're fine as-is.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.