Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-15 Thread Eduardo Habkost
On Mon, Dec 14, 2015 at 07:17:27PM -0500, Raj, Ashok wrote: > On Mon, Dec 14, 2015 at 11:37:16PM +0100, Borislav Petkov wrote: > > On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > > > This is mostly harmless.. since the MCG_CAP space is shared and has no > > > conflict between

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-15 Thread Borislav Petkov
On Mon, Dec 14, 2015 at 07:17:27PM -0500, Raj, Ashok wrote: > I can see how this hurts.. since the poller isn't doing cpu model > specific stuff..? The poller sees mca_cfg.ser set on an AMD guest and then the whole handling/decoding goes wrong. > in the LMCE case, even if you advertise

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-15 Thread Eduardo Habkost
On Mon, Dec 14, 2015 at 07:17:27PM -0500, Raj, Ashok wrote: > On Mon, Dec 14, 2015 at 11:37:16PM +0100, Borislav Petkov wrote: > > On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > > > This is mostly harmless.. since the MCG_CAP space is shared and has no > > > conflict between

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-15 Thread Borislav Petkov
On Mon, Dec 14, 2015 at 07:17:27PM -0500, Raj, Ashok wrote: > I can see how this hurts.. since the poller isn't doing cpu model > specific stuff..? The poller sees mca_cfg.ser set on an AMD guest and then the whole handling/decoding goes wrong. > in the LMCE case, even if you advertise

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Raj, Ashok
On Mon, Dec 14, 2015 at 11:37:16PM +0100, Borislav Petkov wrote: > On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > > This is mostly harmless.. since the MCG_CAP space is shared and has no > > conflict between vendors. Also just the CAP being set has no effect. > > Of course it does

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Borislav Petkov
On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > This is mostly harmless.. since the MCG_CAP space is shared and has no > conflict between vendors. Also just the CAP being set has no effect. Of course it does - we check SER_P in machine_check_poll() and when I emulate an AMD guest

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Eduardo Habkost
On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > On Mon, Dec 14, 2015 at 05:37:38PM +0100, Borislav Petkov wrote: > > > > ... and obviously LMCE is vendor-specific so it cannot be enabled on > > !Intel guests with a define like that. mce_init() in qemu should check > > vendor too. >

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Raj, Ashok
On Mon, Dec 14, 2015 at 05:37:38PM +0100, Borislav Petkov wrote: > > ... and obviously LMCE is vendor-specific so it cannot be enabled on > !Intel guests with a define like that. mce_init() in qemu should check > vendor too. > > The same mistake was done with SER_P but that's much harder to

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Raj, Ashok
Hi Eduardo, Thanks for the feedback. All the suggestions make sense.. On Mon, Dec 14, 2015 at 02:23:56PM -0200, Eduardo Habkost wrote: > > +static void feature_control_init(X86CPU *cpu) > > +{ > > + CPUX86State *cenv = >env; > > + > > + cenv->msr_ia32_feature_control = ((1<<20) | (1<<0)); >

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Borislav Petkov
On Mon, Dec 14, 2015 at 02:23:56PM -0200, Eduardo Habkost wrote: > > -#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P) > > +#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P|MCG_LMCE_P) > > This makes mcg_cap change when upgrading QEMU. > > VMs with MCG_LMCE_P enabled shouldn't be migratable to hosts >

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Eduardo Habkost
Hi, Comments below: On Thu, Dec 10, 2015 at 02:41:21PM -0500, Ashok Raj wrote: > This patch adds basic enumeration, control msr's required to support > Local Machine Check Exception Support (LMCE). > > - Added Local Machine Check definitions, changed MCG_CAP > - Added support for

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Borislav Petkov
On Mon, Dec 14, 2015 at 02:23:56PM -0200, Eduardo Habkost wrote: > > -#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P) > > +#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P|MCG_LMCE_P) > > This makes mcg_cap change when upgrading QEMU. > > VMs with MCG_LMCE_P enabled shouldn't be migratable to hosts >

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Eduardo Habkost
Hi, Comments below: On Thu, Dec 10, 2015 at 02:41:21PM -0500, Ashok Raj wrote: > This patch adds basic enumeration, control msr's required to support > Local Machine Check Exception Support (LMCE). > > - Added Local Machine Check definitions, changed MCG_CAP > - Added support for

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Raj, Ashok
On Mon, Dec 14, 2015 at 05:37:38PM +0100, Borislav Petkov wrote: > > ... and obviously LMCE is vendor-specific so it cannot be enabled on > !Intel guests with a define like that. mce_init() in qemu should check > vendor too. > > The same mistake was done with SER_P but that's much harder to

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Eduardo Habkost
On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > On Mon, Dec 14, 2015 at 05:37:38PM +0100, Borislav Petkov wrote: > > > > ... and obviously LMCE is vendor-specific so it cannot be enabled on > > !Intel guests with a define like that. mce_init() in qemu should check > > vendor too. >

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Raj, Ashok
Hi Eduardo, Thanks for the feedback. All the suggestions make sense.. On Mon, Dec 14, 2015 at 02:23:56PM -0200, Eduardo Habkost wrote: > > +static void feature_control_init(X86CPU *cpu) > > +{ > > + CPUX86State *cenv = >env; > > + > > + cenv->msr_ia32_feature_control = ((1<<20) | (1<<0)); >

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Borislav Petkov
On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > This is mostly harmless.. since the MCG_CAP space is shared and has no > conflict between vendors. Also just the CAP being set has no effect. Of course it does - we check SER_P in machine_check_poll() and when I emulate an AMD guest

Re: [Qemu-devel] [Patch V2 1/2] x86, mce: Basic support to add LMCE support to QEMU

2015-12-14 Thread Raj, Ashok
On Mon, Dec 14, 2015 at 11:37:16PM +0100, Borislav Petkov wrote: > On Mon, Dec 14, 2015 at 02:11:46PM -0500, Raj, Ashok wrote: > > This is mostly harmless.. since the MCG_CAP space is shared and has no > > conflict between vendors. Also just the CAP being set has no effect. > > Of course it does