On Tue, Mar 22, 2022 at 04:49:53PM +0200, Jani Nikula wrote:
On Tue, 22 Mar 2022, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
On Tue, Mar 22, 2022 at 12:21:59PM +0200, Jani Nikula wrote:
On Mon, 21 Mar 2022, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
On Mon, Mar 21, 2022 at 04:34:49PM -0700, Casey Bowman wrote:
Wanted to ping this older thread to find out where we stand with this patch,
Are we OK with the current state of these changes?
With more recent information gathered from feedback on other patches, would
we prefer changing this to a more arch-neutral control flow?
e.g.
#if IS_ENABLED(CONFIG_X86)
...
#else
...
#endif
Would we also prefer this RFC series be merged or would it be preferred to
create a new series instead?
for this specific function, that is used in only 2 places I think it's
ok to do:
static inline bool run_as_guest(void)
{
#if IS_ENABLED(CONFIG_X86)
return !hypervisor_is_type(X86_HYPER_NATIVE);
#else
/* Not supported yet */
return false;
#endif
}
For PCH it doesn't really matter as we don't execute that function
for discrete. For intel_vtd_active() I figure anything other than
x86 would be fine with false here.
Jani, that this look good to you?
It's more important to me to get this out of i915_drv.h, which is not
supposed to be a collection of random stuff anymore. I've sent patches
to this effect but they've stalled a bit.
do you have a patch moving this particular one? got a link?
Yeah, but it was basically shot down by Tvrtko [1], and I stalled there.
I'd just like to get all this cruft out of i915_drv.h. Whenever we have
a file where the name isn't super specific, we seem to have a tendency
of turning it into a dumping ground for random crap. So I'd really like
to move this out of there *before* expanding on it.
ok, I took a look there and it seems there is still some discussion on
where to move it. The place you moved it to is not ideal as
`run_as_guest()` has nothing to do with vt-d, even if it's used by one
of those functions. It's also used by the PCH detection/fallback code.
So, I think this is very much orthogonal to moving it and we are not
really extending: this just doesn't make much sense for other archs.
So my proposal is that we continue with this as is and move it when
we have a rough agreement on where to move it to.
As much as I hate the i915_utils.[ch] and I've been trying to reduce it,
I can't think of a best place. Other than of course come up with an
arch-neutral API to add to the kernel
From some quick grep, include/linux/hypervisor.h might be a good place.
But again, I think it should be orthogonal to what this is doing.
thanks
Lucas De Marchi
BR,
Jani.
[1] https://patchwork.freedesktop.org/series/99852/
--
Jani Nikula, Intel Open Source Graphics Center
