Re: [Xen-devel] [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 10 March 2020 11:02
> To: paul@xxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Kevin Tian' <kevin.tian@xxxxxxxxx>; 
> 'Stefano Stabellini'
> <sstabellini@xxxxxxxxxx>; 'Julien Grall' <julien@xxxxxxx>; 'Wei Liu' 
> <wl@xxxxxxx>; 'Konrad Wilk'
> <konrad.wilk@xxxxxxxxxx>; 'George Dunlap' <George.Dunlap@xxxxxxxxxxxxx>; 
> 'Andrew Cooper'
> <andrew.cooper3@xxxxxxxxxx>; 'Ian Jackson' <ian.jackson@xxxxxxxxxx>
> Subject: Re: [PATCH v2 2/5] IOMMU: iommu_intpost is x86/HVM-only
> 
> On 10.03.2020 11:54, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: 09 March 2020 10:43
> >>
> >> @@ -486,8 +480,10 @@ int __init iommu_setup(void)
> >>          panic("Couldn't enable %s and iommu=required/force\n",
> >>                !iommu_enabled ? "IOMMU" : "Interrupt Remapping");
> >>
> >> +#ifndef iommu_intpost
> >>      if ( !iommu_intremap )
> >>          iommu_intpost = 0;
> >
> > Nit: 0 -> false
> 
> Hmm, I'm not touching this line, and the goal of the patch isn't
> to (also) switch _all_ assignments to the variable.

Yes, but it is in context and you normally ask for fix-ups where they are in 
context. In this case it’s a pretty trivial addition to the patch.

> There is at
> least one more (in vmcs.c), and doing the adjustment here (as
> being not otherwise motivated, e.g. because of touching the line
> anyway) would then, for consistency, seem to call for correcting
> that other instance too.

No, I'm not suggesting a wholesale conversion (although I'm not against it)... 
just tidying as we go.

> This, however, would seem too unrelated
> a change to make here for my taste. Hence ...
> 
> > With that fixed...
> >
> > Reviewed-by: Paul Durrant <paul@xxxxxxx>
> 
> ... please clarify whether I may leave the line untouched.

Since it's in context I'd prefer it fixed, but I'm not going to insist so you 
can keep the R-b.

  Paul

> 
> Jan


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