Received-SPF: pass (google.com: domain of linux-kernel+bounces-20206-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 9 Jan 2024 07:36:22 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<dri-devel@lists.freedesktop.org>, <pbonzini@redhat.com>,
	<seanjc@google.com>, <olvaffe@gmail.com>, <kevin.tian@intel.com>,
	<zhiyuan.lv@intel.com>, <zhenyu.z.wang@intel.com>, <yongwei.ma@intel.com>,
	<vkuznets@redhat.com>, <wanpengli@tencent.com>, <jmattson@google.com>,
	<joro@8bytes.org>, <gurchetansingh@chromium.org>, <kraxel@redhat.com>,
	<zzyiwei@google.com>, <ankita@nvidia.com>, <alex.williamson@redhat.com>,
	<maz@kernel.org>, <oliver.upton@linux.dev>, <james.morse@arm.com>,
	<suzuki.poulose@arm.com>, <yuzenghui@huawei.com>
Subject: Re: [PATCH 0/4] KVM: Honor guest memory types for virtio GPU devices
Message-ID: <ZZyG9n0qZEr6dLlZ@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20240105091237.24577-1-yan.y.zhao@intel.com>
 <20240105195551.GE50406@nvidia.com>
 <ZZuQEQAVX28v7p9Z@yzhao56-desk.sh.intel.com>
 <20240108140250.GJ50406@nvidia.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240108140250.GJ50406@nvidia.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?tMWWZQ/9cgMYQZP+dixDdkrkN9jjXKl7D3w3BUh2odfcaGnf42wxInpwKn8m?=
 =?us-ascii?Q?KQINpPKATuXTuT12fwcMtnwiNeurtc43nTVQXredzPii2y9hnV/+TJrbrfaL?=
 =?us-ascii?Q?9g0dAKe9/qCDTnXqM7C5yoPkD4UE/s+oQiTLu/bEDS/O0NAK+G8y3262AZqq?=
 =?us-ascii?Q?iqbEPyKSaVQjSdZBil28QaH3MQX45KnCPXcTb18sVSfNoeI/q2xeP8fK62Rj?=
 =?us-ascii?Q?e2x2+VKDMsIeuWXu77CRumdoU6Qb7u3Bn/yS5EhLQWPvITCQ/x2dt3zmQTbR?=
 =?us-ascii?Q?TBHBaEiOlORglxS+1XbFfZLAt351QdByzDkJjQFXnI6lvVuqoL/u2j7aDPwD?=
 =?us-ascii?Q?cq4bfeVqQk51lTrTmxaRqVbBMdigjtwaxpAE66ZazAAhghO9sZaftPdu13Qp?=
 =?us-ascii?Q?R2N1bqSPo1sh8db3p0o5TgBbm0eqvWOALyiFSZ0m/sc3paeidtYU1sO5dBIR?=
 =?us-ascii?Q?LOADrgQKXmV6GyOMebnxraY+1N7ISDjIa+Bbswgw1oCdv5VtNSA3Wwlznl3q?=
 =?us-ascii?Q?V2A7Qe4zWyCjjpkq9c585a7BdPwftWtD8Z/SXAB01R5YXWrBc8LuVZHlkm+Y?=
 =?us-ascii?Q?fFeBYRxhA2QJ+ufCTwN6ymGFpBJcIvCYTjx7j/rr2787+i6n+YNLiiVLTg3n?=
 =?us-ascii?Q?Eo7feGS5qOm31VLFjlvOKgm7eloqyLf91DvUsZOHRMZOlV5u6We6Y1ppuRju?=
 =?us-ascii?Q?X6N4hnOYl5OPLebl1dyS5qGdwP2Q7F4TMYOnikv0n4Uq7x9y/nbZ7lneJNKt?=
 =?us-ascii?Q?ssh7mrOYxqfePY4SaT8jO9VYDlYqhXecOnEg8GrhL1k9mefPo8uunbYva1wd?=
 =?us-ascii?Q?WLdjAxKyOUzIMTBfcpfc46MS8H6DLo5DaOYnqSVJ1ce3hWpQVCsA9ESEuNdR?=
 =?us-ascii?Q?KrIBRq1qfHnaLc7D4GHH1/4M6DEB9fZLW7L7dGBQGcrPvau5sEae5TsA7CO7?=
 =?us-ascii?Q?zHvCz5cGI4WWNMIX+e+qzb3ASnOdr10jNxHFqh2Vq5uYmcEqsTYTM5yfAJJB?=
 =?us-ascii?Q?4biK6zxudP5WiIyoC+/l9F1MhugFFaKsDdYAPw+PT33eM7ZnxqHYrf1eFF6p?=
 =?us-ascii?Q?2SPpNEm03icdoHGMzLYrd6dm+nvhuo7123bRz3VtLq1Md3tcic7RjXympu3g?=
 =?us-ascii?Q?ZMvaSGtYTOHA3rFk6Hp78nsz+oBhmy1Z84rkzYdj3+ssHIS0Nu5nIVPMqaM/?=
 =?us-ascii?Q?6c/26nxzEWKcqiXcoDa4bi9uBlF50DQjD2dBcjLy8mhpZi5rDa+vTW3+KWDJ?=
 =?us-ascii?Q?apS6aF9hi5E8Lcopl9YAJIeFGpGNJLGDYojkKEDdKXphABM3J8C5mEU9nchw?=
 =?us-ascii?Q?jCYOxsARi5AO0beuuvXqWrpvY33f0bhK3J8IcxWCqXToodDEm4eVmN3OTBto?=
 =?us-ascii?Q?KRecZN/gesGQmB2VdZheWL51oH7ksxr8aSgeMgoqRTc+NV3yRCEh1mW2Fqg5?=
 =?us-ascii?Q?RyvqPpGAG//3e2iO7WBwGiMR54oPCYp+Uj/k7SXlfWqb6yABhAWXox4btDI3?=
 =?us-ascii?Q?j3HyP/K1C5cl/4CqAE9pV73wQENLaxrnBm8bQKo2AJ3JgByas2E6L9TNyenp?=
 =?us-ascii?Q?2+qsqcYHG6IHrFdRHmADopFJ9754MWHK3xYlHgvn?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 04712176-8e25-4ecc-99f4-08dc10a6ba37
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jan 2024 00:05:45.7184
 (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: uc5IoAkvdNXW0EIgdh6hVmh+tY0uNks9dQmZ6arcsOJ3EIE2lOAGfturXaP9KLD5j3Lkwkgl8atg8niL/c8PMQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5005
X-OriginatorOrg: intel.com

On Mon, Jan 08, 2024 at 10:02:50AM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 08, 2024 at 02:02:57PM +0800, Yan Zhao wrote:
> > On Fri, Jan 05, 2024 at 03:55:51PM -0400, Jason Gunthorpe wrote:
> > > On Fri, Jan 05, 2024 at 05:12:37PM +0800, Yan Zhao wrote:
> > > > This series allow user space to notify KVM of noncoherent DMA status so as
> > > > to let KVM honor guest memory types in specified memory slot ranges.
> > > > 
> > > > Motivation
> > > > ===
> > > > A virtio GPU device may want to configure GPU hardware to work in
> > > > noncoherent mode, i.e. some of its DMAs do not snoop CPU caches.
> > > 
> > > Does this mean some DMA reads do not snoop the caches or does it
> > > include DMA writes not synchronizing the caches too?
> > Both DMA reads and writes are not snooped.
> 
> Oh that sounds really dangerous.
>
But the IOMMU for Intel GPU does not do force-snoop, no matter KVM
honors guest memory type or not.

> > > > This is generally for performance consideration.
> > > > In certain platform, GFX performance can improve 20+% with DMAs going to
> > > > noncoherent path.
> > > > 
> > > > This noncoherent DMA mode works in below sequence:
> > > > 1. Host backend driver programs hardware not to snoop memory of target
> > > >    DMA buffer.
> > > > 2. Host backend driver indicates guest frontend driver to program guest PAT
> > > >    to WC for target DMA buffer.
> > > > 3. Guest frontend driver writes to the DMA buffer without clflush stuffs.
> > > > 4. Hardware does noncoherent DMA to the target buffer.
> > > > 
> > > > In this noncoherent DMA mode, both guest and hardware regard a DMA buffer
> > > > as not cached. So, if KVM forces the effective memory type of this DMA
> > > > buffer to be WB, hardware DMA may read incorrect data and cause misc
> > > > failures.
> > > 
> > > I don't know all the details, but a big concern would be that the
> > > caches remain fully coherent with the underlying memory at any point
> > > where kvm decides to revoke the page from the VM.
> > Ah, you mean, for page migration, the content of the page may not be copied
> > correctly, right?
> 
> Not just migration. Any point where KVM revokes the page from the
> VM. Ie just tearing down the VM still has to make the cache coherent
> with physical or there may be problems.
Not sure what's the mentioned problem during KVM revoking.
In host,
- If the memory type is WB, as the case in intel GPU passthrough,
  the mismatch can only happen when guest memory type is UC/WC/WT/WP, all
  stronger than WB.
  So, even after KVM revoking the page, the host will not get delayed
  data from cache.
- If the memory type is WC, as the case in virtio GPU, after KVM revoking
  the page, the page is still hold in the virtio host side.
  Even though a incooperative guest can cause wrong data in the page,
  the guest can achieve the purpose in a more straight-forward way, i.e.
  writing a wrong data directly to the page.
  So, I don't see the problem in this case too.

>  
> > Currently in x86, we have 2 ways to let KVM honor guest memory types:
> > 1. through KVM memslot flag introduced in this series, for virtio GPUs, in
> >    memslot granularity.
> > 2. through increasing noncoherent dma count, as what's done in VFIO, for
> >    Intel GPU passthrough, for all guest memory.
> 
> And where does all this fixup the coherency problem?
> 
> > This page migration issue should not be the case for virtio GPU, as both host
> > and guest are synced to use the same memory type and actually the pages
> > are not anonymous pages.
> 
> The guest isn't required to do this so it can force the cache to
> become incoherent.
> 
> > > If you allow an incoherence of cache != physical then it opens a
> > > security attack where the observed content of memory can change when
> > > it should not.
> > 
> > In this case, will this security attack impact other guests?
> 
> It impacts the hypervisor potentially. It depends..
Could you elaborate more on how it will impact hypervisor?
We can try to fix it if it's really a case.