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: <4fcc4739-2da9-1b89-209c-876129604d7d@amd.com>
Date:   Mon, 15 Aug 2022 12:14:00 +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
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
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>
In-Reply-To: <8caf3008-dcf3-985a-631e-e019b277c6f0@amd.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?cFFjMnRYVll1ZkRJWHBLYkNhSjE4LzYzYkZlT1hMYnR4eVpoQXB2SUcwUzR6?=
 =?utf-8?B?VEFDL1ExeVhsLyszZTRkaEUyMTB1YkFYZ2ZHWXhrbXZaWkFUKzNLSnhoa3F5?=
 =?utf-8?B?eFljcUxlSUtPclFTbmgwUTlwWWNTZGx1dFZRSlQ1NnhmMk9HOUM1VGtJU1Uz?=
 =?utf-8?B?endvUldLTWQ0SktpVG54anI0ZGtSb0ZUMkkvRXpZTmhjM1JraWdaRnNhZDlG?=
 =?utf-8?B?eHAvaWxTSUZqcGxlVGpkdXZhSFg1SzNtTzMyaVdYRVV2L0pvUm45VTVNUmQ0?=
 =?utf-8?B?aENlTjB3bnNSRWFKUGNjdlg5TnN6VENDMlNDOE1OMFEzVk1YZ0VLazRlL05y?=
 =?utf-8?B?MHBiOU1LRHViZWVid2t4Um1zSHpBdEtZOGYwRTZ5d0JBK0xFay9GZGFFbHpP?=
 =?utf-8?B?VGYxQi9DNm1aN3FjN1g4TE8wZ1NEeXpsOWNDMzZ6dGJNajZWTGhBNmJTMEUv?=
 =?utf-8?B?RzdOVWo4b2NTdFVqb1FqdFNjcVpZWmg3MEpvMWZRWk9qeU5neGNzcFFYcDd2?=
 =?utf-8?B?Zzh1SFRpMHB3a0NkcW5peStFR2F1UG1BMTgyM2lQZnh3ZWtDb3BvdkN2RlV1?=
 =?utf-8?B?ZlNybnlwRlI3cWEwSFVHdHlzWDZoTkRGVDBEbmExbWVJQ09OSEhVTVlMWG5Q?=
 =?utf-8?B?d3JaN21zcndyQXJGdWpRYWZScmEvSGhxSnRqaG1rVTc3NDlXM0lsdHRrblR5?=
 =?utf-8?B?ZUc3dVFBSDJDVXk4c3hNUVZVQlRBWEdZejYzNFY0blQvR0UvOWZPbHNoOHdl?=
 =?utf-8?B?eHREWVNLWGJBajRVeDE4ZnVuaG5COE5NOUFhaVBLVDl2RFRpMHJ4M1FYMVoz?=
 =?utf-8?B?ZERSbnJ6UGJab2VFNlRyZFg4eEhRZTFid29CeXRZZUE5N0U2cVBjRDdDQTEr?=
 =?utf-8?B?TDhBRWNBUUZzd3dWS0RGaDk5Wm5PbHdQNDhoc3RpamJVTkdXVEZWRVJyb3kz?=
 =?utf-8?B?d3VmV0pZZkNBdERsV0IvK3haNnp3amw1VUJuZVd5MVBleGs0OVhvSHRrc0pM?=
 =?utf-8?B?bG44RVFodHozcTNhZm15elZaMVIzMlNMcDVXRmVvN0s4MTRrZU9ieEphOG5P?=
 =?utf-8?B?aFpIZUsreXNMeW1UUkhTYzY3Y3pLc3BjTHFBVy9CRmtNQjZWanpiWEp5OE9i?=
 =?utf-8?B?MVlVd0oycWZnbUFsN1hWK2cyM1NpMkI5RGRMR2dvRFRVNVEwMm44TjRuNzZs?=
 =?utf-8?B?d08wcG1iZ3BBdCtTK3hCWk5BNWZHVEMyTGE2VXNCYjRJVFFxUmIrTWh5N1ll?=
 =?utf-8?B?a3c0WmxDeFp5R3ZhZ2UvR1laK2plZktZYlovOFBQY3RGd2twZS9RS2ZReWpo?=
 =?utf-8?B?ZUlMYWhjNFFMbmh3a1dxQ0krOEIrVDNPcHJyek1HZU42Ynk5RjV1bHcrbEhm?=
 =?utf-8?B?Ny9ZaVkySnpyTXZyWXZxVVIxNy9tL255NXpSN0FwK241STJBNDNwZ2Y2cHZF?=
 =?utf-8?B?WEg1QkdqaENTMHJsUkdZV3pzSjVldkdDUXYrRWdwbzI4MVlWaWp5emFyVTc4?=
 =?utf-8?B?eGhCeXAvOW12b3B5YVRXWWZNSjI0K0IrU2k5NGh2UmFLWXY0TDA5cXd6b1NC?=
 =?utf-8?B?cStRMzMzRGx1ZGpOWTdiekllOUpnOEdoYXZuV3dNN1hKaklSd2swcmJ1UnZM?=
 =?utf-8?B?TnNhYTFqTVlqWDlNWEtpcUU1Wk05THpNbnRFbEM0dEI2dWVVUkxxUE5aa2d6?=
 =?utf-8?B?WXpJQ1ZtNVhVQWxDRUtTNVVabTRMMjNnaUNreWZqUkFPcFVyMlgxaGFkaWdY?=
 =?utf-8?B?c2FpcmJwQ2hPcjlQdWdUYXJYZjlQR1hjWG81RkMzMWlPaC9KcVRpeHBiZUJr?=
 =?utf-8?B?ZEFmbk1HbTk5UGNLZlptaDE1M1c4VkZYRjBMVjQrRnIwYndpN2pETVVUS092?=
 =?utf-8?B?YW5pOTVLM2psS3hBQUZ1S216NlM5bVZ2SU1yTGI1TVQxcCsxeTI3cUFuNE0v?=
 =?utf-8?B?MUZQVzVZZnFkNTZXaUJZajRXbVZ3UDUyWFl1dE5Td1k4azFreVoyN2ZlaVA0?=
 =?utf-8?B?RDh1Mk1uMDFsMmt6cVRJVFd0RkNrbE42Rzd2cDI4R3dWWjM3M093NTRMVlN0?=
 =?utf-8?B?dy9TeGtrS1NNaTNNUWpDN1pWMWFNb1RiMnk1SVN0NmQvYnN0ZGNJOC9wZjJ6?=
 =?utf-8?B?VTFaZ0lvZXBxVHI5VU52empBTVZLWEhUbmpZRzZUR1ArYjBLMGszcitkSGJX?=
 =?utf-8?Q?kXSt3iVvHDCTf5x96uNRYJL8N57zmwhCe6rAN0Gjmp1a?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dec1f74c-2fea-40cb-efdd-08da7ea6e322
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:14:06.8297
 (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: xoNbRTWAH1dqOCNGdTtNe36eb9I9jxOtK1VoX/9t7kCVBBEr2GbRqme58ln/tFGY
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4251
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: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.

Regards,
Christian.

>
>>> Please immediately stop this completely broken approach. We have
>>> discussed this multiple times now.
>> Could you please give me a link to these discussions?
>
> Not of hand, please search the dri-devel list for similar patches. 
> This was brought up multiple times now.
>
> Regards,
> Christian.