Re: [Xen-devel] [PATCH v7 5/6] iommu: tidy up iommu_use_hap_pt() and need_iommu_pt_sync() macros

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 30 August 2019 15:08
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Julien Grall <julien.grall@xxxxxxx>; 
> Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>; 
> Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian 
> Jackson
> <Ian.Jackson@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Konrad 
> Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Wei Liu <wl@xxxxxxx>
> 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 <jbeulich@xxxxxxxx>
> 

Thanks.

  Paul

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