Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     Philip Yang <philip.yang@amd.com>, David Airlie <airlied@linux.ie>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        amd-gfx list <amd-gfx@lists.freedesktop.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        Alex Deucher <alexander.deucher@amd.com>,
        Colin Ian King <colin.king@canonical.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
References: <20210308153359.2513446-1-arnd@kernel.org>
 <176cae4d-33bc-1d51-a7d7-58eeeea5180e@amd.com>
 <CAK8P3a0nUKSJ2+knM6+REp8HXRFbz5-DmNRAusezkT+XzZQa7Q@mail.gmail.com>
 <92a6c999-c20a-2311-4e31-209a2d9229ed@amd.com>
 <CAK8P3a1SRubvaPzn2020Nc_SsL8xBW7ODGOGtF_4UYASGqTq-w@mail.gmail.com>
From:   Felix Kuehling <felix.kuehling@amd.com>
Message-ID: <f2829f2e-6498-53d4-93fa-1613e988803f@amd.com>
Date:   Mon, 8 Mar 2021 15:02:19 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
In-Reply-To: <CAK8P3a1SRubvaPzn2020Nc_SsL8xBW7ODGOGtF_4UYASGqTq-w@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from [192.168.2.100] (142.117.121.176) by YTBPR01CA0003.CANPRD01.PROD.OUTLOOK.COM (2603:10b6:b01:14::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3912.26 via Frontend Transport; Mon, 8 Mar 2021 20:02:20 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-HT: Tenant
X-MS-Office365-Filtering-Correlation-Id: 268fb447-b07b-4c69-9e88-08d8e26d156d
X-MS-TrafficTypeDiagnostic: MN2PR12MB4389:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <MN2PR12MB4389D4485415AD4FEE93C33B92939@MN2PR12MB4389.namprd12.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:9508;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: KK6O1cIrXmdzgBXOd2WfhGqtfy4uJ/N4t50T3/GissahNIPevE8j5Rh5JXCBipDGUemUEob8+zvZU4zH694AJjr6kNbiLSRHl5aiCct8gJUvIHM3lxWasM/HuYmfVU20DEqbdh1I6W4zOXGeSVOiI6gpvmR7xrnqU819gG+BY8XEsHFRS5WV5OxyS8RBY91Xb8tUJMk1c+JyX17gjBPfVe6GY+OssxZmjKSYi/gh546RdEk+Uc0xmoU4mM7yvFQbN8L8TcOOaC+PouygImXXiEKpuEOLPJen63sBi7TdrtUFZKREtpDfl7cJBW7+NoHsTPw3f0DdQTR/8UFXEmhXoQmOx90ubdyOIoe7Ry3h1kk+KbAzJoHT5iMvA/BjUMkZHn0XgAKHWI1202tBde90SGon6ie9ECjUO3nCLcFOxII8pbtPKiTWMa6Fepo8gTZD+GYpZ3jN19BfEBmI63vlOCLcuz7Y7sP+V3VP+eDMlIa99zPoHoFtee7OnGCgBFydEaEZwZzZW2wwTYFx1t/fa1curS+5okloPPfrSS+2gOj7+wZNiit/qcBxUB4kowc73g31EG6uyXZhJbTHX+S6ZgkqVcTdPuFGnL5yBeaHa9cAaO4HGp1GhGIwbxvXoVdPjHYYiMpEpum2A3hXUSx12A==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BL0PR12MB4948.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(396003)(39860400002)(366004)(376002)(346002)(136003)(186003)(86362001)(26005)(36756003)(16526019)(8676002)(66476007)(2906002)(8936002)(2616005)(54906003)(31696002)(4326008)(66556008)(956004)(6486002)(316002)(478600001)(5660300002)(53546011)(44832011)(31686004)(16576012)(966005)(66946007)(6916009)(83380400001)(52116002)(45980500001)(43740500002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?UEV2YklGWVlIanNWZlJKa3o2RkJxVmNueTJKUi8yNUExSFdCUmpCai9ES2cw?=
 =?utf-8?B?WG1xZTExc0dGVVRlSFZIREZ4VnVNQlRBeUFzeEk5MlVFcjNDdnZQeWY4RUhS?=
 =?utf-8?B?dDgwdm1KcU82UDg0N0VpeHR4UE4xRFZqR09UVUxMQVAzRW9OUi9JQkorSHhu?=
 =?utf-8?B?Mk9vNHNmeUUvdkwwWk9KbVUzcUNHZFpyKzhaUmZLUllnMkdkYjRkUXlJMFFk?=
 =?utf-8?B?bUNjRnp2dUovaWJMSjJXOGRkSjNDcmhhMzA2U2gxN0NZTDgwNjZqN2UrektI?=
 =?utf-8?B?bVhHNEd0R2V6VUkyMVVFS0dJK1RkTHQvcStxSEJkcUh1eFpmaDYzS1ZSbElD?=
 =?utf-8?B?MzI5Q2Q4NW1WYk4wazBtQkJ5NU1yeTAvdk0yYi9kWDJySEJ1ZEMvaTh0azVu?=
 =?utf-8?B?NVk1YTI5eUxvSnNaZGdpOHAxN0ZCTzFxcWErQisrSWNMUXl3NFlyQnI5RGR4?=
 =?utf-8?B?MjNtV3NBKyswZ3ZYZVE3UW1zMCtkbmkvMGVRWmo3cUNLY1c2Q2VLekgwZHFG?=
 =?utf-8?B?bExycldpYU5KS0VLWVpZeUhOeUJLQUxSSzc1ODFSREFRSktWd2pQMkNRSitJ?=
 =?utf-8?B?OXQ1NjFyODB4YW12K0l1WTJsTGcwWkIrdnYyQ3VFbUtvUDNjSmhyUWtWNDgx?=
 =?utf-8?B?MmJWcDF1SW8wVmxMeFB0TkplT0I2NTgvNmJOYm92eWVQOEtvTUZrbzQ1cWZp?=
 =?utf-8?B?VE1rTEdhTWJJcFY3L0Z0Y1RNWFdrVnQvcHJnZ3lPY2VteUMwdExFZTMxVnFy?=
 =?utf-8?B?N1dyaGFNSy92SlMyc0pRM3VFNDdrZU9vQ0ZjOVlUaDdvRmFrK3hsNmVHMmE1?=
 =?utf-8?B?a3o3L3BHQm1GSnhEMGIvbGUrTXZEVFREMUpxaGN4d2RheVVYL2VKS3M4NDc4?=
 =?utf-8?B?VTllbVBNVWpaZEtBSVhQUjMzUFgvZ0Z0N3o3c21vb0dqOGZ5QVhmQWJDMGVy?=
 =?utf-8?B?bjg3TWl6b3htT1AvQ1RXQkcrSE1mOWRTeWVnWExpaHdvNzFxVEt4Uld4eWUy?=
 =?utf-8?B?cEJEU1hualRkYzRxY0VmUThMOW5qODBOb3hrSWlCODFrVEYzbFlCQ1pGclIr?=
 =?utf-8?B?TFBlSjdoY1hvbHowWHZPeU1vNUswbFA3Q0RIejhPTW96WVUwZmU1b2FjV08z?=
 =?utf-8?B?VlFMQ3p4bjRGY0pFL1dFdkJoaHdNanlua05jV2NQeTFhdm5uaXpQWkloaEFi?=
 =?utf-8?B?QlFUSHRSUTZ2M0p3cHJXbVBEL1dXVHJkekRodWRDM3ZJN0hmT3Y0Q3VEOFVB?=
 =?utf-8?B?Ujl4WjBsZGF6c3R6Z2pEeXQrWXFRTjRPOFZydGNENFpENGxVTUVueW9ZNEFN?=
 =?utf-8?B?RUxWWURYVUEzU3V6c2xOU1g1QTR2cXVyYk1sSEdtMjBJL3k4UjZwdHdYWjM5?=
 =?utf-8?B?czE1WlVZZC9QOVFmN29NdmRyb1kxa2FybUJ1VGVrdGk4Q0dhR1RwVnpRckFV?=
 =?utf-8?B?bC91aW90dkhFOWNQMlg0RzYvNnNIRS84VmlRbCtVdGpsbjcvdGZTR3g2aUFQ?=
 =?utf-8?B?MlFNM2xsUExYTCtPaGp6R2dGcjVIa01IUVhvaWJaUElDZ3o5dnFCMjM1MXJo?=
 =?utf-8?B?N3JZK1pnNHVWc2JlcDJ6TnNhWWZ1TG1sdVloMnJuMURmY2IzNlNDMGsvVVl3?=
 =?utf-8?B?U3hOZGlON2Y5VmdGUkl3U3ZXTlFtL2tzNzBxemFSNzhRVDRFbUlNYWlGWEYw?=
 =?utf-8?B?QmdKVG5kblV0SE03NDdMaTlBRU1Ga0FqUGl2bWlDVTZNM0lrcC8wNlVwVGFL?=
 =?utf-8?B?ZFNHU3g0K2NreEhoRXg5a2w0N2gzbUVJYjBpTG1OSnpPNkIxcnhJU0JlZEY3?=
 =?utf-8?B?dlZIYXNDYlErUFZma3NHdz09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 268fb447-b07b-4c69-9e88-08d8e26d156d
X-MS-Exchange-CrossTenant-AuthSource: BL0PR12MB4948.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2021 20:02:21.3774
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /lB8/cDZM8oOLzcYx/fIFcRv0UeA6+OSFhoTA9/JEc3twV3/UR0xrDlHaqRVK/LFslAfINpm0O845V4Yew/USg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4389
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>> have this condition:
>>>>
>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>> endif
>>>>
>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>
>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>> ... function declarations ...
>>>> #else
>>>> ... stubs ...
>>>> #endif
>>> Right, that is the problem I tried to explain in my patch description.
>>>
>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>
>>>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>
>>> I can send a v2 after some testing if you prefer this version.
>> No. My point is, there should not be a hard dependency. The build should
>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>> working for you. It looks like you're building kfd_iommu.o, which should
>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>> your kernel config.
> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
> a loadable module, while AMDGPU is configured as built-in.
I'm sorry, I didn't read it carefully. And I thought "imply" was meant
to fix exactly this kind of issue.

I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
it, because it is only really needed for a small number of AMD APUs, and
even there it is now optional for more recent ones.

Is there a better way to avoid build failures without creating a hard
dependency?  The documentation in
Documentation/kbuild/kconfig-language.rst suggests using if
(IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
calls. I think more generally, we could guard all of kfd_iommu.c with

    #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)

And use the same condition to define the stubs in kfd_iommu.h.

Regards,
  Felix


>
> The causes a link failure for the vmlinux file, because the linker cannot
> resolve addresses of loadable modules at compile time -- they have
> not been loaded yet.
>
>       Arnd
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel