From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2C49C4332F for ; Wed, 2 Nov 2022 21:49:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230368AbiKBVtP (ORCPT ); Wed, 2 Nov 2022 17:49:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46408 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229770AbiKBVtM (ORCPT ); Wed, 2 Nov 2022 17:49:12 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id D8D5CE003; Wed, 2 Nov 2022 14:49:11 -0700 (PDT) Received: from skinsburskii-cloud-desktop.rtlyha0sdvfehj3ppc5ptuaytc.xx.internal.cloudapp.net (unknown [20.120.152.163]) by linux.microsoft.com (Postfix) with ESMTPSA id 28CA820C28B1; Wed, 2 Nov 2022 14:49:11 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 28CA820C28B1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1667425751; bh=vFhP36XRDpEGRi+gltuc+VdLwPELF2AWSpHAKsqU7BA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dg3AT9MQIGb+zxcA2grXl9uAU32Dt4kQkvv3KrSf7URA8+gLAippcslCGe9bYefnw QZyTFKRYQldBbqQfkV7WbffeyTg9eiFxfqFaWJ9DzDUNYP/0yBbbavCT8bE9YI1Fhe E2Stu25ZbaT7SKeyZwwp8xtuMYUOy9m+on4LAM0A= Date: Wed, 2 Nov 2022 21:49:06 +0000 From: Stanislav Kinsburskii To: "Michael Kelley (LINUX)" Cc: Stanislav Kinsburskiy , Anirudh Rayabharam , KY Srinivasan , Haiyang Zhang , Wei Liu , Dexuan Cui , Daniel Lezcano , Thomas Gleixner , "linux-hyperv@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 2/4] drivers/clocksource/hyper-v: Introduce TSC MSR register structure Message-ID: References: <166732356767.9827.4925884794177179249.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net> <166732386986.9827.12356845572628674464.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 02, 2022 at 09:27:07PM +0000, Michael Kelley (LINUX) wrote: > From: Stanislav Kinsburskiy Sent: Wednesday, November 2, 2022 1:58 PM > > > ср, 2 нояб. 2022 г. в 13:30, Michael Kelley (LINUX) : > > From: Stanislav Kinsburskiy   Sent: Wednesday, November 2, 2022 12:26 PM > > > > > > It makes sense to have the tsc_page global variable so that we can > > > > handle the root partition and guest partition cases with common code, > > > > even though the TSC page memory originates differently in the two cases. > > > > > > > > But do we also need a tsc_pfn global variable and getter function?  When > > > > the PFN is needed, conversion from the tsc_page virtual address to the PFN > > > > isn't hard, and such a conversion is needed in only a couple of places.  To me, > > > > it's simpler to keep a single global variable and getter function (i.e., > > > > hv_get_tsc_page), and do the conversions where needed.   Adding tsc_pfn > > > > and the getter function introduces a fair amount of code churn for not much > > > > benefit.  It's a judgment call, but that's my $.02. > > > > > > As I replied to Anirudh , AFAIK virt_to_phys doesn't work for remapped pages. > > > Another option would be to read the MSR each time PFN has to be returned to > > > the vvar mapping function (i.e. on every fork), which introduces unnecessary > > > performance regression.. > > > Another modification would be to make pfn a static variable and initialize it > > > once in hv_get_tsc_pfn() on first access. But I like this implementation less. > > > > Valid point about virt_to_phys().  But virt_to_hvpfn() does the right thing.  It > > > distinguishes between kernel addresses in the main linear mapping and > > > vmalloc() addresses, which is what you get from memremap().  But I haven't > > > looked through all the places virt_to_hvpfn() would be needed to make sure > > > it's safe to use. > > > > Yeah, I guess virt_to_hvpfn() will do. > > But I'm not sure it the current code should be reworked to use it: it would save only a > > few lines of code, but will remove the explicit distinguishment between root and guest > > partitions, currently reflected in the code. > > Please, let me know if you insist on reworking the series to use virt_to_hvpfn(). > > Your call. I'm OK with leaving things "as is" due to the additional complexity > of dealing with the vmalloc() address that comes from memremap(). >   I'll keep as it is then. Thanks. > > > However, thinking about virt_to_hvpfn(), there's a problem with Anirudh's > > > earlier patch set that started using __phys_to_pfn().  That won't work correctly > > > if the guest page size is not 4K because it will return a PFN based on the guest > > > page size, not based on Hyper-V's expectation that the PFN is based on a > > > 4K page size.  So that needs to be fixed either way. > > > Could you elaborate more on the problem?  >   > The key is to recognize that PFNs are inherently interpreted in the context > of the page size. Consider Guest Physical Address (GPA) 0x12340000. > If the page size is 4096, the PFN is 0x12340. If the page size is 64K, the PFN > is 0x1234. Hyper-V is always expecting PFNs in the context of a page size > of 4096. But Linux guests running on Hyper-V on ARM64 could have a > guest page size of 64K, even though Hyper-V itself is using a page size > of 4096. For my example, in an ARM64 VM with 64K page size, > __phys_to_pfn(0x12340000) would return 0x1234. If that value is > stored in the PFN field of the MSR, Hyper-V will think the GPA is > 0x1234000 when it should be 0x12340000. > Thank you for the verbose explanation. Stas > Michael >