Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Date:   Tue, 10 Oct 2023 11:46:24 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     Like Xu <like.xu.linux@gmail.com>, <pbonzini@redhat.com>,
        <chao.gao@intel.com>, <kai.huang@intel.com>,
        <robert.hoo.linux@gmail.com>, <yuan.yao@linux.intel.com>,
        kvm list <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 01/12] KVM: x86/mmu: helpers to return if KVM honors
 guest MTRRs
Message-ID: <ZSTJEJepdnmC5PA5@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230714064656.20147-1-yan.y.zhao@intel.com>
 <20230714065006.20201-1-yan.y.zhao@intel.com>
 <553e3a0f-156b-e5d2-037b-2d9acaf52329@gmail.com>
 <ZSRZ_y64UPXBG6lA@google.com>
 <ZSRwNO4xWU6Dx1ne@google.com>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZSRwNO4xWU6Dx1ne@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OGxhdm9palhQLzdiM09vYmRYSHpLOHBhSmQ0eUFWYzJIZVNIYlVJYUc3a1VN?=
 =?utf-8?B?R1JweHFGN0puSjBObWJtVWlaTElOV3dqbXFpdEhEanZpcjdXenZ1d2NoRGh3?=
 =?utf-8?B?MFNtYUVhVWJyUlE5K2N2RWc5TWZwdFduZ2tKdUVmNFNCLzJFQmxUZFc3OXUz?=
 =?utf-8?B?RThUWE14bktIZ3FEeHNIVUJhbkdHSGthYjdzb1dmQTIvdFRic3RXYXR2WDJn?=
 =?utf-8?B?UUpnUm5FcmJJTllUbUcxRzFEZ3NJa0FjekxORlhnK0tTWWtnZEczM2cwTDli?=
 =?utf-8?B?WjJxMitpcXY0aWhWbWFiV1EwdEVxcUZkbGRwMDJPNGJmOHhxM01IZEdTUWlv?=
 =?utf-8?B?RGVnOStpb1BaMDh4Y3YvZmdlTjY4c29MSUwzbjNuSUw3c3o0cHAzdUxxZFdT?=
 =?utf-8?B?cFRQRk1PL3pvbzRIMGNlZDN1ZURCcjFCOEpZUkI1a0R0WHdsd1ZMSVE3Z3dv?=
 =?utf-8?B?QU42c01KSDd0NnFmemdhTkFaTXpjZDltVUxCNXg0YXpDd1J5RWtzRDg3eFlJ?=
 =?utf-8?B?akYwUDhKWDJVQ1dTdllzT2h5TzkvSXU5Z0xkYlViaXUvR2ZjK3dIazlsalYr?=
 =?utf-8?B?OStUa2xsOThXbjdDM0xsTk5pdGJaVVQwZ2NlZTdBUCsvemFyZjdOOXRaTVZx?=
 =?utf-8?B?aUVPRkJMRVhqc3BQT0RNTitlSGdXelZtVzRoNEYzS2lOQnlncmtITkI3aGxm?=
 =?utf-8?B?WFl0MGdncGZWdG85ZUxxZmV1UkpFTDdtNE1FMHhKNFgvaTM2NFhXSGI5SGpv?=
 =?utf-8?B?ZHFEOCt5UXZhdjUyUmZTWDE0WU55OVRNMjRoTnl0L3d4ZmR1dFhDSHUvQjRH?=
 =?utf-8?B?WlZqSHBnN2xsVUtYeUpMUHpGdDRrZ2d5V1Z4NTVQeHNZVnB5bDlnNThjaFlo?=
 =?utf-8?B?bnRCWG9LZ1YxNHg4YUFzcHVVa1hKY0hxUUJQbUJMQm5QcWZHT0daRGUxWlZT?=
 =?utf-8?B?S2ZmQmtMMXUyMkVhK2xiLzlDeFFKd0JzVU5mVWs3QmhkRmN5bXd1dXFXNmhB?=
 =?utf-8?B?Wnk3UlZqTjFUMEM1M01HSmYwSjFXYWNYLzQ1NisyaCtTNndnTkJHTm9IQW0w?=
 =?utf-8?B?Qmp2Rlo0L0h2SXQ3WElPODlDNmNPRkVDMVNaQzJwL3hEcDFLb1JRUkxPcFdz?=
 =?utf-8?B?MVVOT01UQnhvRVNseURvWmtoelQ2UmlWUGlQeWZUNUJ5b1FIZmhwT1JnS0sz?=
 =?utf-8?B?YzZuVENlNnpVa0syNHJESllHQ2gwVkxmQ3d4ZzFlSkhVTnF5ZVlBYURzRHRW?=
 =?utf-8?B?TlhyV2Fwd21Jem1BMzV2SHRiTWN5aUhaL0tWUjlzUy92bzIydHkxbE1HZ2Ix?=
 =?utf-8?B?bGV1cTlDNUZTaXIxbXNrNkxWbnhreDJqN3VOS3o4bVpvVURUZG8yZURqUDNK?=
 =?utf-8?B?N1B0c1grZnVCbTR2WmRXMUZSYzhaTDd0TDAzZlRsQll1aGU4NjROZHpUUWpL?=
 =?utf-8?B?em1tRVJ2NGF1QStMN3JZemNyaitPZFhkUUFZRERNQ25EdzZsTmZpN2VhbTY2?=
 =?utf-8?B?NkxnK2tTcU1FMnBEZ3V3UzFWaU0vR1F1VE9FRWRwYS9nNDZCNGlWTkxSK0VI?=
 =?utf-8?B?b1JpbFltV0UyQ0NuQTJkSDZWYUNudEc5L1paZWU1QmJ2TGZYUjlvTEltQ1l4?=
 =?utf-8?B?Qm8wVThLcE1TSG9VbGxZMXNlODhDL0ptRnBjVlBDbmRqWlhLaSt5MlY5TDNE?=
 =?utf-8?B?RmlQUmNMa0FEQzdvZzVRZzdpRjdTaU9WdlpHMThVREhTQ2dDMDlrbGl3dHBX?=
 =?utf-8?B?ZmJaVTdCNUJHVlRnV0pwQ3hRSzE3d29PK3dSTkNjZGVqeHdRVUxNbWJiTFh5?=
 =?utf-8?B?TERhcVhrdll6YmtFQk1sZC92ZUNvSnN3K2pqT3NGV3VjcHlManZKK3pBaXBl?=
 =?utf-8?B?cHFkRjhNOGYrQi9xUlIxbHppQTZEZWN5V0hybzlPbXI3SXVReHZNWlUweWtL?=
 =?utf-8?B?SXR3djAxOXBZSnM3d1l6ZG5zQVoxZVdiM0QvWDNEaDZGNVFPWldCU1pkazJp?=
 =?utf-8?B?VWFzbUpSL3VpL0h5c0o3dWYyelpwT2ZkVjVCSXp3ZUIxbFhhMFBaRUYzNjlZ?=
 =?utf-8?B?VERNMVY0N0orbkt4a0diSjRSWDVSU2dXNEhVQWpsbk1qbmZiK0QvSFY4bkxM?=
 =?utf-8?Q?wQXEzpNrhes/QV6yto/Qknn3r?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0b7b95c7-40be-4fb3-47ba-08dbc94765e7
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Oct 2023 04:14:29.3907
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TPlly5LBzePY6NBvLnkXe9WtciFkUxl51RD99SfD8w/6wFMUu6yxwoSZoWAk6jilSJkedbF4yCDSPkLXFbfIAQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8231
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=2.7 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,
	URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 09 Oct 2023 21:14:51 -0700 (PDT)
X-Spam-Level: **

On Mon, Oct 09, 2023 at 02:27:16PM -0700, Sean Christopherson wrote:
> On Mon, Oct 09, 2023, Sean Christopherson wrote:
> > On Sat, Oct 07, 2023, Like Xu wrote:
> > > On 14/7/2023 2:50 pm, Yan Zhao wrote:
> > > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> > > > index 92d5a1924fc1..38bd449226f6 100644
> > > > --- a/arch/x86/kvm/mmu.h
> > > > +++ b/arch/x86/kvm/mmu.h
> > > > @@ -235,6 +235,13 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
> > > >   	return -(u32)fault & errcode;
> > > >   }
> > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma);
> > > > +
> > > > +static inline bool kvm_mmu_honors_guest_mtrrs(struct kvm *kvm)
> > > > +{
> > > > +	return __kvm_mmu_honors_guest_mtrrs(kvm, kvm_arch_has_noncoherent_dma(kvm));
> > > > +}
> > > > +
> > > >   void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end);
> > > >   int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu);
> > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > > index 1e5db621241f..b4f89f015c37 100644
> > > > --- a/arch/x86/kvm/mmu/mmu.c
> > > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > > @@ -4516,6 +4516,21 @@ static int kvm_tdp_mmu_page_fault(struct kvm_vcpu *vcpu,
> > > >   }
> > > >   #endif
> > > > +bool __kvm_mmu_honors_guest_mtrrs(struct kvm *kvm, bool vm_has_noncoherent_dma)
> > > 
> > > According to the motivation provided in the comment, the function will no
> > > longer need to be passed the parameter "struct kvm *kvm" but will rely on
> > > the global parameters (plus vm_has_noncoherent_dma), removing "*kvm" ?
> > 
> > Yeah, I'll fixup the commit to drop @kvm from the inner helper.  Thanks!
> 
> Gah, and I gave more bad advice when I suggested this idea.  There's no need to
> explicitly check tdp_enabled, as shadow_memtype_mask is set to zero if TDP is
> disabled.  And that must be the case, e.g. make_spte() would generate a corrupt
> shadow_memtype_mask were non-zero on Intel with shadow paging.
> 
> Yan, can you take a look at what I ended up with (see below) to make sure it
> looks sane/acceptable to you?
yes, tested working on my side.
I think why we added the checking of tdp_enabled was because of the existing check
in patch 3. As noncoherent DMAs checking is not on hot paths, the previous double
checking is also good :)

BTW, as param "kvm" is now removed from the helper, better to remove the word
"second" in comment in patch 4, i.e.

-        * So, specify the second parameter as true here to indicate
-        * non-coherent DMAs are/were involved and TDP zap might be
-        * necessary.
+        * So, specify the parameter as true here to indicate non-coherent
+        * DMAs are/were involved and TDP zap might be necessary.

Sorry and thanks a lot for helps on this series!