| Message ID | 20170323180819.26255-1-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 23.03.17 at 19:08, <wei.liu2@citrix.com> wrote: > PVH guest is actually an translated guest. It should be able to > manipulate page table for other domains when acting as Dom0. The same was true for PVHv1, so I'm afraid there's a little more to this. > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -3041,12 +3041,6 @@ static struct domain *get_pg_owner(domid_t domid) > goto out; > } > > - if ( unlikely(paging_mode_translate(curr)) ) > - { > - MEM_LOG("Cannot mix foreign mappings with translated domains"); > - goto out; > - } Prior to Roger's recent removal of PVHv1 code this was if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) Instead of removing the left side, I think it should have been converted to !is_hvm_domain() (or is_pv_domain()). Protection against this being used on other than PV domains as target luckily looks to be there: - mmuext and mmu_update already have respective (albeit somewhat inconsistent) checks, - do_update_va_mapping_otherdomain() is not wired up for HVM. Jan
On Fri, Mar 24, 2017 at 02:12:47AM -0600, Jan Beulich wrote: > >>> On 23.03.17 at 19:08, <wei.liu2@citrix.com> wrote: > > PVH guest is actually an translated guest. It should be able to > > manipulate page table for other domains when acting as Dom0. > > The same was true for PVHv1, so I'm afraid there's a little more to > this. > > > --- a/xen/arch/x86/mm.c > > +++ b/xen/arch/x86/mm.c > > @@ -3041,12 +3041,6 @@ static struct domain *get_pg_owner(domid_t domid) > > goto out; > > } > > > > - if ( unlikely(paging_mode_translate(curr)) ) > > - { > > - MEM_LOG("Cannot mix foreign mappings with translated domains"); > > - goto out; > > - } > > Prior to Roger's recent removal of PVHv1 code this was > > if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) > > Instead of removing the left side, I think it should have been > converted to !is_hvm_domain() (or is_pv_domain()). > DYM: if ( !is_hvm_domain(curr) && unlikely(paging_mode_translate(curr)) ) ? Here is something I'm not entirely sure of: is autotranslated PV guest still a thing? I thought Andrew removed it already? > Protection against this being used on other than PV domains > as target luckily looks to be there: > - mmuext and mmu_update already have respective (albeit > somewhat inconsistent) checks, > - do_update_va_mapping_otherdomain() is not wired up for > HVM. Yes. That's right. Wei. > > Jan >
>>> On 24.03.17 at 12:08, <wei.liu2@citrix.com> wrote: > On Fri, Mar 24, 2017 at 02:12:47AM -0600, Jan Beulich wrote: >> >>> On 23.03.17 at 19:08, <wei.liu2@citrix.com> wrote: >> > PVH guest is actually an translated guest. It should be able to >> > manipulate page table for other domains when acting as Dom0. >> >> The same was true for PVHv1, so I'm afraid there's a little more to >> this. >> >> > --- a/xen/arch/x86/mm.c >> > +++ b/xen/arch/x86/mm.c >> > @@ -3041,12 +3041,6 @@ static struct domain *get_pg_owner(domid_t domid) >> > goto out; >> > } >> > >> > - if ( unlikely(paging_mode_translate(curr)) ) >> > - { >> > - MEM_LOG("Cannot mix foreign mappings with translated domains"); >> > - goto out; >> > - } >> >> Prior to Roger's recent removal of PVHv1 code this was >> >> if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) >> >> Instead of removing the left side, I think it should have been >> converted to !is_hvm_domain() (or is_pv_domain()). >> > > DYM: > > if ( !is_hvm_domain(curr) && unlikely(paging_mode_translate(curr)) ) > > ? Yes. > Here is something I'm not entirely sure of: is autotranslated PV guest > still a thing? I thought Andrew removed it already? From the tools, iirc, but not from the hypervisor, at least not entirely (as you see from the change you want to make). Jan
On Fri, Mar 24, 2017 at 05:17:51AM -0600, Jan Beulich wrote: > >>> On 24.03.17 at 12:08, <wei.liu2@citrix.com> wrote: > > On Fri, Mar 24, 2017 at 02:12:47AM -0600, Jan Beulich wrote: > >> >>> On 23.03.17 at 19:08, <wei.liu2@citrix.com> wrote: > >> > PVH guest is actually an translated guest. It should be able to > >> > manipulate page table for other domains when acting as Dom0. > >> > >> The same was true for PVHv1, so I'm afraid there's a little more to > >> this. > >> > >> > --- a/xen/arch/x86/mm.c > >> > +++ b/xen/arch/x86/mm.c > >> > @@ -3041,12 +3041,6 @@ static struct domain *get_pg_owner(domid_t domid) > >> > goto out; > >> > } > >> > > >> > - if ( unlikely(paging_mode_translate(curr)) ) > >> > - { > >> > - MEM_LOG("Cannot mix foreign mappings with translated domains"); > >> > - goto out; > >> > - } > >> > >> Prior to Roger's recent removal of PVHv1 code this was > >> > >> if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) > >> > >> Instead of removing the left side, I think it should have been > >> converted to !is_hvm_domain() (or is_pv_domain()). > >> > > > > DYM: > > > > if ( !is_hvm_domain(curr) && unlikely(paging_mode_translate(curr)) ) > > > > ? > > Yes. > > > Here is something I'm not entirely sure of: is autotranslated PV guest > > still a thing? I thought Andrew removed it already? > > From the tools, iirc, but not from the hypervisor, at least not > entirely (as you see from the change you want to make). > OK, I will update the patch per your suggestion. > Jan >
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index a6b2649cda..3635764df8 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -3041,12 +3041,6 @@ static struct domain *get_pg_owner(domid_t domid) goto out; } - if ( unlikely(paging_mode_translate(curr)) ) - { - MEM_LOG("Cannot mix foreign mappings with translated domains"); - goto out; - } - switch ( domid ) { case DOMID_IO:
PVH guest is actually an translated guest. It should be able to manipulate page table for other domains when acting as Dom0. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/mm.c | 6 ------ 1 file changed, 6 deletions(-)