Re: [Xen-devel] [RFC PATCH V3] xen/arm: Restrict "p2m_ipa_bits" according to the IOMMU requirements

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 11/09/2019 18:59, Oleksandr Tyshchenko wrote:
> ---
>   xen/arch/arm/p2m.c                       | 41 ++++++++++++++++++++++++++++----
>   xen/arch/arm/setup.c                     |  9 +++++--
>   xen/drivers/passthrough/arm/ipmmu-vmsa.c | 18 ++------------
>   xen/drivers/passthrough/arm/smmu.c       | 11 +++------
>   xen/include/asm-arm/p2m.h                |  9 +++++++
>   5 files changed, 58 insertions(+), 30 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2374e92..d5e2539 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -34,7 +34,11 @@ static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
>   
>   #define P2M_ROOT_PAGES    (1<<P2M_ROOT_ORDER)
>   
> -unsigned int __read_mostly p2m_ipa_bits;
> +/*
> + * Set larger than any possible value, so the number of IPA bits can be
> + * restricted by external entity (e.g. IOMMU).
> + */
> +unsigned int __read_mostly p2m_ipa_bits = 64;
>   
>   /* Helpers to lookup the properties of each level */
>   static const paddr_t level_masks[] =
> @@ -1912,6 +1916,16 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
> vaddr_t va,
>       return page;
>   }
>   
> +void __init p2m_restrict_ipa_bits(unsigned int ipa_bits)
> +{
> +    /*
> +     * Calculate the minimum of the maximum IPA bits that any external entity
> +     * can support.
> +     */
> +    if ( ipa_bits < p2m_ipa_bits )
> +        p2m_ipa_bits = ipa_bits;
> +}
> +
>   /* VTCR value to be configured by all CPUs. Set only once by the boot CPU */
>   static uint32_t __read_mostly vtcr;
>   
> @@ -1966,15 +1980,24 @@ void __init setup_virt_paging(void)
>           [7] = { 0 }  /* Invalid */
>       };
>   
> -    unsigned int cpu;
> +    unsigned int i, cpu;
>       unsigned int pa_range = 0x10; /* Larger than any possible value */
>       bool vmid_8_bit = false;
>   
> +    if ( p2m_ipa_bits < 40 )
> +        panic("P2M IPA size must be at least 40-bit (p2m_ipa_bits=%u)\n",
> +              p2m_ipa_bits);
Isn't this check meant to be for Arm32? If so, this path is not called by arm32. 
See the #ifdef CONFIG_ARM_32 above.
Also, I would suggest to reword the message to:

"P2M: Not able to support %⁻bit IPA at the moment.\n"

The rest of the code looks good to me.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel