[gem5-dev] Change in gem5/gem5[develop]: cpu-kvm: refactor x86 fixup before kvm_run

2021-03-11 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42301 )


Change subject: cpu-kvm: refactor x86 fixup before kvm_run
..

cpu-kvm: refactor x86 fixup before kvm_run

Since kvmRun still does lots of thing before really going to KVM, to
make the fixup more precise, I change ioctlRun to a virtual function and
make the derived class overrides it if needed.

Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42301
Tested-by: kokoro 
Reviewed-by: Andreas Sandberg 
Reviewed-by: Jason Lowe-Power 
Maintainer: Andreas Sandberg 
---
M src/cpu/kvm/base.hh
M src/cpu/kvm/x86_cpu.cc
M src/cpu/kvm/x86_cpu.hh
3 files changed, 25 insertions(+), 28 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/cpu/kvm/base.hh b/src/cpu/kvm/base.hh
index d97845b..68eda46 100644
--- a/src/cpu/kvm/base.hh
+++ b/src/cpu/kvm/base.hh
@@ -569,6 +569,8 @@
 }
 /** @} */

+/** Execute the KVM_RUN ioctl */
+virtual void ioctlRun();

 /**
  * KVM memory port.  Uses default RequestPort behavior and provides an
@@ -678,9 +680,6 @@
 /** Try to drain the CPU if a drain is pending */
 bool tryDrain();

-/** Execute the KVM_RUN ioctl */
-void ioctlRun();
-
 /** KVM vCPU file descriptor */
 int vcpuFD;
 /** Size of MMAPed kvm_run area */
diff --git a/src/cpu/kvm/x86_cpu.cc b/src/cpu/kvm/x86_cpu.cc
index 4a7d21b..bae5a29 100644
--- a/src/cpu/kvm/x86_cpu.cc
+++ b/src/cpu/kvm/x86_cpu.cc
@@ -1225,7 +1225,7 @@
 if (_status == Idle)
 return 0;
 else
-return kvmRunWrapper(ticks);
+return BaseKvmCPU::kvmRun(ticks);
 }

 Tick
@@ -1245,33 +1245,14 @@
 // Limit the run to 1 millisecond. That is hopefully enough to
 // reach an interrupt window. Otherwise, we'll just try again
 // later.
-return kvmRunWrapper(1 * SimClock::Float::ms);
+return BaseKvmCPU::kvmRun(1 * SimClock::Float::ms);
 } else {
 DPRINTF(Drain, "kvmRunDrain: Delivering pending IO\n");

-return kvmRunWrapper(0);
+return BaseKvmCPU::kvmRun(0);
 }
 }

-Tick
-X86KvmCPU::kvmRunWrapper(Tick ticks)
-{
-struct kvm_run _run(*getKvmRunState());
-
-// Synchronize the APIC base and CR8 here since they are present
-// in the kvm_run struct, which makes the synchronization really
-// cheap.
-kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-const Tick run_ticks(BaseKvmCPU::kvmRun(ticks));
-
-tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-return run_ticks;
-}
-
 uint64_t
 X86KvmCPU::getHostCycles() const
 {
@@ -1404,6 +1385,23 @@
 return !pending_events;
 }

+void
+X86KvmCPU::ioctlRun()
+{
+struct kvm_run _run(*getKvmRunState());
+
+// Synchronize the APIC base and CR8 here since they are present
+// in the kvm_run struct, which makes the synchronization really
+// cheap.
+kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+
+BaseKvmCPU::ioctlRun();
+
+tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+}
+
 static struct kvm_cpuid_entry2
 makeKvmCpuid(uint32_t function, uint32_t index,
  CpuidResult )
diff --git a/src/cpu/kvm/x86_cpu.hh b/src/cpu/kvm/x86_cpu.hh
index a60d597..a114a8a 100644
--- a/src/cpu/kvm/x86_cpu.hh
+++ b/src/cpu/kvm/x86_cpu.hh
@@ -78,9 +78,6 @@
  */
 Tick kvmRunDrain() override;

-/** Wrapper that synchronizes state in kvm_run */
-Tick kvmRunWrapper(Tick ticks);
-
 uint64_t getHostCycles() const override;

 /**
@@ -158,6 +155,9 @@
  */
 bool archIsDrained() const override;

+/** Override for synchronizing state in kvm_run */
+void ioctlRun() override;
+
   private:
 /**
  * Support routines to update the state of the KVM CPU from gem5's

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42301
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
Gerrit-Change-Number: 42301
Gerrit-PatchSet: 4
Gerrit-Owner: Yu-hsin Wang 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Earl Ou 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Yu-hsin Wang 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email 

[gem5-dev] Change in gem5/gem5[develop]: cpu-kvm: refactor x86 fixup before kvm_run

2021-03-05 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42301 )



Change subject: cpu-kvm: refactor x86 fixup before kvm_run
..

cpu-kvm: refactor x86 fixup before kvm_run

Since kvmRun still does lots of thing before really going to KVM, to
make the fixup more precise, I change ioctlRun to a virtual function and
make the derived class overrides it if needed.

Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
---
M src/cpu/kvm/base.hh
M src/cpu/kvm/x86_cpu.cc
M src/cpu/kvm/x86_cpu.hh
3 files changed, 25 insertions(+), 28 deletions(-)



diff --git a/src/cpu/kvm/base.hh b/src/cpu/kvm/base.hh
index d97845b..68eda46 100644
--- a/src/cpu/kvm/base.hh
+++ b/src/cpu/kvm/base.hh
@@ -569,6 +569,8 @@
 }
 /** @} */

+/** Execute the KVM_RUN ioctl */
+virtual void ioctlRun();

 /**
  * KVM memory port.  Uses default RequestPort behavior and provides an
@@ -678,9 +680,6 @@
 /** Try to drain the CPU if a drain is pending */
 bool tryDrain();

-/** Execute the KVM_RUN ioctl */
-void ioctlRun();
-
 /** KVM vCPU file descriptor */
 int vcpuFD;
 /** Size of MMAPed kvm_run area */
diff --git a/src/cpu/kvm/x86_cpu.cc b/src/cpu/kvm/x86_cpu.cc
index 4a7d21b..bae5a29 100644
--- a/src/cpu/kvm/x86_cpu.cc
+++ b/src/cpu/kvm/x86_cpu.cc
@@ -1225,7 +1225,7 @@
 if (_status == Idle)
 return 0;
 else
-return kvmRunWrapper(ticks);
+return BaseKvmCPU::kvmRun(ticks);
 }

 Tick
@@ -1245,33 +1245,14 @@
 // Limit the run to 1 millisecond. That is hopefully enough to
 // reach an interrupt window. Otherwise, we'll just try again
 // later.
-return kvmRunWrapper(1 * SimClock::Float::ms);
+return BaseKvmCPU::kvmRun(1 * SimClock::Float::ms);
 } else {
 DPRINTF(Drain, "kvmRunDrain: Delivering pending IO\n");

-return kvmRunWrapper(0);
+return BaseKvmCPU::kvmRun(0);
 }
 }

-Tick
-X86KvmCPU::kvmRunWrapper(Tick ticks)
-{
-struct kvm_run _run(*getKvmRunState());
-
-// Synchronize the APIC base and CR8 here since they are present
-// in the kvm_run struct, which makes the synchronization really
-// cheap.
-kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-const Tick run_ticks(BaseKvmCPU::kvmRun(ticks));
-
-tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-return run_ticks;
-}
-
 uint64_t
 X86KvmCPU::getHostCycles() const
 {
@@ -1404,6 +1385,23 @@
 return !pending_events;
 }

+void
+X86KvmCPU::ioctlRun()
+{
+struct kvm_run _run(*getKvmRunState());
+
+// Synchronize the APIC base and CR8 here since they are present
+// in the kvm_run struct, which makes the synchronization really
+// cheap.
+kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+
+BaseKvmCPU::ioctlRun();
+
+tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+}
+
 static struct kvm_cpuid_entry2
 makeKvmCpuid(uint32_t function, uint32_t index,
  CpuidResult )
diff --git a/src/cpu/kvm/x86_cpu.hh b/src/cpu/kvm/x86_cpu.hh
index a60d597..a114a8a 100644
--- a/src/cpu/kvm/x86_cpu.hh
+++ b/src/cpu/kvm/x86_cpu.hh
@@ -78,9 +78,6 @@
  */
 Tick kvmRunDrain() override;

-/** Wrapper that synchronizes state in kvm_run */
-Tick kvmRunWrapper(Tick ticks);
-
 uint64_t getHostCycles() const override;

 /**
@@ -158,6 +155,9 @@
  */
 bool archIsDrained() const override;

+/** Override for synchronizing state in kvm_run */
+void ioctlRun() override;
+
   private:
 /**
  * Support routines to update the state of the KVM CPU from gem5's

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42301
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
Gerrit-Change-Number: 42301
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-hsin Wang 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s