> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 30 August 2019 15:08
> To: Paul Durrant <paul.durr...@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Julien Grall <julien.gr...@arm.com>;
> Andrew Cooper
> <andrew.coop...@citrix.com>; Roger Pau Monne <roger....@citrix.com>;
> Volodymyr Babchuk
> <volodymyr_babc...@epam.com>; George Dunlap <george.dun...@citrix.com>; Ian
> Jackson
> <ian.jack...@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>; Konrad
> Rzeszutek Wilk
> <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org>; Wei Liu <w...@xen.org>
> Subject: Re: [PATCH v7 5/6] iommu: tidy up iommu_use_hap_pt() and
> need_iommu_pt_sync() macros
>
> On 30.08.2019 10:29, Paul Durrant wrote:
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -49,7 +49,11 @@ int8_t __hwdom_initdata iommu_hwdom_reserved = -1;
> > * default until we find a good solution to resolve it.
> > */
> > bool_t __read_mostly iommu_intpost;
> > -bool_t __read_mostly iommu_hap_pt_share = 1;
> > +
> > +#ifndef CONFIG_ARM
> > +bool __read_mostly iommu_hap_pt_share = true;
> > +#endif
>
> The #idef here should be in line with ...
>
> > @@ -102,8 +106,10 @@ static int __init parse_iommu_param(const char *s)
> > iommu_hwdom_passthrough = val;
> > else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
> > iommu_hwdom_strict = val;
> > +#ifndef iommu_hap_pt_share
> > else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
> > iommu_hap_pt_share = val;
> > +#endif
>
> ... the one here, i.e. neither should be Arm-specific. What a specific
> architecture wants should be controlled in a single place (in a header).
Ah, ok. I obviously misunderstood what you meant.
>
> > @@ -268,6 +274,17 @@ struct domain_iommu {
> > #define iommu_set_feature(d, f) set_bit(f, dom_iommu(d)->features)
> > #define iommu_clear_feature(d, f) clear_bit(f, dom_iommu(d)->features)
> >
> > +/* Are we using the domain P2M table as its IOMMU pagetable? */
> > +#define iommu_use_hap_pt(d) \
> > + (hap_enabled(d) && is_iommu_enabled(d) && iommu_hap_pt_share)
> > +
> > +/* Does the IOMMU pagetable need to be kept synchronized with the P2M */
> > +#ifdef CONFIG_HAS_PASSTHROUGH
> > +#define need_iommu_pt_sync(d) (dom_iommu(d)->need_sync)
> > +#else
> > +#define need_iommu_pt_sync(d) ({ (void)d; false; })
>
> "d" wants to be parenthesized.
>
Ok.
> With these taken care of (possibly while committing)
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
>
Thanks.
Paul
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
