[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 2/8] x86/vm-event/monitor: relocate code-motion more appropriately
On 7/8/2016 2:53 PM, Jan Beulich wrote:
On 08.07.16 at 13:33, <czuzu@xxxxxxxxxxxxxxx> wrote:
On 7/8/2016 1:37 PM, Jan Beulich wrote:
On 08.07.16 at 12:22, <czuzu@xxxxxxxxxxxxxxx> wrote:
On 7/8/2016 10:21 AM, Jan Beulich wrote:
On 06.07.16 at 17:50, <czuzu@xxxxxxxxxxxxxxx> wrote:
+ /*
+ * If CR0.PE=0, CR3 load exiting must remain enabled.
+ * See vmx_update_guest_cr code motion for cr = 0.
+ */
+ if ( cr3_ldexit && !hvm_paging_enabled(v) &&
!vmx_unrestricted_guest(v) )
+ continue;
The first sentence of the comment should be brought in line with
this condition.
Would this do (aligned with the above observation):
"
/*
* If CR3 load-exiting was enabled and CR0.PE=0, then it must remain
* enabled (see vmx_update_guest_cr(v, cr) function when cr = 0).
*/
"
?
Not really: The condition checks whether paging is enabled and
whether it's an unrestricted guest. The comment talks about
protected mode being enabled.
Hah you're right, I only now notice, that comment has actually been
adopted (although I don't remember from where, I wonder if it was
meantime removed and I only now see), I always thought it said "CR0.PG =
0"...
So...
"
/*
* If domain paging is disabled (CR0.PG=0) and
* the domain is not in real-mode, then CR3 load-exiting
* must remain enabled (see vmx_update_guest_cr(v, cr) when cr = 0).
*/
"
?
Looks reasonable.
+static inline void write_ctrlreg_adjust_traps(struct domain *d, uint8_t
index)
Unless there is a particular reason for this uint8_t, please convert to
unsigned int.
The particular reason is cr-indexes being uint8_t typed (see
typeof(xen_domctl_monitor_op.mov_to_cr.index)).
But I will change it to unsigned int if you prefer (maybe you could
explain the preference though).
No use of fixed width types when fixed width types aren't really
required. Generally generated code is less efficient when having
to deal with fixed width types.
Strange, I would have thought the compiler would properly (and easily)
deal with such efficiency issues.
In this case the compiler may well do (as the function is static
inline), but in other cases it's simply not allowed to. In order to
not misguide people cloning existing code we should thus try to
limit the number of bad examples (which in particular mean not
introducing any new ones).
Jan
Ack.
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
