[PATCH v2] x86/PV: make post-migration page state consistent

[PATCH v2] x86/PV: make post-migration page state consistent

[Jan Beulich]

When a page table page gets de-validated, its type reference count drops
to zero (and PGT_validated gets cleared), but its type remains intact.
XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
such pages. An intermediate write to such a page via e.g.
MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
return. In libxc the decision which pages to normalize / localize
depends solely on the type returned from the domctl. As a result without
further precautions the guest won't be able to tell whether such a page
has had its (apparent) PTE entries transitioned to the new MFNs.

Add a check of PGT_validated, thus consistently avoiding normalization /
localization in the tool stack.

Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
open coding it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Don't change type's type.

--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -215,7 +215,7 @@ long arch_do_domctl(
 
         for ( i = 0; i < num; ++i )
         {
-            unsigned long gfn = 0, type = 0;
+            unsigned long gfn = 0, type = XEN_DOMCTL_PFINFO_NOTAB;
             struct page_info *page;
             p2m_type_t t;
 
@@ -255,6 +255,8 @@ long arch_do_domctl(
 
                 if ( page->u.inuse.type_info & PGT_pinned )
                     type |= XEN_DOMCTL_PFINFO_LPINTAB;
+                else if ( !(page->u.inuse.type_info & PGT_validated) )
+                    type = XEN_DOMCTL_PFINFO_NOTAB;
 
                 if ( page->count_info & PGC_broken )
                     type = XEN_DOMCTL_PFINFO_BROKEN;

Ping: [PATCH v2] x86/PV: make post-migration page state consistent

[Jan Beulich]

On 04.11.2020 08:56, Jan Beulich wrote:
> When a page table page gets de-validated, its type reference count drops
> to zero (and PGT_validated gets cleared), but its type remains intact.
> XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
> such pages. An intermediate write to such a page via e.g.
> MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
> PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
> return. In libxc the decision which pages to normalize / localize
> depends solely on the type returned from the domctl. As a result without
> further precautions the guest won't be able to tell whether such a page
> has had its (apparent) PTE entries transitioned to the new MFNs.
> 
> Add a check of PGT_validated, thus consistently avoiding normalization /
> localization in the tool stack.
> 
> Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
> open coding it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Don't change type's type.

Ping?

> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -215,7 +215,7 @@ long arch_do_domctl(
>  
>          for ( i = 0; i < num; ++i )
>          {
> -            unsigned long gfn = 0, type = 0;
> +            unsigned long gfn = 0, type = XEN_DOMCTL_PFINFO_NOTAB;
>              struct page_info *page;
>              p2m_type_t t;
>  
> @@ -255,6 +255,8 @@ long arch_do_domctl(
>  
>                  if ( page->u.inuse.type_info & PGT_pinned )
>                      type |= XEN_DOMCTL_PFINFO_LPINTAB;
> +                else if ( !(page->u.inuse.type_info & PGT_validated) )
> +                    type = XEN_DOMCTL_PFINFO_NOTAB;
>  
>                  if ( page->count_info & PGC_broken )
>                      type = XEN_DOMCTL_PFINFO_BROKEN;
> 

Re: Ping: [PATCH v2] x86/PV: make post-migration page state consistent

[Andrew Cooper]

On 20/11/2020 12:48, Jan Beulich wrote:
> On 04.11.2020 08:56, Jan Beulich wrote:
>> When a page table page gets de-validated, its type reference count drops
>> to zero (and PGT_validated gets cleared), but its type remains intact.
>> XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
>> such pages. An intermediate write to such a page via e.g.
>> MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
>> PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
>> return. In libxc the decision which pages to normalize / localize
>> depends solely on the type returned from the domctl. As a result without
>> further precautions the guest won't be able to tell whether such a page
>> has had its (apparent) PTE entries transitioned to the new MFNs.
>>
>> Add a check of PGT_validated, thus consistently avoiding normalization /
>> localization in the tool stack.
>>
>> Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
>> open coding it.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Don't change type's type.
> Ping?

Ping what?  There is still nothing addressing my concerns from v1.

To re-iterate - this is a very subtle change, in a very complicated
piece of migration.  As the problems described do not manifest in
practice, it is vital to understand why.

~Andrew

Re: Ping: [PATCH v2] x86/PV: make post-migration page state consistent

[Jan Beulich]

On 23.11.2020 13:26, Andrew Cooper wrote:
> On 20/11/2020 12:48, Jan Beulich wrote:
>> On 04.11.2020 08:56, Jan Beulich wrote:
>>> When a page table page gets de-validated, its type reference count drops
>>> to zero (and PGT_validated gets cleared), but its type remains intact.
>>> XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
>>> such pages. An intermediate write to such a page via e.g.
>>> MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
>>> PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
>>> return. In libxc the decision which pages to normalize / localize
>>> depends solely on the type returned from the domctl. As a result without
>>> further precautions the guest won't be able to tell whether such a page
>>> has had its (apparent) PTE entries transitioned to the new MFNs.
>>>
>>> Add a check of PGT_validated, thus consistently avoiding normalization /
>>> localization in the tool stack.
>>>
>>> Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
>>> open coding it.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> v2: Don't change type's type.
>> Ping?
> 
> Ping what?  There is still nothing addressing my concerns from v1.

I did reply to your concerns on Sep 11th, and then replied to this
reply of mine another time on Sep 28th. Neither of these got any
response from you, hence I had to conclude - after two further
pings on v1 - that they were satisfactory to you. Now you say they
weren't, but without saying in which way, so I still wouldn't know
what to change in the description.

On the code change itself you did say "... so this is probably a
good change", so I was further understanding that your concern is
merely with the description. Maybe I misunderstood this aspect,
too?

> To re-iterate - this is a very subtle change, in a very complicated
> piece of migration.  As the problems described do not manifest in
> practice, it is vital to understand why.

Until now it has been my understanding that they just don't happen
to manifest, because guests know to behave themselves (read: pin,
first and foremost, all their page tables, which means we wouldn't
in practice run into ones with an in-flight state change).

Jan

Ping²: [PATCH v2] x86/PV: make post-migration page state consistent

[Jan Beulich]

On 23.11.2020 13:50, Jan Beulich wrote:
> On 23.11.2020 13:26, Andrew Cooper wrote:
>> On 20/11/2020 12:48, Jan Beulich wrote:
>>> On 04.11.2020 08:56, Jan Beulich wrote:
>>>> When a page table page gets de-validated, its type reference count drops
>>>> to zero (and PGT_validated gets cleared), but its type remains intact.
>>>> XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
>>>> such pages. An intermediate write to such a page via e.g.
>>>> MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
>>>> PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
>>>> return. In libxc the decision which pages to normalize / localize
>>>> depends solely on the type returned from the domctl. As a result without
>>>> further precautions the guest won't be able to tell whether such a page
>>>> has had its (apparent) PTE entries transitioned to the new MFNs.
>>>>
>>>> Add a check of PGT_validated, thus consistently avoiding normalization /
>>>> localization in the tool stack.
>>>>
>>>> Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
>>>> open coding it.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> v2: Don't change type's type.
>>> Ping?
>>
>> Ping what?  There is still nothing addressing my concerns from v1.
> 
> I did reply to your concerns on Sep 11th, and then replied to this
> reply of mine another time on Sep 28th. Neither of these got any
> response from you, hence I had to conclude - after two further
> pings on v1 - that they were satisfactory to you. Now you say they
> weren't, but without saying in which way, so I still wouldn't know
> what to change in the description.
> 
> On the code change itself you did say "... so this is probably a
> good change", so I was further understanding that your concern is
> merely with the description. Maybe I misunderstood this aspect,
> too?
> 
>> To re-iterate - this is a very subtle change, in a very complicated
>> piece of migration.  As the problems described do not manifest in
>> practice, it is vital to understand why.
> 
> Until now it has been my understanding that they just don't happen
> to manifest, because guests know to behave themselves (read: pin,
> first and foremost, all their page tables, which means we wouldn't
> in practice run into ones with an in-flight state change).

Another example where I think I have waited long enough for a reply.
Roger has acked the change, so unless I hear otherwise by then I'm
intending to commit this, too, once the tree is fully open again.

Jan

Re: [PATCH v2] x86/PV: make post-migration page state consistent

[Roger Pau Monné]

On Wed, Nov 04, 2020 at 08:56:50AM +0100, Jan Beulich wrote:
> When a page table page gets de-validated, its type reference count drops
> to zero (and PGT_validated gets cleared), but its type remains intact.
> XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
> such pages. An intermediate write to such a page via e.g.
> MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
> PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
> return. In libxc the decision which pages to normalize / localize
> depends solely on the type returned from the domctl. As a result without
> further precautions the guest won't be able to tell whether such a page
> has had its (apparent) PTE entries transitioned to the new MFNs.
> 
> Add a check of PGT_validated, thus consistently avoiding normalization /
> localization in the tool stack.
> 
> Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
> open coding it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Maybe the switch could be avoided if the page is not validated or
broken? Not that I care that much.

Thanks, Roger.

Re: [PATCH v2] x86/PV: make post-migration page state consistent

[Jan Beulich]

On 23.11.2020 12:48, Roger Pau Monné wrote:
> On Wed, Nov 04, 2020 at 08:56:50AM +0100, Jan Beulich wrote:
>> When a page table page gets de-validated, its type reference count drops
>> to zero (and PGT_validated gets cleared), but its type remains intact.
>> XEN_DOMCTL_getpageframeinfo3, therefore, so far reported prior usage for
>> such pages. An intermediate write to such a page via e.g.
>> MMU_NORMAL_PT_UPDATE, however, would transition the page's type to
>> PGT_writable_page, thus altering what XEN_DOMCTL_getpageframeinfo3 would
>> return. In libxc the decision which pages to normalize / localize
>> depends solely on the type returned from the domctl. As a result without
>> further precautions the guest won't be able to tell whether such a page
>> has had its (apparent) PTE entries transitioned to the new MFNs.
>>
>> Add a check of PGT_validated, thus consistently avoiding normalization /
>> localization in the tool stack.
>>
>> Also use XEN_DOMCTL_PFINFO_NOTAB in the variable's initializer instead
>> open coding it.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> Maybe the switch could be avoided if the page is not validated or
> broken? Not that I care that much.

It certainly could be, but it didn't seem worth the code churn
to me.

Jan