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

2015-05-24 Thread Jan Kiszka
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.kis...@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



signature.asc
Description: OpenPGP digital signature


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

2015-04-21 Thread 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.kis...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-21 Thread 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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-21 Thread 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.kis...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-21 Thread 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.kis...@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);
   }
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-21 Thread 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.kis...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-21 Thread 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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-20 Thread 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.kis...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-20 Thread 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.kis...@siemens.com
 ---
 
 Changes in v2:
  - add mark_dirty as found missing by Radim

Thanks.

Reviewed-by: Radim Krčmář rkrc...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html