Received-SPF: pass (google.com: domain of linux-kernel+bounces-170918-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 7 May 2024 17:12:40 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
CC: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>, "alex.williamson@redhat.com"
	<alex.williamson@redhat.com>, "jgg@nvidia.com" <jgg@nvidia.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>, "pbonzini@redhat.com"
	<pbonzini@redhat.com>, "seanjc@google.com" <seanjc@google.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"luto@kernel.org" <luto@kernel.org>, "peterz@infradead.org"
	<peterz@infradead.org>, "tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de" <bp@alien8.de>,
	"hpa@zytor.com" <hpa@zytor.com>, "corbet@lwn.net" <corbet@lwn.net>,
	"joro@8bytes.org" <joro@8bytes.org>, "will@kernel.org" <will@kernel.org>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>, "baolu.lu@linux.intel.com"
	<baolu.lu@linux.intel.com>, "Liu, Yi L" <yi.l.liu@intel.com>
Subject: Re: [PATCH 1/5] x86/pat: Let pat_pfn_immune_to_uc_mtrr() check MTRR
 for untracked PAT range
Message-ID: <ZjnwiKcmdpDAjMQ5@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20240507061802.20184-1-yan.y.zhao@intel.com>
 <20240507061924.20251-1-yan.y.zhao@intel.com>
 <BN9PR11MB5276DA8F389AAE7237C7F48E8CE42@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <BN9PR11MB5276DA8F389AAE7237C7F48E8CE42@BN9PR11MB5276.namprd11.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Ilg+FHYBYRMB+jN3NUjfleDcUJStNx7srnn9+yqLpsPAQWg4+mlf+Uke58SJ?=
 =?us-ascii?Q?S+oJ0CFSpfI3+Vtii4gzVg9SUp2HMCBSJgqNQB8VQM0I+YqP3JUsVDDvmvV6?=
 =?us-ascii?Q?SrxVdA/qSchwPwMZC9ieLRudXojkbTMTT4KIVqx7Sa71DEgJRDj90LcAUE/G?=
 =?us-ascii?Q?5ABEKQsbaU8tMV2J0HgMbqvUsBWLzvnisw527kl8Z7GrnZ48UzkWHSqTskCt?=
 =?us-ascii?Q?Z9pMZtPom4U6bPkTJE69kpiaGRCErurESN+MN7HYTR3NtDjpP2NNQyXEZH0R?=
 =?us-ascii?Q?TNpoGA941bweuFgyAn2jBcirsmZMfN14uGXCURR6ugu7SeYZFT6gQOs/qJvv?=
 =?us-ascii?Q?x0XLJ6mr6uM5bGZulAEpMGDm4maX10V9MywUBZAygXnlQZFRD4R1jZOf1aeB?=
 =?us-ascii?Q?Uq9G03UBf6p+xGCG3uYDlUx1t5yl9Sw2W75d0v4O9w1AHgxgWWu9zvkCnYkN?=
 =?us-ascii?Q?gusAHM9EmJjHRvmyt9iOUNapAHcyseSyDofaxIboZ9gLODiegbjw0fJEY0E5?=
 =?us-ascii?Q?mE1zdTD0ygf771kW/R8ZINJ0TutVAz0IIUIKvTM8TwRNTQDNvVFMsLoZCPVO?=
 =?us-ascii?Q?cPNau+aaPi7JaNXg5b39E05iDNwClyySwGGyufEOWUbJhz7CioN6UVK/UtVR?=
 =?us-ascii?Q?swgLXnZqvoaALBOYl7uB8Ou7+k6KyXKze5ieWDs4OGqe9orSAX0Qa5PQj2oR?=
 =?us-ascii?Q?L8zdtoeAms4aP3xEdRJxO9/bTzH+PcDcpTzfl/W9Htflk4aT8aToAHA7ILe1?=
 =?us-ascii?Q?BxqR66OGsiwdTWtwHAaFu0T7NQoRr86jdR2b6lsApA42LX4N/p7x232OTvl6?=
 =?us-ascii?Q?2kocci5SoqLeVTSuZrZdGZRNTBwUbnUpVJ8Vk7eqH6oQIceUWxwAoQQge3hV?=
 =?us-ascii?Q?VSaR0kXdBgjlnT/zo2/ptY9fSQh8LjcW7oemCuVfqi3NsHpDq/OtBmJ6B4Ah?=
 =?us-ascii?Q?o+2E6K9gxq1GC+3HM7JnFv4L9orBJYpyYzNbxHdpqtJModqSVOJsTTyVUeoY?=
 =?us-ascii?Q?/EKBXfDA1/0hdC9eHPv2Pp/1M7v1WjOq1R+OU4TQp/xOtjGClwV3TGKhXOTX?=
 =?us-ascii?Q?PNKlVgSGXk8WQ9SOLEaFKwaRAamuL3jP24Mf+3xy3in65QHimdqfswNZNumm?=
 =?us-ascii?Q?spiIh61eNaHRFKIkwJW8197kZ0EwYfZ0Lyw10gyuqgu2dYESzfDCWcpdbbxR?=
 =?us-ascii?Q?pU5IxFe2NgEZ9i46oNErik87cXl/rA5ELrktqqSF9VfDDZKWzpurxM2q+isL?=
 =?us-ascii?Q?C3lbAXPo/T1Cn/aH+CLNsvW6mcHfIzM4B1rM09UXv+zTPC/5cs52aY0I7crm?=
 =?us-ascii?Q?O0TobUIQ70mXNBLq6H2eCOwwqzX+v/p6vOo3QHsphGYlngk66INfoD5O1MfE?=
 =?us-ascii?Q?XJKYh3Mp81K7mRDOVjs1eCAdBGLl2B+1/7rPpxaEX8Y5DXB0RxNdau0kw32N?=
 =?us-ascii?Q?gQjUI6xAJS6ZemzVMEsu4NkiT2BiinSP33V9JmqW4P6KPdH4JlOsmqJqyiyP?=
 =?us-ascii?Q?Q6ruxvX+w0M9HalYMK2uLreAi0Gcb+JEL3YbWvEtOlLYcD0hSlxi0m7pq75G?=
 =?us-ascii?Q?ObfXYk6BVGGh3CG4DcFeWHD68gKZAgdhu3xsxmdF?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 67a31a43-01f4-4c7c-f56b-08dc6e75f28e
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 09:13:24.0681
 (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: OCnSwAlYDgevrxyrAIJgrtmWOG+6GlKkkXj8r/gPCVCFlrGUnRQ8yhjNWioccitsjA3YgFmsoOLucKjcSQ/cvA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB6621
X-OriginatorOrg: intel.com

On Tue, May 07, 2024 at 04:26:37PM +0800, Tian, Kevin wrote:
> > From: Zhao, Yan Y <yan.y.zhao@intel.com>
> > Sent: Tuesday, May 7, 2024 2:19 PM
> > 
> > However, lookup_memtype() defaults to returning WB for PFNs within the
> > untracked PAT range, regardless of their actual MTRR type. This behavior
> > could lead KVM to misclassify the PFN as non-MMIO, permitting cacheable
> > guest access. Such access might result in MCE on certain platforms, (e.g.
> > clflush on VGA range (0xA0000-0xBFFFF) triggers MCE on some platforms).
> 
> the VGA range is not exposed to any guest today. So is it just trying to
> fix a theoretical problem?

Yes. Not sure if VGA range is allowed to be exposed to guest in future, given
we have VFIO variant drivers.

> 
> > @@ -705,7 +705,17 @@ static enum page_cache_mode
> > lookup_memtype(u64 paddr)
> >   */
> >  bool pat_pfn_immune_to_uc_mtrr(unsigned long pfn)
> >  {
> > -	enum page_cache_mode cm = lookup_memtype(PFN_PHYS(pfn));
> > +	u64 paddr = PFN_PHYS(pfn);
> > +	enum page_cache_mode cm;
> > +
> > +	/*
> > +	 * Check MTRR type for untracked pat range since lookup_memtype()
> > always
> > +	 * returns WB for this range.
> > +	 */
> > +	if (x86_platform.is_untracked_pat_range(paddr, paddr + PAGE_SIZE))
> > +		cm = pat_x_mtrr_type(paddr, paddr + PAGE_SIZE,
> > _PAGE_CACHE_MODE_WB);
> 
> doing so violates the name of this function. The PAT of the untracked
> range is still WB and not immune to UC MTRR.
Right.
Do you think we can rename this function to something like
pfn_of_uncachable_effective_memory_type() and make it work under !pat_enabled()
too?

> 
> > +	else
> > +		cm = lookup_memtype(paddr);
> > 
> >  	return cm == _PAGE_CACHE_MODE_UC ||
> >  	       cm == _PAGE_CACHE_MODE_UC_MINUS ||
>