Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2e64c037-ee5a-0c60-5dba-2e48bb437014@amd.com>
Date:   Mon, 15 Aug 2022 12:42:22 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v1] drm/ttm: Refcount allocated tail pages
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        David Airlie <airlied@linux.ie>, Huang Rui <ray.huang@amd.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Trigger Huang <Trigger.Huang@gmail.com>,
        Gert Wollny <gert.wollny@collabora.com>,
        Antonio Caggiano <antonio.caggiano@collabora.com>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Dmitry Osipenko <digetx@gmail.com>, kvm@vger.kernel.org,
        kernel@collabora.com, virtualization@lists.linux-foundation.org
References: <20220815095423.11131-1-dmitry.osipenko@collabora.com>
 <8230a356-be38-f228-4a8e-95124e8e8db6@amd.com>
 <134bce02-58d6-8553-bb73-42dfda18a595@collabora.com>
 <8caf3008-dcf3-985a-631e-e019b277c6f0@amd.com>
 <4fcc4739-2da9-1b89-209c-876129604d7d@amd.com>
 <14be3b22-1d60-732b-c695-ddacc6b21055@collabora.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <14be3b22-1d60-732b-c695-ddacc6b21055@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?R0J0Tm5BRDB2WXZnWjB4MnBndmUvL0gxNTVMVlFRUWNHOFZZdEtSU1dac0pZ?=
 =?utf-8?B?bzJXMWdVcHNDMkc2MnUwdldocUxYdTd3MHVuakxTNkYzOU5NUy8wL1NVbi9j?=
 =?utf-8?B?d3pyalBlSjVZS1pYVlR6Q3dPdmltUjEvMEhJUk5jblcxT3lzODRBckszWHZI?=
 =?utf-8?B?RFIzc1paOGZFNTNRV2hjQnVMbjFBTllBOGZXL2xxS0tlUkVLVEhrMDJ5aUx5?=
 =?utf-8?B?U0lhSldvZFRieS81ZG9oZWU2N0U2amM4bEFSelJ1TmtLdFJ1MFhsc1FLbmVW?=
 =?utf-8?B?a1VwMnA5aWh0SWdueWVUWS8yNlBhUFE3SzQzTThNazRwZkJLUDhqcTU5U24x?=
 =?utf-8?B?OFV1bTBBVTdDcVVzSVJ0aldhTTlxYTFiVWx4MWE0cEkxNWJGenBLYmd5RDFP?=
 =?utf-8?B?Z2NCU2FXNHFLK1lnaWlpL1hPVGMrQzV2K29IUS8zV2dmd21MbWhjU1dFT3Vu?=
 =?utf-8?B?K3g3SWNYZDdQR3lsc2JyVmgrWlhzaHJTWEVrWWpJc1JoUW8vNkI5MmY2UklK?=
 =?utf-8?B?QXdHNTBZaXgweGpKRFpESWVlOUJKL2ZTMWlKbjFNTEFNR0xpL3NXMjIyb1c4?=
 =?utf-8?B?VFRGeER2QXc1SC81T3V4ak91cFBRZHc0NVE3WUVpK1ZJWGlKTnYxUVl4WmRX?=
 =?utf-8?B?SHZ2KzU0RkNRb2dmVUZhYU1UeE9IQnZqd1BpZXlHK0ppYSsrMGRyK1RjUzJS?=
 =?utf-8?B?aHVXcTVDbU4vRFFiVGM2c0d2a2dzaE9wc2VVRGtoMk5aODU2M2oraVlpc0pi?=
 =?utf-8?B?MitIQUVma3FXR0RDVjdOa3FTdlpvbGNHWGhITzFmdnZWNU1rZWt1WXJpV2th?=
 =?utf-8?B?NlJtUWVaZUREU0pEYS9zUHNCVlozR1RBcU5CNGlnK21tQmx0ZHNxUUs2b0pu?=
 =?utf-8?B?Z05sYU1tZEZJc3VHaExkNXcrVnh0N3gyRzJnK1V2MXJoKzQxNWhRdzZ6VTlx?=
 =?utf-8?B?SHlsQkp3NnU4aHRkR0M0QWVoTUhqMHlrMjhVcHhxa1N4eDYxeUx1UVhScjJu?=
 =?utf-8?B?bkFnTGpseEhPckI5OC9mSXBLNmNzcE56R0tPL0M3SVJMSUlpS3d0WlJwdTdj?=
 =?utf-8?B?YXRIN0srUFpqYi9sbnViWGVNdlZNVG1jUVhGcmFtblNmekFZT1RKTGxtZkJp?=
 =?utf-8?B?Z0hEQzcxZG5pV2Z4YjN1ZXl2cmNzVEhTNWFQUTU0b1pmb1krb24yVjNrbkxM?=
 =?utf-8?B?Q0hCdU1LcDI3ZUsvSU5jaXFtRXZyUnZ0OWFLQ0dLQjhycW55cjFaQnJpeTlO?=
 =?utf-8?B?TjRyY1NXbnNZaXE0YVFReVJ0RnF3TXU1aHYvY1RNeHhFc2k2UitlVndVV2ts?=
 =?utf-8?B?UUk5RllsTisrbDZSalI5SHowV2tqNnlkNFU2RGNVVGh3S1M5UUFyZEZuRXBz?=
 =?utf-8?B?OTY3eDdIQU51QmhBaHY4N0hIT01pTDViT0NEVU4rSnZWK0duTHN0UVpBZzNJ?=
 =?utf-8?B?NUY2cUM4STlORFQ5akdJMTUrMXFVdUJlWFRzM3EzcmIvc3lvODFjUkRmdWln?=
 =?utf-8?B?VzM0Y2ZWWXp4aXRid1g1aEk3WHJhSVF1OTVJbXkvZStqOHpJZVlGYW5LMGRS?=
 =?utf-8?B?WEhaby85eDM5NHN2ZzBPRmdvY2R0OVgzWkRrdWkzZ0RYUUpTQVd4VjF3ZHZF?=
 =?utf-8?B?clpHNHZtU2c3ZVovdHlEUno0cGl4SGRocWFTOW9rQW8zd2U5YWNmVUZjWjJE?=
 =?utf-8?B?RlVvRndoVkJCVU8rU2RaK1ptZk1qc0VZaXk5U3ZWMFM5YmtnRHIvSHN4UDhr?=
 =?utf-8?B?VDlEeEhIRFRHMmxodmQrN1hwOW5FM05OTDVUNDA4ZVN2NnVLMHZZT1JtSm00?=
 =?utf-8?B?NW5zc01ITDZoSjdOZWdsNkRFUVo1Sk1oZ2hSOTdWMExxcEJsVjJCZVBXMms1?=
 =?utf-8?B?QUlsc1ZJTXo3b2tBOGZHaDRxS3dNZWh6ZTU5UDJueXhDampjbzY4Qy9GbFFL?=
 =?utf-8?B?VkJneVpHMVFxeEp6eDRwSTE5ZU82VkJiZ01TN25pazB3OXdUNUU5V0hqTkVO?=
 =?utf-8?B?ditsSXFDSTFNWEVaNS9UR2JRaW96cm4rb1RGejhTVTZqVUErVmJSZVlob013?=
 =?utf-8?B?WURXV1BnVEtHTCtzMERSUFgyWVlOdTlzL0lHTURVZjBRYXNGSzNGSHJuNFNY?=
 =?utf-8?B?V0JZbXp4QVROZWtIaFVOZUtTNGYzZEIyRGNyVm5KTGdUTmExQUhEM21RbjZo?=
 =?utf-8?Q?/7EJIuQoqDnytSBLbzxqttYe/3ZQX7sqz1dz388IBcaA?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b7de8b93-6189-4f70-ccba-08da7eaad995
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Aug 2022 10:42:28.8859
 (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: Vpg0KmSUOQ5zTLiowY8cdDJhWJPf+5Hlz7L0ez4awi7PQ7FBGKGtpoCg19GGe/Ej
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6158
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 15.08.22 um 12:18 schrieb Dmitry Osipenko:
> On 8/15/22 13:14, Christian König wrote:
>> Am 15.08.22 um 12:11 schrieb Christian König:
>>> Am 15.08.22 um 12:09 schrieb Dmitry Osipenko:
>>>> On 8/15/22 13:05, Christian König wrote:
>>>>> Am 15.08.22 um 11:54 schrieb Dmitry Osipenko:
>>>>>> Higher order pages allocated using alloc_pages() aren't refcounted and
>>>>>> they
>>>>>> need to be refcounted, otherwise it's impossible to map them by
>>>>>> KVM. This
>>>>>> patch sets the refcount of the tail pages and fixes the KVM memory
>>>>>> mapping
>>>>>> faults.
>>>>>>
>>>>>> Without this change guest virgl driver can't map host buffers into
>>>>>> guest
>>>>>> and can't provide OpenGL 4.5 profile support to the guest. The host
>>>>>> mappings are also needed for enabling the Venus driver using host GPU
>>>>>> drivers that are utilizing TTM.
>>>>>>
>>>>>> Based on a patch proposed by Trigger Huang.
>>>>> Well I can't count how often I have repeated this: This is an
>>>>> absolutely
>>>>> clear NAK!
>>>>>
>>>>> TTM pages are not reference counted in the first place and because of
>>>>> this giving them to virgl is illegal.
>>>> A? The first page is refcounted when allocated, the tail pages are not.
>>> No they aren't. The first page is just by coincident initialized with
>>> a refcount of 1. This refcount is completely ignored and not used at all.
>>>
>>> Incrementing the reference count and by this mapping the page into
>>> some other address space is illegal and corrupts the internal state
>>> tracking of TTM.
>> See this comment in the source code as well:
>>
>>          /* Don't set the __GFP_COMP flag for higher order allocations.
>>           * Mapping pages directly into an userspace process and calling
>>           * put_page() on a TTM allocated page is illegal.
>>           */
>>
>> I have absolutely no idea how somebody had the idea he could do this.
> I saw this comment, but it doesn't make sense because it doesn't explain
> why it's illegal. Hence it looks like a bogus comment since the
> refcouting certainly works, at least to a some degree because I haven't
> noticed any problems in practice, maybe by luck :)

Well exactly that's the problem. It does not work, you are just lucky :)

I will provide a patch to set the reference count to zero even for 
non-compound pages. Maybe that will yield more backtrace to abusers of 
this interface.

Regards,
Christian.

>
> I'll try to dig out the older discussions, thank you for the quick reply!
>