Re: [Xen-devel] [PATCH v1 05/10] iommu/arm: Re-define iommu_use_hap_pt(d) as iommu_hap_pt_share

[Julien Grall <julien.grall@xxxxxxx>]

On 11/05/17 15:38, Oleksandr Tyshchenko wrote:
> On Thu, May 11, 2017 at 2:28 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > Hi Oleksandr,
> Hi Julien
Hi Oleksandr,

> > On 10/05/17 15:03, Oleksandr Tyshchenko wrote:
> > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > >
> > > Not every integrated into ARM SoCs IOMMU can share page tables
> > > with the CPU and as result the iommu_use_hap_pt(d) is not always true.
> > > Reuse x86's iommu_hap_pt_share flag to indicate whether the IOMMU
> > > page table is shared or not.
> > >
> > > Now all IOMMU drivers on ARM are able to change this flag
> > > according to their possibilities like x86-variants do.
> > > Therefore set iommu_hap_pt_share flag for SMMU because it always shares
> > > page table with the CPU.
> > >
> > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > ---
> > >  xen/drivers/passthrough/arm/smmu.c | 3 +++
> > >  xen/include/asm-arm/iommu.h        | 7 +++++--
> > >  2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/xen/drivers/passthrough/arm/smmu.c
> > > b/xen/drivers/passthrough/arm/smmu.c
> > > index 527a592..86ee12a 100644
> > > --- a/xen/drivers/passthrough/arm/smmu.c
> > > +++ b/xen/drivers/passthrough/arm/smmu.c
> > > @@ -2870,6 +2870,9 @@ static __init int arm_smmu_dt_init(struct
> > > dt_device_node *dev,
> > >
> > >         platform_features &= smmu->features;
> > >
> > > +       /* Always share P2M table between the CPU and the SMMU */
> > > +       iommu_hap_pt_share = true;
> > > +
> >
> > I would prefer to bail-out if someone try to unshare the page-table rather
> > than overriding. This would help us to know if someone are try to do that.
> >
> > So I would do:
> >
> > if ( !iommu_hap_pt_share )
> > {
> >         printk(....)
> >         return -EINVAL;
> > }
> I got it for SMMU.
>
> But, for IPMMU we will override since iommu_hap_pt_share is true by
> default. Right?
>
> /*
> * The IPMMU can't reuse P2M table since it only supports
> * stage-1 page tables.
> */
> iommu_hap_pt_share = false;
Good point. Looking at the other driver, they print a message to notify 
the override. (See drivers/passthrough/amd/iommu_init.c for instance).
Cheers,

--
Julien Grall

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