* [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()
@ 2017-08-30 12:19 Andrew Cooper
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Andrew Cooper @ 2017-08-30 12:19 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
No functional change (as confirmed by diffing the assembly).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
For some reason best known to GCC, there is one single change:
@@ -145835,7 +145835,7 @@
ffff82d0802864f8: 85 c0 test %eax,%eax
ffff82d0802864fa: 75 e0 jne ffff82d0802864dc <new_guest_cr3+0x7c>
ffff82d0802864fc: 4c 8b ad 08 0b 00 00 mov 0xb08(%rbp),%r13
-ffff82d080286503: 49 39 dd cmp %rbx,%r13
+ffff82d080286503: 4c 39 eb cmp %r13,%rbx
ffff82d080286506: 0f 84 74 01 00 00 je ffff82d080286680 <new_guest_cr3+0x220>
ffff82d08028650c: 41 f6 84 24 19 06 00 testb $0x8,0x619(%r12)
ffff82d080286513: 00 08
This is from the mfn_eq() alteration, and must be a side effect from using an
inline function. The net result is still correct, as only the zero flag is
checked.
---
xen/arch/x86/mm.c | 35 ++++++++++++++++++-----------------
xen/arch/x86/pv/emul-priv-op.c | 2 +-
xen/include/asm-x86/mm.h | 2 +-
3 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1f23470..dc07b4f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2772,23 +2772,23 @@ int vcpu_destroy_pagetables(struct vcpu *v)
return rc != -EINTR ? rc : -ERESTART;
}
-int new_guest_cr3(unsigned long mfn)
+int new_guest_cr3(mfn_t mfn)
{
struct vcpu *curr = current;
struct domain *d = curr->domain;
int rc;
- unsigned long old_base_mfn;
+ mfn_t old_base_mfn;
if ( is_pv_32bit_domain(d) )
{
- unsigned long gt_mfn = pagetable_get_pfn(curr->arch.guest_table);
- l4_pgentry_t *pl4e = map_domain_page(_mfn(gt_mfn));
+ mfn_t mmfn = pagetable_get_mfn(curr->arch.guest_table);
+ l4_pgentry_t *pl4e = map_domain_page(mmfn);
rc = mod_l4_entry(pl4e,
- l4e_from_pfn(mfn,
+ l4e_from_mfn(mfn,
(_PAGE_PRESENT | _PAGE_RW |
_PAGE_USER | _PAGE_ACCESSED)),
- gt_mfn, 0, curr);
+ mfn_x(mmfn), 0, curr);
unmap_domain_page(pl4e);
switch ( rc )
{
@@ -2800,7 +2800,7 @@ int new_guest_cr3(unsigned long mfn)
default:
gdprintk(XENLOG_WARNING,
"Error while installing new compat baseptr %" PRI_mfn "\n",
- mfn);
+ mfn_x(mfn));
return rc;
}
@@ -2814,20 +2814,20 @@ int new_guest_cr3(unsigned long mfn)
if ( unlikely(rc) )
return rc;
- old_base_mfn = pagetable_get_pfn(curr->arch.guest_table);
+ old_base_mfn = pagetable_get_mfn(curr->arch.guest_table);
/*
* This is particularly important when getting restarted after the
* previous attempt got preempted in the put-old-MFN phase.
*/
- if ( old_base_mfn == mfn )
+ if ( mfn_eq(old_base_mfn, mfn) )
{
write_ptbase(curr);
return 0;
}
rc = paging_mode_refcounts(d)
- ? (get_page_from_mfn(_mfn(mfn), d) ? 0 : -EINVAL)
- : get_page_and_type_from_mfn(_mfn(mfn), PGT_root_page_table, d, 0, 1);
+ ? (get_page_from_mfn(mfn, d) ? 0 : -EINVAL)
+ : get_page_and_type_from_mfn(mfn, PGT_root_page_table, d, 0, 1);
switch ( rc )
{
case 0:
@@ -2837,22 +2837,23 @@ int new_guest_cr3(unsigned long mfn)
return -ERESTART;
default:
gdprintk(XENLOG_WARNING,
- "Error while installing new baseptr %" PRI_mfn "\n", mfn);
+ "Error while installing new baseptr %" PRI_mfn "\n",
+ mfn_x(mfn));
return rc;
}
invalidate_shadow_ldt(curr, 0);
if ( !VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) )
- fill_ro_mpt(_mfn(mfn));
- curr->arch.guest_table = pagetable_from_pfn(mfn);
+ fill_ro_mpt(mfn);
+ curr->arch.guest_table = pagetable_from_mfn(mfn);
update_cr3(curr);
write_ptbase(curr);
- if ( likely(old_base_mfn != 0) )
+ if ( likely(mfn_x(old_base_mfn) != 0) )
{
- struct page_info *page = mfn_to_page(_mfn(old_base_mfn));
+ struct page_info *page = mfn_to_page(old_base_mfn);
if ( paging_mode_refcounts(d) )
put_page(page);
@@ -3180,7 +3181,7 @@ long do_mmuext_op(
else if ( unlikely(paging_mode_translate(currd)) )
rc = -EINVAL;
else
- rc = new_guest_cr3(op.arg1.mfn);
+ rc = new_guest_cr3(_mfn(op.arg1.mfn));
break;
case MMUEXT_NEW_USER_BASEPTR: {
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index af1624a..54a63c2 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -774,7 +774,7 @@ static int priv_op_write_cr(unsigned int reg, unsigned long val,
page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
if ( !page )
break;
- rc = new_guest_cr3(mfn_x(page_to_mfn(page)));
+ rc = new_guest_cr3(page_to_mfn(page));
put_page(page);
switch ( rc )
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index ec7ce3c..4c03a33 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -539,7 +539,7 @@ void audit_domains(void);
#endif
-int new_guest_cr3(unsigned long pfn);
+int new_guest_cr3(mfn_t mfn);
void make_cr3(struct vcpu *v, unsigned long mfn);
void update_cr3(struct vcpu *v);
int vcpu_destroy_pagetables(struct vcpu *);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] x86/mm: Use mfn_t for make_cr3()
2017-08-30 12:19 [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Andrew Cooper
@ 2017-08-30 12:19 ` Andrew Cooper
2017-08-30 13:08 ` Wei Liu
` (2 more replies)
2017-08-30 13:08 ` [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Wei Liu
` (2 subsequent siblings)
3 siblings, 3 replies; 11+ messages in thread
From: Andrew Cooper @ 2017-08-30 12:19 UTC (permalink / raw)
To: Xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu, Jan Beulich, Tim Deegan
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Tim Deegan <tim@xen.org>
CC: George Dunlap <george.dunlap@eu.citrix.com>
---
xen/arch/x86/mm.c | 10 +++++-----
xen/arch/x86/mm/hap/hap.c | 2 +-
xen/arch/x86/mm/shadow/common.c | 8 ++++----
xen/arch/x86/mm/shadow/multi.c | 4 ++--
xen/include/asm-x86/mm.h | 2 +-
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index dc07b4f..f0c81e7 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -498,9 +498,9 @@ void free_shared_domheap_page(struct page_info *page)
free_domheap_page(page);
}
-void make_cr3(struct vcpu *v, unsigned long mfn)
+void make_cr3(struct vcpu *v, mfn_t mfn)
{
- v->arch.cr3 = mfn << PAGE_SHIFT;
+ v->arch.cr3 = mfn_x(mfn) << PAGE_SHIFT;
}
void write_ptbase(struct vcpu *v)
@@ -518,7 +518,7 @@ void write_ptbase(struct vcpu *v)
*/
void update_cr3(struct vcpu *v)
{
- unsigned long cr3_mfn;
+ mfn_t cr3_mfn;
if ( paging_mode_enabled(v->domain) )
{
@@ -527,9 +527,9 @@ void update_cr3(struct vcpu *v)
}
if ( !(v->arch.flags & TF_kernel_mode) )
- cr3_mfn = pagetable_get_pfn(v->arch.guest_table_user);
+ cr3_mfn = pagetable_get_mfn(v->arch.guest_table_user);
else
- cr3_mfn = pagetable_get_pfn(v->arch.guest_table);
+ cr3_mfn = pagetable_get_mfn(v->arch.guest_table);
make_cr3(v, cr3_mfn);
}
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 15e4877..6946fde 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -719,7 +719,7 @@ static void hap_update_paging_modes(struct vcpu *v)
{
mfn_t mmfn = hap_make_monitor_table(v);
v->arch.monitor_table = pagetable_from_mfn(mmfn);
- make_cr3(v, mfn_x(mmfn));
+ make_cr3(v, mmfn);
hvm_update_host_cr3(v);
}
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index e8ee6db..3926ed6 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2961,7 +2961,7 @@ static void sh_update_paging_modes(struct vcpu *v)
{
mfn_t mmfn = v->arch.paging.mode->shadow.make_monitor_table(v);
v->arch.monitor_table = pagetable_from_mfn(mmfn);
- make_cr3(v, mfn_x(mmfn));
+ make_cr3(v, mmfn);
hvm_update_host_cr3(v);
}
@@ -3004,7 +3004,7 @@ static void sh_update_paging_modes(struct vcpu *v)
/* Don't be running on the old monitor table when we
* pull it down! Switch CR3, and warn the HVM code that
* its host cr3 has changed. */
- make_cr3(v, mfn_x(new_mfn));
+ make_cr3(v, new_mfn);
if ( v == current )
write_ptbase(v);
hvm_update_host_cr3(v);
@@ -3380,9 +3380,9 @@ static int shadow_one_bit_disable(struct domain *d, u32 mode)
if ( v->arch.paging.mode )
v->arch.paging.mode->shadow.detach_old_tables(v);
if ( !(v->arch.flags & TF_kernel_mode) )
- make_cr3(v, pagetable_get_pfn(v->arch.guest_table_user));
+ make_cr3(v, pagetable_get_mfn(v->arch.guest_table_user));
else
- make_cr3(v, pagetable_get_pfn(v->arch.guest_table));
+ make_cr3(v, pagetable_get_mfn(v->arch.guest_table));
#if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
{
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index c5c0af8..f7efe66 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -4273,7 +4273,7 @@ sh_update_cr3(struct vcpu *v, int do_locking)
///
if ( shadow_mode_external(d) )
{
- make_cr3(v, pagetable_get_pfn(v->arch.monitor_table));
+ make_cr3(v, pagetable_get_mfn(v->arch.monitor_table));
}
else // not shadow_mode_external...
{
@@ -4287,7 +4287,7 @@ sh_update_cr3(struct vcpu *v, int do_locking)
v->arch.cr3 = virt_to_maddr(&v->arch.paging.shadow.l3table);
#else
/* 4-on-4: Just use the shadow top-level directly */
- make_cr3(v, pagetable_get_pfn(v->arch.shadow_table[0]));
+ make_cr3(v, pagetable_get_mfn(v->arch.shadow_table[0]));
#endif
}
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 4c03a33..dce3f16 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -540,7 +540,7 @@ void audit_domains(void);
#endif
int new_guest_cr3(mfn_t mfn);
-void make_cr3(struct vcpu *v, unsigned long mfn);
+void make_cr3(struct vcpu *v, mfn_t mfn);
void update_cr3(struct vcpu *v);
int vcpu_destroy_pagetables(struct vcpu *);
void *do_page_walk(struct vcpu *v, unsigned long addr);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86/mm: Use mfn_t for make_cr3()
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
@ 2017-08-30 13:08 ` Wei Liu
2017-08-30 15:47 ` Jan Beulich
2017-08-30 18:59 ` Tim Deegan
2017-09-01 17:30 ` George Dunlap
2 siblings, 1 reply; 11+ messages in thread
From: Wei Liu @ 2017-08-30 13:08 UTC (permalink / raw)
To: Andrew Cooper; +Cc: George Dunlap, Tim Deegan, Wei Liu, Jan Beulich, Xen-devel
On Wed, Aug 30, 2017 at 01:19:33PM +0100, Andrew Cooper wrote:
> No functional change.
>
> 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] 11+ messages in thread
* Re: [PATCH 2/2] x86/mm: Use mfn_t for make_cr3()
2017-08-30 13:08 ` Wei Liu
@ 2017-08-30 15:47 ` Jan Beulich
0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2017-08-30 15:47 UTC (permalink / raw)
To: Andrew Cooper, WeiLiu; +Cc: George Dunlap, Tim Deegan, Xen-devel
>>> On 30.08.17 at 15:08, <wei.liu2@citrix.com> wrote:
> On Wed, Aug 30, 2017 at 01:19:33PM +0100, Andrew Cooper wrote:
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
As a side note, I can see instances of mmfn here too, but here (other
than in patch 1) the first m quite clearly stands for "monitor". There's
nothing formally called "monitor" on that other path, though.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86/mm: Use mfn_t for make_cr3()
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
2017-08-30 13:08 ` Wei Liu
@ 2017-08-30 18:59 ` Tim Deegan
2017-09-01 17:30 ` George Dunlap
2 siblings, 0 replies; 11+ messages in thread
From: Tim Deegan @ 2017-08-30 18:59 UTC (permalink / raw)
To: Andrew Cooper; +Cc: George Dunlap, Wei Liu, Jan Beulich, Xen-devel
At 13:19 +0100 on 30 Aug (1504099173), Andrew Cooper wrote:
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] x86/mm: Use mfn_t for make_cr3()
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
2017-08-30 13:08 ` Wei Liu
2017-08-30 18:59 ` Tim Deegan
@ 2017-09-01 17:30 ` George Dunlap
2 siblings, 0 replies; 11+ messages in thread
From: George Dunlap @ 2017-09-01 17:30 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Tim Deegan, Wei Liu, Jan Beulich, Xen-devel
On Wed, Aug 30, 2017 at 1:19 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Tim Deegan <tim@xen.org>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> xen/arch/x86/mm.c | 10 +++++-----
> xen/arch/x86/mm/hap/hap.c | 2 +-
> xen/arch/x86/mm/shadow/common.c | 8 ++++----
> xen/arch/x86/mm/shadow/multi.c | 4 ++--
> xen/include/asm-x86/mm.h | 2 +-
> 5 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index dc07b4f..f0c81e7 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -498,9 +498,9 @@ void free_shared_domheap_page(struct page_info *page)
> free_domheap_page(page);
> }
>
> -void make_cr3(struct vcpu *v, unsigned long mfn)
> +void make_cr3(struct vcpu *v, mfn_t mfn)
> {
> - v->arch.cr3 = mfn << PAGE_SHIFT;
> + v->arch.cr3 = mfn_x(mfn) << PAGE_SHIFT;
> }
>
> void write_ptbase(struct vcpu *v)
> @@ -518,7 +518,7 @@ void write_ptbase(struct vcpu *v)
> */
> void update_cr3(struct vcpu *v)
> {
> - unsigned long cr3_mfn;
> + mfn_t cr3_mfn;
>
> if ( paging_mode_enabled(v->domain) )
> {
> @@ -527,9 +527,9 @@ void update_cr3(struct vcpu *v)
> }
>
> if ( !(v->arch.flags & TF_kernel_mode) )
> - cr3_mfn = pagetable_get_pfn(v->arch.guest_table_user);
> + cr3_mfn = pagetable_get_mfn(v->arch.guest_table_user);
> else
> - cr3_mfn = pagetable_get_pfn(v->arch.guest_table);
> + cr3_mfn = pagetable_get_mfn(v->arch.guest_table);
>
> make_cr3(v, cr3_mfn);
> }
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index 15e4877..6946fde 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -719,7 +719,7 @@ static void hap_update_paging_modes(struct vcpu *v)
> {
> mfn_t mmfn = hap_make_monitor_table(v);
> v->arch.monitor_table = pagetable_from_mfn(mmfn);
> - make_cr3(v, mfn_x(mmfn));
> + make_cr3(v, mmfn);
> hvm_update_host_cr3(v);
> }
>
> diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
> index e8ee6db..3926ed6 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -2961,7 +2961,7 @@ static void sh_update_paging_modes(struct vcpu *v)
> {
> mfn_t mmfn = v->arch.paging.mode->shadow.make_monitor_table(v);
> v->arch.monitor_table = pagetable_from_mfn(mmfn);
> - make_cr3(v, mfn_x(mmfn));
> + make_cr3(v, mmfn);
> hvm_update_host_cr3(v);
> }
>
> @@ -3004,7 +3004,7 @@ static void sh_update_paging_modes(struct vcpu *v)
> /* Don't be running on the old monitor table when we
> * pull it down! Switch CR3, and warn the HVM code that
> * its host cr3 has changed. */
> - make_cr3(v, mfn_x(new_mfn));
> + make_cr3(v, new_mfn);
> if ( v == current )
> write_ptbase(v);
> hvm_update_host_cr3(v);
> @@ -3380,9 +3380,9 @@ static int shadow_one_bit_disable(struct domain *d, u32 mode)
> if ( v->arch.paging.mode )
> v->arch.paging.mode->shadow.detach_old_tables(v);
> if ( !(v->arch.flags & TF_kernel_mode) )
> - make_cr3(v, pagetable_get_pfn(v->arch.guest_table_user));
> + make_cr3(v, pagetable_get_mfn(v->arch.guest_table_user));
> else
> - make_cr3(v, pagetable_get_pfn(v->arch.guest_table));
> + make_cr3(v, pagetable_get_mfn(v->arch.guest_table));
>
> #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
> {
> diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
> index c5c0af8..f7efe66 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -4273,7 +4273,7 @@ sh_update_cr3(struct vcpu *v, int do_locking)
> ///
> if ( shadow_mode_external(d) )
> {
> - make_cr3(v, pagetable_get_pfn(v->arch.monitor_table));
> + make_cr3(v, pagetable_get_mfn(v->arch.monitor_table));
> }
> else // not shadow_mode_external...
> {
> @@ -4287,7 +4287,7 @@ sh_update_cr3(struct vcpu *v, int do_locking)
> v->arch.cr3 = virt_to_maddr(&v->arch.paging.shadow.l3table);
> #else
> /* 4-on-4: Just use the shadow top-level directly */
> - make_cr3(v, pagetable_get_pfn(v->arch.shadow_table[0]));
> + make_cr3(v, pagetable_get_mfn(v->arch.shadow_table[0]));
> #endif
> }
>
> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> index 4c03a33..dce3f16 100644
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -540,7 +540,7 @@ void audit_domains(void);
> #endif
>
> int new_guest_cr3(mfn_t mfn);
> -void make_cr3(struct vcpu *v, unsigned long mfn);
> +void make_cr3(struct vcpu *v, mfn_t mfn);
> void update_cr3(struct vcpu *v);
> int vcpu_destroy_pagetables(struct vcpu *);
> void *do_page_walk(struct vcpu *v, unsigned long addr);
> --
> 2.1.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()
2017-08-30 12:19 [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Andrew Cooper
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
@ 2017-08-30 13:08 ` Wei Liu
2017-08-30 15:45 ` Jan Beulich
2017-09-01 17:29 ` George Dunlap
3 siblings, 0 replies; 11+ messages in thread
From: Wei Liu @ 2017-08-30 13:08 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Jan Beulich, Xen-devel
On Wed, Aug 30, 2017 at 01:19:32PM +0100, Andrew Cooper wrote:
> No functional change (as confirmed by diffing the assembly).
>
> 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] 11+ messages in thread
* Re: [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()
2017-08-30 12:19 [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Andrew Cooper
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
2017-08-30 13:08 ` [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Wei Liu
@ 2017-08-30 15:45 ` Jan Beulich
2017-08-30 15:57 ` Andrew Cooper
2017-09-01 17:29 ` George Dunlap
3 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2017-08-30 15:45 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Xen-devel
>>> On 30.08.17 at 14:19, <andrew.cooper3@citrix.com> wrote:
> @@ -2772,23 +2772,23 @@ int vcpu_destroy_pagetables(struct vcpu *v)
> return rc != -EINTR ? rc : -ERESTART;
> }
>
> -int new_guest_cr3(unsigned long mfn)
> +int new_guest_cr3(mfn_t mfn)
> {
> struct vcpu *curr = current;
> struct domain *d = curr->domain;
> int rc;
> - unsigned long old_base_mfn;
> + mfn_t old_base_mfn;
>
> if ( is_pv_32bit_domain(d) )
> {
> - unsigned long gt_mfn = pagetable_get_pfn(curr->arch.guest_table);
> - l4_pgentry_t *pl4e = map_domain_page(_mfn(gt_mfn));
> + mfn_t mmfn = pagetable_get_mfn(curr->arch.guest_table);
> + l4_pgentry_t *pl4e = map_domain_page(mmfn);
What was wrong with "gt_mfn" for "guest table MFN"? I can't help
thinking mmfn is a typo, where you've hit the m key one too many
times. What is that first m supposed to stand for?
Other than this the patch is certainly fine.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()
2017-08-30 15:45 ` Jan Beulich
@ 2017-08-30 15:57 ` Andrew Cooper
2017-08-30 16:04 ` Jan Beulich
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Cooper @ 2017-08-30 15:57 UTC (permalink / raw)
To: Jan Beulich; +Cc: Wei Liu, Xen-devel
On 30/08/17 16:45, Jan Beulich wrote:
>>>> On 30.08.17 at 14:19, <andrew.cooper3@citrix.com> wrote:
>> @@ -2772,23 +2772,23 @@ int vcpu_destroy_pagetables(struct vcpu *v)
>> return rc != -EINTR ? rc : -ERESTART;
>> }
>>
>> -int new_guest_cr3(unsigned long mfn)
>> +int new_guest_cr3(mfn_t mfn)
>> {
>> struct vcpu *curr = current;
>> struct domain *d = curr->domain;
>> int rc;
>> - unsigned long old_base_mfn;
>> + mfn_t old_base_mfn;
>>
>> if ( is_pv_32bit_domain(d) )
>> {
>> - unsigned long gt_mfn = pagetable_get_pfn(curr->arch.guest_table);
>> - l4_pgentry_t *pl4e = map_domain_page(_mfn(gt_mfn));
>> + mfn_t mmfn = pagetable_get_mfn(curr->arch.guest_table);
>> + l4_pgentry_t *pl4e = map_domain_page(mmfn);
> What was wrong with "gt_mfn" for "guest table MFN"? I can't help
> thinking mmfn is a typo, where you've hit the m key one too many
> times. What is that first m supposed to stand for?
mmfn is the shadow code nomenclature for the monitor mfn. I can move
back to gt_mfn if you think thats clearer.
~Andrew
>
> Other than this the patch is certainly fine.
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()
2017-08-30 15:57 ` Andrew Cooper
@ 2017-08-30 16:04 ` Jan Beulich
0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2017-08-30 16:04 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Xen-devel
>>> On 30.08.17 at 17:57, <andrew.cooper3@citrix.com> wrote:
> On 30/08/17 16:45, Jan Beulich wrote:
>>>>> On 30.08.17 at 14:19, <andrew.cooper3@citrix.com> wrote:
>>> @@ -2772,23 +2772,23 @@ int vcpu_destroy_pagetables(struct vcpu *v)
>>> return rc != -EINTR ? rc : -ERESTART;
>>> }
>>>
>>> -int new_guest_cr3(unsigned long mfn)
>>> +int new_guest_cr3(mfn_t mfn)
>>> {
>>> struct vcpu *curr = current;
>>> struct domain *d = curr->domain;
>>> int rc;
>>> - unsigned long old_base_mfn;
>>> + mfn_t old_base_mfn;
>>>
>>> if ( is_pv_32bit_domain(d) )
>>> {
>>> - unsigned long gt_mfn = pagetable_get_pfn(curr->arch.guest_table);
>>> - l4_pgentry_t *pl4e = map_domain_page(_mfn(gt_mfn));
>>> + mfn_t mmfn = pagetable_get_mfn(curr->arch.guest_table);
>>> + l4_pgentry_t *pl4e = map_domain_page(mmfn);
>> What was wrong with "gt_mfn" for "guest table MFN"? I can't help
>> thinking mmfn is a typo, where you've hit the m key one too many
>> times. What is that first m supposed to stand for?
>
> mmfn is the shadow code nomenclature for the monitor mfn. I can move
> back to gt_mfn if you think thats clearer.
I'm not going to insist, but I'd appreciate keeping the original name.
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3()
2017-08-30 12:19 [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Andrew Cooper
` (2 preceding siblings ...)
2017-08-30 15:45 ` Jan Beulich
@ 2017-09-01 17:29 ` George Dunlap
3 siblings, 0 replies; 11+ messages in thread
From: George Dunlap @ 2017-09-01 17:29 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Jan Beulich, Xen-devel
On Wed, Aug 30, 2017 at 1:19 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> No functional change (as confirmed by diffing the assembly).
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
>
> For some reason best known to GCC, there is one single change:
>
> @@ -145835,7 +145835,7 @@
> ffff82d0802864f8: 85 c0 test %eax,%eax
> ffff82d0802864fa: 75 e0 jne ffff82d0802864dc <new_guest_cr3+0x7c>
> ffff82d0802864fc: 4c 8b ad 08 0b 00 00 mov 0xb08(%rbp),%r13
> -ffff82d080286503: 49 39 dd cmp %rbx,%r13
> +ffff82d080286503: 4c 39 eb cmp %r13,%rbx
> ffff82d080286506: 0f 84 74 01 00 00 je ffff82d080286680 <new_guest_cr3+0x220>
> ffff82d08028650c: 41 f6 84 24 19 06 00 testb $0x8,0x619(%r12)
> ffff82d080286513: 00 08
>
> This is from the mfn_eq() alteration, and must be a side effect from using an
> inline function. The net result is still correct, as only the zero flag is
> checked.
> ---
> xen/arch/x86/mm.c | 35 ++++++++++++++++++-----------------
> xen/arch/x86/pv/emul-priv-op.c | 2 +-
> xen/include/asm-x86/mm.h | 2 +-
> 3 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 1f23470..dc07b4f 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2772,23 +2772,23 @@ int vcpu_destroy_pagetables(struct vcpu *v)
> return rc != -EINTR ? rc : -ERESTART;
> }
>
> -int new_guest_cr3(unsigned long mfn)
> +int new_guest_cr3(mfn_t mfn)
> {
> struct vcpu *curr = current;
> struct domain *d = curr->domain;
> int rc;
> - unsigned long old_base_mfn;
> + mfn_t old_base_mfn;
>
> if ( is_pv_32bit_domain(d) )
> {
> - unsigned long gt_mfn = pagetable_get_pfn(curr->arch.guest_table);
> - l4_pgentry_t *pl4e = map_domain_page(_mfn(gt_mfn));
> + mfn_t mmfn = pagetable_get_mfn(curr->arch.guest_table);
> + l4_pgentry_t *pl4e = map_domain_page(mmfn);
>
> rc = mod_l4_entry(pl4e,
> - l4e_from_pfn(mfn,
> + l4e_from_mfn(mfn,
> (_PAGE_PRESENT | _PAGE_RW |
> _PAGE_USER | _PAGE_ACCESSED)),
> - gt_mfn, 0, curr);
> + mfn_x(mmfn), 0, curr);
> unmap_domain_page(pl4e);
> switch ( rc )
> {
> @@ -2800,7 +2800,7 @@ int new_guest_cr3(unsigned long mfn)
> default:
> gdprintk(XENLOG_WARNING,
> "Error while installing new compat baseptr %" PRI_mfn "\n",
> - mfn);
> + mfn_x(mfn));
> return rc;
> }
>
> @@ -2814,20 +2814,20 @@ int new_guest_cr3(unsigned long mfn)
> if ( unlikely(rc) )
> return rc;
>
> - old_base_mfn = pagetable_get_pfn(curr->arch.guest_table);
> + old_base_mfn = pagetable_get_mfn(curr->arch.guest_table);
> /*
> * This is particularly important when getting restarted after the
> * previous attempt got preempted in the put-old-MFN phase.
> */
> - if ( old_base_mfn == mfn )
> + if ( mfn_eq(old_base_mfn, mfn) )
> {
> write_ptbase(curr);
> return 0;
> }
>
> rc = paging_mode_refcounts(d)
> - ? (get_page_from_mfn(_mfn(mfn), d) ? 0 : -EINVAL)
> - : get_page_and_type_from_mfn(_mfn(mfn), PGT_root_page_table, d, 0, 1);
> + ? (get_page_from_mfn(mfn, d) ? 0 : -EINVAL)
> + : get_page_and_type_from_mfn(mfn, PGT_root_page_table, d, 0, 1);
> switch ( rc )
> {
> case 0:
> @@ -2837,22 +2837,23 @@ int new_guest_cr3(unsigned long mfn)
> return -ERESTART;
> default:
> gdprintk(XENLOG_WARNING,
> - "Error while installing new baseptr %" PRI_mfn "\n", mfn);
> + "Error while installing new baseptr %" PRI_mfn "\n",
> + mfn_x(mfn));
> return rc;
> }
>
> invalidate_shadow_ldt(curr, 0);
>
> if ( !VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) )
> - fill_ro_mpt(_mfn(mfn));
> - curr->arch.guest_table = pagetable_from_pfn(mfn);
> + fill_ro_mpt(mfn);
> + curr->arch.guest_table = pagetable_from_mfn(mfn);
> update_cr3(curr);
>
> write_ptbase(curr);
>
> - if ( likely(old_base_mfn != 0) )
> + if ( likely(mfn_x(old_base_mfn) != 0) )
> {
> - struct page_info *page = mfn_to_page(_mfn(old_base_mfn));
> + struct page_info *page = mfn_to_page(old_base_mfn);
>
> if ( paging_mode_refcounts(d) )
> put_page(page);
> @@ -3180,7 +3181,7 @@ long do_mmuext_op(
> else if ( unlikely(paging_mode_translate(currd)) )
> rc = -EINVAL;
> else
> - rc = new_guest_cr3(op.arg1.mfn);
> + rc = new_guest_cr3(_mfn(op.arg1.mfn));
> break;
>
> case MMUEXT_NEW_USER_BASEPTR: {
> diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
> index af1624a..54a63c2 100644
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -774,7 +774,7 @@ static int priv_op_write_cr(unsigned int reg, unsigned long val,
> page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
> if ( !page )
> break;
> - rc = new_guest_cr3(mfn_x(page_to_mfn(page)));
> + rc = new_guest_cr3(page_to_mfn(page));
> put_page(page);
>
> switch ( rc )
> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> index ec7ce3c..4c03a33 100644
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -539,7 +539,7 @@ void audit_domains(void);
>
> #endif
>
> -int new_guest_cr3(unsigned long pfn);
> +int new_guest_cr3(mfn_t mfn);
> void make_cr3(struct vcpu *v, unsigned long mfn);
> void update_cr3(struct vcpu *v);
> int vcpu_destroy_pagetables(struct vcpu *);
> --
> 2.1.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-09-01 17:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 12:19 [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Andrew Cooper
2017-08-30 12:19 ` [PATCH 2/2] x86/mm: Use mfn_t for make_cr3() Andrew Cooper
2017-08-30 13:08 ` Wei Liu
2017-08-30 15:47 ` Jan Beulich
2017-08-30 18:59 ` Tim Deegan
2017-09-01 17:30 ` George Dunlap
2017-08-30 13:08 ` [PATCH 1/2] x86/mm: Use mfn_t for new_guest_cr3() Wei Liu
2017-08-30 15:45 ` Jan Beulich
2017-08-30 15:57 ` Andrew Cooper
2017-08-30 16:04 ` Jan Beulich
2017-09-01 17:29 ` George Dunlap
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.