Bug 848328

Summary:	kvm emulates instructions with rip-relative addressing incorrectly
Product:	Red Hat Enterprise Linux 6	Reporter:	Avi Kivity <avi>
Component:	kernel	Assignee:	Andrew Jones <drjones>
Status:	CLOSED NOTABUG	QA Contact:	Virtualization Bugs <virt-bugs>
Severity:	low	Docs Contact:
Priority:	unspecified
Version:	6.4	CC:	areis, gleb, knoel, mkenneth, rhod, virt-maint
Target Milestone:	rc
Target Release:	---
Hardware:	x86_64
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:	848325	Environment:
Last Closed:	2013-04-18 07:18:08 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	848325
Bug Blocks:

Description Avi Kivity 2012-08-15 09:34:56 UTC

+++ This bug was initially created as a clone of Bug #848325 +++

Description of problem:

The KVM x86 emulator does calculates rip-relative addresses incorrectly: if they contain an immediate operand, then the memory operand's address will be off by the size of the immediate operand.

Example:

  movl $0, somewhere(%rip)

Version-Release number of selected component (if applicable):
kvm-83-259.el5

How reproducible:
Never

Steps to Reproduce:
1. Find a guest which accesses mmio using an rip-relative instruction with an immediate
2. Run guest
3. Watch guest malfunction
  
Actual results:

Guest malfunctions

Expected results:

Guest works correctly

Additional info:

--- Additional comment from avi on 2012-08-15 12:33:58 IDT ---

Upstream fix:

commit cb16c348760ad2bc79b67b20aefac05529569ed7
Author: Avi Kivity <avi>
Date:   Sun Jun 19 19:21:11 2011 +0300

    KVM: x86 emulator: fix %rip-relative addressing with immediate source operand
    
    %rip-relative addressing is relative to the first byte of the next instruction,
    so we need to add %rip only after we've fetched any immediate bytes.
    
    Based on original patch by Li Xin <xin.li>.
    
    Signed-off-by: Avi Kivity <avi>
    Acked-by: Li Xin <xin.li>
    Signed-off-by: Marcelo Tosatti <mtosatti>

Comment 3 Andrew Jones 2013-04-02 13:48:23 UTC

afaict, rhel6 doesn't need this patch. rhel6's arch/x86/kvm/emulate.c is missing 69f55cb11e8d78, which was the patch that moved the 'effective address += rip' up above the immediate fetching. Thus, we shouldn't have to move it back down again (which is what cb16c348760ad does).

Setting needinfo on Gleb to ack that analysis. If acked we can close as NOTABUG.

Comment 4 Gleb Natapov 2013-04-18 06:03:18 UTC

Yes, it looks like you are correct. Good thing we have a unit test for this case now. You can run emulator.flat to be absolutely sure.

Comment 5 Andrew Jones 2013-04-22 16:02:20 UTC

(In reply to comment #4)
> Yes, it looks like you are correct. Good thing we have a unit test for this
> case now. You can run emulator.flat to be absolutely sure.

Thanks for the pointer. I ran it and the rip_relative test passed.