Re: [PATCH v2] KVM: SVM: Sync g_pat with guest-written PAT value
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
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
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 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
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
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
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
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 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