[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v3 6/8] admin: Add theory of operation for write recording commands
> From: Jason Wang <jasowang@redhat.com> > Sent: Monday, November 13, 2023 9:11 AM > > On Fri, Nov 10, 2023 at 2:46âPM Parav Pandit <parav@nvidia.com> wrote: > > > > Hi Michael, > > > > > From: Michael S. Tsirkin <mst@redhat.com> > > > Sent: Thursday, November 9, 2023 1:29 PM > > > > [..] > > > > Besides the issue of performance, it's also racy, assuming we are > > > > logging > > > IOVA. > > > > > > > > 0) device log IOVA > > > > 1) hypervisor fetches IOVA from log buffer > > > > 2) guest map IOVA to a new GPA > > > > 3) hypervisor traverse guest table to get IOVA to new GPA > > > > > > > > Then we lost the old GPA. > > > > > > Interesting and a good point. And by the way e.g. vhost has the same > > > issue. You need to flush dirty tracking info when changing the > > > mappings somehow. Parav what's the plan for this? Should be addressed in > the spec too. > > > > > As you listed the flush is needed for vhost or device-based DPT. > > What does DPT mean? Device Page Table? Let's not invent terminology which is > not known by others please. > Sorry for using the acronym. I meant dirty page tracking. > We have discussed it many times. You can't just depend on ATS or reinventing > wheels in virtio. The dependency is on the iommu which would have the mapping of GIOVA to GPA like any sw implementation. No dependency on ATS. > > What's more, please try not to give me the impression that the proposal is > optimized for a specific vendor (like device IOMMU stuffs). > You should stop calling this specific vendor thing. One can equally say that suspend bit proposal is for the sw_vendor device who is forcing virtio hw device to only implement ioqueues + PASID + non_unified interface for PF, VF, SIOVs + non_TDISP based devices. > > The necessary plumbing is already covered for this in the query (read and > clear) command of this v3 proposal. > > The issue is logging via IOVA ... I don't see how "read and clear" can help. > Read and clear helps that ensures that all the dirty pages are reported, hence there is no mapping/unmapping race. As everything is reported. > > It is listed in Device Write Records Read Command. > > Please explain how your proposal can solve the above race. > In below manner. 1. guest has GIOVA to GPA_1 mapping 2. RX packets occurred to GIOVA 3. device reported dirty page log for GIOVA (hypervisor is yet to read) 4. guest requested mapping change from GIOVA to GPA_2 4.1 During this IOTLB is invalidated and dirty page report is queried ensuring, it can change the mapping > > > > When the page write record is fully read, it is flushed. > > How/when to use, I think its hypervisor specific, so we probably better off not > documenting those details. > > Well, as the author of this proposal, at least you need to know how a hypervisor > can work with your proposal, no? > Likely yes, but it is not the scope of the spec to list those paths etc. > > May be such read is needed in some other path too depending on how > hypervisor implemented. > > What do you mean by "May be ... some other path" here? You're inventing a > mechanism that you don't know how a hypervisor can use? No. I meant hypervisor may have more operations that map/unmap/flush where it may need to implement it. Some one may call it set_map(), some may say dma_map()...
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]