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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.06.18 at 17:42, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

What's wrong with utilizing the little bit of sharing that's possible at
this point, and extend it (if suitable) later? Any bit of avoided
redundancy is helpful imo. Let me say it that way - I'm not going to
make my R-b dependent on which route you go, but I'd like to ask
that you please consider the suggested alternative another time.

Jan



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