Message-ID: <5e415baf-94d9-2723-3770-9a35f9fa6278@amd.com>
Date:   Mon, 30 Jan 2023 13:58:32 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH drm-next 05/14] drm/nouveau: new VM_BIND uapi interfaces
Content-Language: en-US
To:     David Airlie <airlied@redhat.com>
Cc:     Danilo Krummrich <dakr@redhat.com>,
        Matthew Brost <matthew.brost@intel.com>, daniel@ffwll.ch,
        bskeggs@redhat.com, jason@jlekstrand.net, tzimmermann@suse.de,
        mripard@kernel.org, corbet@lwn.net, nouveau@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-doc@vger.kernel.org
References: <20230118061256.2689-1-dakr@redhat.com>
 <20230118061256.2689-6-dakr@redhat.com>
 <Y9MjSeMcsd18r9vM@DUT025-TGLU.fm.intel.com>
 <7c046ff9-728d-7634-9d77-8536308c7481@redhat.com>
 <c2256c7d-e768-ae3f-d465-b9f8080d111b@amd.com>
 <2427a918-5348-d1ef-ccae-a29c1ff33c83@redhat.com>
 <a214b28b-043c-a8bb-69da-b4d8216fce56@amd.com>
 <3a76bfa9-8ee5-a7d9-b9fb-a98181baec0b@redhat.com>
 <49ac3f95-6eda-9009-4b28-0167213301b2@amd.com>
 <CAMwc25oGap_hEg5AquM_L2eH42PcG=pM3KPS6HNTNj-SzeHu+g@mail.gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <CAMwc25oGap_hEg5AquM_L2eH42PcG=pM3KPS6HNTNj-SzeHu+g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eWNGZFFVbE5MUmsrc1lPWVlZaEl3dVVoWmRNamdDL3Z6KzdLVWJXV3h4QXEv?=
 =?utf-8?B?ZGpGMWRtNEMvMHAxL0p4TzJFQ21Db1BpTWJRajJ5QUNNMkZGRU5ibGlhQ3I2?=
 =?utf-8?B?SFNkdGk3R3VFZld5SHROSk8zQyt0ZHhVbjdkdGVIRzdDZ1BQbFNCSDRCbnEy?=
 =?utf-8?B?NXVCNFRGTDJreVpjQWl5TW85bUE0REpYdWJ5OG5qOGljNlpVK0RxUFVuckRJ?=
 =?utf-8?B?TG9OaGphWmFqSkt5Q2RYSXo3Sy9qbCtRRml6Q1lqZzdaRDNtVEtXNTlNckYy?=
 =?utf-8?B?ay9tN3c4TEtuYVlzcXpNRURTS29JQmMyZlNhQy9yMUkxTGdJRzUrSEpmVmJ4?=
 =?utf-8?B?b1ovTmhIdzhzNVBlVWFySWhGM0FENnluOXMrVUxRYUoyN3JaaXN2ampmbENQ?=
 =?utf-8?B?U1ZiRUtMd1duNWpodVF4K3lQdVNvbzc0UldYV3hrVDBSZjdmYldLRW8vejlk?=
 =?utf-8?B?VzhhRlVtcks0SlRmYmdpQXVSdmUyUDdPd2g0aldINHZpclByTWVkcFlUd2FY?=
 =?utf-8?B?ZmZZQjd3K3BZdmZ4YnFRR1Ftb2tOcTdtLzh5TEt5S0EzZHRUUzdaSEJFdnVF?=
 =?utf-8?B?OWs0UTlibDBYSFhTQW8rajlDekp1NnV1ZG1rU3Q4d0FEYUJzU3RneHE1emVs?=
 =?utf-8?B?VUZSRVd6djNxdDZiQWVFOHB1MC9sMStzQ2xSNWFVYVlxOU1GWVRtSHZNaFhF?=
 =?utf-8?B?SGI3TXpoczAycGFoYXNXdmUycktUcWN4c3FhdGJkQ0Zib0UwM0FGaU9BWnps?=
 =?utf-8?B?V204bmJYWUJsZHpGOGhjcGdNUzZsNEhlRWJxTEN4WStweG5YaCtCQ0N6SjVV?=
 =?utf-8?B?QkFhZS9hVDRweEJvS3h2OFBsSnhFRTd3OG5WUG5VYncxZjYva0tTeUxHenI4?=
 =?utf-8?B?MTZkczlzbm56UTU1TUwwb0pDajdPVGJDbzFPQkZKOEM1azVKTWZFOWFOekdR?=
 =?utf-8?B?MlREWUVHcEZESmRBVmZoZ0krTzFsMFk4anV3YU9rbWJ1K1AvZzdMbTZDWDZm?=
 =?utf-8?B?ZjZqQWx0QXYwb1hpNVlGaHFqZTZVT2tSV3M4WDY0VFR6UzBOenQ3MEwzaW5p?=
 =?utf-8?B?OHJ0ekVJa2VlWTcycElscUpta3ZzUERHaVVJMU5UdlBtTVdJYmVLa2I5V09M?=
 =?utf-8?B?TEZ6YWFlZDE5SjZUVElFbVE2QjAyQk5LM25hWTBIMFFXaWxENWZsOFlPQXFu?=
 =?utf-8?B?TkIzS3VJYjZvOFZJUEVTbWdWa1V0d2dsa2hUa0Z2ZGtabzVyUE1ZdE1MR1lz?=
 =?utf-8?B?YnYxV1ZyNnVFL0hvQldoRitKYVdVMCtSTXdiQ29uRzZlRHVlQ0pzWksxN25m?=
 =?utf-8?B?NlpVRGRzV1NyZzFFZThKQWIyaGJDL2FzWU15bitJODdkT0VJWjFrQy9UK3M2?=
 =?utf-8?B?U0RCR3JqOUozQ1JyN1ZLK0hGNjZJdWdQKzZ0bkdHQWtZb1B6VytJSFUwRXNV?=
 =?utf-8?B?d3hndjVTZXl2RVhid3o3Tm5aMVBDTUdwYlhhYmlWak11a2llams1cEdMeC9D?=
 =?utf-8?B?Yzd2TU9PMzk2UzYzV3h4Y1hxdjFOWjFmSVltd01CMDZtUS9OUVd1REN2b1lG?=
 =?utf-8?B?UzY1VXJObldzazNSM05hQzc4aDN5YzF3WUNjSk1STzlTS0ZoTnBaZVk1Ryto?=
 =?utf-8?B?YjVYR1pERUM2REw2K1IxN3NBVkJvL1JJakNnY1lvNUwrRk8yUmtCTWhjODVI?=
 =?utf-8?B?aFFvbnRERWErWVdqMGRFOU9LUy9RUStnTUtHTndmZUVqdG1wNnBDcWFIT3cv?=
 =?utf-8?B?SzBkUERVZEJLVlJXNEV0SnlnTG0wa2xtNkJSUUU5clZMeHk2MDZmNW8yeElO?=
 =?utf-8?B?WEp1U2hOZEtXdndINUR3aUlKUmNSMTFTcFM0b1hSN2hmTlRwOW1jMSt1SlJG?=
 =?utf-8?B?VzFJUitXYWFmMittSHRaYVF3Wm1MUGp6bFQzMG5sMVVhYTl1MnZIa1BuRWVz?=
 =?utf-8?B?NWNKbGVFSjZHSEMvanFYYkpsdEtTYTNSZVRXV25pbzZGeHgxM0JYdmtRbEVH?=
 =?utf-8?B?MXpyOXR1dVZBRjlZL3pVWGlJOGI4WFMwSmh2UnZoYW9mUWJxT3dGRFNqUHFF?=
 =?utf-8?B?M2F3MWVZYmNVTEp5VFdlM0FubmhnQkxPYTZ1R3cwckM1NnpiZ1RoWUtGTVhy?=
 =?utf-8?Q?ocDLiPnK9y8pRfyt5nGjjc08E?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: eb9d9079-9e2b-449b-df7c-08db02c1b276
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2023 12:58:35.1166
 (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: IAIsQ9NKq8gCQBOKhgz4sT+k+o6TJ9kmF4mvJWu7ofpp2j+ru4AnGZbromQLOcCk
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB7156
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 27.01.23 um 21:25 schrieb David Airlie:
> [SNIP]
>> What we have inside the kernel is the information what happens if an
>> address X is accessed. On AMD HW this can be:
>>
>> 1. Route to the PCIe bus because the mapped BO is stored in system memory.
>> 2. Route to the internal MC because the mapped BO is stored in local memory.
>> 3. Route to other GPUs in the same hive.
>> 4. Route to some doorbell to kick of other work.
>> ...
>> x. Ignore write, return 0 on reads (this is what is used for sparse
>> mappings).
>> x+1. Trigger a recoverable page fault. This is used for things like SVA.
>> x+2. Trigger a non-recoverable page fault. This is used for things like
>> unmapped regions where access is illegal.
>>
>> All this is plus some hw specific caching flags.
>>
>> When Vulkan allocates a sparse VKBuffer what should happen is the following:
>>
>> 1. The Vulkan driver somehow figures out a VA region A..B for the
>> buffer. This can be in userspace (libdrm_amdgpu) or kernel (drm_mm), but
>> essentially is currently driver specific.
> There are NO plans to have drm_mm do VA region management, VA region
> management will be in userspace in Mesa. Can we just not bring that up again?

If we are talking about Mesa drivers then yes that should work because 
they can then implement all the hw specific quirks you need for VA 
allocation. If the VA allocation should be hw independent then we have a 
major problem here.

At least on AMD hw we have four different address spaces and even if you 
know of hand from which one you want to allocate you need to share your 
address space between Vulkan, VA-API and potentially even things like 
ROCm/OpenCL.

If we don't properly do that then the AMD user space tools for debugging 
and profiling (RMV, UMR etc...) won't work any more.

> This is for GPU VA tracking not management if that makes it easier we
> could rename it.
>
>> 2. The kernel gets a request to map the VA range A..B as sparse, meaning
>> that it updates the page tables from A..B with the sparse setting.
>>
>> 3. User space asks kernel to map a couple of memory backings at location
>> A+1, A+10, A+15 etc....
> 3.5?
>
> Userspace asks the kernel to unmap A+1 so it can later map something
> else in there?
>
> What happens in that case, with a set of queued binds, do you just do
> a new sparse mapping for A+1, does userspace decide that?

Yes, exactly that. Essentially there are no unmap operation from the 
kernel pov.

You just tell the kernel what should happen when the hw tries to resolve 
address X.

This what can happen can potentially be resolve to some buffer memory, 
ignored for sparse binding or generate a fault. This is stuff which is 
most likely common to all drivers.

But then at least on newer AMD hardware we also have things like raise a 
debug trap on access, wait forever until a debugger tells you to 
continue.....

It would be great if we could have the common stuff for a VA update 
IOCTL common for all drivers, e.g. in/out fences, range description 
(start, offset, end....), GEM handle in a standardized structure while 
still be able to handle all that hw specific stuff as well.

Christian.

>
> Dave.
>
>> 4. The VKBuffer is de-allocated, userspace asks kernel to update region
>> A..B to not map anything (usually triggers a non-recoverable fault).
>>
>> When you want to unify this between hw drivers I strongly suggest to
>> completely start from scratch once more.
>>
>> First of all don't think about those mappings as VMAs, that won't work
>> because VMAs are usually something large. Think of this as individual
>> PTEs controlled by the application. similar how COW mappings and struct
>> pages are handled inside the kernel.
>>
>> Then I would start with the VA allocation manager. You could probably
>> base that on drm_mm. We handle it differently in amdgpu currently, but I
>> think this is something we could change.
>>
>> Then come up with something close to the amdgpu VM system. I'm pretty
>> sure that should work for Nouveau and Intel XA as well. In other words
>> you just have a bunch of very very small structures which represents
>> mappings and a larger structure which combine all mappings of a specific
>> type, e.g. all mappings of a BO or all sparse mappings etc...
>>
>> Merging of regions is actually not mandatory. We don't do it in amdgpu
>> and can live with the additional mappings pretty well. But I think this
>> can differ between drivers.
>>
>> Regards,
>> Christian.
>>