| Message ID | 1302085803-25399-1-git-send-email-joerg.roedel@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/06/2011 01:30 PM, Joerg Roedel wrote: > When the emulation of vmload or vmsave fails because the > guest passed an unsupported physical address it gets an #GP > with rip pointing to the instruction after vmsave/vmload. > This is a bug and fixed by this patch. > Applied, thanks.
On 04/06/2011 02:21 PM, Avi Kivity wrote: > On 04/06/2011 01:30 PM, Joerg Roedel wrote: >> When the emulation of vmload or vmsave fails because the >> guest passed an unsupported physical address it gets an #GP >> with rip pointing to the instruction after vmsave/vmload. >> This is a bug and fixed by this patch. >> > > Applied, thanks. > btw, I think the actual address check is incorrect, need to check MAXPHYADDR and not hardcoded 0xffff000000000000.
On Wed, Apr 06, 2011 at 07:28:24AM -0400, Avi Kivity wrote: > On 04/06/2011 02:21 PM, Avi Kivity wrote: > > On 04/06/2011 01:30 PM, Joerg Roedel wrote: > >> When the emulation of vmload or vmsave fails because the > >> guest passed an unsupported physical address it gets an #GP > >> with rip pointing to the instruction after vmsave/vmload. > >> This is a bug and fixed by this patch. > >> > > > > Applied, thanks. > > > > btw, I think the actual address check is incorrect, need to check > MAXPHYADDR and not hardcoded 0xffff000000000000. There is a difference. MAX_PHYSADDR_BITS is the maximum Linux can support while the mask above is the current limit the hardware supports. It is the same on real hardware, when rax is >= (1<<48) there is a #GP (and no intercept if in guest-mode). Here is btw. a difference between nested-svm and hardware-svm, if rax contains a physical address which is supported but not backed by RAM the machine will just freeze on real hardware where as in nested-svm it causes a #GP (should be fine because the behavior in this case is undefined). Joerg
On 04/06/2011 03:27 PM, Roedel, Joerg wrote: > On Wed, Apr 06, 2011 at 07:28:24AM -0400, Avi Kivity wrote: > > On 04/06/2011 02:21 PM, Avi Kivity wrote: > > > On 04/06/2011 01:30 PM, Joerg Roedel wrote: > > >> When the emulation of vmload or vmsave fails because the > > >> guest passed an unsupported physical address it gets an #GP > > >> with rip pointing to the instruction after vmsave/vmload. > > >> This is a bug and fixed by this patch. > > >> > > > > > > Applied, thanks. > > > > > > > btw, I think the actual address check is incorrect, need to check > > MAXPHYADDR and not hardcoded 0xffff000000000000. > > There is a difference. MAX_PHYSADDR_BITS is the maximum Linux can > support while the mask above is the current limit the hardware > supports. I'm talking about MAXPHYADDR, the result of cpuid(0x80000008).eax[0:7]. (IIRC with the current page table format the absolute limit is 53 bits while the current limit is 48 bits). > It is the same on real hardware, when rax is>= (1<<48) there is a #GP > (and no intercept if in guest-mode). Here is btw. a difference between > nested-svm and hardware-svm, if rax contains a physical address which is > supported but not backed by RAM the machine will just freeze on real > hardware where as in nested-svm it causes a #GP (should be fine because > the behavior in this case is undefined). > In this case the behaviour before the patch was correct too :)
On Wed, Apr 06, 2011 at 08:43:47AM -0400, Avi Kivity wrote: > On 04/06/2011 03:27 PM, Roedel, Joerg wrote: > > On Wed, Apr 06, 2011 at 07:28:24AM -0400, Avi Kivity wrote: > > > On 04/06/2011 02:21 PM, Avi Kivity wrote: > > > > On 04/06/2011 01:30 PM, Joerg Roedel wrote: > > > >> When the emulation of vmload or vmsave fails because the > > > >> guest passed an unsupported physical address it gets an #GP > > > >> with rip pointing to the instruction after vmsave/vmload. > > > >> This is a bug and fixed by this patch. > > > >> > > > > > > > > Applied, thanks. > > > > > > > > > > btw, I think the actual address check is incorrect, need to check > > > MAXPHYADDR and not hardcoded 0xffff000000000000. > > > > There is a difference. MAX_PHYSADDR_BITS is the maximum Linux can > > support while the mask above is the current limit the hardware > > supports. > > I'm talking about MAXPHYADDR, the result of cpuid(0x80000008).eax[0:7]. > > (IIRC with the current page table format the absolute limit is 53 bits > while the current limit is 48 bits). Architectural limit is 52 bits, hardware implementation limit is 48 currently on most AMD processors (was 40 on K8 and is 36 on Ontario). And yes, this is the correct limit to check against. > > It is the same on real hardware, when rax is>= (1<<48) there is a #GP > > (and no intercept if in guest-mode). Here is btw. a difference between > > nested-svm and hardware-svm, if rax contains a physical address which is > > supported but not backed by RAM the machine will just freeze on real > > hardware where as in nested-svm it causes a #GP (should be fine because > > the behavior in this case is undefined). > > > > In this case the behaviour before the patch was correct too :) Well ... true :-) Joerg
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 779b091..f0c8721 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -2407,13 +2407,13 @@ static int vmload_interception(struct vcpu_svm *svm) if (nested_svm_check_permissions(svm)) return 1; - svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); - nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax, &page); if (!nested_vmcb) return 1; + svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; + skip_emulated_instruction(&svm->vcpu); + nested_svm_vmloadsave(nested_vmcb, svm->vmcb); nested_svm_unmap(page); @@ -2428,13 +2428,13 @@ static int vmsave_interception(struct vcpu_svm *svm) if (nested_svm_check_permissions(svm)) return 1; - svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; - skip_emulated_instruction(&svm->vcpu); - nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax, &page); if (!nested_vmcb) return 1; + svm->next_rip = kvm_rip_read(&svm->vcpu) + 3; + skip_emulated_instruction(&svm->vcpu); + nested_svm_vmloadsave(svm->vmcb, nested_vmcb); nested_svm_unmap(page);
When the emulation of vmload or vmsave fails because the guest passed an unsupported physical address it gets an #GP with rip pointing to the instruction after vmsave/vmload. This is a bug and fixed by this patch. Cc: stable@kernel.org Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/kvm/svm.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)