Found workaround/fix for ntp on AMD systems with PCI passthrough

Found workaround/fix for ntp on AMD systems with PCI passthrough

[geoff]

Hi All,

I have spent the last few days attempting to track down the cause of the
performance degradation when ntp is enabled on AMD-Vi systems and VGA
pass through.

A few hours ago I identified why Intel is working and AMD is not and 
found a
work around that fixes the problem, I am now seeing near native 
performance
with ntp enabled on an AMD Ryzen platform.

https://www.3dmark.com/3dm/22878932
https://www.3dmark.com/3dm/22879024

Since my understanding of hardware at this level on this platform is 
limited it
needs a guru to take this and implement a fix or perhaps even a quirk 
that can
be enabled to work around this problem.

In svm.c, by just changing the line in `init_vmcb` that reads:

    save->g_pat = svm->vcpu.arch.pat;

To:

    save->g_pat = 0x0606060606060606;

The problem is resolved. From what I understand this is setting a MTTR 
value
that enables Write Back (WB). Since finding this gem I found the 
following patch
set also:

https://patchwork.kernel.org/patch/6748441/

This doesn't fix the problem but it seems to be heading in the right 
direction.

-Geoff

Re: Found workaround/fix for ntp on AMD systems with PCI passthrough

[Paolo Bonzini]

On 24/10/2017 23:50, geoff@hostfission.com wrote:
> In svm.c, by just changing the line in `init_vmcb` that reads:
> 
>    save->g_pat = svm->vcpu.arch.pat;
> 
> To:
> 
>    save->g_pat = 0x0606060606060606;
> 
> The problem is resolved. From what I understand this is setting a
> MTTR value that enables Write Back (WB).

That's cool, you certainly are onto something.  Currently, SVM is
disregarding the guest PAT setting (PA0=PA4=WB, PA1=PA5=WT, PA2=PA6=UC-,
PA3=UC).  The guest might be using a different setting so you're
getting slow accesses (UC- or UC, i.e. uncacheable) instead of fast
accesses (WB or WC, respectively writeback and write combining).

It would be great if you could proceed with the following tests:

1) see if this patch has any effect

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index af256b786a70..b2e4b912f053 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3626,6 +3626,12 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	u32 ecx = msr->index;
 	u64 data = msr->data;
 	switch (ecx) {
+	case MSR_IA32_CR_PAT:
+		if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
+			return 1;
+		vcpu->arch.pat = data;
+		svm->vmcb->save.g_pat = data;
+		break;
 	case MSR_IA32_TSC:
 		kvm_write_tsc(vcpu, msr);
 		break;

2) if it doesn't, add a printk("%#016lx", data); to the patch and get the
last value written by the guest.  Hard-code it in the "save->g_pat = ..."
line where you've been using 0x0606060606060606 successfully.  Test that
things work (though they should still be slow).

3) starting from the rightmost byte, change one byte to 0x06, test that
and see if things get fast.  For each byte you change, take a note of the
full value and whether things are slow or fast.

Thank you very much!

Paolo

Re: Found workaround/fix for ntp on AMD systems with PCI passthrough

[Radim Krčmář]

2017-10-25 07:42+0200, Paolo Bonzini:
> On 24/10/2017 23:50, geoff@hostfission.com wrote:
> > In svm.c, by just changing the line in `init_vmcb` that reads:
> > 
> >    save->g_pat = svm->vcpu.arch.pat;
> > 
> > To:
> > 
> >    save->g_pat = 0x0606060606060606;
> > 
> > The problem is resolved. From what I understand this is setting a
> > MTTR value that enables Write Back (WB).
> 
> That's cool, you certainly are onto something.  Currently, SVM is
> disregarding the guest PAT setting (PA0=PA4=WB, PA1=PA5=WT, PA2=PA6=UC-,
> PA3=UC).  The guest might be using a different setting so you're
> getting slow accesses (UC- or UC, i.e. uncacheable) instead of fast
> accesses (WB or WC, respectively writeback and write combining).
> 
> It would be great if you could proceed with the following tests:
> 
> 1) see if this patch has any effect
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index af256b786a70..b2e4b912f053 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3626,6 +3626,12 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  	u32 ecx = msr->index;
>  	u64 data = msr->data;
>  	switch (ecx) {
> +	case MSR_IA32_CR_PAT:
> +		if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
> +			return 1;
> +		vcpu->arch.pat = data;
> +		svm->vmcb->save.g_pat = data;

Great progress!  SVM might cache the value and adding

+		mark_dirty(svm->vmcb, VMCB_NPT);

here should result in the same behavior as doing (2).

> +		break;
>  	case MSR_IA32_TSC:
>  		kvm_write_tsc(vcpu, msr);
>  		break;
> 
> 2) if it doesn't, add a printk("%#016lx", data); to the patch and get the
> last value written by the guest.  Hard-code it in the "save->g_pat = ..."
> line where you've been using 0x0606060606060606 successfully.  Test that
> things work (though they should still be slow).
> 
> 3) starting from the rightmost byte, change one byte to 0x06, test that
> and see if things get fast.  For each byte you change, take a note of the
> full value and whether things are slow or fast.
> 
> Thank you very much!
> 
> Paolo

Re: Found workaround/fix for ntp on AMD systems with PCI passthrough

[geoff]

On 2017-10-25 16:42, Paolo Bonzini wrote:
> On 24/10/2017 23:50, geoff@hostfission.com wrote:
>> In svm.c, by just changing the line in `init_vmcb` that reads:
>> 
>>    save->g_pat = svm->vcpu.arch.pat;
>> 
>> To:
>> 
>>    save->g_pat = 0x0606060606060606;
>> 
>> The problem is resolved. From what I understand this is setting a
>> MTTR value that enables Write Back (WB).
> 
> That's cool, you certainly are onto something.  Currently, SVM is
> disregarding the guest PAT setting (PA0=PA4=WB, PA1=PA5=WT, 
> PA2=PA6=UC-,
> PA3=UC).  The guest might be using a different setting so you're
> getting slow accesses (UC- or UC, i.e. uncacheable) instead of fast
> accesses (WB or WC, respectively writeback and write combining).
> 
> It would be great if you could proceed with the following tests:
> 
> 1) see if this patch has any effect
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index af256b786a70..b2e4b912f053 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3626,6 +3626,12 @@ static int svm_set_msr(struct kvm_vcpu *vcpu,
> struct msr_data *msr)
>  	u32 ecx = msr->index;
>  	u64 data = msr->data;
>  	switch (ecx) {
> +	case MSR_IA32_CR_PAT:
> +		if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
> +			return 1;
> +		vcpu->arch.pat = data;
> +		svm->vmcb->save.g_pat = data;
> +		break;
>  	case MSR_IA32_TSC:
>  		kvm_write_tsc(vcpu, msr);
>  		break;
> 

Confirmed! this has corrected the fault without the need to hard code 
the
value.

> 2) if it doesn't, add a printk("%#016lx", data); to the patch and get 
> the
> last value written by the guest.  Hard-code it in the "save->g_pat = 
> ..."
> line where you've been using 0x0606060606060606 successfully.  Test 
> that
> things work (though they should still be slow).
> 
> 3) starting from the rightmost byte, change one byte to 0x06, test that
> and see if things get fast.  For each byte you change, take a note of 
> the
> full value and whether things are slow or fast.
> 
> Thank you very much!
> 
> Paolo