Re: [Xen-devel] [PATCH v3 6/7] xen/dom0: Drop iommu_hwdom_inclusive entirely

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jan 17, 2019 at 06:32:01AM -0700, Jan Beulich wrote:
> >>> On 16.01.19 at 10:00, <andrew.cooper3@xxxxxxxxxx> wrote:
> > This option is unique to x86 PV dom0's, but it is not sensible to have a
> > catch-all which blindly maps all non-RAM regions into the IOMMU.
> > 
> > The map-reserved option remains, and covers all the buggy firmware issues 
> > that
> > I am aware of.
> 
> The final part of this sentence is the main culprit, resulting in me
> being uncertain about this move. I'm not outright opposed, since
> I agree with the "is not sensible" part above. But there surely
> was a reason why the option was introduced. May I suggest as
> an alternative to take an intermediate step for at least one
> release cycle, and attach a warning_add() to any use of this
> option?
> 
> > --- a/xen/drivers/passthrough/x86/iommu.c
> > +++ b/xen/drivers/passthrough/x86/iommu.c
> > @@ -172,10 +172,10 @@ static bool __hwdom_init hwdom_iommu_map(const struct 
> > domain *d,
> >      default:
> >          if ( type & RAM_TYPE_RESERVED )
> >          {
> > -            if ( !iommu_hwdom_inclusive && !iommu_hwdom_reserved )
> > +            if ( !iommu_hwdom_reserved )
> >                  return false;
> >          }
> > -        else if ( is_hvm_domain(d) || !iommu_hwdom_inclusive || pfn > 
> > max_pfn )
> > +        else if ( is_hvm_domain(d) || pfn > max_pfn )
> >              return false;
> >      }
> 
> Removal of this option could be imagined taking an intermediate
> step where the variable resolves to constant false. This suggests
> to me that the first of these two changes is correct, but the
> second is not. Instead the "else if()" ought to collapse to just
> "else", which in turn would suggest to consider folding if()/else
> into a single if().

I've also noted that the whole switch can be simplified in a reply to
a previous version:

https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg00217.html

Thanks, Roger.

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