Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:
> This commit moves the altp2m-related code from x86 to ARM.
Looking at the code in the follow-up patches, I have the impression that 
the code is very similar (if not exactly) to the x86 code. If so, we 
should move the HVMOP for altp2m in common code rather than duplicating 
the code.
> Functions
> that are no yet supported notify the caller or print a BUG message
> stating their absence.
>
> Also, the struct arch_domain is extended with the altp2m_active
> attribute, represeting the current altp2m activity configuration of the
s/represeting/representing/

> domain.
>
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>   xen/arch/arm/hvm.c           | 82 ++++++++++++++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/altp2m.h | 22 ++++++++++--
>   xen/include/asm-arm/domain.h |  3 ++
>   3 files changed, 105 insertions(+), 2 deletions(-)
[..]

> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
> index a87747a..16ae9d6 100644
> --- a/xen/include/asm-arm/altp2m.h
> +++ b/xen/include/asm-arm/altp2m.h
> @@ -2,6 +2,7 @@
>    * Alternate p2m
>    *
>    * Copyright (c) 2014, Intel Corporation.
> + * Copyright (c) 2016, Sergej Proskurin <proskurin@xxxxxxxxxxxxx>.
>    *
>    * This program is free software; you can redistribute it and/or modify it
>    * under the terms and conditions of the GNU General Public License,
> @@ -24,8 +25,7 @@
>   /* Alternate p2m on/off per domain */
>   static inline bool_t altp2m_active(const struct domain *d)
>   {
> -    /* Not implemented on ARM. */
> -    return 0;
> +    return d->arch.altp2m_active;
>   }
>
>   /* Alternate p2m VCPU */
> @@ -36,4 +36,22 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
>       return 0;
>   }
>
> +static inline void altp2m_vcpu_initialise(struct vcpu *v)
> +{
> +    /* Not implemented on ARM, should not be reached. */
> +    BUG();
> +}
> +
> +static inline void altp2m_vcpu_destroy(struct vcpu *v)
> +{
> +    /* Not implemented on ARM, should not be reached. */
> +    BUG();
> +}
> +
> +static inline void altp2m_vcpu_reset(struct vcpu *v)
> +{
> +    /* Not implemented on ARM, should not be reached. */
> +    BUG();
> +}
Those 3 helpers are not used by anyone in the code so far and you 
replace them by another implementation in patch #5.
So I would prefer if you introduce the helpers and implementation only 
when they will be used.
>   #endif /* __ASM_ARM_ALTP2M_H */
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 979f7de..2039f16 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -127,6 +127,9 @@ struct arch_domain
>       paddr_t efi_acpi_gpa;
>       paddr_t efi_acpi_len;
>   #endif
> +
> +    /* altp2m: allow multiple copies of host p2m */
> +    bool_t altp2m_active;
>   }  __cacheline_aligned;
>
>   struct arch_vcpu
Regards,

--
Julien Grall

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