* [PATCH] x86/mm: Rearrange guest_get_eff_{, kern_}l1e() not be void
@ 2017-08-30 13:22 Andrew Cooper
2017-08-30 13:29 ` Wei Liu
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2017-08-30 13:22 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
Coverity complains that gl1e.l1 may be used while uninitialised in
map_ldt_shadow_page(). This isn't actually accurate as guest_get_eff_l1e()
will always write to its parameter.
However, having a void function which returns a 64bit value via pointer is
rather silly. Rearrange the functions to return l1_pgentry_t.
No functional change, but hopefully should help Coverity not to come to the
wrong conclusion.
Bloat-o-meter also reports a modest improvement:
add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-71 (-71)
function old new delta
guest_get_eff_l1e 82 75 -7
mmio_ro_do_page_fault 530 514 -16
map_ldt_shadow_page 501 485 -16
ptwr_do_page_fault 615 583 -32
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
xen/arch/x86/mm.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f0c81e7..fe1529e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -568,34 +568,41 @@ static inline void guest_unmap_l1e(void *p)
}
/* 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)
+static l1_pgentry_t guest_get_eff_l1e(unsigned long linear)
{
+ l1_pgentry_t l1e;
+
ASSERT(!paging_mode_translate(current->domain));
ASSERT(!paging_mode_external(current->domain));
if ( unlikely(!__addr_ok(linear)) ||
- __copy_from_user(eff_l1e,
+ __copy_from_user(&l1e,
&__linear_l1_table[l1_linear_offset(linear)],
sizeof(l1_pgentry_t)) )
- *eff_l1e = l1e_empty();
+ l1e = l1e_empty();
+
+ return l1e;
}
/*
* Read the guest's l1e that maps this address, from the kernel-mode
* page tables.
*/
-static void guest_get_eff_kern_l1e(unsigned long linear, l1_pgentry_t *eff_l1e)
+static l1_pgentry_t guest_get_eff_kern_l1e(unsigned long linear)
{
struct vcpu *curr = current;
const bool user_mode = !(curr->arch.flags & TF_kernel_mode);
+ l1_pgentry_t l1e;
if ( user_mode )
toggle_guest_mode(curr);
- guest_get_eff_l1e(linear, eff_l1e);
+ l1e = guest_get_eff_l1e(linear);
if ( user_mode )
toggle_guest_mode(curr);
+
+ return l1e;
}
static inline void page_set_tlbflush_timestamp(struct page_info *page)
@@ -693,7 +700,7 @@ bool map_ldt_shadow_page(unsigned int offset)
if ( is_pv_32bit_domain(d) )
linear = (uint32_t)linear;
- guest_get_eff_kern_l1e(linear, &gl1e);
+ gl1e = guest_get_eff_kern_l1e(linear);
if ( unlikely(!(l1e_get_flags(gl1e) & _PAGE_PRESENT)) )
return false;
@@ -5192,7 +5199,7 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long addr,
int rc;
/* Attempt to read the PTE that maps the VA being accessed. */
- guest_get_eff_l1e(addr, &pte);
+ pte = guest_get_eff_l1e(addr);
/* We are looking only for read-only mappings of p.t. pages. */
if ( ((l1e_get_flags(pte) & (_PAGE_PRESENT|_PAGE_RW)) != _PAGE_PRESENT) ||
@@ -5347,7 +5354,7 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long addr,
int rc;
/* Attempt to read the PTE that maps the VA being accessed. */
- guest_get_eff_l1e(addr, &pte);
+ pte = guest_get_eff_l1e(addr);
/* We are looking only for read-only mappings of MMIO pages. */
if ( ((l1e_get_flags(pte) & (_PAGE_PRESENT|_PAGE_RW)) != _PAGE_PRESENT) )
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/mm: Rearrange guest_get_eff_{, kern_}l1e() not be void
2017-08-30 13:22 [PATCH] x86/mm: Rearrange guest_get_eff_{, kern_}l1e() not be void Andrew Cooper
@ 2017-08-30 13:29 ` Wei Liu
2017-08-30 15:51 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Wei Liu @ 2017-08-30 13:29 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Jan Beulich, Xen-devel
On Wed, Aug 30, 2017 at 02:22:05PM +0100, Andrew Cooper wrote:
> Coverity complains that gl1e.l1 may be used while uninitialised in
> map_ldt_shadow_page(). This isn't actually accurate as guest_get_eff_l1e()
> will always write to its parameter.
>
> However, having a void function which returns a 64bit value via pointer is
> rather silly. Rearrange the functions to return l1_pgentry_t.
>
> No functional change, but hopefully should help Coverity not to come to the
> wrong conclusion.
>
> Bloat-o-meter also reports a modest improvement:
> add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-71 (-71)
> function old new delta
> guest_get_eff_l1e 82 75 -7
> mmio_ro_do_page_fault 530 514 -16
> map_ldt_shadow_page 501 485 -16
> ptwr_do_page_fault 615 583 -32
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/mm: Rearrange guest_get_eff_{, kern_}l1e() not be void
2017-08-30 13:29 ` Wei Liu
@ 2017-08-30 15:51 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2017-08-30 15:51 UTC (permalink / raw)
To: Andrew Cooper, WeiLiu; +Cc: Xen-devel
>>> On 30.08.17 at 15:29, <wei.liu2@citrix.com> wrote:
> On Wed, Aug 30, 2017 at 02:22:05PM +0100, Andrew Cooper wrote:
>> Coverity complains that gl1e.l1 may be used while uninitialised in
>> map_ldt_shadow_page(). This isn't actually accurate as guest_get_eff_l1e()
>> will always write to its parameter.
>>
>> However, having a void function which returns a 64bit value via pointer is
>> rather silly. Rearrange the functions to return l1_pgentry_t.
>>
>> No functional change, but hopefully should help Coverity not to come to the
>> wrong conclusion.
>>
>> Bloat-o-meter also reports a modest improvement:
>> add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-71 (-71)
>> function old new delta
>> guest_get_eff_l1e 82 75 -7
>> mmio_ro_do_page_fault 530 514 -16
>> map_ldt_shadow_page 501 485 -16
>> ptwr_do_page_fault 615 583 -32
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-30 15:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 13:22 [PATCH] x86/mm: Rearrange guest_get_eff_{, kern_}l1e() not be void Andrew Cooper
2017-08-30 13:29 ` Wei Liu
2017-08-30 15:51 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.