KVM: How does is PAT emulation supposed to work?

KVM: How does is PAT emulation supposed to work?

[Jan Kiszka]

Hi all,

while digging into the PAT topic for Jailhouse, I also wondered how KVM
deals with it. And I'm still not getting it complete - or there is a bug:

KVM intercepts all guest writes to the PAT MSR and instead keeps the
guest value in vcpu->arch.pat. But, besides returning that value back on
read accesses, arch.pat has no other purpose.

On Intel, we only seem to have proper emulation - through hardware -
when VMX supports PAT switching (see vmx_set_msr). On AMD, the situation
is even worse as the g_pat save field is not updated at all on PAT
writes. That seems to be a low hanging fruit to bring svm on the same
support level as vmx.

Or am I missing something?

Jan

PS: If someone has a good idea for a simple test case on machines
without IOMMU (like my current boxes), thus without a chance to use
device pass-through to stress guest PAT settings, I would be all ears.

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

[RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

When hardware supports the g_pat VMCB field, we can use it for emulating
the PAT configuration that the guest configures by writing to the
corresponding MSR.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

RFC because it is only compile-tested.

 arch/x86/kvm/svm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ce741b8..9439c6c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3245,6 +3245,15 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	case MSR_VM_IGNNE:
 		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
 		break;
+	case MSR_IA32_CR_PAT:
+		if (npt_enabled) {
+			if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
+				return 1;
+			svm->vmcb->save.g_pat = data;
+			vcpu->arch.pat = data;
+			break;
+		}
+		/* fall through */
 	default:
 		return kvm_set_msr_common(vcpu, msr);
 	}
-- 
2.1.4

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-13 08:58+0200, Jan Kiszka:
> When hardware supports the g_pat VMCB field, we can use it for emulating
> the PAT configuration that the guest configures by writing to the
> corresponding MSR.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-13 08:58+0200, Jan Kiszka:
> When hardware supports the g_pat VMCB field, we can use it for emulating
> the PAT configuration that the guest configures by writing to the
> corresponding MSR.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> RFC because it is only compile-tested.
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> @@ -3245,6 +3245,15 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  	case MSR_VM_IGNNE:
>  		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
>  		break;
> +	case MSR_IA32_CR_PAT:
> +		if (npt_enabled) {
> +			if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
> +				return 1;
> +			svm->vmcb->save.g_pat = data;
> +			vcpu->arch.pat = data;

Disregarding my Reviewed-by, the code is missing:

  mark_dirty(svm->vmcb, VMCB_NPT);

Also,

Tested-by: Radim Krčmář <rkrcmar@redhat.com>

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-20 18:14+0200, Radim Krčmář:
> Tested-by: Radim Krčmář <rkrcmar@redhat.com>

Uncached accesses were roughly 20x slower.
In case anyone wanted to reproduce, I used this as a kvm-unit-test:

---
#include "processor.h"

#define NR_TOP_LOOPS 24
#define NR_MEM_LOOPS 10
#define MEM_ELEMENTS 1024

static volatile u64 pat_test_memory[MEM_ELEMENTS];

static void flush_tlb(void)
{
	write_cr3(read_cr3());
}

static void set_pat(u64 val)
{
	wrmsr(0x277, val);
	flush_tlb();

}

static u64 time_memory_accesses(void)
{
	u64 tsc_before = rdtsc();

	for (unsigned loop = 0; loop < NR_MEM_LOOPS; loop++)
		for (unsigned i = 0; i < MEM_ELEMENTS; i++)
			pat_test_memory[i]++;

	return rdtsc() - tsc_before;
}

int main(int argc, char **argv)
{
	unsigned error = 0;

	for (unsigned loop = 0; loop < NR_TOP_LOOPS; loop++) {
		u64 time_uc, time_wb;

		set_pat(0);
		time_uc = time_memory_accesses();

		set_pat(0x0606060606060606ULL);
		time_wb = time_memory_accesses();

		if (time_uc < time_wb * 4)
			error++;

		printf("%02d uc: %10lld wb: %8lld\n", loop, time_uc, time_wb);
	}

	report("guest PAT", !error);

	return report_summary();
}

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

On 2015-04-20 19:16, Radim Krčmář wrote:
> 2015-04-20 18:14+0200, Radim Krčmář:
>> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
> 
> Uncached accesses were roughly 20x slower.
> In case anyone wanted to reproduce, I used this as a kvm-unit-test:
> 
> ---
> #include "processor.h"
> 
> #define NR_TOP_LOOPS 24
> #define NR_MEM_LOOPS 10
> #define MEM_ELEMENTS 1024
> 
> static volatile u64 pat_test_memory[MEM_ELEMENTS];
> 
> static void flush_tlb(void)
> {
> 	write_cr3(read_cr3());
> }
> 
> static void set_pat(u64 val)
> {
> 	wrmsr(0x277, val);
> 	flush_tlb();
> 
> }
> 
> static u64 time_memory_accesses(void)
> {
> 	u64 tsc_before = rdtsc();
> 
> 	for (unsigned loop = 0; loop < NR_MEM_LOOPS; loop++)
> 		for (unsigned i = 0; i < MEM_ELEMENTS; i++)
> 			pat_test_memory[i]++;
> 
> 	return rdtsc() - tsc_before;
> }
> 
> int main(int argc, char **argv)
> {
> 	unsigned error = 0;
> 
> 	for (unsigned loop = 0; loop < NR_TOP_LOOPS; loop++) {
> 		u64 time_uc, time_wb;
> 
> 		set_pat(0);
> 		time_uc = time_memory_accesses();
> 
> 		set_pat(0x0606060606060606ULL);
> 		time_wb = time_memory_accesses();
> 
> 		if (time_uc < time_wb * 4)
> 			error++;
> 
> 		printf("%02d uc: %10lld wb: %8lld\n", loop, time_uc, time_wb);
> 	}
> 
> 	report("guest PAT", !error);
> 
> 	return report_summary();
> }
> 

Great, thanks. Will you push it to the unit tests? Could raise
motivations to fix the !NPT/EPT case.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-20 19:21+0200, Jan Kiszka:
> On 2015-04-20 19:16, Radim Krčmář wrote:
> > 2015-04-20 18:14+0200, Radim Krčmář:
> >> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
> > 
> > Uncached accesses were roughly 20x slower.
> > In case anyone wanted to reproduce, I used this as a kvm-unit-test:
> > 
> > ---
| [code]
> 
> Great, thanks. Will you push it to the unit tests? Could raise
> motivations to fix the !NPT/EPT case.

It can't be included in `run_tests.sh`, because we intenionally ignore
PAT for normal RAM on VMX and the test does "fail" ...

I'll think how to make the test use fool-proof first, and also look how
to fix the !NPT/EPT without affecting the case we care about too much.
(And if we can do a similar trick with NPT.)

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

On 2015-04-20 19:33, Radim Krčmář wrote:
> 2015-04-20 19:21+0200, Jan Kiszka:
>> On 2015-04-20 19:16, Radim Krčmář wrote:
>>> 2015-04-20 18:14+0200, Radim Krčmář:
>>>> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
>>>
>>> Uncached accesses were roughly 20x slower.
>>> In case anyone wanted to reproduce, I used this as a kvm-unit-test:
>>>
>>> ---
> | [code]
>>
>> Great, thanks. Will you push it to the unit tests? Could raise
>> motivations to fix the !NPT/EPT case.
> 
> It can't be included in `run_tests.sh`, because we intenionally ignore
> PAT for normal RAM on VMX and the test does "fail" ...

That ignoring is encoded into the EPT? Hmm... Maybe we can create a
ivshmem device and use that as test target.

> 
> I'll think how to make the test use fool-proof first, and also look how
> to fix the !NPT/EPT without affecting the case we care about too much.
> (And if we can do a similar trick with NPT.)
> 

OK.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

On 2015-04-20 19:37, Jan Kiszka wrote:
> On 2015-04-20 19:33, Radim Krčmář wrote:
>> 2015-04-20 19:21+0200, Jan Kiszka:
>>> On 2015-04-20 19:16, Radim Krčmář wrote:
>>>> 2015-04-20 18:14+0200, Radim Krčmář:
>>>>> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
>>>>
>>>> Uncached accesses were roughly 20x slower.
>>>> In case anyone wanted to reproduce, I used this as a kvm-unit-test:
>>>>
>>>> ---
>> | [code]
>>>
>>> Great, thanks. Will you push it to the unit tests? Could raise
>>> motivations to fix the !NPT/EPT case.
>>
>> It can't be included in `run_tests.sh`, because we intenionally ignore
>> PAT for normal RAM on VMX and the test does "fail" ...
> 
> That ignoring is encoded into the EPT? Hmm... Maybe we can create a
> ivshmem device and use that as test target.

And do you also know why is it ignored on Intel? Side effects on the host?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-20 19:45+0200, Jan Kiszka:
> On 2015-04-20 19:37, Jan Kiszka wrote:
> > On 2015-04-20 19:33, Radim Krčmář wrote:
> >> 2015-04-20 19:21+0200, Jan Kiszka:
> >>> On 2015-04-20 19:16, Radim Krčmář wrote:
> >>>> 2015-04-20 18:14+0200, Radim Krčmář:
> >>>>> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
> >>>>
> >>>> Uncached accesses were roughly 20x slower.
> >>>> In case anyone wanted to reproduce, I used this as a kvm-unit-test:
> >>>>
> >>>> ---
> >> | [code]
> >>>
> >>> Great, thanks. Will you push it to the unit tests? Could raise
> >>> motivations to fix the !NPT/EPT case.
> >>
> >> It can't be included in `run_tests.sh`, because we intenionally ignore
> >> PAT for normal RAM on VMX and the test does "fail" ...
> > 
> > That ignoring is encoded into the EPT?

Yes, it's the VMX_EPT_IPAT_BIT.

> And do you also know why is it ignored on Intel? Side effects on the host?

I think it is an optimization exclusive to Intel.
We know that the other side is not real hardware, which could avoid CPU
caches when accessing memory, so there is little reason to slow the
guest down.

> >                                        Hmm... Maybe we can create a
> > ivshmem device and use that as test target.

Good idea, thanks.
(Haven't used it yet, so its parts might be able to do what is needed
 without creating a dependency on the whole ivshmem system.)

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

On 2015-04-20 20:33, Radim Krčmář wrote:
> 2015-04-20 19:45+0200, Jan Kiszka:
>> On 2015-04-20 19:37, Jan Kiszka wrote:
>>> On 2015-04-20 19:33, Radim Krčmář wrote:
>>>> 2015-04-20 19:21+0200, Jan Kiszka:
>>>>> On 2015-04-20 19:16, Radim Krčmář wrote:
>>>>>> 2015-04-20 18:14+0200, Radim Krčmář:
>>>>>>> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
>>>>>>
>>>>>> Uncached accesses were roughly 20x slower.
>>>>>> In case anyone wanted to reproduce, I used this as a kvm-unit-test:
>>>>>>
>>>>>> ---
>>>> | [code]
>>>>>
>>>>> Great, thanks. Will you push it to the unit tests? Could raise
>>>>> motivations to fix the !NPT/EPT case.
>>>>
>>>> It can't be included in `run_tests.sh`, because we intenionally ignore
>>>> PAT for normal RAM on VMX and the test does "fail" ...
>>>
>>> That ignoring is encoded into the EPT?
> 
> Yes, it's the VMX_EPT_IPAT_BIT.
> 
>> And do you also know why is it ignored on Intel? Side effects on the host?
> 
> I think it is an optimization exclusive to Intel.
> We know that the other side is not real hardware, which could avoid CPU
> caches when accessing memory, so there is little reason to slow the
> guest down.

If the guest pushes data for DMA into RAM, it may assume that it lands
there directly, without the need for explicit flushes, because it has
caching disabled - no?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-20 20:41+0200, Jan Kiszka:
> On 2015-04-20 20:33, Radim Krčmář wrote:
> > 2015-04-20 19:45+0200, Jan Kiszka:
> >> On 2015-04-20 19:37, Jan Kiszka wrote:
> >>> On 2015-04-20 19:33, Radim Krčmář wrote:
> >>>> 2015-04-20 19:21+0200, Jan Kiszka:
> >>>>> On 2015-04-20 19:16, Radim Krčmář wrote:
> >>>>>> 2015-04-20 18:14+0200, Radim Krčmář:
> >>>>>>> Tested-by: Radim Krčmář <rkrcmar@redhat.com>
> >>>>>>
> >>>>>> Uncached accesses were roughly 20x slower.
> >>>>>> In case anyone wanted to reproduce, I used this as a kvm-unit-test:
> >>>>>>
> >>>>>> ---
> >>>> | [code]
> >>>>>
> >>>>> Great, thanks. Will you push it to the unit tests? Could raise
> >>>>> motivations to fix the !NPT/EPT case.
> >>>>
> >>>> It can't be included in `run_tests.sh`, because we intenionally ignore
> >>>> PAT for normal RAM on VMX and the test does "fail" ...
> >>>
> >>> That ignoring is encoded into the EPT?
> > 
> > Yes, it's the VMX_EPT_IPAT_BIT.
> > 
> >> And do you also know why is it ignored on Intel? Side effects on the host?
> > 
> > I think it is an optimization exclusive to Intel.
> > We know that the other side is not real hardware, which could avoid CPU
> > caches when accessing memory, so there is little reason to slow the
> > guest down.
> 
> If the guest pushes data for DMA into RAM, it may assume that it lands
> there directly, without the need for explicit flushes, because it has
> caching disabled - no?

Yes, the guest can't tell a difference.

In the presence of an assigned device, we always consider PAT;
with VFIO, ignoring PAT is decided by coherent DMA capability.
Emulated DMA requests are handled by the host, so the IPAT bit optimizes
cases where we know that caches are always right, but the guest doesn't.

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Paolo Bonzini]

On 20/04/2015 20:41, Jan Kiszka wrote:
> If the guest pushes data for DMA into RAM, it may assume that it lands
> there directly, without the need for explicit flushes, because it has
> caching disabled - no?

Yes, but Intel IOMMUs can have snooping control and in this case you can
just set memory to WB.

On Intel, KVM trusts the guest's PAT if the IOMMU is in use, and you do
not have an Intel IOMMU with snooping control.  In this case
kvm_arch_has_noncoherent_dma(vcpu->kvm) returns true.

The same should work for AMD, so you can set the gPAT:

- to the guest's value if kvm_arch_has_noncoherent_dma(vcpu->kvm), and
then you return cachemode2protval(kvm_get_guest_memory_type(...)) from
svm_get_mt_mask to layer the guest MTRRs on top of the guest PAT.

- otherwise, to all WB (0x0606060606060606), and then you can return
either 0 or _PAGE_NOCACHE from svm_get_mt_mask to achieve either UC (for
MMIO regions) or WB (for everything else).

To sum up you have:

               IOMMU?                      no IOMMU?
guest PAT      obeyed                      ignored (all WB)
guest MTRR     obeyed (svm_get_mt_mask)    ignored (UC if MMIO, else WB)
host PAT       always ignored              always ignored
host MTRR      always obeyed               always obeyed

I think that kvm_arch_has_noncoherent_dma() can be resampled, and gPAT
updated, in wbinvd_interception.

Paolo

Re: [RFC][PATCH] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-20 19:16+0200, Radim Krčmář:
> Uncached accesses were roughly 20x slower.

Sorry, a zero is missing there ... they were 200 times slower.

[PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

When hardware supports the g_pat VMCB field, we can use it for emulating
the PAT configuration that the guest configures by writing to the
corresponding MSR.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - add mark_dirty as found missing by Radim

 arch/x86/kvm/svm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ce741b8..68fdddc 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3245,6 +3245,16 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	case MSR_VM_IGNNE:
 		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
 		break;
+	case MSR_IA32_CR_PAT:
+		if (npt_enabled) {
+			if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
+				return 1;
+			svm->vmcb->save.g_pat = data;
+			mark_dirty(svm->vmcb, VMCB_NPT);
+			vcpu->arch.pat = data;
+			break;
+		}
+		/* fall through */
 	default:
 		return kvm_set_msr_common(vcpu, msr);
 	}
-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-20 19:25+0200, Jan Kiszka:
> When hardware supports the g_pat VMCB field, we can use it for emulating
> the PAT configuration that the guest configures by writing to the
> corresponding MSR.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Changes in v2:
>  - add mark_dirty as found missing by Radim

Thanks.

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Paolo Bonzini]

On 20/04/2015 19:25, Jan Kiszka wrote:
> When hardware supports the g_pat VMCB field, we can use it for emulating
> the PAT configuration that the guest configures by writing to the
> corresponding MSR.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

I'm not sure about this.  The problem is that, unlike Intel, AMD has no
way for the host to force its PAT value and ignore the guest's.  I'm
worried about potential performance problems in the guest.

This is not as bad as on ARM, because the guest cannot disable the cache
snooping protocol and thus cache coherency is guaranteed (see tables
7-10 and 15-20 in the AMD docs), but still I think I'd prefer having
some knob (module parameter) to enable/disable gPAT.  It's okay to make
it enabled by default.

Paolo

> ---
> 
> Changes in v2:
>  - add mark_dirty as found missing by Radim
> 
>  arch/x86/kvm/svm.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index ce741b8..68fdddc 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3245,6 +3245,16 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  	case MSR_VM_IGNNE:
>  		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
>  		break;
> +	case MSR_IA32_CR_PAT:
> +		if (npt_enabled) {
> +			if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
> +				return 1;
> +			svm->vmcb->save.g_pat = data;
> +			mark_dirty(svm->vmcb, VMCB_NPT);
> +			vcpu->arch.pat = data;
> +			break;
> +		}
> +		/* fall through */
>  	default:
>  		return kvm_set_msr_common(vcpu, msr);
>  	}

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

On 2015-04-21 13:09, Paolo Bonzini wrote:
> 
> 
> On 20/04/2015 19:25, Jan Kiszka wrote:
>> When hardware supports the g_pat VMCB field, we can use it for emulating
>> the PAT configuration that the guest configures by writing to the
>> corresponding MSR.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> I'm not sure about this.  The problem is that, unlike Intel, AMD has no
> way for the host to force its PAT value and ignore the guest's.  I'm
> worried about potential performance problems in the guest.

I think the guest needs to get what it requests - see my remark in
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/135271.

> 
> This is not as bad as on ARM, because the guest cannot disable the cache

You mean AMD, I guess.

> snooping protocol and thus cache coherency is guaranteed (see tables
> 7-10 and 15-20 in the AMD docs), but still I think I'd prefer having
> some knob (module parameter) to enable/disable gPAT.  It's okay to make
> it enabled by default.

I still don't get the scenario where we want to override the guest
settings. Maybe you can help out - would be valuable for the reasoning
in code or commit logs as well.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Paolo Bonzini]

On 21/04/2015 13:25, Jan Kiszka wrote:
> On 2015-04-21 13:09, Paolo Bonzini wrote:
>>
>>
>> On 20/04/2015 19:25, Jan Kiszka wrote:
>>> When hardware supports the g_pat VMCB field, we can use it for emulating
>>> the PAT configuration that the guest configures by writing to the
>>> corresponding MSR.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> I'm not sure about this.  The problem is that, unlike Intel, AMD has no
>> way for the host to force its PAT value and ignore the guest's.  I'm
>> worried about potential performance problems in the guest.
> 
> I think the guest needs to get what it requests - see my remark in
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/135271.
> 
>>
>> This is not as bad as on ARM, because the guest cannot disable the cache
> 
> You mean AMD, I guess.

No, I meant ARM. :)  On ARM the guest can even disable the cache
snooping protocol, making things particularly messy when QEMU accesses
the processor caches and the guest doesn't.  At least on AMD you cannot
do this.

>> snooping protocol and thus cache coherency is guaranteed (see tables
>> 7-10 and 15-20 in the AMD docs), but still I think I'd prefer having
>> some knob (module parameter) to enable/disable gPAT.  It's okay to make
>> it enabled by default.
> 
> I still don't get the scenario where we want to override the guest
> settings. Maybe you can help out - would be valuable for the reasoning
> in code or commit logs as well.

Basically it's an optimization.  The guest can set the UC memory type on
PCI BARs that are actually backed by RAM in QEMU, and then accesses to
these BARs will be unnecessarily slow.  It would be particularly bad if,
for example, access to ivshmem were slowed down because the guest PAT
says the memory is uncacheable.

Paolo

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

On 2015-04-21 13:32, Paolo Bonzini wrote:
> Basically it's an optimization.  The guest can set the UC memory type on
> PCI BARs that are actually backed by RAM in QEMU, and then accesses to
> these BARs will be unnecessarily slow.  It would be particularly bad if,
> for example, access to ivshmem were slowed down because the guest PAT
> says the memory is uncacheable.

ivshmem is pv anyway - why shouldn't the guest driver take this room for
optimization into account and ask for a cached mapping?

Is that that only use case?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Paolo Bonzini]

On 21/04/2015 13:56, Jan Kiszka wrote:
> > Basically it's an optimization.  The guest can set the UC memory type on
> > PCI BARs that are actually backed by RAM in QEMU, and then accesses to
> > these BARs will be unnecessarily slow.  It would be particularly bad if,
> > for example, access to ivshmem were slowed down because the guest PAT
> > says the memory is uncacheable.
> 
> ivshmem is pv anyway - why shouldn't the guest driver take this room for
> optimization into account and ask for a cached mapping?
> 
> Is that that only use case?

I guess a frame buffer would be affected as well, though probably the
guest would set it to WC so it's less bad.

Paolo

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Radim Krčmář]

2015-04-21 13:09+0200, Paolo Bonzini:
> 
> 
> On 20/04/2015 19:25, Jan Kiszka wrote:
> > When hardware supports the g_pat VMCB field, we can use it for emulating
> > the PAT configuration that the guest configures by writing to the
> > corresponding MSR.
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> I'm not sure about this.  The problem is that, unlike Intel, AMD has no
> way for the host to force its PAT value and ignore the guest's.  I'm
> worried about potential performance problems in the guest.

We already set g_pat to 0x0007040600070406ULL in init_vmcb().
This patch uses caching that the guest expects, which might improve
performance as well.  I think it's a step in right direction even if we
somehow optimize cache coherent cases later.

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On 2015-04-21 14:21, Radim Krčmář wrote:
> 2015-04-21 13:09+0200, Paolo Bonzini:
>>
>>
>> On 20/04/2015 19:25, Jan Kiszka wrote:
>>> When hardware supports the g_pat VMCB field, we can use it for emulating
>>> the PAT configuration that the guest configures by writing to the
>>> corresponding MSR.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> I'm not sure about this.  The problem is that, unlike Intel, AMD has no
>> way for the host to force its PAT value and ignore the guest's.  I'm
>> worried about potential performance problems in the guest.
> 
> We already set g_pat to 0x0007040600070406ULL in init_vmcb().
> This patch uses caching that the guest expects, which might improve
> performance as well.  I think it's a step in right direction even if we
> somehow optimize cache coherent cases later.

This topic is still open - and the patch still applies.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On 2015-05-24 17:28, Jan Kiszka wrote:
> On 2015-04-21 14:21, Radim Krčmář wrote:
>> 2015-04-21 13:09+0200, Paolo Bonzini:
>>>
>>>
>>> On 20/04/2015 19:25, Jan Kiszka wrote:
>>>> When hardware supports the g_pat VMCB field, we can use it for emulating
>>>> the PAT configuration that the guest configures by writing to the
>>>> corresponding MSR.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> I'm not sure about this.  The problem is that, unlike Intel, AMD has no
>>> way for the host to force its PAT value and ignore the guest's.  I'm
>>> worried about potential performance problems in the guest.
>>
>> We already set g_pat to 0x0007040600070406ULL in init_vmcb().
>> This patch uses caching that the guest expects, which might improve
>> performance as well.  I think it's a step in right direction even if we
>> somehow optimize cache coherent cases later.
> 
> This topic is still open - and the patch still applies.

Just rebased by kvm patch queue for some new entries - in this old one
is still there. What can we do about it?

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: KVM: How does is PAT emulation supposed to work?

[Radim Krčmář]

2015-04-13 07:16+0200, Jan Kiszka:
> Hi all,
> 
> while digging into the PAT topic for Jailhouse, I also wondered how KVM
> deals with it. And I'm still not getting it complete - or there is a bug:
> 
> KVM intercepts all guest writes to the PAT MSR and instead keeps the
> guest value in vcpu->arch.pat. But, besides returning that value back on
> read accesses, arch.pat has no other purpose.
> 
> On Intel, we only seem to have proper emulation - through hardware -
> when VMX supports PAT switching (see vmx_set_msr). On AMD, the situation
> is even worse as the g_pat save field is not updated at all on PAT
> writes. That seems to be a low hanging fruit to bring svm on the same
> support level as vmx.
> 
> Or am I missing something?

I don't think so, it looks buggy ... we could switch PAT manually, if it
isn't accelerated by hardware.

> PS: If someone has a good idea for a simple test case on machines
> without IOMMU (like my current boxes), thus without a chance to use
> device pass-through to stress guest PAT settings, I would be all ears.

Not a good one:  KVM sets VMX_EPT_IPAT_BIT for RAM unless
kvm_arch_has_noncoherent_dma().  You can comment the line in
vmx_get_mt_mask(), or call kvm_arch_register_noncoherent_dma(),
for guest PAT to work on normal memory.

Re: KVM: How does is PAT emulation supposed to work?

[Jan Kiszka]

On 2015-04-17 18:43, Radim Krčmář wrote:
> 2015-04-13 07:16+0200, Jan Kiszka:
>> PS: If someone has a good idea for a simple test case on machines
>> without IOMMU (like my current boxes), thus without a chance to use
>> device pass-through to stress guest PAT settings, I would be all ears.
> 
> Not a good one:  KVM sets VMX_EPT_IPAT_BIT for RAM unless
> kvm_arch_has_noncoherent_dma().  You can comment the line in
> vmx_get_mt_mask(), or call kvm_arch_register_noncoherent_dma(),
> for guest PAT to work on normal memory.

That's for VMX (where I do have IOMMUs), but I would need something for
AMD. :)

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: KVM: How does is PAT emulation supposed to work?

[Radim Krčmář]

2015-04-17 19:12+0200, Jan Kiszka:
> On 2015-04-17 18:43, Radim Krčmář wrote:
> > 2015-04-13 07:16+0200, Jan Kiszka:
> >> PS: If someone has a good idea for a simple test case on machines
> >> without IOMMU (like my current boxes), thus without a chance to use
> >> device pass-through to stress guest PAT settings, I would be all ears.
> > 
> > Not a good one:  KVM sets VMX_EPT_IPAT_BIT for RAM unless
> > kvm_arch_has_noncoherent_dma().  You can comment the line in
> > vmx_get_mt_mask(), or call kvm_arch_register_noncoherent_dma(),
> > for guest PAT to work on normal memory.
> 
> That's for VMX (where I do have IOMMUs), but I would need something for
> AMD. :)

Ah, your patch should be sufficient on AMD ...
(At least the documentation doesn't list other NPT switches.)

The simple test case is 'wrmsr(0x277, 0x0707070707070707)' and RAM
accesses in a bounded loop -- they aren't significantly slower?