On Tue, Dec 19, 2023 at 12:26:45PM +0800, Yan Zhao wrote:
> On Mon, Dec 18, 2023 at 07:08:51AM -0800, Sean Christopherson wrote:
> > > > Implementation Consideration
> > > > ===
> > > > There is a previous series [1] from google to serve the same purpose to
> > > > let KVM be aware of virtio GPU's noncoherent DMA status. That series
> > > > requires a new memslot flag, and special memslots in user space.
> > > >
> > > > We don't choose to use memslot flag to request honoring guest memory
> > > > type.
> > >
> > > memslot flag has the potential to restrict the impact e.g. when using
> > > clflush-before-read in migration?
> >
> > Yep, exactly. E.g. if KVM needs to ensure coherency when freeing memory back to
> > the host kernel, then the memslot flag will allow for a much more targeted
> > operation.
> >
> > > Of course the implication is to honor guest type only for the selected slot
> > > in KVM instead of applying to the entire guest memory as in previous series
> > > (which selects this way because vmx_get_mt_mask() is in perf-critical path
> > > hence not good to check memslot flag?)
> >
> > Checking a memslot flag won't impact performance. KVM already has the memslot
> > when creating SPTEs, e.g. the sole caller of vmx_get_mt_mask(), make_spte(), has
> > access to the memslot.
> >
> > That isn't coincidental, KVM _must_ have the memslot to construct the SPTE, e.g.
> > to retrieve the associated PFN, update write-tracking for shadow pages, etc.
> >
> Hi Sean,
> Do you prefer to introduce a memslot flag KVM_MEM_DMA or KVM_MEM_WC?
> For KVM_MEM_DMA, KVM needs to
> (a) search VMA for vma->vm_page_prot and convert it to page cache mode (with
> pgprot2cachemode()? ), or
> (b) look up memtype of the PFN, by calling lookup_memtype(), similar to that in
> pat_pfn_immune_to_uc_mtrr().
>
> But pgprot2cachemode() and lookup_memtype() are not exported by x86 code now.
>
> For KVM_MEM_WC, it requires user to ensure the memory is actually mapped
> to WC, right?
>
> Then, vmx_get_mt_mask() just ignores guest PAT and programs host PAT as EPT type
> for the special memslot only, as below.
> Is this understanding correct?
>
> static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
> {
> if (is_mmio)
> return MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT;
>
> if (gfn_in_dma_slot(vcpu->kvm, gfn)) {
> u8 type = MTRR_TYPE_WRCOMB;
> //u8 type = pat_pfn_memtype(pfn);
> return (type << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;
> }
>
> if (!kvm_arch_has_noncoherent_dma(vcpu->kvm))
> return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;
>
> if (kvm_read_cr0_bits(vcpu, X86_CR0_CD)) {
> if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_CD_NW_CLEARED))
> return MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT;
> else
> return (MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT) |
> VMX_EPT_IPAT_BIT;
> }
>
> return kvm_mtrr_get_guest_memory_type(vcpu, gfn) << VMX_EPT_MT_EPTE_SHIFT;
> }
>
> BTW, since the special memslot must be exposed to guest as virtio GPU BAR in
> order to prevent other guest drivers from access, I wonder if it's better to
> include some keyword like VIRTIO_GPU_BAR in memslot flag name.
Another choice is to add a memslot flag KVM_MEM_HONOR_GUEST_PAT, then user
(e.g. QEMU) does special treatment to this kind of memslots (e.g. skipping
reading/writing to them in general paths).
@@ -7589,26 +7589,29 @@ static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
if (is_mmio)
return MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT;
+ if (in_slot_honor_guest_pat(vcpu->kvm, gfn))
+ return kvm_mtrr_get_guest_memory_type(vcpu, gfn) << VMX_EPT_MT_EPTE_SHIFT;
+
if (!kvm_arch_has_noncoherent_dma(vcpu->kvm))
return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;
if (kvm_read_cr0_bits(vcpu, X86_CR0_CD)) {
if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_CD_NW_CLEARED))
return MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT;
else
return (MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT) |
VMX_EPT_IPAT_BIT;
}
return kvm_mtrr_get_guest_memory_type(vcpu, gfn) << VMX_EPT_MT_EPTE_SHIFT;
}
