Thanks Jerome for the the correct HMM config option, only select HMM_MIRROR is not good enough because CONFIG_HMM option maybe missing, add depends on ARCH_HAS_HMM will solve the issue. I will submit new patch to fix the compilation error if HMM_MIRROR config is missing and the HMM config dependency issue. Philip On 2019-02-20 7:25 p.m., Jerome Glisse wrote: > On Thu, Feb 21, 2019 at 12:17:33AM +0000, Kuehling, Felix wrote: >> On 2019-02-20 6:34 p.m., Jerome Glisse wrote: >>> On Wed, Feb 20, 2019 at 10:39:49PM +0000, Kuehling, Felix wrote: >>>> On 2019-02-20 5:12 p.m., Jerome Glisse wrote: >>>>> On Wed, Feb 20, 2019 at 07:18:17PM +0000, Kuehling, Felix wrote: >>>>>> [+Jerome] >>>>>> >>>>>> Why to we need ZONE_DEVICE. I didn't think this was needed for mirroring >>>>>> CPU page tables to device page tables. >>>>>> >>>>>> ARCH_HAS_HMM depends on (X86_64 || PPC64). Do we have some alternative >>>>>> for ARM support? >>>>>> >>>>>> Also, the name ARCH_HAS_HMM looks like it's meant to be selected by the >>>>>> CPU architecture rather than any driver. Jerome, do you have any advice? >>>>> This patch is wrong you need to depend on ARCH_HAS_HMM and >>>> Who selects ARCH_HAS_HMM? Currently I don't see this selected anywhere. >>>> So any config option that depends on it will be invisible in menuconfig. >>>> Do we need ARCH_HAS_HMM somewhere in the arch/x86/Kconfig and >>>> arch/powerpc/Kconfig? >>>> >>>> Also, ARCH_HAS_HMM does not currently support ARM. Does that mean we >>>> can't have ARM support in AMDGPU if we start using HMM? >>> ARCH_HAS_HMM is defined by architecture that support HMM. So par x86 >>> and PPC. It should not be hard to add it to ARM (i can not remember if >>> ARM has DAX yet or not, if ARM does not have DAX then you need to add >>> that first). >> >> Not having ARM support is a bummer. I just enabled KFD on ARM a few >> weeks ago. Now depending on HMM makes KFD unusable on ARM. [+Mark FYI] I >> hope this is only a temporary setback. > > It should not be hard to add in fact all it might need is a Kconfig > patch. I have no easy access to ARM with PCIE so i have not tackle > this yet. > >> >> >>>> Finally, ARCH_HAS_HMM has a bunch of dependencies. If they are not met, >>>> I guess it can't be enabled. Should those be "select"s instead? >>> No they should not be selected, people configuring their system need >>> to have the freedom of doing so. All those option are selected in all >>> the big distribution. >> As far as I can tell, the arch/x86/Kconfig doesn't select ARCH_HAS_HMM. >> Its default is "y", so it should be enabled on anything that meets the >> dependencies. But ZONE_DEVICE was not enabled by default. I think that's >> what broke our kernel configs. >> >> We'll fix our own kernel configs to enable ZONE_DEVICE and ARCH_HAS_HMM >> to get our internal builds to work again. > > You seem to be doing weird thing with your kconfig ... > >> >> I suspect other users with their own kernel configs will stumble over >> this and wonder why KFD and userptr support are disabled in their builds. > > Patch to improve kconfig are welcome but they should not force select > thing. Configuration is there to give user freedom to select fewature > they want to give up. > > Maybe following would help: > ARCH_HAS_HMM > - bool > - default y > + def_bool y > > Cheers, > Jérôme > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
