Re: [Xen-devel] [PATCH] x86/pv: Minor improvements to guest_get_eff_{, kern}_l1e()
On 24/08/17 13:35, Jan Beulich wrote: On 24.08.17 at 13:13, wrote: >> On 24/08/17 09:44, Jan Beulich wrote: >>> >>> On 23.08.17 at 21:09, wrote: * These functions work in terms of linear addresses, not virtual addresses. Update the comments and parameter names. * Drop unnecessary inlines. * Drop vcpu parameter from guest_get_eff_kern_l1e(). Its sole caller passes current, and its callee strictly operates on current. >>> I'm not entirely convinced of this part, as I think the intention was >>> to save the re-latching of "current", but anyway: >> It'll be inlined anyway, given a sole caller. > But the inlining won't help with the re-fetching; the compiler can't > know it can re-use the earlier fetched value. On that side of things, in the past I tried experimenting with attribute pure and const for things behind current. Such annotations should allow subexpression elimination and reordering, even across inlining boundaries, but I failed to get any change in generated code. I don't know if it was because I was annotating incorrectly, or GCC decided that elimination wasn't worth doing, but it might be something worth re-investigating. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Minor improvements to guest_get_eff_{, kern}_l1e()
>>> On 24.08.17 at 13:13, wrote: > On 24/08/17 09:44, Jan Beulich wrote: >> >>> On 23.08.17 at 21:09, wrote: >>> * These functions work in terms of linear addresses, not virtual addresses. >>>Update the comments and parameter names. >>> * Drop unnecessary inlines. >>> * Drop vcpu parameter from guest_get_eff_kern_l1e(). Its sole caller >>> passes >>>current, and its callee strictly operates on current. >> I'm not entirely convinced of this part, as I think the intention was >> to save the re-latching of "current", but anyway: > > It'll be inlined anyway, given a sole caller. But the inlining won't help with the re-fetching; the compiler can't know it can re-use the earlier fetched value. > We could have struct vcpu *curr as a parameter if you prefer, but having > just a plain v gives the false impression it can be used with something > other than current. That would be an option, yes, but as said, the problem is minor enough for the patch to be good as is. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Minor improvements to guest_get_eff_{, kern}_l1e()
On 24/08/17 09:44, Jan Beulich wrote: > >>> On 23.08.17 at 21:09, wrote: >> * These functions work in terms of linear addresses, not virtual addresses. >>Update the comments and parameter names. >> * Drop unnecessary inlines. >> * Drop vcpu parameter from guest_get_eff_kern_l1e(). Its sole caller passes >>current, and its callee strictly operates on current. > I'm not entirely convinced of this part, as I think the intention was > to save the re-latching of "current", but anyway: It'll be inlined anyway, given a sole caller. We could have struct vcpu *curr as a parameter if you prefer, but having just a plain v gives the false impression it can be used with something other than current. ~Andrew > >> * Switch guest_get_eff_kern_l1e()'s parameter from void * to l1_pgentry_t >> *. >>Both its caller and callee already use the correct type already. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Jan Beulich > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Minor improvements to guest_get_eff_{, kern}_l1e()
On Wed, Aug 23, 2017 at 08:09:35PM +0100, Andrew Cooper wrote: > * These functions work in terms of linear addresses, not virtual addresses. >Update the comments and parameter names. > * Drop unnecessary inlines. > * Drop vcpu parameter from guest_get_eff_kern_l1e(). Its sole caller passes >current, and its callee strictly operates on current. > * Switch guest_get_eff_kern_l1e()'s parameter from void * to l1_pgentry_t *. >Both its caller and callee already use the correct type already. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Minor improvements to guest_get_eff_{, kern}_l1e()
>>> On 23.08.17 at 21:09, wrote: > * These functions work in terms of linear addresses, not virtual addresses. >Update the comments and parameter names. > * Drop unnecessary inlines. > * Drop vcpu parameter from guest_get_eff_kern_l1e(). Its sole caller passes >current, and its callee strictly operates on current. I'm not entirely convinced of this part, as I think the intention was to save the re-latching of "current", but anyway: > * Switch guest_get_eff_kern_l1e()'s parameter from void * to l1_pgentry_t > *. >Both its caller and callee already use the correct type already. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/pv: Minor improvements to guest_get_eff_{, kern}_l1e()
* These functions work in terms of linear addresses, not virtual addresses.
Update the comments and parameter names.
* Drop unnecessary inlines.
* Drop vcpu parameter from guest_get_eff_kern_l1e(). Its sole caller passes
current, and its callee strictly operates on current.
* Switch guest_get_eff_kern_l1e()'s parameter from void * to l1_pgentry_t *.
Both its caller and callee already use the correct type already.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
---
xen/arch/x86/mm.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 3262499..8993e6d 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -561,15 +561,15 @@ static inline void guest_unmap_l1e(void *p)
unmap_domain_page(p);
}
-/* Read a PV guest's l1e that maps this virtual address. */
-static inline void guest_get_eff_l1e(unsigned long addr, l1_pgentry_t *eff_l1e)
+/* Read a PV guest's l1e that maps this linear address. */
+static void guest_get_eff_l1e(unsigned long linear, l1_pgentry_t *eff_l1e)
{
ASSERT(!paging_mode_translate(current->domain));
ASSERT(!paging_mode_external(current->domain));
-if ( unlikely(!__addr_ok(addr)) ||
+if ( unlikely(!__addr_ok(linear)) ||
__copy_from_user(eff_l1e,
- &__linear_l1_table[l1_linear_offset(addr)],
+ &__linear_l1_table[l1_linear_offset(linear)],
sizeof(l1_pgentry_t)) )
*eff_l1e = l1e_empty();
}
@@ -578,18 +578,18 @@ static inline void guest_get_eff_l1e(unsigned long addr,
l1_pgentry_t *eff_l1e)
* Read the guest's l1e that maps this address, from the kernel-mode
* page tables.
*/
-static inline void guest_get_eff_kern_l1e(struct vcpu *v, unsigned long addr,
- void *eff_l1e)
+static void guest_get_eff_kern_l1e(unsigned long linear, l1_pgentry_t *eff_l1e)
{
-const bool user_mode = !(v->arch.flags & TF_kernel_mode);
+struct vcpu *curr = current;
+const bool user_mode = !(curr->arch.flags & TF_kernel_mode);
if ( user_mode )
-toggle_guest_mode(v);
+toggle_guest_mode(curr);
-guest_get_eff_l1e(addr, eff_l1e);
+guest_get_eff_l1e(linear, eff_l1e);
if ( user_mode )
-toggle_guest_mode(v);
+toggle_guest_mode(curr);
}
static inline void page_set_tlbflush_timestamp(struct page_info *page)
@@ -676,7 +676,7 @@ int map_ldt_shadow_page(unsigned int off)
if ( is_pv_32bit_domain(d) )
gva = (u32)gva;
-guest_get_eff_kern_l1e(v, gva, &l1e);
+guest_get_eff_kern_l1e(gva, &l1e);
if ( unlikely(!(l1e_get_flags(l1e) & _PAGE_PRESENT)) )
return 0;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
