On 22/03/2022 15:18, Tvrtko Ursulin wrote:
On 22/03/2022 14:49, 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.
Sounds like we had agreement on what tweaks to make and I conceded to
live for now with the IMO wrongly named intel_vtd_run_as_guest.
(I mean I really disagree with file name being trumps, which I think
this example illustrates - this is i915 asking whether the kernel is
running as guest so intel_vtd_ prefix is just wrong. Intel VT-d is the
iommu thingy so it makes no sense when called from PCH detection. But I
have no better ideas at the moment. We can call it i915_run_as_guest, to
signify function belongs to i915, but then we lose the first parameter
names the function rule.)
But in any case I don't see that I created any blockers in this thread.
AFAICS just a respin with intel_vtd_active taking struct device is
needed and job done.
Sorry now I see I also suggested moving intel_scanout_needs_vtd_wa,
intel_ggtt_update_needs_vtd_wa and intel_vm_no_concurrent_access_wa all
to their respective files. Which I think is also correct. They are all
higher components which are asking intel_vtd a question and basing a
decision upon the answer. I don't think intel_vtd.h should contain
knowledge about a mix of other driver components.
Regards,
Tvrtko
