Re: [PATCH 2/4] introduce arch_get_features()

[Julien Grall <julien.grall@linaro.org>]

Hi Juergen,

On 11/14/2014 09:37 AM, Juergen Gross wrote:
> The XENVER_get_features sub command of the xen_version hypercall is
> handled completely in common/kernel.c despite of some architecture
> dependant parts.
> 
> Move the architecture dependant parts in an own function in
> arch/*/domain.c
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

For the ARM part:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

> ---
>  xen/arch/arm/domain.c    |  5 +++++
>  xen/arch/x86/domain.c    | 30 ++++++++++++++++++++++++++++++
>  xen/common/kernel.c      | 22 ++--------------------
>  xen/include/xen/domain.h |  2 ++
>  4 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 7221bc8..dc5a3fb 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -823,6 +823,11 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
>          vcpu_unblock(current);
>  }
>  
> +uint32_t arch_get_features(struct domain *d, unsigned int submap_idx)
> +{
> +    return 0;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index ae0a344..d98aabd 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -2166,6 +2166,36 @@ static int __init init_vcpu_kick_softirq(void)
>  }
>  __initcall(init_vcpu_kick_softirq);
>  
> +uint32_t arch_get_features(struct domain *d, unsigned int submap_idx)
> +{
> +    uint32_t submap = 0;
> +
> +    switch ( submap_idx )
> +    {
> +    case 0:
> +        switch ( d->guest_type )
> +        {
> +        case guest_type_pv:
> +            submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
> +                      (1U << XENFEAT_highmem_assist) |
> +                      (1U << XENFEAT_gnttab_map_avail_bits);
> +            break;
> +        case guest_type_pvh:
> +            submap |= (1U << XENFEAT_hvm_safe_pvclock) |
> +                      (1U << XENFEAT_supervisor_mode_kernel) |
> +                      (1U << XENFEAT_hvm_callback_vector);
> +            break;
> +        case guest_type_hvm:
> +            submap |= (1U << XENFEAT_hvm_safe_pvclock) |
> +                      (1U << XENFEAT_hvm_callback_vector) |
> +                      (1U << XENFEAT_hvm_pirqs);
> +            break;
> +        }
> +        break;
> +    }
> +
> +    return submap;
> +}
>  
>  /*
>   * Local variables:
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index d23c422..d22a860 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -312,31 +312,13 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>                  fi.submap |= 1U << XENFEAT_supervisor_mode_kernel;
>              if ( is_hardware_domain(current->domain) )
>                  fi.submap |= 1U << XENFEAT_dom0;
> -#ifdef CONFIG_X86
> -            switch ( d->guest_type )
> -            {
> -            case guest_type_pv:
> -                fi.submap |= (1U << XENFEAT_mmu_pt_update_preserve_ad) |
> -                             (1U << XENFEAT_highmem_assist) |
> -                             (1U << XENFEAT_gnttab_map_avail_bits);
> -                break;
> -            case guest_type_pvh:
> -                fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
> -                             (1U << XENFEAT_supervisor_mode_kernel) |
> -                             (1U << XENFEAT_hvm_callback_vector);
> -                break;
> -            case guest_type_hvm:
> -                fi.submap |= (1U << XENFEAT_hvm_safe_pvclock) |
> -                             (1U << XENFEAT_hvm_callback_vector) |
> -                             (1U << XENFEAT_hvm_pirqs);
> -                break;
> -            }
> -#endif
>              break;
>          default:
>              return -EINVAL;
>          }
>  
> +        fi.submap |= arch_get_features(d, fi.submap_idx);
> +
>          if ( copy_to_guest(arg, &fi, 1) )
>              return -EFAULT;
>          return 0;
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 9215b0e..0d12dc0 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -80,6 +80,8 @@ extern spinlock_t vcpu_alloc_lock;
>  bool_t domctl_lock_acquire(void);
>  void domctl_lock_release(void);
>  
> +uint32_t arch_get_features(struct domain *d, unsigned int submap_idx);
> +
>  /*
>   * Continue the current hypercall via func(data) on specified cpu.
>   * If this function returns 0 then the function is guaranteed to run at some
> 


-- 
Julien Grall