On 1/29/2016 6:47 PM, Lengyel, Tamas wrote:
by leaving there only the x86-specific part, i.e.:
struct {
uint8_t mov_to_msr_enabled : 1;
uint8_t mov_to_msr_extended : 1;
} monitor;
and moving the rest directly into the domain structure, i.e. add @
the end of struct domain [@ xen/include/xen/sched.h]:
struct {
uint16_t write_ctrlreg_enabled : 4;
uint16_t write_ctrlreg_sync : 4;
uint16_t write_ctrlreg_onchangeonly : 4;
uint16_t singlestep_enabled : 1;
uint16_t software_breakpoint_enabled : 1;
uint16_t guest_request_enabled : 1;
uint16_t guest_request_sync : 1;
} monitor;
Beside guest_request_enable/sync I'm fairly sure all the other bits
are x86 specific. For example the ctrlreg fields are 4 bits wide to
correspond to the 4 x86 CR regs for which we can get hardware events
(VM_EVENT_X86_*). You should not be moving those in that form into
common. For ARM you should create an arch specific monitor structure
for events that you can actually capture (SMC, etc.). So IMHO for 2
bits in common it's a waste to have things moved up from arch.
Tamas
Conceptually speaking, they are not X86-specific. Single-stepping,
software-breakpoints, guest-to-hypervisor notifications, control/system
registers - these are concepts that are not strictly confined
exclusively within a single architecture, whereas for example MSRs are.
It's true that originally the need for defining 4 bits for 4
control-registers came because those 4 registers were the X86 CR0, CR3,
CR4 & XCR0 registers, but I was not suggesting to keep the same
significance after this change.
Rather, my proposition was-to-be (when sending the move-to-common patch)
either:
1). to change that significance to "for now, 4 bits are enough to cover
all the monitored control-registers for all the architectures that
implement vm-event control register monitoring. If, in the future an
implementation change of control-register monitoring for an architecture
will require more than the # of bits @ that time, then that # can be
adjusted."
2). define a single upper limit for control-registers count for all
architectures, e.g. smth like #define MONITOR_CTRLREG_MAX_COUNT 8 -
maybe paired w/ ASSERT/BUG_ON_BUILD-like checks to enforce that and/or
avoid undesired behavior.
Or maybe some other way, nonetheless, as I said, IMO the place for these
bits could be here, in the common code, since conceptually they could
apply to any architecture.
I was actually hoping to approach this matter after the move-to-common
patch because I have another patch I would like to put forward that
implements control-registers monitoring on ARM as well and incidentally
there are also 4 of them in that patch, as on X86. But that's to come :).
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
