Re: [PATCH v3] KVM: x86 emulator: access GPRs on demand
On 08/26/2012 10:04 AM, Marcelo Tosatti wrote:
On Thu, Aug 23, 2012 at 05:14:27AM -0300, Marcelo Tosatti wrote:
On Sun, Aug 19, 2012 at 12:32:36PM +0300, Avi Kivity wrote:
On 08/17/2012 08:29 PM, Marcelo Tosatti wrote:
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct
x86_emulate_ctxt *ctxt,
{
int rc;
+ invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index,
reason,
has_error_code, error_code);
- if (rc == X86EMUL_CONTINUE)
+ if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+ writeback_registers(ctxt);
+ }
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED :
EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
It is cleared when emulation starts. For the non-insn-emulation entry
points, there is an explicit invalidate. For the emulation entry point,
there is a memset() that clears everything up to _regs, which includes
the cache. This discrepancy isn't nice, but it preexists. I don't know
whether we should decompose the memset() or not, it is rather efficient.
Concern is that emulator can start with cached registers marked as
valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
See the memset() in init_decode_cache().
Right. Applied, thanks.
Actually, had to revert because autotest was failing.
Was it failing because of this patch? Or what?
Now it rejects:
4 out of 49 hunks FAILED -- saving rejects to file
arch/x86/kvm/emulate.c.rej
Please regenerate.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
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 v3] KVM: x86 emulator: access GPRs on demand
On 08/27/2012 01:22 PM, Avi Kivity wrote:
On 08/26/2012 10:04 AM, Marcelo Tosatti wrote:
On Thu, Aug 23, 2012 at 05:14:27AM -0300, Marcelo Tosatti wrote:
On Sun, Aug 19, 2012 at 12:32:36PM +0300, Avi Kivity wrote:
On 08/17/2012 08:29 PM, Marcelo Tosatti wrote:
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct
x86_emulate_ctxt *ctxt,
{
int rc;
+invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index,
reason,
has_error_code, error_code);
-if (rc == X86EMUL_CONTINUE)
+if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+writeback_registers(ctxt);
+}
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED :
EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation
start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
It is cleared when emulation starts. For the non-insn-emulation entry
points, there is an explicit invalidate. For the emulation entry point,
there is a memset() that clears everything up to _regs, which includes
the cache. This discrepancy isn't nice, but it preexists. I don't know
whether we should decompose the memset() or not, it is rather efficient.
Concern is that emulator can start with cached registers marked as
valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
See the memset() in init_decode_cache().
Right. Applied, thanks.
Actually, had to revert because autotest was failing.
Was it failing because of this patch? Or what?
I see, the rsp mask fix.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
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 v3] KVM: x86 emulator: access GPRs on demand
On Mon, Aug 27, 2012 at 05:53:32PM -0300, Marcelo Tosatti wrote: With the fix, it rejects. About to merge the big real mode patchset, so its not a bad idea to wait for that before resending. Nevermind this sentence. -- 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 v3] KVM: x86 emulator: access GPRs on demand
On Mon, Aug 27, 2012 at 01:22:55PM -0700, Avi Kivity wrote:
On 08/26/2012 10:04 AM, Marcelo Tosatti wrote:
On Thu, Aug 23, 2012 at 05:14:27AM -0300, Marcelo Tosatti wrote:
On Sun, Aug 19, 2012 at 12:32:36PM +0300, Avi Kivity wrote:
On 08/17/2012 08:29 PM, Marcelo Tosatti wrote:
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct
x86_emulate_ctxt *ctxt,
{
int rc;
+invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index,
reason,
has_error_code, error_code);
-if (rc == X86EMUL_CONTINUE)
+if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+writeback_registers(ctxt);
+}
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED :
EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation
start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
It is cleared when emulation starts. For the non-insn-emulation entry
points, there is an explicit invalidate. For the emulation entry point,
there is a memset() that clears everything up to _regs, which includes
the cache. This discrepancy isn't nice, but it preexists. I don't know
whether we should decompose the memset() or not, it is rather efficient.
Concern is that emulator can start with cached registers marked as
valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
See the memset() in init_decode_cache().
Right. Applied, thanks.
Actually, had to revert because autotest was failing.
Was it failing because of this patch? Or what?
No, due to lack of stack size fix.
With the fix, it rejects. About to merge the big real mode
patchset, so its not a bad idea to wait for that before
resending.
--
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 v3] KVM: x86 emulator: access GPRs on demand
On Thu, Aug 23, 2012 at 05:14:27AM -0300, Marcelo Tosatti wrote:
On Sun, Aug 19, 2012 at 12:32:36PM +0300, Avi Kivity wrote:
On 08/17/2012 08:29 PM, Marcelo Tosatti wrote:
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct x86_emulate_ctxt
*ctxt,
{
int rc;
+invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index,
reason,
has_error_code, error_code);
-if (rc == X86EMUL_CONTINUE)
+if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+writeback_registers(ctxt);
+}
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED :
EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
It is cleared when emulation starts. For the non-insn-emulation entry
points, there is an explicit invalidate. For the emulation entry point,
there is a memset() that clears everything up to _regs, which includes
the cache. This discrepancy isn't nice, but it preexists. I don't know
whether we should decompose the memset() or not, it is rather efficient.
Concern is that emulator can start with cached registers marked as valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
See the memset() in init_decode_cache().
Right. Applied, thanks.
Actually, had to revert because autotest was failing.
Now it rejects:
4 out of 49 hunks FAILED -- saving rejects to file
arch/x86/kvm/emulate.c.rej
Please regenerate.
--
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 v3] KVM: x86 emulator: access GPRs on demand
On Sun, Aug 19, 2012 at 12:32:36PM +0300, Avi Kivity wrote:
On 08/17/2012 08:29 PM, Marcelo Tosatti wrote:
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct x86_emulate_ctxt
*ctxt,
{
int rc;
+ invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index, reason,
has_error_code, error_code);
- if (rc == X86EMUL_CONTINUE)
+ if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+ writeback_registers(ctxt);
+ }
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
It is cleared when emulation starts. For the non-insn-emulation entry
points, there is an explicit invalidate. For the emulation entry point,
there is a memset() that clears everything up to _regs, which includes
the cache. This discrepancy isn't nice, but it preexists. I don't know
whether we should decompose the memset() or not, it is rather efficient.
Concern is that emulator can start with cached registers marked as valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
See the memset() in init_decode_cache().
Right. Applied, thanks.
--
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 v3] KVM: x86 emulator: access GPRs on demand
On 08/17/2012 08:29 PM, Marcelo Tosatti wrote:
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct x86_emulate_ctxt
*ctxt,
{
int rc;
+invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index, reason,
has_error_code, error_code);
-if (rc == X86EMUL_CONTINUE)
+if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+writeback_registers(ctxt);
+}
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
It is cleared when emulation starts. For the non-insn-emulation entry
points, there is an explicit invalidate. For the emulation entry point,
there is a memset() that clears everything up to _regs, which includes
the cache. This discrepancy isn't nice, but it preexists. I don't know
whether we should decompose the memset() or not, it is rather efficient.
Concern is that emulator can start with cached registers marked as valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
See the memset() in init_decode_cache().
--
error compiling committee.c: too many arguments to function
--
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 v3] KVM: x86 emulator: access GPRs on demand
On Thu, Aug 16, 2012 at 05:54:49PM +0300, Avi Kivity wrote:
Instead of populating the the entire register file, read in registers
as they are accessed, and write back only the modified ones. This
saves a VMREAD and VMWRITE on Intel (for rsp, since it is not usually
used during emulation), and a two 128-byte copies for the registers.
Signed-off-by: Avi Kivity a...@redhat.com
---
v3:
fix misplaced parentheses in em_loop() and em_jcxz(), unbreaking those
instructions.
v2:
add APIs for managing the register cache. This reduces the potential for
confusion
between ctxt-regs_dirty and vcpu-arch.regs_dirty.
move cache management to the entry points
add missing writebacks to int and task switch emulation
arch/x86/include/asm/kvm_emulate.h | 20 ++-
arch/x86/kvm/emulate.c | 305
++---
arch/x86/kvm/x86.c | 45 +++---
3 files changed, 223 insertions(+), 147 deletions(-)
@@ -2715,14 +2764,17 @@ int emulator_task_switch(struct x86_emulate_ctxt
*ctxt,
{
int rc;
+ invalidate_registers(ctxt);
ctxt-_eip = ctxt-eip;
ctxt-dst.type = OP_NONE;
rc = emulator_do_task_switch(ctxt, tss_selector, idt_index, reason,
has_error_code, error_code);
- if (rc == X86EMUL_CONTINUE)
+ if (rc == X86EMUL_CONTINUE) {
ctxt-eip = ctxt-_eip;
+ writeback_registers(ctxt);
+ }
return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
}
No clear point when emulator register cache is active, when it is
not (AFAICS this patch does not invalidate registers on emulation start
(the above being one of the exceptions) does not clear valid bit on
writeback-to-vcpu-cache on emulation exit).
Concern is that emulator can start with cached registers marked as valid
but in fact are invalid from previous emulation round.
Maybe move invalidate() to init_emulate_ctxt?
--
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
