* Bug in smpboot.c? @ 2011-06-09 15:49 John McDermott (U.S. Navy Employee) 2011-06-09 22:09 ` Keir Fraser 2011-06-10 7:30 ` Bug in smpboot.c? Keir Fraser 0 siblings, 2 replies; 11+ messages in thread From: John McDermott (U.S. Navy Employee) @ 2011-06-09 15:49 UTC (permalink / raw) To: xen-devel Xen Developers, In C function cpu_add(), in xen/arch/x86/smpboot.c, if acpi_id == MAX_MADT_ENTRIES, won't this write past the end of array x86_acpiid_toapicid[MAX_MADT_ENTRIES]? I am looking at xen-unstable. It looks like the guard is not catching this 1 case? Sincerely, John McDermott ---- What is the formal meaning of the one-line program #include "/dev/tty" J.P. McDermott building 12 Code 5542 mcdermott@itd.nrl.navy.mil Naval Research Laboratory voice: +1 202.404.8301 Washington, DC 20375, US fax: +1 202.404.7942 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug in smpboot.c? 2011-06-09 15:49 Bug in smpboot.c? John McDermott (U.S. Navy Employee) @ 2011-06-09 22:09 ` Keir Fraser 2011-06-10 5:36 ` Addback capability check for non-initial features Dong, Eddie 2011-06-10 7:30 ` Bug in smpboot.c? Keir Fraser 1 sibling, 1 reply; 11+ messages in thread From: Keir Fraser @ 2011-06-09 22:09 UTC (permalink / raw) To: John McDermott (U.S. Navy Employee), xen-devel On 09/06/2011 16:49, "John McDermott (U.S. Navy Employee)" <john.mcdermott@nrl.navy.mil> wrote: > Xen Developers, > > In C function cpu_add(), in xen/arch/x86/smpboot.c, if acpi_id == > MAX_MADT_ENTRIES, won't this write past the end of array > x86_acpiid_toapicid[MAX_MADT_ENTRIES]? I am looking at xen-unstable. It looks > like the guard is not catching this 1 case? Looks like it. Also the check against MAX_APICS, and the pxm value against 256, are all similarly off by one. Thanks for spotting it! -- Keir > Sincerely, > > John McDermott > ---- > What is the formal meaning of the one-line program > #include "/dev/tty" > > J.P. McDermott building 12 > Code 5542 mcdermott@itd.nrl.navy.mil > Naval Research Laboratory voice: +1 202.404.8301 > Washington, DC 20375, US fax: +1 202.404.7942 > > > > > > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Addback capability check for non-initial features 2011-06-09 22:09 ` Keir Fraser @ 2011-06-10 5:36 ` Dong, Eddie 2011-06-10 5:50 ` Dong, Eddie 0 siblings, 1 reply; 11+ messages in thread From: Dong, Eddie @ 2011-06-10 5:36 UTC (permalink / raw) To: Keir Fraser; +Cc: xen-devel@lists.xensource.com, Dong, Eddie [-- Attachment #1: Type: text/plain, Size: 1879 bytes --] add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. Besides initial configuration, adjust_vmx_controls is responsible for hardware capibility check as well. This patch add back the check. Signed-off-by: Eddie Dong <eddie.dong@intel.com> diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) MSR_IA32_VMX_PINBASED_CTLS, &mismatch); min = (CPU_BASED_HLT_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING | +#ifdef __x86_64__ + CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING | +#endif CPU_BASED_INVLPG_EXITING | CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING | @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) _vmx_cpu_based_exec_control = adjust_vmx_controls( "CPU-Based Exec Control", min, opt, MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING); #ifdef __x86_64__ if ( !(_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) ) - { - min |= CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING; - _vmx_cpu_based_exec_control = adjust_vmx_controls( - "CPU-Based Exec Control", min, opt, - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - } + _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING); #endif if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS ) [-- Attachment #2: cap3.patch --] [-- Type: application/octet-stream, Size: 1836 bytes --] add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. Besides initial configuration, adjust_vmx_controls is responsible for hardware capibility check as well. This patch add back the check. Signed-off-by: Eddie Dong <eddie.dong@intel.com> diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) MSR_IA32_VMX_PINBASED_CTLS, &mismatch); min = (CPU_BASED_HLT_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING | +#ifdef __x86_64__ + CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING | +#endif CPU_BASED_INVLPG_EXITING | CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING | @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) _vmx_cpu_based_exec_control = adjust_vmx_controls( "CPU-Based Exec Control", min, opt, MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING); #ifdef __x86_64__ if ( !(_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) ) - { - min |= CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING; - _vmx_cpu_based_exec_control = adjust_vmx_controls( - "CPU-Based Exec Control", min, opt, - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - } + _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING); #endif if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS ) [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Addback capability check for non-initial features 2011-06-10 5:36 ` Addback capability check for non-initial features Dong, Eddie @ 2011-06-10 5:50 ` Dong, Eddie 2011-06-10 6:05 ` Keir Fraser 2011-06-10 7:33 ` Keir Fraser 0 siblings, 2 replies; 11+ messages in thread From: Dong, Eddie @ 2011-06-10 5:50 UTC (permalink / raw) To: Keir Fraser; +Cc: xen-devel@lists.xensource.com, Dong, Eddie [-- Attachment #1: Type: text/plain, Size: 1879 bytes --] add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. Besides initial configuration, adjust_vmx_controls is responsible for hardware capibility check as well. This patch add back the check. Signed-off-by: Eddie Dong <eddie.dong@intel.com> diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) MSR_IA32_VMX_PINBASED_CTLS, &mismatch); min = (CPU_BASED_HLT_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING | +#ifdef __x86_64__ + CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING | +#endif CPU_BASED_INVLPG_EXITING | CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING | @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) _vmx_cpu_based_exec_control = adjust_vmx_controls( "CPU-Based Exec Control", min, opt, MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING); #ifdef __x86_64__ if ( !(_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) ) - { - min |= CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING; - _vmx_cpu_based_exec_control = adjust_vmx_controls( - "CPU-Based Exec Control", min, opt, - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - } + _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING); #endif if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS ) [-- Attachment #2: cap3.patch --] [-- Type: application/octet-stream, Size: 1836 bytes --] add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. Besides initial configuration, adjust_vmx_controls is responsible for hardware capibility check as well. This patch add back the check. Signed-off-by: Eddie Dong <eddie.dong@intel.com> diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) MSR_IA32_VMX_PINBASED_CTLS, &mismatch); min = (CPU_BASED_HLT_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING | +#ifdef __x86_64__ + CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING | +#endif CPU_BASED_INVLPG_EXITING | CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING | @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) _vmx_cpu_based_exec_control = adjust_vmx_controls( "CPU-Based Exec Control", min, opt, MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | + CPU_BASED_VIRTUAL_INTR_PENDING); #ifdef __x86_64__ if ( !(_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) ) - { - min |= CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING; - _vmx_cpu_based_exec_control = adjust_vmx_controls( - "CPU-Based Exec Control", min, opt, - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); - } + _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | + CPU_BASED_CR8_STORE_EXITING); #endif if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS ) [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Addback capability check for non-initial features 2011-06-10 5:50 ` Dong, Eddie @ 2011-06-10 6:05 ` Keir Fraser 2011-06-10 6:33 ` Dong, Eddie 2011-06-10 7:33 ` Keir Fraser 1 sibling, 1 reply; 11+ messages in thread From: Keir Fraser @ 2011-06-10 6:05 UTC (permalink / raw) To: Dong, Eddie; +Cc: xen-devel@lists.xensource.com On 10/06/2011 06:50, "Dong, Eddie" <eddie.dong@intel.com> wrote: > > add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. > > Besides initial configuration, adjust_vmx_controls is responsible for > hardware capibility check as well. This patch add back the check. I suppose the CPU_BASED_VIRTUAL_INTR_PENDING addition is correct, for what it's worth (surely every VMX-capable CPU ever has and will support that). The change to CR8 detection looks mad and incorrect. You've inverted it so that CR8 exits get enabled when TPR_SHADOW is available, rather than when it isn't, surely? And that can't be correct. I don't see how the CR8-exit detection and enabling is wrong, as it is already. -- Keir > Signed-off-by: Eddie Dong <eddie.dong@intel.com> > > diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c > --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 > +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 > @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) > MSR_IA32_VMX_PINBASED_CTLS, &mismatch); > > min = (CPU_BASED_HLT_EXITING | > + CPU_BASED_VIRTUAL_INTR_PENDING | > +#ifdef __x86_64__ > + CPU_BASED_CR8_LOAD_EXITING | > + CPU_BASED_CR8_STORE_EXITING | > +#endif > CPU_BASED_INVLPG_EXITING | > CPU_BASED_CR3_LOAD_EXITING | > CPU_BASED_CR3_STORE_EXITING | > @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) > _vmx_cpu_based_exec_control = adjust_vmx_controls( > "CPU-Based Exec Control", min, opt, > MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); > - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; > + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | > + CPU_BASED_VIRTUAL_INTR_PENDING); > #ifdef __x86_64__ > if ( !(_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) ) > - { > - min |= CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING; > - _vmx_cpu_based_exec_control = adjust_vmx_controls( > - "CPU-Based Exec Control", min, opt, > - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); > - } > + _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | > + CPU_BASED_CR8_STORE_EXITING); > #endif > > if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS > ) ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: Addback capability check for non-initial features 2011-06-10 6:05 ` Keir Fraser @ 2011-06-10 6:33 ` Dong, Eddie 2011-06-10 6:58 ` Keir Fraser 0 siblings, 1 reply; 11+ messages in thread From: Dong, Eddie @ 2011-06-10 6:33 UTC (permalink / raw) To: Keir Fraser; +Cc: xen-devel@lists.xensource.com, Dong, Eddie > > > > add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. > > > > Besides initial configuration, adjust_vmx_controls is responsible for > > hardware capibility check as well. This patch add back the check. > > I suppose the CPU_BASED_VIRTUAL_INTR_PENDING addition is correct, for > what > it's worth (surely every VMX-capable CPU ever has and will support that). > > The change to CR8 detection looks mad and incorrect. You've inverted it so > that CR8 exits get enabled when TPR_SHADOW is available, rather than > when it CR8 exit is removed later on if TPR_SHADOW exist:) The only difference is that if there are processors that support TPR_SHADOW only, I can check internally if this is the concern. Current nested vmx is assuming CR8 exiting is presented to emulate L1 guest CR8 exiting. TPR_SHAOW can't trap CR8 read though cr8 write trap is OK w/ TPR shadow. Eventually I want to have a minimal common set of capability that is supported by all HW and is presented to L1 guest. > isn't, surely? And that can't be correct. I don't see how the CR8-exit > detection and enabling is wrong, as it is already. The original code for CR8 exit is correct too :) Thx, Eddie ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Addback capability check for non-initial features 2011-06-10 6:33 ` Dong, Eddie @ 2011-06-10 6:58 ` Keir Fraser 0 siblings, 0 replies; 11+ messages in thread From: Keir Fraser @ 2011-06-10 6:58 UTC (permalink / raw) To: Dong, Eddie; +Cc: xen-devel@lists.xensource.com On 10/06/2011 07:33, "Dong, Eddie" <eddie.dong@intel.com> wrote: >>> >>> add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. >>> >>> Besides initial configuration, adjust_vmx_controls is responsible for >>> hardware capibility check as well. This patch add back the check. >> >> I suppose the CPU_BASED_VIRTUAL_INTR_PENDING addition is correct, for >> what >> it's worth (surely every VMX-capable CPU ever has and will support that). >> >> The change to CR8 detection looks mad and incorrect. You've inverted it so >> that CR8 exits get enabled when TPR_SHADOW is available, rather than >> when it > > CR8 exit is removed later on if TPR_SHADOW exist:) Not in your patch. You remove it later if TPR_SHADOW *doesn't* exist. > The only difference is that if there are processors that support TPR_SHADOW > only, I can check internally if this is the concern. > Current nested vmx is assuming CR8 exiting is presented to emulate L1 guest > CR8 exiting. TPR_SHAOW can't trap CR8 read though cr8 write trap is OK w/ TPR > shadow. Hmm okay. > Eventually I want to have a minimal common set of capability that is supported > by all HW and is presented to L1 guest. > >> isn't, surely? And that can't be correct. I don't see how the CR8-exit >> detection and enabling is wrong, as it is already. > > The original code for CR8 exit is correct too :) More correct than yours :) -- Keir > Thx, Eddie ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Addback capability check for non-initial features 2011-06-10 5:50 ` Dong, Eddie 2011-06-10 6:05 ` Keir Fraser @ 2011-06-10 7:33 ` Keir Fraser 2011-06-10 8:05 ` Dong, Eddie 1 sibling, 1 reply; 11+ messages in thread From: Keir Fraser @ 2011-06-10 7:33 UTC (permalink / raw) To: Dong, Eddie; +Cc: xen-devel@lists.xensource.com On 10/06/2011 06:50, "Dong, Eddie" <eddie.dong@intel.com> wrote: > > add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. > > Besides initial configuration, adjust_vmx_controls is responsible for > hardware capibility check as well. This patch add back the check. I've fixed this and then applied it as xen-unstable:23508. Please take a look. -- Keir > Signed-off-by: Eddie Dong <eddie.dong@intel.com> > > diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c > --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 > +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 > @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) > MSR_IA32_VMX_PINBASED_CTLS, &mismatch); > > min = (CPU_BASED_HLT_EXITING | > + CPU_BASED_VIRTUAL_INTR_PENDING | > +#ifdef __x86_64__ > + CPU_BASED_CR8_LOAD_EXITING | > + CPU_BASED_CR8_STORE_EXITING | > +#endif > CPU_BASED_INVLPG_EXITING | > CPU_BASED_CR3_LOAD_EXITING | > CPU_BASED_CR3_STORE_EXITING | > @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) > _vmx_cpu_based_exec_control = adjust_vmx_controls( > "CPU-Based Exec Control", min, opt, > MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); > - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; > + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | > + CPU_BASED_VIRTUAL_INTR_PENDING); > #ifdef __x86_64__ > if ( !(_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) ) > - { > - min |= CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING; > - _vmx_cpu_based_exec_control = adjust_vmx_controls( > - "CPU-Based Exec Control", min, opt, > - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); > - } > + _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | > + CPU_BASED_CR8_STORE_EXITING); > #endif > > if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS > ) ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: Addback capability check for non-initial features 2011-06-10 7:33 ` Keir Fraser @ 2011-06-10 8:05 ` Dong, Eddie 0 siblings, 0 replies; 11+ messages in thread From: Dong, Eddie @ 2011-06-10 8:05 UTC (permalink / raw) To: Keir Fraser; +Cc: xen-devel@lists.xensource.com, Dong, Eddie Thanks! You are correct. Eddie > -----Original Message----- > From: Keir Fraser [mailto:keir.xen@gmail.com] On Behalf Of Keir Fraser > Sent: Friday, June 10, 2011 3:34 PM > To: Dong, Eddie > Cc: xen-devel@lists.xensource.com > Subject: Re: Addback capability check for non-initial features > > On 10/06/2011 06:50, "Dong, Eddie" <eddie.dong@intel.com> wrote: > > > > > add back missing capability check of MSR_IA32_VMX_PROCBASED_CTLS. > > > > Besides initial configuration, adjust_vmx_controls is responsible for > > hardware capibility check as well. This patch add back the check. > > I've fixed this and then applied it as xen-unstable:23508. Please take a > look. > > -- Keir > > > Signed-off-by: Eddie Dong <eddie.dong@intel.com> > > > > diff -r 43a06a43e60b xen/arch/x86/hvm/vmx/vmcs.c > > --- a/xen/arch/x86/hvm/vmx/vmcs.c Thu Jun 09 16:30:34 2011 +0800 > > +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jun 10 13:28:49 2011 +0800 > > @@ -148,6 +148,11 @@ static int vmx_init_vmcs_config(void) > > MSR_IA32_VMX_PINBASED_CTLS, &mismatch); > > > > min = (CPU_BASED_HLT_EXITING | > > + CPU_BASED_VIRTUAL_INTR_PENDING | > > +#ifdef __x86_64__ > > + CPU_BASED_CR8_LOAD_EXITING | > > + CPU_BASED_CR8_STORE_EXITING | > > +#endif > > CPU_BASED_INVLPG_EXITING | > > CPU_BASED_CR3_LOAD_EXITING | > > CPU_BASED_CR3_STORE_EXITING | > > @@ -164,15 +169,12 @@ static int vmx_init_vmcs_config(void) > > _vmx_cpu_based_exec_control = adjust_vmx_controls( > > "CPU-Based Exec Control", min, opt, > > MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); > > - _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING; > > + _vmx_cpu_based_exec_control &= ~(CPU_BASED_RDTSC_EXITING | > > + > CPU_BASED_VIRTUAL_INTR_PENDING); > > #ifdef __x86_64__ > > if ( !(_vmx_cpu_based_exec_control & > CPU_BASED_TPR_SHADOW) ) > > - { > > - min |= CPU_BASED_CR8_LOAD_EXITING | > CPU_BASED_CR8_STORE_EXITING; > > - _vmx_cpu_based_exec_control = adjust_vmx_controls( > > - "CPU-Based Exec Control", min, opt, > > - MSR_IA32_VMX_PROCBASED_CTLS, &mismatch); > > - } > > + _vmx_cpu_based_exec_control &= > ~(CPU_BASED_CR8_LOAD_EXITING | > > + > CPU_BASED_CR8_STORE_EXITING); > > #endif > > > > if ( _vmx_cpu_based_exec_control & > CPU_BASED_ACTIVATE_SECONDARY_CONTROLS > > ) > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug in smpboot.c? 2011-06-09 15:49 Bug in smpboot.c? John McDermott (U.S. Navy Employee) 2011-06-09 22:09 ` Keir Fraser @ 2011-06-10 7:30 ` Keir Fraser 2011-06-10 11:31 ` John McDermott CIV 1 sibling, 1 reply; 11+ messages in thread From: Keir Fraser @ 2011-06-10 7:30 UTC (permalink / raw) To: John McDermott (U.S. Navy Employee), xen-devel On 09/06/2011 16:49, "John McDermott (U.S. Navy Employee)" <john.mcdermott@nrl.navy.mil> wrote: > Xen Developers, > > In C function cpu_add(), in xen/arch/x86/smpboot.c, if acpi_id == > MAX_MADT_ENTRIES, won't this write past the end of array > x86_acpiid_toapicid[MAX_MADT_ENTRIES]? I am looking at xen-unstable. It looks > like the guard is not catching this 1 case? Fixed in xen-unstable:23505. Fortunately this function is only accessible from the TCB so it's not exploitable. Thanks, -- Keir > Sincerely, > > John McDermott > ---- > What is the formal meaning of the one-line program > #include "/dev/tty" > > J.P. McDermott building 12 > Code 5542 mcdermott@itd.nrl.navy.mil > Naval Research Laboratory voice: +1 202.404.8301 > Washington, DC 20375, US fax: +1 202.404.7942 > > > > > > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Bug in smpboot.c? 2011-06-10 7:30 ` Bug in smpboot.c? Keir Fraser @ 2011-06-10 11:31 ` John McDermott CIV 0 siblings, 0 replies; 11+ messages in thread From: John McDermott CIV @ 2011-06-10 11:31 UTC (permalink / raw) To: Keir Fraser; +Cc: xen-devel Keir, Thanks; yes, we see no way to unravel it. I'm just paranoid. Sincerely, John On Jun 10, 2011, at 3:30 AM, Keir Fraser wrote: > On 09/06/2011 16:49, "John McDermott (U.S. Navy Employee)" > <john.mcdermott@nrl.navy.mil> wrote: > >> Xen Developers, >> >> In C function cpu_add(), in xen/arch/x86/smpboot.c, if acpi_id == >> MAX_MADT_ENTRIES, won't this write past the end of array >> x86_acpiid_toapicid[MAX_MADT_ENTRIES]? I am looking at xen-unstable. It looks >> like the guard is not catching this 1 case? > > Fixed in xen-unstable:23505. Fortunately this function is only accessible > from the TCB so it's not exploitable. > > Thanks, > -- Keir > >> Sincerely, >> >> John McDermott >> ---- >> What is the formal meaning of the one-line program >> #include "/dev/tty" >> >> J.P. McDermott building 12 >> Code 5542 mcdermott@itd.nrl.navy.mil >> Naval Research Laboratory voice: +1 202.404.8301 >> Washington, DC 20375, US fax: +1 202.404.7942 >> >> >> >> >> >> >> >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > ---- What is the formal meaning of the one-line program #include "/dev/tty" J.P. McDermott building 12 Code 5542 mcdermott@itd.nrl.navy.mil Naval Research Laboratory voice: +1 202.404.8301 Washington, DC 20375, US fax: +1 202.404.7942 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-06-10 11:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-09 15:49 Bug in smpboot.c? John McDermott (U.S. Navy Employee) 2011-06-09 22:09 ` Keir Fraser 2011-06-10 5:36 ` Addback capability check for non-initial features Dong, Eddie 2011-06-10 5:50 ` Dong, Eddie 2011-06-10 6:05 ` Keir Fraser 2011-06-10 6:33 ` Dong, Eddie 2011-06-10 6:58 ` Keir Fraser 2011-06-10 7:33 ` Keir Fraser 2011-06-10 8:05 ` Dong, Eddie 2011-06-10 7:30 ` Bug in smpboot.c? Keir Fraser 2011-06-10 11:31 ` John McDermott CIV
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).