Re: [PATCH v6 1/8] iommu: provide early initialisation hook for IOMMU drivers

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3933 bytes --]

On Fri, Dec 05, 2014 at 01:21:31PM +0000, Grant Likely wrote:
> On Fri, Dec 5, 2014 at 1:18 PM, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Fri, Dec 05, 2014 at 01:06:52PM +0000, Grant Likely wrote:
> >> On Fri, Dec 5, 2014 at 12:35 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> >> > Hi Will,
> >> >
> >> > On 05/12/14 12:10, Will Deacon wrote:
> >> > [...]
> >> >>>>
> >> >>>> Do you expect drivers to modify that *priv pointer after the ops
> >> >>>> structure is registered? I'd be very surprised if that was the use
> >> >>>> case. It's fine for the driver to register a non-const version, but
> >> >>>> once it is registered, the infrastructure can treat it as const from
> >> >>>> then on.
> >> >>>
> >> >>>
> >> >>> Possibly not - certainly my current port of the ARM SMMU which makes use
> >> >>> of *priv is only ever reading it - although we did also wave around
> >> >>> reasons for mutable ops like dynamically changing the pgsize_bitmap and
> >> >>> possibly even swizzling individual ops for runtime reconfiguration. On
> >> >>> consideration though, I'd agree that things like that are mad enough to
> >> >>> stay well within individual drivers if they did ever happen, and
> >> >>> certainly shouldn't apply to this bit of the infrastructure at any rate.
> >> >>
> >> >>
> >> >> I certainly need to update the pgsize_bitmap at runtime because I don't
> >> >> know the supported page sizes until I've both (a) probed the hardware
> >> >> and (b) allocated page tables for a domain. We've already discussed
> >> >> moving the pgsize_bitmap out of the ops, but moving it somewhere where
> >> >> it remains const doesn't really help.
> >> >
> >> >
> >> > We can safely cast the call to get_ops in the SMMU driver though, since
> >> > we'll know that we put a mutable per-instance ops in there in the first
> >> > place. At least that way drivers that aren't taking advantage and just pass
> >> > their static const ops around shouldn't provoke warnings. I deliberately
> >> > didn't touch anything beyond get_ops as that would be too disruptive.
> >> >
> >> >> Can I just take the patch that Grant acked, in the interest of getting
> >> >> something merged? As you say, there's plenty of planned changes in this
> >> >> area anyway. I plan to send Olof a pull request this afternoon.
> >> >
> >> >
> >> > Grant, Thierry? Personally I'm not fussed either way - the sooner something
> >> > goes in, the sooner I can carry on working at replacing it :D
> >>
> >> I've already acked it. Why are we still talking about it?  :-D
> >
> > Am I missing something? Why is there a need to rush things? Are there
> > actually drivers that depend on this that will be merged during the 3.19
> > merge window? It seems like that'd be cutting it really close given
> > where we are in the release cycle.
> >
> > If that's not the case, why even bother getting this hack into 3.19 if
> > nobody uses it and we're going to change it in 3.20 anyway?
> 
> I also acked the non-hack version, the patch that doesn't try to make
> everything const. I assumed that was the one that we are talking about
> merging.

Actually not making everything const would be a hack. Drivers already
mark their struct iommu_ops as const.

But I'm more referring to the series as a whole. It seems like there are
various issues that still need to be ironed out, and there's committment
to do that before 3.20, so unless there are drivers that need any of the
unfinished patches for 3.19 I don't see why we should be merging them in
the first place.

If getting them into 3.19 is merely to resolve dependencies then it's
not going to work well anyway. Since this is all going to change in 3.20
anyway we'd likely have new dependencies that need to be handled, so
might just as well do it properly at that time.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]