Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/08/18 15:52, Jan Beulich wrote:
>>>> On 28.08.18 at 19:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 28.08.18 at 19:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>> The trailing _vcpu suffix is redundant, but adds to code volume.  Drop it.
>>
>> Reflow lines as appropriate, and switch to using the new XFREE/etc wrappers
>> where applicable.
> I couldn't find any such conversion, so perhaps better to delete that
> part of the description.

Fixed.

>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

>
>> @@ -3888,19 +3886,19 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t 
>> cs, uint16_t ip)
>>      v->arch.user_regs.rip = ip;
>>      memset(&v->arch.debugreg, 0, sizeof(v->arch.debugreg));
>>  
>> -    v->arch.hvm_vcpu.guest_cr[0] = X86_CR0_ET;
>> +    v->arch.hvm.guest_cr[0] = X86_CR0_ET;
>>      hvm_update_guest_cr(v, 0);
>>  
>> -    v->arch.hvm_vcpu.guest_cr[2] = 0;
>> +    v->arch.hvm.guest_cr[2] = 0;
>>      hvm_update_guest_cr(v, 2);
>>  
>> -    v->arch.hvm_vcpu.guest_cr[3] = 0;
>> +    v->arch.hvm.guest_cr[3] = 0;
>>      hvm_update_guest_cr(v, 3);
>>  
>> -    v->arch.hvm_vcpu.guest_cr[4] = 0;
>> +    v->arch.hvm.guest_cr[4] = 0;
>>      hvm_update_guest_cr(v, 4);
>>  
>> -    v->arch.hvm_vcpu.guest_efer = 0;
>> +    v->arch.hvm.guest_efer = 0;
>>      hvm_update_guest_efer(v);
> Noticing this, a thought unrelated to this series: Wouldn't we be better off
> setting all the structure fields first, and only then invoke all the 
> functions?
> Just like arch_set_info_hvm_guest() does?

Actually, arch_set_info_hvm_guest() is broken in a related way.  When
nested virt is in the mix, the usual rules for which control bits can be
changed are relaxed, and you can get into a number of corner cases where
these helpers don't do the correct thing.  (e.g. when a 32bit PAE guest
tries vmexiting to a PCID hypervisor).

Resolving this mess is on the todo list, which includes this function,
and others.

~Andrew

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