Re: [PATCH] drm/i915: Expose LLC size to user space

[Ben Widawsky <ben@bwidawsk.net>]

On Wed, Jul 10, 2013 at 07:00:53PM +0100, Chris Wilson wrote:
> On Wed, Jul 10, 2013 at 10:46:47AM -0700, Ben Widawsky wrote:
> > On Wed, Jul 10, 2013 at 06:40:02PM +0100, Chris Wilson wrote:
> > > On Wed, Jul 10, 2013 at 09:40:18AM -0700, Ben Widawsky wrote:
> > > > On Wed, Jul 10, 2013 at 09:59:01AM +0100, Chris Wilson wrote:
> > > > > On Tue, Jul 09, 2013 at 07:58:02PM -0700, Ben Widawsky wrote:
> > > > > > The algorithm/information was originally written by Chad, though I
> > > > > > changed the control flow, and I think his original code had a couple of
> > > > > > bugs, though I didn't look very hard before rewriting. That could have
> > > > > > also been different interpretations of the spec.
> > > > > 
> > > > > Just a cpuid query that can already be done more simply from userspace
> > > > > (i.e. with no syscalls)? I was expecting a lot more magic.
> > > > > -Chris
> > > > > 
> > > > > -- 
> > > > > Chris Wilson, Intel Open Source Technology Centre
> > > > 
> > > > Chad wrote this originally for mesa. And yes, it's doable from
> > > > userspace. Chatting with Daniel, we thought maybe other GPU clients
> > > > might want this info, and so a central place to put the code would be
> > > > nice, in case there are quirks or anything like that (I've had a
> > > > particularly hard time figuring out if Xeon really has L3 or not).
> > > > 
> > > > So what's a central place? Libdrm, everybody uses that right? You can
> > > > read /proc/cpuinfo as far as I know, but then you still need to query
> > > > the HAS_LLC getparam to figure out what kind of L3 (or do your own
> > > > chipset ID check).
> > > > 
> > > > In addition to the centrality argument, I noticed while poking around
> > > > cpuid in the kernel that it is a vitalized function. I'm not sure what
> > > > the purpose of this is, but it's something you can't fake in userspace.
> > > 
> > > In fact, isn't this functionality already part of arch/x86, and isn't the
> > > value already exposed via /proc/cpuinfo?
> > > -Chris
> > > 
> > > -- 
> > > Chris Wilson, Intel Open Source Technology Centre
> > >
> > Yes to the /proc/cpuinfo bit (I said that). If the implication is just
> > to use that code, I did a quick search and couldn't find it. I can go
> > back and check again. Assuming it's externally exposed to drivers, I
> > would rather use that.
> > 
> > We still need the HAS_LLC check to differentiate the L3 though.
> 
> You have to be careful about repurposing PARAM_HAS_LLC though. At the
> moment it has a dual meaning that the kernel/gpu supports LLC and that
> we default to placing objects in LLC.
> 
> If you must expose cache_size through a param, make it a new one and
> make it explicit. I would also rather just use a param than parse
> /proc/cpuinfo (especially as some paranoid setups prevent access to
> /proc/cpuinfo).

I did a search, and I think my change is backward compatible, but if you
want a new param, I don't care.

> 
> But we really should not be calling cpuid from our driver, that just
> looks very very ugly, and given cpuid's history will be a maintenance
> burden.
> -Chris

Okay, let me try again to find where it is parsed by the arch/x86 code.

> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ben Widawsky, Intel Open Source Technology Center