| Message ID | 1494424994-26232-6-git-send-email-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Oleksandr, On 10/05/17 15:03, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > 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@epam.com> > --- > 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; } > return 0; > } > > diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h > index 57d9b1e..10a6f23 100644 > --- a/xen/include/asm-arm/iommu.h > +++ b/xen/include/asm-arm/iommu.h > @@ -20,8 +20,11 @@ struct arch_iommu > void *priv; > }; > > -/* Always share P2M Table between the CPU and the IOMMU */ > -#define iommu_use_hap_pt(d) (1) > +/* > + * The ARM domain always has a P2M table, but not every integrated into > + * ARM SoCs IOMMU can use it as page table. The first part: "ARM domain has a P2M table" is pretty obvious. I would instead say: "Not every ARM SoCs IOMMU use the same page-table format as the processor.". > + */ > +#define iommu_use_hap_pt(d) (iommu_hap_pt_share) > > const struct iommu_ops *iommu_get_ops(void); > void __init iommu_set_ops(const struct iommu_ops *ops); > Cheers,
On Thu, May 11, 2017 at 2:28 PM, Julien Grall <julien.grall@arm.com> wrote: > Hi Oleksandr, Hi Julien > > On 10/05/17 15:03, Oleksandr Tyshchenko wrote: >> >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> 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@epam.com> >> --- >> 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; > >> return 0; >> } >> >> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h >> index 57d9b1e..10a6f23 100644 >> --- a/xen/include/asm-arm/iommu.h >> +++ b/xen/include/asm-arm/iommu.h >> @@ -20,8 +20,11 @@ struct arch_iommu >> void *priv; >> }; >> >> -/* Always share P2M Table between the CPU and the IOMMU */ >> -#define iommu_use_hap_pt(d) (1) >> +/* >> + * The ARM domain always has a P2M table, but not every integrated into >> + * ARM SoCs IOMMU can use it as page table. > > > The first part: "ARM domain has a P2M table" is pretty obvious. I would > instead say: "Not every ARM SoCs IOMMU use the same page-table format as the > processor.". Agree. > >> + */ >> +#define iommu_use_hap_pt(d) (iommu_hap_pt_share) >> >> const struct iommu_ops *iommu_get_ops(void); >> void __init iommu_set_ops(const struct iommu_ops *ops); >> > > Cheers, > > -- > Julien Grall
On 11/05/17 15:38, Oleksandr Tyshchenko wrote: > On Thu, May 11, 2017 at 2:28 PM, Julien Grall <julien.grall@arm.com> wrote: >> Hi Oleksandr, > Hi Julien Hi Oleksandr, >> >> On 10/05/17 15:03, Oleksandr Tyshchenko wrote: >>> >>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >>> >>> 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@epam.com> >>> --- >>> 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,
On Thu, May 11, 2017 at 8:58 PM, Julien Grall <julien.grall@arm.com> wrote: > > > On 11/05/17 15:38, Oleksandr Tyshchenko wrote: >> >> On Thu, May 11, 2017 at 2:28 PM, Julien Grall <julien.grall@arm.com> >> wrote: >>> >>> Hi Oleksandr, >> >> Hi Julien > > > Hi Oleksandr, > > >>> >>> On 10/05/17 15:03, Oleksandr Tyshchenko wrote: >>>> >>>> >>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >>>> >>>> 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@epam.com> >>>> --- >>>> 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). I will print a message too. > > Cheers, > > -- > Julien Grall
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; + return 0; } diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h index 57d9b1e..10a6f23 100644 --- a/xen/include/asm-arm/iommu.h +++ b/xen/include/asm-arm/iommu.h @@ -20,8 +20,11 @@ struct arch_iommu void *priv; }; -/* Always share P2M Table between the CPU and the IOMMU */ -#define iommu_use_hap_pt(d) (1) +/* + * The ARM domain always has a P2M table, but not every integrated into + * ARM SoCs IOMMU can use it as page table. + */ +#define iommu_use_hap_pt(d) (iommu_hap_pt_share) const struct iommu_ops *iommu_get_ops(void); void __init iommu_set_ops(const struct iommu_ops *ops);