Re: [Xen-devel] [PATCH v3 13/25] x86emul: adjust_bnd() should check XCR0
On 02/02/18 16:19, Jan Beulich wrote:
On 02.02.18 at 14:30, wrote:
>> On 07/12/17 14:08, Jan Beulich wrote:
>>> Experimentally MPX instructions have been confirmed to behave as NOPs
>>> unless both related XCR0 bits are set to 1. By implication branches
>>> then also don't clear BNDn.
>>>
>>> Signed-off-by: Jan Beulich
>>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -2143,12 +2143,16 @@ static bool umip_active(struct x86_emula
>>> static void adjust_bnd(struct x86_emulate_ctxt *ctxt,
>>> const struct x86_emulate_ops *ops, enum vex_pfx pfx)
>>> {
>>> -uint64_t bndcfg;
>>> +uint64_t xcr0, bndcfg;
>>> int rc;
>>>
>>> if ( pfx == vex_f2 || !cpu_has_mpx || !vcpu_has_mpx() )
>>> return;
>>>
>>> +if ( !ops->read_xcr || ops->read_xcr(0, &xcr0, ctxt) != X86EMUL_OKAY ||
>>> + !(xcr0 & XSTATE_BNDREGS) || !(xcr0 & XSTATE_BNDCSR) )
>> !(xcr0 & (XSTATE_BNDREGS | XSTATE_BNDCSR)) ?
> No, I mean "if either bit is clear", not "if both bits are clear". I think
> we had discussed before that both bits need to be 1 in order for
> bounds checking to actually work.
>
>> Otherwise, Reviewed-by: Andrew Cooper
> Please clarify this in light of the above.
Architecturally, they can't be different, which is why the above logic
looks suspicious.
Given that the actual isn't wrong, I won't object, but it does look
wrong to compare them individually.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 13/25] x86emul: adjust_bnd() should check XCR0
>>> On 02.02.18 at 14:30, wrote:
> On 07/12/17 14:08, Jan Beulich wrote:
>> Experimentally MPX instructions have been confirmed to behave as NOPs
>> unless both related XCR0 bits are set to 1. By implication branches
>> then also don't clear BNDn.
>>
>> Signed-off-by: Jan Beulich
>>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -2143,12 +2143,16 @@ static bool umip_active(struct x86_emula
>> static void adjust_bnd(struct x86_emulate_ctxt *ctxt,
>> const struct x86_emulate_ops *ops, enum vex_pfx pfx)
>> {
>> -uint64_t bndcfg;
>> +uint64_t xcr0, bndcfg;
>> int rc;
>>
>> if ( pfx == vex_f2 || !cpu_has_mpx || !vcpu_has_mpx() )
>> return;
>>
>> +if ( !ops->read_xcr || ops->read_xcr(0, &xcr0, ctxt) != X86EMUL_OKAY ||
>> + !(xcr0 & XSTATE_BNDREGS) || !(xcr0 & XSTATE_BNDCSR) )
>
> !(xcr0 & (XSTATE_BNDREGS | XSTATE_BNDCSR)) ?
No, I mean "if either bit is clear", not "if both bits are clear". I think
we had discussed before that both bits need to be 1 in order for
bounds checking to actually work.
> Otherwise, Reviewed-by: Andrew Cooper
Please clarify this in light of the above.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 13/25] x86emul: adjust_bnd() should check XCR0
On 07/12/17 14:08, Jan Beulich wrote:
> Experimentally MPX instructions have been confirmed to behave as NOPs
> unless both related XCR0 bits are set to 1. By implication branches
> then also don't clear BNDn.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -2143,12 +2143,16 @@ static bool umip_active(struct x86_emula
> static void adjust_bnd(struct x86_emulate_ctxt *ctxt,
> const struct x86_emulate_ops *ops, enum vex_pfx pfx)
> {
> -uint64_t bndcfg;
> +uint64_t xcr0, bndcfg;
> int rc;
>
> if ( pfx == vex_f2 || !cpu_has_mpx || !vcpu_has_mpx() )
> return;
>
> +if ( !ops->read_xcr || ops->read_xcr(0, &xcr0, ctxt) != X86EMUL_OKAY ||
> + !(xcr0 & XSTATE_BNDREGS) || !(xcr0 & XSTATE_BNDCSR) )
!(xcr0 & (XSTATE_BNDREGS | XSTATE_BNDCSR)) ?
Otherwise, Reviewed-by: Andrew Cooper
> +return;
> +
> if ( !mode_ring0() )
> bndcfg = read_bndcfgu();
> else if ( !ops->read_msr ||
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
