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

[Oleksandr <olekstysh@xxxxxxxxx>]

On 24.09.19 12:36, Julien Grall wrote:
> Hi,

Hi, Julien


> 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.
I will move under #ifdef CONFIG_ARM_32


> Also, I would suggest to reword the message to:
>
> "P2M: Not able to support %⁻bit IPA at the moment.\n"
ok


> The rest of the code looks good to me.

Shall the non-RFC patch be split into "adding possibility to restrict" 
and "restrict by the IOMMU drivers"?

--
Regards,

Oleksandr Tyshchenko


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