Re: [Xen-devel] [PATCH v3 07/32] xen/x86: fix arch_set_info_guest for HVM guests

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

El 06/07/15 a les 14.58, Andrew Cooper ha escrit:
> On 03/07/15 12:34, Roger Pau Monne wrote:
>> Add checks for ignored vcpu fields in HVM mode. HVM vCPUs (BSP and APs) are
>> always started in 32bit protected mode with paging disabled.
>>
>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>>  xen/arch/x86/domain.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index a8fe046..a112953 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -795,6 +795,15 @@ int arch_set_info_guest(
>>                c.nat->fs_base || c.nat->gs_base_user)) )
>>              return -EINVAL;
>>      }
>> +    else if ( is_hvm_domain(d) )
>> +    {
>> +        if ( c(ctrlreg[0]) || c(ctrlreg[1]) || c(ctrlreg[2]) ||
>> +             c(ctrlreg[3]) || c(ctrlreg[4]) || c(ctrlreg[5]) ||
> 
> I am not sure that the cr0 and cr4 checks are sensible.  While the new
> ABI doesn't need them, there is nothing inherently wrong with setting
> them via this interface.

They will be ignored anyway, so I think the right thing to do is to
notify the caller that setting CR4 and CR0 is not possible.

IMHO this should be as limited as possible, and CR0/CR4 can be easily
setup by the startup AP code in the guest.

>> +             c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) ||
>> +             c(ldt_ents) || c(kernel_ss) || c(kernel_sp) ||
>> +             c(gdt_ents) )
>> +            return -EINVAL;
>> +    }
>>  
>>      v->fpu_initialised = !!(flags & VGCF_I387_VALID);
>>  
>> @@ -1064,15 +1073,14 @@ int arch_set_info_guest(
>>      if ( v->vcpu_id == 0 )
>>          update_domain_wallclock_time(d);
>>  
>> -    /* Don't redo final setup */
>> -    v->is_initialised = 1;
>> -
> 
> Is this a bugfix, or spurious movement?

Since this hypercall was never used before with HVM guests we need to
move the is_initialized setting to the "out" label, so it's also set for
HVM guests (HVM guests jump to the out label after setting the user and
debug registers).

Roger.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel