Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
At 11:05 +0100 on 24 Aug (1503572736), Wei Liu wrote:
> On Thu, Aug 24, 2017 at 11:02:33AM +0100, Tim Deegan wrote:
> > At 16:58 +0100 on 23 Aug (1503507504), Wei Liu wrote:
> > > After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
> > > PG_refcounts"), PG_refcounts and PG_translate always need to be set
> > > together.
> > >
> > > Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
> > > are replaced by calls to paging_mode_translate.
> >
> > Would it be a good idea to merge all three and have only PG_external?
> >
>
> My reverse-engineering is that when PV guest is migrated it has
> PG_external and (the new) PG_translate.
>
> I would be happy to squash all three into one if you tell me I'm wrong.
> :-)
I _think_ you're wrong :) but can't check right now as I'm
semi-offline. Migrating PV guests should have paging enabled but
not any of external, translate or refcounts.
There was once code that used PG_translate with PV guests but never
AFAIK checked in to upstream Xen. There was some talk of using that
combination for PVH at one point but IIRC PVH now uses
translate|refcounts|external like HVM.
Cheers,
Tim.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
On Thu, Aug 24, 2017 at 11:09:38AM +0100, Andrew Cooper wrote:
> On 24/08/17 11:07, Wei Liu wrote:
> > On Thu, Aug 24, 2017 at 11:05:36AM +0100, Wei Liu wrote:
> >> On Thu, Aug 24, 2017 at 11:02:33AM +0100, Tim Deegan wrote:
> >>> At 16:58 +0100 on 23 Aug (1503507504), Wei Liu wrote:
> After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
> PG_refcounts"), PG_refcounts and PG_translate always need to be set
> together.
>
> Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
> are replaced by calls to paging_mode_translate.
> >>> Would it be a good idea to merge all three and have only PG_external?
> >>>
> >> My reverse-engineering is that when PV guest is migrated it has
> >> PG_external and (the new) PG_translate.
> > Sorry, I meant (!PG_external && PG_translate)
>
> PV guests migrating should get logdirty and none of
> PG_external|PG_translate|PG_refcounts
>
OK, in that case I will squash PG_external as well.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
On 24/08/17 11:07, Wei Liu wrote:
> On Thu, Aug 24, 2017 at 11:05:36AM +0100, Wei Liu wrote:
>> On Thu, Aug 24, 2017 at 11:02:33AM +0100, Tim Deegan wrote:
>>> At 16:58 +0100 on 23 Aug (1503507504), Wei Liu wrote:
After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
PG_refcounts"), PG_refcounts and PG_translate always need to be set
together.
Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
are replaced by calls to paging_mode_translate.
>>> Would it be a good idea to merge all three and have only PG_external?
>>>
>> My reverse-engineering is that when PV guest is migrated it has
>> PG_external and (the new) PG_translate.
> Sorry, I meant (!PG_external && PG_translate)
PV guests migrating should get logdirty and none of
PG_external|PG_translate|PG_refcounts
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
On Thu, Aug 24, 2017 at 11:05:36AM +0100, Wei Liu wrote:
> On Thu, Aug 24, 2017 at 11:02:33AM +0100, Tim Deegan wrote:
> > At 16:58 +0100 on 23 Aug (1503507504), Wei Liu wrote:
> > > After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
> > > PG_refcounts"), PG_refcounts and PG_translate always need to be set
> > > together.
> > >
> > > Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
> > > are replaced by calls to paging_mode_translate.
> >
> > Would it be a good idea to merge all three and have only PG_external?
> >
>
> My reverse-engineering is that when PV guest is migrated it has
> PG_external and (the new) PG_translate.
Sorry, I meant (!PG_external && PG_translate)
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
On Thu, Aug 24, 2017 at 11:02:33AM +0100, Tim Deegan wrote:
> At 16:58 +0100 on 23 Aug (1503507504), Wei Liu wrote:
> > After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
> > PG_refcounts"), PG_refcounts and PG_translate always need to be set
> > together.
> >
> > Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
> > are replaced by calls to paging_mode_translate.
>
> Would it be a good idea to merge all three and have only PG_external?
>
My reverse-engineering is that when PV guest is migrated it has
PG_external and (the new) PG_translate.
I would be happy to squash all three into one if you tell me I'm wrong.
:-)
> > Signed-off-by: Wei Liu
>
> Acked-by: Tim Deegan
>
> with one adjustment:
>
> > /* common paging mode bits */
> > #define PG_mode_shift 10
> > -/* Refcounts based on shadow tables instead of guest tables */
> > -#define PG_refcounts (XEN_DOMCTL_SHADOW_ENABLE_REFCOUNT << PG_mode_shift)
> > /* Enable log dirty mode */
> > #define PG_log_dirty (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY <<
> > PG_mode_shift)
> > /* Xen does p2m translation, not guest */
>
> Please add "and handles refcounts" to the comment describing PG_translate.
>
Sure.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
At 16:58 +0100 on 23 Aug (1503507504), Wei Liu wrote:
> After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
> PG_refcounts"), PG_refcounts and PG_translate always need to be set
> together.
>
> Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
> are replaced by calls to paging_mode_translate.
Would it be a good idea to merge all three and have only PG_external?
> Signed-off-by: Wei Liu
Acked-by: Tim Deegan
with one adjustment:
> /* common paging mode bits */
> #define PG_mode_shift 10
> -/* Refcounts based on shadow tables instead of guest tables */
> -#define PG_refcounts (XEN_DOMCTL_SHADOW_ENABLE_REFCOUNT << PG_mode_shift)
> /* Enable log dirty mode */
> #define PG_log_dirty (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)
> /* Xen does p2m translation, not guest */
Please add "and handles refcounts" to the comment describing PG_translate.
Cheers,
Tim.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
On Wed, Aug 23, 2017 at 04:58:24PM +0100, Wei Liu wrote:
> After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
> PG_refcounts"), PG_refcounts and PG_translate always need to be set
> together.
>
> Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
> are replaced by calls to paging_mode_translate.
>
> Signed-off-by: Wei Liu
> ---
> Cc: George Dunlap
> Cc: Tim Deegan
> Cc: Jan Beulich
> Cc: Andrew Cooper
> ---
> xen/arch/x86/domain.c| 6 ++
> xen/arch/x86/hvm/hvm.c | 2 +-
> xen/arch/x86/mm.c| 20 ++--
> xen/arch/x86/mm/paging.c | 8 +++-
> xen/include/asm-x86/paging.h | 5 +
> xen/include/asm-x86/shadow.h | 2 +-
> 6 files changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 9b4b9596d8..bbe545b165 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1017,8 +1017,6 @@ int arch_set_info_guest(
>
> if ( !cr3_page )
> rc = -EINVAL;
> -else if ( paging_mode_refcounts(d) )
> -/* nothing */;
It appears I accidentally deleted this hunk, but my tests were still
happy...
Given this function is rather convoluted I intend to add it back and
deal with that later.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH RFC 2/2] x86/mm: PG_translate implies PG_refcounts
After 404595352 ("x86/paging: Enforce PG_external == PG_translate ==
PG_refcounts"), PG_refcounts and PG_translate always need to be set
together.
Squash PG_refcounts to simplify code. All calls paging_mode_refcounts
are replaced by calls to paging_mode_translate.
Signed-off-by: Wei Liu
---
Cc: George Dunlap
Cc: Tim Deegan
Cc: Jan Beulich
Cc: Andrew Cooper
---
xen/arch/x86/domain.c| 6 ++
xen/arch/x86/hvm/hvm.c | 2 +-
xen/arch/x86/mm.c| 20 ++--
xen/arch/x86/mm/paging.c | 8 +++-
xen/include/asm-x86/paging.h | 5 +
xen/include/asm-x86/shadow.h | 2 +-
6 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 9b4b9596d8..bbe545b165 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1017,8 +1017,6 @@ int arch_set_info_guest(
if ( !cr3_page )
rc = -EINVAL;
-else if ( paging_mode_refcounts(d) )
-/* nothing */;
else if ( cr3_page == v->arch.old_guest_table )
{
v->arch.old_guest_table = NULL;
@@ -1040,7 +1038,7 @@ int arch_set_info_guest(
break;
case 0:
if ( !compat && !VM_ASSIST(d, m2p_strict) &&
- !paging_mode_refcounts(d) )
+ !paging_mode_translate(d) )
fill_ro_mpt(cr3_gfn);
break;
default:
@@ -1061,7 +1059,7 @@ int arch_set_info_guest(
if ( !cr3_page )
rc = -EINVAL;
-else if ( !paging_mode_refcounts(d) )
+else if ( !paging_mode_translate(d) )
{
rc = get_page_type_preemptible(cr3_page, PGT_root_page_table);
switch ( rc )
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 6cb903def5..eee0ff422e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -589,7 +589,7 @@ int hvm_domain_initialise(struct domain *d, unsigned long
domcr_flags,
hvm_init_cacheattr_region_list(d);
-rc = paging_enable(d, PG_refcounts|PG_translate|PG_external);
+rc = paging_enable(d, PG_translate|PG_external);
if ( rc != 0 )
goto fail0;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index ed77270586..8adb7b6649 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1925,7 +1925,7 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t
nl1e,
if ( unlikely(__copy_from_user(, pl1e, sizeof(ol1e)) != 0) )
return -EFAULT;
-ASSERT(!paging_mode_refcounts(pt_dom));
+ASSERT(!paging_mode_translate(pt_dom));
if ( l1e_get_flags(nl1e) & _PAGE_PRESENT )
{
@@ -2257,7 +2257,7 @@ int get_page(struct page_info *page, struct domain
*domain)
if ( likely(owner == domain) )
return 1;
-if ( !paging_mode_refcounts(domain) && !domain->is_dying )
+if ( !paging_mode_translate(domain) && !domain->is_dying )
gprintk(XENLOG_INFO,
"Error mfn %"PRI_mfn": rd=%d od=%d caf=%08lx taf=%"
PRtype_info "\n",
mfn_x(page_to_mfn(page)), domain->domain_id,
@@ -2719,7 +2719,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
if ( mfn )
{
page = mfn_to_page(_mfn(mfn));
-if ( paging_mode_refcounts(v->domain) )
+if ( paging_mode_translate(v->domain) )
put_page(page);
else
rc = put_page_and_type_preemptible(page);
@@ -2740,7 +2740,7 @@ int vcpu_destroy_pagetables(struct vcpu *v)
if ( mfn )
{
page = mfn_to_page(_mfn(mfn));
-if ( paging_mode_refcounts(v->domain) )
+if ( paging_mode_translate(v->domain) )
put_page(page);
else
rc = put_page_and_type_preemptible(page);
@@ -2811,7 +2811,7 @@ int new_guest_cr3(unsigned long mfn)
return 0;
}
-rc = paging_mode_refcounts(d)
+rc = paging_mode_translate(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);
switch ( rc )
@@ -2829,7 +2829,7 @@ int new_guest_cr3(unsigned long mfn)
invalidate_shadow_ldt(curr, 0);
-if ( !VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) )
+if ( !VM_ASSIST(d, m2p_strict) && !paging_mode_translate(d) )
fill_ro_mpt(mfn);
curr->arch.guest_table = pagetable_from_pfn(mfn);
update_cr3(curr);
@@ -2840,7 +2840,7 @@ int new_guest_cr3(unsigned long mfn)
{
struct page_info *page = mfn_to_page(_mfn(old_base_mfn));
-if ( paging_mode_refcounts(d) )
+if ( paging_mode_translate(d) )
put_page(page);
else
switch ( rc = put_page_and_type_preemptible(page) )
@@ -3059,7 +3059,7 @@ long do_mmuext_op(
if ( (op.cmd - MMUEXT_PIN_L1_TABLE) > (CONFIG_PAGING_LEVELS - 1)
