Re: [PATCH] KVM: x86/mmu: treat WC memory as MMIO

Re: [PATCH] KVM: x86/mmu: treat WC memory as MMIO

[francisco flynn]

On 2024/3/12 00:20, Sean Christopherson wrote:
> On Mon, Mar 11, 2024, francisco_flynn wrote:
>> when doing kvm_tdp_mmu_map for WC memory, such as pages
>> allocated by amdgpu ttm driver for ttm_write_combined
>> caching mode(e.g. host coherent in vulkan),
>> the spte would be set to WB, in this case, vcpu write
>> to these pages would goes to cache first, and never
>> be write-combined and host-coherent anymore. so
>> WC memory should be treated as MMIO, and the effective
>> memory type is depending on guest PAT.
> 
> No, the effective memtype is not fully guest controlled.  By forcing the EPT memtype
> to UC, the guest can only use UC or WC.  I don't know if there's a use case for

Well,it's actually the host mapping memory WC and guest uses WC,
one use case is virtio-gpu host blob, which is to map physical GPU buffers into guest

> the host mapping memory WC while the guest uses WB, but it should be a moot point,
> because this this series should do what you want (allow guest to map GPU buffers
> as WC).
> 
> https://lore.kernel.org/all/20240309010929.1403984-1-seanjc@google.com
> 

yes, this is what i want, but for virtio-gpu device, if we mapping WC typed 
GPU buffer into guest, kvm_arch_has_noncoherent_dma would return false, 
so on cpu without self-snoop support, guest PAT will be ignored, the effective
memory type would be set to WB, causing data inconsistency.

+	if (!static_cpu_has(X86_FEATURE_SELFSNOOP) &&
+	    !kvm_arch_has_noncoherent_dma(vcpu->kvm))
                return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;


>> Signed-off-by: francisco_flynn <francisco_flynn@foxmail.com>
>> ---
>>  arch/x86/include/asm/memtype.h | 2 ++
>>  arch/x86/kvm/mmu/spte.c        | 5 +++--
> 
> Please use get_maintainers.pl.

sure.

> 
>>  arch/x86/mm/pat/memtype.c      | 8 ++++++++
>>  3 files changed, 13 insertions(+), 2 deletions(-)

Re: [PATCH] KVM: x86/mmu: treat WC memory as MMIO

[Sean Christopherson]

On Tue, Mar 12, 2024, francisco flynn wrote:
> On 2024/3/12 00:20, Sean Christopherson wrote:
> > On Mon, Mar 11, 2024, francisco_flynn wrote:
> >> when doing kvm_tdp_mmu_map for WC memory, such as pages
> >> allocated by amdgpu ttm driver for ttm_write_combined
> >> caching mode(e.g. host coherent in vulkan),
> >> the spte would be set to WB, in this case, vcpu write
> >> to these pages would goes to cache first, and never
> >> be write-combined and host-coherent anymore. so
> >> WC memory should be treated as MMIO, and the effective
> >> memory type is depending on guest PAT.
> > 
> > No, the effective memtype is not fully guest controlled.  By forcing the EPT memtype
> > to UC, the guest can only use UC or WC.  I don't know if there's a use case for
> 
> Well,it's actually the host mapping memory WC and guest uses WC,

No, when the guest is running, the host, i.e. KVM, sets the EPT memory type to UC

  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;

which effectively makes the guest "MTRR" memtype UC, and thus restricts the guest
to using UC or WC.

Your use case wants to map the memory as WC in the guest, but there are zero
guarantees that *every* use case wants to access such memory as WC (or UC),
i.e. forcing UC could cause performance regressions for existing use cases.

Ideally, KVM would force the EPT memtype to match the host PAT memtype while still
honoring guest PAT, but if we wanted to go that route, then KVM should (a) stuff
the exact memtype, (b) stuff the memtype for all of guest memory, and (c) do so
for all flavors of KVM on x86, not just EPT on VMX.

Unfortunatley, making that change now risks breaking 15+ years of KVM ABI.  And
there's also the whole "unsafe on Intel CPUs without self-snoop" problem.

> one use case is virtio-gpu host blob, which is to map physical GPU buffers into guest
> 
> > the host mapping memory WC while the guest uses WB, but it should be a moot point,
> > because this this series should do what you want (allow guest to map GPU buffers
> > as WC).
> > 
> > https://lore.kernel.org/all/20240309010929.1403984-1-seanjc@google.com
> > 
> 
> yes, this is what i want, but for virtio-gpu device, if we mapping WC typed 
> GPU buffer into guest, kvm_arch_has_noncoherent_dma would return false, 
> so on cpu without self-snoop support, guest PAT will be ignored, the effective
> memory type would be set to WB, causing data inconsistency.

My understanding is that every Intel CPU released in the last 8+ years supports
self-snoop.  See check_memory_type_self_snoop_errata().

IMO, that's a perfectly reasonable line to draw: if you want virtio-gpu support,
upgrade to Ivy Bridge or later.

Re: [PATCH] KVM: x86/mmu: treat WC memory as MMIO

[francisco flynn]

On 2024/3/12 23:21, Sean Christopherson wrote:
> On Tue, Mar 12, 2024, francisco flynn wrote:
>> On 2024/3/12 00:20, Sean Christopherson wrote:
>>> On Mon, Mar 11, 2024, francisco_flynn wrote:
>>>> when doing kvm_tdp_mmu_map for WC memory, such as pages
>>>> allocated by amdgpu ttm driver for ttm_write_combined
>>>> caching mode(e.g. host coherent in vulkan),
>>>> the spte would be set to WB, in this case, vcpu write
>>>> to these pages would goes to cache first, and never
>>>> be write-combined and host-coherent anymore. so
>>>> WC memory should be treated as MMIO, and the effective
>>>> memory type is depending on guest PAT.
>>>
>>> No, the effective memtype is not fully guest controlled.  By forcing the EPT memtype
>>> to UC, the guest can only use UC or WC.  I don't know if there's a use case for
>>
>> Well,it's actually the host mapping memory WC and guest uses WC,
> 
> No, when the guest is running, the host, i.e. KVM, sets the EPT memory type to UC
> 
>   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;
> 
> which effectively makes the guest "MTRR" memtype UC, and thus restricts the guest
> to using UC or WC.
> 
> Your use case wants to map the memory as WC in the guest, but there are zero
> guarantees that *every* use case wants to access such memory as WC (or UC),
> i.e. forcing UC could cause performance regressions for existing use cases.
> 

yes, this may cause performance regressions in some cases.

> Ideally, KVM would force the EPT memtype to match the host PAT memtype while still
> honoring guest PAT, but if we wanted to go that route, then KVM should (a) stuff
> the exact memtype, (b) stuff the memtype for all of guest memory, and (c) do so
> for all flavors of KVM on x86, not just EPT on VMX.
> 

it's true.

> Unfortunatley, making that change now risks breaking 15+ years of KVM ABI.  And
> there's also the whole "unsafe on Intel CPUs without self-snoop" problem.
> 
>> one use case is virtio-gpu host blob, which is to map physical GPU buffers into guest
>>
>>> the host mapping memory WC while the guest uses WB, but it should be a moot point,
>>> because this this series should do what you want (allow guest to map GPU buffers
>>> as WC).
>>>
>>> https://lore.kernel.org/all/20240309010929.1403984-1-seanjc@google.com
>>>
>>
>> yes, this is what i want, but for virtio-gpu device, if we mapping WC typed 
>> GPU buffer into guest, kvm_arch_has_noncoherent_dma would return false, 
>> so on cpu without self-snoop support, guest PAT will be ignored, the effective
>> memory type would be set to WB, causing data inconsistency.
> 
> My understanding is that every Intel CPU released in the last 8+ years supports
> self-snoop.  See check_memory_type_self_snoop_errata().
> 
> IMO, that's a perfectly reasonable line to draw: if you want virtio-gpu support,
> upgrade to Ivy Bridge or later.

it make sense.