| Message ID | 96396a5ea9e1acc527705f71e9cd657b16585a75.1553935727.git.puwen@hygon.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Hygon Dhyana Family 18h processor | expand |
>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: > There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly > return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. I think it would have been nice if you had mentioned the real reason why you want to bypass the MSR accesses. This way it sounds as if the change was only cosmetic, and hence could be left out. > Signed-off-by: Pu Wen <puwen@hygon.cn> Acked-by: Jan Beulich <jbeulich@suse.com> Andrew, I'd like to ask for explicit clarification that you don't object to this adjustment. But if you do, please clarify why. Thanks, Jan > --- a/xen/arch/x86/cpu/common.c > +++ b/xen/arch/x86/cpu/common.c > @@ -116,6 +116,11 @@ bool __init probe_cpuid_faulting(void) > uint64_t val; > int rc; > > + if ((boot_cpu_data.x86_vendor == X86_VENDOR_HYGON || > + boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && > + !cpu_has_hypervisor) > + return false; > + > if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0) > raw_msr_policy.plaform_info.cpuid_faulting = > val & MSR_PLATFORM_INFO_CPUID_FAULTING; > -- > 2.7.4
On 2019/4/1 16:41, Jan Beulich wrote: > On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. > > I think it would have been nice if you had mentioned the real > reason why you want to bypass the MSR accesses. This way it > sounds as if the change was only cosmetic, and hence could be > left out. Okay, how about the new description: There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read this MSR will stop the Xen initialization process or produce GPF(0). So directly return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor.
>>> On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: > On 2019/4/1 16:41, Jan Beulich wrote: >> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >>> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. >> >> I think it would have been nice if you had mentioned the real >> reason why you want to bypass the MSR accesses. This way it >> sounds as if the change was only cosmetic, and hence could be >> left out. > > Okay, how about the new description: > There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read > this MSR will stop the Xen initialization process "... for some early Hygon steppings"(?). I'm unaware of AMD CPUs having this issue - are you telling us otherwise? Jan > or produce GPF(0). > So directly return false in the function probe_cpuid_faulting() if > !cpu_has_hypervisor. > > -- > Regards, > Pu Wen
On 01/04/2019 09:40, Jan Beulich wrote: >>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. > I think it would have been nice if you had mentioned the real > reason why you want to bypass the MSR accesses. This way it > sounds as if the change was only cosmetic, and hence could be > left out. > >> Signed-off-by: Pu Wen <puwen@hygon.cn> > Acked-by: Jan Beulich <jbeulich@suse.com> > > Andrew, I'd like to ask for explicit clarification that you don't object > to this adjustment. But if you do, please clarify why. We deliberately emulate MSR_INTEL_PLATFORM_INFO on all systems This is to support pv-shim, so the L1 Xen can exert faulting control over the L2 PV guest, so L2 doesn't see L1's HVM CPUID leaves and choke. I suppose its fine to have a !cpu_has_hypervisor exclusion for non-Intel systems, but I also don't see much value in it. ~Andrew
On 2019/4/2 18:20, Jan Beulich wrote: > On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: >> On 2019/4/1 16:41, Jan Beulich wrote: >>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >>>> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. >>> >>> I think it would have been nice if you had mentioned the real >>> reason why you want to bypass the MSR accesses. This way it >>> sounds as if the change was only cosmetic, and hence could be >>> left out. >> >> Okay, how about the new description: >> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read >> this MSR will stop the Xen initialization process > > "... for some early Hygon steppings"(?). I'm unaware of AMD CPUs Yes,for some early Hygon steppings. > having this issue - are you telling us otherwise? I tested with an AMD CPU(Family 17h, Model 1, Stepping 1) today, and it also stopped when reading the MSR_INTEL_PLATFORM_INFO instead of producing #GP(0).
>>> On 02.04.19 at 12:30, <andrew.cooper3@citrix.com> wrote: > On 01/04/2019 09:40, Jan Beulich wrote: >>>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >>> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. >> I think it would have been nice if you had mentioned the real >> reason why you want to bypass the MSR accesses. This way it >> sounds as if the change was only cosmetic, and hence could be >> left out. >> >>> Signed-off-by: Pu Wen <puwen@hygon.cn> >> Acked-by: Jan Beulich <jbeulich@suse.com> >> >> Andrew, I'd like to ask for explicit clarification that you don't object >> to this adjustment. But if you do, please clarify why. > > We deliberately emulate MSR_INTEL_PLATFORM_INFO on all systems > > This is to support pv-shim, so the L1 Xen can exert faulting control > over the L2 PV guest, so L2 doesn't see L1's HVM CPUID leaves and choke. > > I suppose its fine to have a !cpu_has_hypervisor exclusion for non-Intel > systems, but I also don't see much value in it. But you've see Pu Wen's explanation (of their CPUs hanging on that RDMSR is attempted)? In a later reply he even claims that this also happens on some AMD CPUs. Jan
>>> On 02.04.19 at 13:58, <puwen@hygon.cn> wrote: > On 2019/4/2 18:20, Jan Beulich wrote: >> On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: >>> On 2019/4/1 16:41, Jan Beulich wrote: >>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >>>>> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. >>>> >>>> I think it would have been nice if you had mentioned the real >>>> reason why you want to bypass the MSR accesses. This way it >>>> sounds as if the change was only cosmetic, and hence could be >>>> left out. >>> >>> Okay, how about the new description: >>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read >>> this MSR will stop the Xen initialization process >> >> "... for some early Hygon steppings"(?). I'm unaware of AMD CPUs > > Yes,for some early Hygon steppings. > >> having this issue - are you telling us otherwise? > > I tested with an AMD CPU(Family 17h, Model 1, Stepping 1) today, and > it also stopped when reading the MSR_INTEL_PLATFORM_INFO instead of > producing #GP(0). I've yet to try it out on my Rome system, but I have to admit I find this hard to believe. Andrew - you've tried to boot Xen on a Rome already. Iirc you said it crashed, but did it perhaps get to (and past) this point? Jan
On 02/04/2019 16:46, Jan Beulich wrote: >>>> On 02.04.19 at 13:58, <puwen@hygon.cn> wrote: >> On 2019/4/2 18:20, Jan Beulich wrote: >>> On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: >>>> On 2019/4/1 16:41, Jan Beulich wrote: >>>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>>>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >>>>>> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. >>>>> I think it would have been nice if you had mentioned the real >>>>> reason why you want to bypass the MSR accesses. This way it >>>>> sounds as if the change was only cosmetic, and hence could be >>>>> left out. >>>> Okay, how about the new description: >>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read >>>> this MSR will stop the Xen initialization process >>> "... for some early Hygon steppings"(?). I'm unaware of AMD CPUs >> Yes,for some early Hygon steppings. >> >>> having this issue - are you telling us otherwise? >> I tested with an AMD CPU(Family 17h, Model 1, Stepping 1) today, and >> it also stopped when reading the MSR_INTEL_PLATFORM_INFO instead of >> producing #GP(0). > I've yet to try it out on my Rome system, but I have to admit I > find this hard to believe. Andrew - you've tried to boot Xen on a > Rome already. Iirc you said it crashed, but did it perhaps get to > (and past) this point? I've got Fam10, 15, 17 (both Naples and Rome) and haven't encountered any problems. Whatever is going on, it sounds like a microcode bug. ~Andrew
On 2019/4/2 23:46, Jan Beulich wrote: > On 02.04.19 at 13:58, <puwen@hygon.cn> wrote: >> On 2019/4/2 18:20, Jan Beulich wrote: >>> On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: >>>> On 2019/4/1 16:41, Jan Beulich wrote: >>>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>>>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly >>>>>> return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. >>>>> >>>>> I think it would have been nice if you had mentioned the real >>>>> reason why you want to bypass the MSR accesses. This way it >>>>> sounds as if the change was only cosmetic, and hence could be >>>>> left out. >>>> >>>> Okay, how about the new description: >>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read >>>> this MSR will stop the Xen initialization process >>> >>> "... for some early Hygon steppings"(?). I'm unaware of AMD CPUs >> >> Yes,for some early Hygon steppings. >> >>> having this issue - are you telling us otherwise? >> >> I tested with an AMD CPU(Family 17h, Model 1, Stepping 1) today, and >> it also stopped when reading the MSR_INTEL_PLATFORM_INFO instead of >> producing #GP(0). > > I've yet to try it out on my Rome system, but I have to admit I We have Rome system too, I'll have a try on it as well. > find this hard to believe. Andrew - you've tried to boot Xen on a > Rome already. Iirc you said it crashed, but did it perhaps get to > (and past) this point?
On 02/04/2019 17:14, Pu Wen wrote: > On 2019/4/2 23:46, Jan Beulich wrote: >> On 02.04.19 at 13:58, <puwen@hygon.cn> wrote: >>> On 2019/4/2 18:20, Jan Beulich wrote: >>>> On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: >>>>> On 2019/4/1 16:41, Jan Beulich wrote: >>>>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>>>>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. >>>>>>> So directly >>>>>>> return false in the function probe_cpuid_faulting() if >>>>>>> !cpu_has_hypervisor. >>>>>> >>>>>> I think it would have been nice if you had mentioned the real >>>>>> reason why you want to bypass the MSR accesses. This way it >>>>>> sounds as if the change was only cosmetic, and hence could be >>>>>> left out. >>>>> >>>>> Okay, how about the new description: >>>>> There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. Read >>>>> this MSR will stop the Xen initialization process >>>> >>>> "... for some early Hygon steppings"(?). I'm unaware of AMD CPUs >>> >>> Yes,for some early Hygon steppings. >>> >>>> having this issue - are you telling us otherwise? >>> >>> I tested with an AMD CPU(Family 17h, Model 1, Stepping 1) today, and >>> it also stopped when reading the MSR_INTEL_PLATFORM_INFO instead of >>> producing #GP(0). >> >> I've yet to try it out on my Rome system, but I have to admit I > > We have Rome system too, I'll have a try on it as well. I've just spoken to a contact at AMD, and they've never encountered an issue like this. If it behaviour does manifest, then it is very concerning. ~Andrew
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index db1ebf1..a08d48f 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -116,6 +116,11 @@ bool __init probe_cpuid_faulting(void) uint64_t val; int rc; + if ((boot_cpu_data.x86_vendor == X86_VENDOR_HYGON || + boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && + !cpu_has_hypervisor) + return false; + if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0) raw_msr_policy.plaform_info.cpuid_faulting = val & MSR_PLATFORM_INFO_CPUID_FAULTING;
There is no MSR_INTEL_PLATFORM_INFO for AMD and Hygon families. So directly return false in the function probe_cpuid_faulting() if !cpu_has_hypervisor. Signed-off-by: Pu Wen <puwen@hygon.cn> --- xen/arch/x86/cpu/common.c | 5 +++++ 1 file changed, 5 insertions(+)