Re: [Xen-devel] [PATCH 03/11] x86: Initialise debug registers correctly

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/06/18 14:56, Jan Beulich wrote:
>>>> On 04.06.18 at 15:59, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -322,6 +322,14 @@ void free_vcpu_struct(struct vcpu *v)
>>      free_xenheap_page(v);
>>  }
>>  
>> +static void initialise_registers(struct vcpu *v)
>> +{
>> +    v->arch.user_regs.eflags = X86_EFLAGS_MBS;
> If you used ->rflags here, you could (and imo better would) also ...
>
>> @@ -3907,7 +3908,10 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t 
>> cs, uint16_t ip)
>>      v->arch.user_regs.rflags = X86_EFLAGS_MBS;
>>      v->arch.user_regs.rdx = 0x00000f00;
>>      v->arch.user_regs.rip = ip;
>> -    memset(&v->arch.debugreg, 0, sizeof(v->arch.debugreg));
>> +
>> +    memset(&v->arch.debugreg, 0, sizeof(v->arch.debugreg) - 16);
>> +    v->arch.debugreg[6] = X86_DR6_DEFAULT;
>> +    v->arch.debugreg[7] = X86_DR7_DEFAULT;
> ... call that function from here (dropping the setting of .rflags
> visible in context).
>
> If you decide to go that route,
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Unforunately, I don't agree.  Large quantities of hvm_vcpu_reset_state()
would logically live in initialise_registers(), but definitely don't
want to be in the vcpu_initialise() path at this point.

initialise_registers() is restricted to the simple registers, and in
particular cannot cope with control registers which are wildly different
between PV and HVM guests.

I can't find a reasonable split (or for that matter, name) of a shared
helper function between part of hvm_vcpu_reset_state() and the vcpu
creation path.  For the sake of two constants, I'm not sure trying to
combine the paths is useful, unless you've got a better suggestion.

~Andrew

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