From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH] x86/amd: fix crash as Xen Dom0 on AMD Trinity systems Date: Thu, 31 May 2012 14:24:24 +0200 Message-ID: <4FC762F8.902@amd.com> References: <20120530144851.GA12184@jshin-Toonie> <20120530145005.GI3207@phenom.dumpdata.com> <20120530150334.GA13349@jshin-Toonie> <20120530171754.GA5115@phenom.dumpdata.com> <20120530173247.GC15635@x1.osrc.amd.com> <4FC65D34.1060803@zytor.com> <20120530175150.GE15438@x1.osrc.amd.com> <4FC66037.6020506@zytor.com> <20120530181722.GF15438@x1.osrc.amd.com> <4FC664E1.9050504@zytor.com> <20120530223334.GB28417@andromeda.dapyr.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120530223334.GB28417@andromeda.dapyr.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: jeremy@goop.org, xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk , Jacob Shin , linux-kernel@vger.kernel.org, Borislav Petkov , Jan Beulich , "H. Peter Anvin" , mingo@elte.hu, tglx@linutronix.de List-Id: xen-devel@lists.xenproject.org On 05/31/2012 12:33 AM, Konrad Rzeszutek Wilk wrote: >>> Now, someone probably needs to paravirt the *safe_regs variants in case >>> something else decides to use them. I don't know what to do here, do I >>> want more paravirt code in there? No. I guess if this is done carefully >>> and cleanly, then it should be ok but it can't be done like that - it >>> needs to adhere to the current pv_cpu_ops thing which is already there. > > Using the native variant seems the right thing to do. >>> >> >> I thought I was being told that Xen would trap and emulate the >> rdmsr/wrmsr instructions. I guess they don't want to do that for the > > It does. >> handful of performance-sensitive MSRs there are, but those don't use the >> *_regs variants. > > The underlaying issue (as I understand) was that .rdmsr_regs > (and the corresponding write) was NULL and that caused the crash. > This tiny patch should do it: > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index 75f33b2..e74df95 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1116,7 +1116,10 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { > .wbinvd = native_wbinvd, > > .read_msr = native_read_msr_safe, > + .rdmsr_regs = native_rdmsr_safe_regs, > .write_msr = xen_write_msr_safe, > + .wrmsr_regs = native_wrmsr_safe_regs, > + > .read_tsc = native_read_tsc, > .read_pmc = native_read_pmc, > > Acked-by: Andre Przywara This works on the test machine. Though I'd still like to have my original patch applied, because it makes the thing a bit cleaner. And I made a patch to remove the {rd,wr}msr_regs hooks from paravirt_ops completely. Shall I send it out or do you want to make this part of larger patch series to clean up pvops? Regards, Andre. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany