On Fri, 2015-08-07 at 16:26 -0700, Luis R. Rodriguez wrote: > On Fri, Aug 7, 2015 at 4:08 PM, Toshi Kani <toshi.kani@xxxxxx> wrote: > > On Fri, 2015-08-07 at 15:23 -0700, Luis R. Rodriguez wrote: > > > On Fri, Aug 7, 2015 at 2:56 PM, Toshi Kani <toshi.kani@xxxxxx> wrote: > > > > On Fri, 2015-08-07 at 13:25 -0700, Luis R. Rodriguez wrote: > > > > > On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@xxxxxx> > > > > > wrote: > > > > > > On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote: > > > > > > > On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@xxxxxx > > > > > > > > > > > > > > > wrote: : > > > > > > > > > > Its a bit more than that though. Since you agree that the OS can > > > > > live without MTRR code I was hoping to then see if we can fold out > > > > > PAT Linux code from under the MTRR dependency on Linux and make > > > > > PAT a first class citizen, maybe at least for x86-64. Right now > > > > > you can only get PAT support on Linux if you have MTRR code, but > > > > > I'd like to see if instead we can rip MTRR code out completely > > > > > under its own Kconfig and let it start rotting away. > > > > > > > > > > Code-wise the only issue I saw was that PAT code also relies on > > > > > mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I > > > > > found no other obvious issues. > > > > > > > > We can rip of the MTTR code that modifies the MTRR setup, but not > > > > mtrr_type_lookup(). This function provides necessary checks per > > > > documented > > > > in commit 7f0431e3dc89 as follows. > > > > > > > > 1) reserve_memtype() tracks an effective memory type in case > > > > a request type is WB (ex. /dev/mem blindly uses WB). Missing > > > > to track with its effective type causes a subsequent request > > > > to map the same range with the effective type to fail. > > > > > > > > 2) pud_set_huge() and pmd_set_huge() check if a requested range > > > > has any overlap with MTRRs. Missing to detect an overlap may > > > > cause a performance penalty or undefined behavior. > > > > > > > > mtrr_type_lookup() is still admittedly awkward, but I do not think > > > > we > > > > have an immediate issue in PAT code calling it. I do not think it > > > > makes > > > > PAT code a second class citizen. > > > > > > OK since we know that if MTRR set up code ends up disabled and would > > > return MTRR_TYPE_INVALID what if we just static inline this for the > > > no-MTRR Kconfig build option immediately, and only then have the full > > > blown implementation for the case where MTRR Kconfig option is > > > enabled? > > > > Yes, the MTRR code could be disabled by Kconfig with such inline stubs > > OK thanks. > > > as > > long as the kernel is built specifically for a particular platform with > > MTRR disabled, such as Xen guest kernel. > > Sure. > > > However, since MTRR is a CPU feature enabled on most of the systems, I > > am not sure if it makes sense to be configurable with Kconfig, though. > > To me this is about making PAT a first class citizen in code though > and validating through Kconfig the option then to opt-out of MTRR from > OS code. Perhaps we can recommend to enable it but having the options > to split out PAT from MTRR is what I was aiming for. Since we have CONFIG_MTRR already, we do not need to argue over this option. :-) It makes sense since when MTRR code was introduced, there were CPUs without this capability... > > > > > Platform firmware and SMIs seems to be the only other possible > > > > > issue. More on this below. > > > > > > > > > > > > For those type of OSes... > > > > > > > could it be possible to negotiate or hint to the platform > > > > > > > through an attribute somehow that the OS has such capability > > > > > > > to not use MTRR? > > > > > > > > > > > > The OS can disable MTRR. However, this can also cause a problem > > > > > > in firmware, which may rely on MTRR. > > > > > > > > > > Can you describe what type of issues we could expect ? I tend to > > > > > care more about this for 64-bit systems so if 32-bit platforms > > > > > would be more of the ones which could cause an issue would > > > > > restricting disabling MTRR only for 64-bit help? > > > > > > > > The SMI handler runs in real-mode and relies on MTRR being effective > > > > to provide right cache types. It does not matter if it is 64-bit or > > > > not. > > > > > > I see... since I have no visibility to what goes under the hood, can > > > you provide one example use case where an SMI handler would require > > > getting a cache type through MTRR ? I realize this can vary, vendor by > > > vendor, but any example would do just to satisfy my curiosity. > > > > For fan control, it would need UC access to its registers. > > OK thanks! To follow up with the example, since the platform firmware > would have set up the MTRRs anyway, the SMI should still work, even if > the OS didn't do anything, right? Yes, MTRR works without the OS code. However, mtrr_type_lookup() is necessary to make sure that OS mapping requests are aligned with with the MTRR setup. > > > > > > Is there any issue for Linux to use MTRR set by firmware? > > > > > > > > > > Even though we don't have the Kconfig option right now to disable > > > > > MTRR cod explicitly I'll note that there are a few other cases > > > > > that could flip Linux to note use MTRR: > > > > > > > > > > a) Some BIOSes could let MTRR get disabled > > > > > b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which > > > > > disables MTRR on Linux > > > > > > > > > > If these environments can exist it'd be good to understand > > > > > possible issues that could creep up as a result of the OS not > > > > > having MTRR enabled. If this is a reasonable thing for x86-64 I > > > > > was hoping we could just let users opt-in to a similar build > > > > > configuration through the OS by letting PAT not depend on MTRR. > > > > > > > > Case a) and b) do not cause any issue. They simply lead > > > > mtrr_type_lookup() to return immediately with MTRR_TYPE_INVALID > > > > (i.e. MTRR disable), and the callers handle this value properly. > > > > These cases are only problematic when the OS tries to modify MTRR. > > > > > > OK if the OS returns MTRR_TYPE_INVALID, for folks who do not want MTRR > > > code on their kernel, we should be OK? > > > > Technically OK. Not sure if we want such a Kconfig option, though. > > Its more of me wanting to get PAT out from under MTRR. Does that make > sense? It makes sense if you need to make the kernel size a bit smaller, and you build kernels specific to Xen guests. Leaving the MTRR code enabled on Xen guests does not cause you any issue, though. Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
