Re: [Xen-devel] [PATCH v3 16/25] x86emul: support SWAPGS

[Jan Beulich]

>>> On 02.02.18 at 14:41,  wrote:
> On 07/12/17 14:11, Jan Beulich wrote:
>> Signed-off-by: Jan Beulich 
>> ---
>> v3: New.
>>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -5047,6 +5047,24 @@ x86_emulate(
>>  goto done;
>>  break;
>>  
>> +case 0xf8: /* swapgs */
>> +generate_exception_if(!mode_64bit(), EXC_UD);
>> +generate_exception_if(!mode_ring0(), EXC_GP, 0);
>> +fail_if(!ops->read_segment || !ops->read_msr ||
>> +!ops->write_segment || !ops->write_msr);
>> +if ( (rc = ops->read_segment(x86_seg_gs, ,
>> + ctxt)) != X86EMUL_OKAY ||
>> + (rc = ops->read_msr(MSR_SHADOW_GS_BASE, _val,
>> + ctxt)) != X86EMUL_OKAY ||
>> + (rc = ops->write_msr(MSR_SHADOW_GS_BASE, sreg.base,
>> +  ctxt)) != X86EMUL_OKAY )
> 
> We need to unwind this write in the case of write_segment failing, or
> when the instruction restarts, state will be corrupt.

We don't do similar restoring anywhere else iirc, so I'm not sure I
want to start doing so here. Multi-element updates really need to
be converted to go through a staging layer, where the checks
are done right away, but the commit happens only at the end.
One of the reasons I decided against doing what you suggest
(indeed I had considered that) is that this other write may then
be failing, too. Let me know.

Jan

>> +goto done;
>> +sreg.base = msr_val;
>> +if ( (rc = ops->write_segment(x86_seg_gs, ,
>> +  ctxt)) != X86EMUL_OKAY )
>> +goto done;
>> +break;
>> +
>>  case 0xf9: /* rdtscp */
>>  fail_if(ops->read_msr == NULL);
>>  if ( (rc = ops->read_msr(MSR_TSC_AUX,
>>
>>
>>




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 16/25] x86emul: support SWAPGS

[Andrew Cooper]

On 07/12/17 14:11, Jan Beulich wrote:
> Signed-off-by: Jan Beulich 
> ---
> v3: New.
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -5047,6 +5047,24 @@ x86_emulate(
>  goto done;
>  break;
>  
> +case 0xf8: /* swapgs */
> +generate_exception_if(!mode_64bit(), EXC_UD);
> +generate_exception_if(!mode_ring0(), EXC_GP, 0);
> +fail_if(!ops->read_segment || !ops->read_msr ||
> +!ops->write_segment || !ops->write_msr);
> +if ( (rc = ops->read_segment(x86_seg_gs, ,
> + ctxt)) != X86EMUL_OKAY ||
> + (rc = ops->read_msr(MSR_SHADOW_GS_BASE, _val,
> + ctxt)) != X86EMUL_OKAY ||
> + (rc = ops->write_msr(MSR_SHADOW_GS_BASE, sreg.base,
> +  ctxt)) != X86EMUL_OKAY )

We need to unwind this write in the case of write_segment failing, or
when the instruction restarts, state will be corrupt.

~Andrew

> +goto done;
> +sreg.base = msr_val;
> +if ( (rc = ops->write_segment(x86_seg_gs, ,
> +  ctxt)) != X86EMUL_OKAY )
> +goto done;
> +break;
> +
>  case 0xf9: /* rdtscp */
>  fail_if(ops->read_msr == NULL);
>  if ( (rc = ops->read_msr(MSR_TSC_AUX,
>
>
>


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel