Re: [Xen-devel] [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

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

On 19/11/13 11:34, George Dunlap wrote:
> On 11/18/2013 06:18 PM, Roger Pau Monne wrote:
>> The aim of this patch is to define a stable way in which PVH is
>> going to do AP bringup.
>>
>> Since we are running inside of a HVM container, PVH should only need
>> to set flags, cr3 and user_regs in order to bring up a vCPU, the rest
>> can be set once the vCPU is started using the bare metal methods.
>> Additionally, the guest can also set cr0 and cr4, and those values
>> will be appended to the default values set by Xen.
>>
>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>> Cc: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> Cc: Jan Beulich <JBeulich@xxxxxxxx>
>> Cc: Tim Deegan <tim@xxxxxxx>
>> Cc: Keir Fraser <keir@xxxxxxx>
>> ---
>>   xen/arch/x86/domain.c             |   24 ++++++++++++++++--------
>>   xen/arch/x86/hvm/vmx/vmx.c        |    6 +-----
>>   xen/include/asm-x86/hvm/hvm.h     |    6 +++---
>>   xen/include/public/arch-x86/xen.h |    8 ++++----
>>   4 files changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index a3868f9..51311fc 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -51,6 +51,7 @@
>>   #include <asm/fixmap.h>
>>   #include <asm/hvm/hvm.h>
>>   #include <asm/hvm/support.h>
>> +#include <asm/hvm/nestedhvm.h>
>>   #include <asm/debugreg.h>
>>   #include <asm/msr.h>
>>   #include <asm/traps.h>
>> @@ -704,9 +705,13 @@ int arch_set_info_guest(
>>           /* PVH 32bitfixme */
>>           ASSERT(!compat);
>>
>> -        if ( c(ctrlreg[1]) || c(ldt_base) || c(ldt_ents) ||
>> +        if ( (c(ctrlreg[0]) & HVM_CR0_GUEST_RESERVED_BITS) ||
>> +             (c(ctrlreg[4]) & HVM_CR4_GUEST_RESERVED_BITS(v)) ||
> 
> Good idea. :-)
> 
>> +             c(ctrlreg[1]) || c(ctrlreg[2]) || c(ctrlreg[5]) ||
>> +             c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) ||
>> c(ldt_ents) ||
>>                c(user_regs.cs) || c(user_regs.ss) || c(user_regs.es) ||
>>                c(user_regs.ds) || c(user_regs.fs) || c(user_regs.gs) ||
>> +             c(kernel_ss) || c(kernel_sp) || c.nat->gs_base_kernel ||
>>                c.nat->gdt_ents || c.nat->fs_base || c.nat->gs_base_user )
>>               return -EINVAL;
>>       }
>> @@ -745,17 +750,20 @@ int arch_set_info_guest(
>>
>>       if ( has_hvm_container_vcpu(v) )
>>       {
>> -        /*
>> -         * NB: TF_kernel_mode is set unconditionally for HVM guests,
>> -         * so we always use the gs_base_kernel here. If we change this
>> -         * function to imitate the PV functionality, we'll need to
>> -         * make it pay attention to the kernel bit.
>> -         */
>> -        hvm_set_info_guest(v, compat ? 0 : c.nat->gs_base_kernel);
> 
> I think we still need to call hvm_set_info_guest() here -- there is
> other stuff being set up, and it was called (without the extra argument
> for gs_base_kernel) before the PVH patch series went in.
> 
> (See c/s 35b1e07.)

I see, I will restore the hvm_set_info_guest call, good catch.

> 
>>
>>           if ( is_hvm_vcpu(v) || v->is_initialised )
>>               goto out;
>>
>> +        if ( c.nat->ctrlreg[0] ) {
>> +            v->arch.hvm_vcpu.guest_cr[0] |= c.nat->ctrlreg[0];
>> +            hvm_update_guest_cr(v, 0);
>> +        }
>> +
>> +        if ( c.nat->ctrlreg[4] ) {
>> +            v->arch.hvm_vcpu.guest_cr[4] |= c.nat->ctrlreg[4];
>> +            hvm_update_guest_cr(v, 4);
>> +        }
> 
> I thought the idea was that he would set cr0, cr3, and cr4 for all HVM
> guests, rather than special-casing PVH guests?

Well, I was just trying to make the PVH AP bringup interface stable,
without messing much with HVM (not that we shouldn't aim to do it for
HVM guests also), but I think the main concern now is releasing 4.4 with
a stable interface for PVH guests.

Thanks, Roger.


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