[Intel-gfx] [RFC PATCH v3 1/1] i915/drm: Split out x86/arm64 for run_as_guest

[Lucas De Marchi]

On Tue, Mar 22, 2022 at 04:49:53PM +0200, Jani Nikula wrote:
>On Tue, 22 Mar 2022, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> On Tue, Mar 22, 2022 at 12:21:59PM +0200, Jani Nikula wrote:
>>>On Mon, 21 Mar 2022, Lucas De Marchi <lucas.demarchi at intel.com> 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