Re: [PATCH 1/2] xen, libxc: Introduced XEN_DOMCTL_emulate_each_rep

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 09/15/2015 06:36 PM, Julien Grall wrote:
> Hi Razvan,
> 
> On 15/09/15 10:19, Razvan Cojocaru wrote:
>> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
>> index 9e0fef5..214a22a 100644
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -1180,6 +1180,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>              copyback = 1;
>>          break;
>>  
>> +    case XEN_DOMCTL_emulate_each_rep:
>> +        d->arch.hvm_domain.emulate_each_rep = !!op->u.emulate_each_rep.op;
> 
> The common code should never access directly to an arch field.
> 
> Actually this will break on ARM because you introduced the field only
> for x86.
> 
> Please move this new domctl in arch_do_domctl.

Of course, thanks for pointing that out!

>> +        ret = 0;
>> +        break;
>> +
>>      default:
>>          ret = arch_do_domctl(op, d, u_domctl);
>>          break;
>> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
>> index 992d5d1..b8fbe5e 100644
>> --- a/xen/include/asm-x86/hvm/domain.h
>> +++ b/xen/include/asm-x86/hvm/domain.h
>> @@ -136,6 +136,7 @@ struct hvm_domain {
>>      bool_t                 mem_sharing_enabled;
>>      bool_t                 qemu_mapcache_invalidate;
>>      bool_t                 is_s3_suspended;
>> +    bool_t                 emulate_each_rep;
>>  
>>      /*
>>       * TSC value that VCPUs use to calculate their tsc_offset value.
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>> index 794d4d5..efc42a8 100644
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -1063,6 +1063,12 @@ struct xen_domctl_psr_cat_op {
>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>  
>> +struct xen_domctl_emulate_each_rep {
>> +    int32_t op;
>> +};
>> +typedef struct xen_domctl_emulate_each_rep xen_domctl_emulate_each_rep_t;
>> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_emulate_each_rep_t);
>> +
>>  struct xen_domctl {
>>      uint32_t cmd;
>>  #define XEN_DOMCTL_createdomain                   1
>> @@ -1140,6 +1146,7 @@ struct xen_domctl {
>>  #define XEN_DOMCTL_monitor_op                    77
>>  #define XEN_DOMCTL_psr_cat_op                    78
>>  #define XEN_DOMCTL_soft_reset                    79
>> +#define XEN_DOMCTL_emulate_each_rep              80
>>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
>> @@ -1202,6 +1209,7 @@ struct xen_domctl {
>>          struct xen_domctl_psr_cmt_op        psr_cmt_op;
>>          struct xen_domctl_monitor_op        monitor_op;
>>          struct xen_domctl_psr_cat_op        psr_cat_op;
>> +        struct xen_domctl_emulate_each_rep  emulate_each_rep;
> 
> IHMO this should be exposed only for x86 as we don't have a such
> instruction on ARM.

Sure, I'll #ifdef it with the other __i386__ and __x86_64__ things.


Thanks for the quick review,
Razvan