>From: Tim Deegan
>Sent: 2008年2月21日 22:13
>So, the idea seems sound, and avoids the shadow lock altogether on a
>bunch more pagefaults, which is nice.
>I think that since PV pagetables are guaranteed to be read-only above
>l1e, the guest_map_l1e and guest_get_eff_l1e functions can be
>drop the shadow lock and call guest_walk_tables with shadow_op == 0.
>That would mean that there are no callers left setting shadow_op to 1,
>and then the shadow_op argument (and the whole mechanism for calling
>remove_write_access from guest_walk_tables) could be removed.
Thanks for clarification.
>I think that in guest_walk_tables, you need to add an rmb() after
>reading the version number to stop the compiler from hoisting the
>pagetable reads to before the version read.
Yes, my overlook and thanks for pointing out.
> - I'd like to see the "version" field called something more
> descriptive, and moved into the shadow-specific domain state,
> since HAP won't be using it.
Done and renamed to gtable_dirty_version. Sorry if this is still not a
good name and please feel free help polish a better name. :-)
> - In shadow_check_gwalk, maybe return 1 at the top of the function if
> the version number hasn't changed, rather than putting most of the
> function inside an if()?
> - You've added a second "not a shadow_fault" printout without removing
> the first.
The first one is branched with lock held, and some audit functions also
required lock held. To differentiate, I put a new label. Now I just removed
the new label since only few places use it.
> - We can remove the comment at the top of multi.c about reworking the
> guest_walk TLB flush logic. :)
Then updated version attached, and please help check again.
Xen-devel mailing list