* 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: 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: 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-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).