On Tue, 2023-11-21 at 18:02 +0000, Paul Durrant wrote: > From: Paul Durrant <pdurrant@xxxxxxxxxx> > > As noted in [1] the KVM_GUEST_USES_PFN usage flag is never set by any > callers of kvm_gpc_init(), which also makes the 'vcpu' argument redundant. > Moreover, all existing callers specify KVM_HOST_USES_PFN so the usage > check in hva_to_pfn_retry() and hence the 'usage' argument to > kvm_gpc_init() are also redundant. > Remove the pfn_cache_usage enumeration and remove the redundant arguments, > fields of struct gfn_to_hva_cache, and all the related code. > > [1] https://lore.kernel.org/all/ZQiR8IpqOZrOpzHC@xxxxxxxxxx/ > > Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx> I think it's https://lore.kernel.org/all/ZBEEQtmtNPaEqU1i@xxxxxxxxxx/ which is the key reference. I'm not sure I'm 100% on board, but I never got round to replying to Sean's email because it was one of those "put up or shut up situations" and I didn't have the bandwidth to actually write the code to prove my point. I think it *is* important to support non-pinned pages. There's a reason we even made the vapic page migratable. We want to support memory hotplug, we want to cope with machine checks telling us to move certain pages (which I suppose is memory hotplug). See commit 38b9917350cb ("kvm: vmx: Implement set_apic_access_page_addr") for example. I agree that in the first round of the nVMX code there were bugs. And sure, of *course* it isn't sufficient to wire up the invalidation without either a KVM_REQ_SOMETHIMG to put it back, or just a *check* on the corresponding gpc on the way back into the guest. We'd have worked that out. And yes, the gpc has had bugs as we implemented it, but the point was that we got to something which *is* working, and forms a usable building block. So I'm not really sold on the idea of ditching KVM_GUEST_USES_PFN. I think we could get it working, and I think it's worth it. But my opinion is worth very little unless I express it in 'diff -up' form instead of prose, and reverting this particular patch is the least of my barriers to doing so, so reluctantly... Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
