| Message ID | 1503511888-9769-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 23.08.17 at 20:11, <andrew.cooper3@citrix.com> wrote: > This is leftovers from the 32bit hypervisor days. The only Xen content in > this virtual range for 32bit PV guests is the compat M2P. It is not critical > that the mapping is present, nor is it critical that the slot is unshared. Hmm, while technically correct this would allow 32-bit guests to run on newer hypervisors that would fail on older ones. I'm not sure this is a good idea, but I'm also not entirely opposed to it. > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -1429,46 +1429,6 @@ static int alloc_l1_table(struct page_info *page) > return ret; > } > > -static int create_pae_xen_mappings(struct domain *d, l3_pgentry_t *pl3e) > -{ > - struct page_info *page; > - l3_pgentry_t l3e3; > - > - if ( !is_pv_32bit_domain(d) ) > - return 1; > - > - pl3e = (l3_pgentry_t *)((unsigned long)pl3e & PAGE_MASK); > - > - /* 3rd L3 slot contains L2 with Xen-private mappings. It *must* exist. */ > - l3e3 = pl3e[3]; > - if ( !(l3e_get_flags(l3e3) & _PAGE_PRESENT) ) > - { > - gdprintk(XENLOG_WARNING, "PAE L3 3rd slot is empty\n"); > - return 0; > - } > - > - /* > - * The Xen-private mappings include linear mappings. The L2 thus cannot > - * be shared by multiple L3 tables. The test here is adequate because: > - * 1. Cannot appear in slots != 3 because get_page_type() checks the > - * PGT_pae_xen_l2 flag, which is asserted iff the L2 appears in slot 3 > - * 2. Cannot appear in another page table's L3: > - * a. alloc_l3_table() calls this function and this check will fail > - * b. mod_l3_entry() disallows updates to slot 3 in an existing table For consistency wouldn't you then better also delete the check in mod_l3_entry() that's being referred to here? And relax the respective checking in alloc_l3_table() to at least allow the entry to be non-present? Jan
On 08/24/2017 09:40 AM, Jan Beulich wrote: >>>> On 23.08.17 at 20:11, <andrew.cooper3@citrix.com> wrote: >> This is leftovers from the 32bit hypervisor days. The only Xen content in >> this virtual range for 32bit PV guests is the compat M2P. It is not critical >> that the mapping is present, nor is it critical that the slot is unshared. > > Hmm, while technically correct this would allow 32-bit guests to > run on newer hypervisors that would fail on older ones. I'm not > sure this is a good idea, but I'm also not entirely opposed to it. I think I'm missing something; why would some 32-bit guests fail on hypervisors which still had this mapping (which is what I presume you mean by "older hypervisors")? -George
>>> On 24.08.17 at 10:52, <george.dunlap@citrix.com> wrote: > On 08/24/2017 09:40 AM, Jan Beulich wrote: >>>>> On 23.08.17 at 20:11, <andrew.cooper3@citrix.com> wrote: >>> This is leftovers from the 32bit hypervisor days. The only Xen content in >>> this virtual range for 32bit PV guests is the compat M2P. It is not critical >>> that the mapping is present, nor is it critical that the slot is unshared. >> >> Hmm, while technically correct this would allow 32-bit guests to >> run on newer hypervisors that would fail on older ones. I'm not >> sure this is a good idea, but I'm also not entirely opposed to it. > > I think I'm missing something; why would some 32-bit guests fail on > hypervisors which still had this mapping (which is what I presume you > mean by "older hypervisors")? Oh, I'm sorry - I mean a guest not playing by the rules being enforced here, i.e. e.g. one leaving the 3rd slot empty. Testing such a guest on an up-to-date hypervisor would not turn up any issues, yet it would fail once run on an older one. Jan
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index ed77270..3262499 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -1429,46 +1429,6 @@ static int alloc_l1_table(struct page_info *page) return ret; } -static int create_pae_xen_mappings(struct domain *d, l3_pgentry_t *pl3e) -{ - struct page_info *page; - l3_pgentry_t l3e3; - - if ( !is_pv_32bit_domain(d) ) - return 1; - - pl3e = (l3_pgentry_t *)((unsigned long)pl3e & PAGE_MASK); - - /* 3rd L3 slot contains L2 with Xen-private mappings. It *must* exist. */ - l3e3 = pl3e[3]; - if ( !(l3e_get_flags(l3e3) & _PAGE_PRESENT) ) - { - gdprintk(XENLOG_WARNING, "PAE L3 3rd slot is empty\n"); - return 0; - } - - /* - * The Xen-private mappings include linear mappings. The L2 thus cannot - * be shared by multiple L3 tables. The test here is adequate because: - * 1. Cannot appear in slots != 3 because get_page_type() checks the - * PGT_pae_xen_l2 flag, which is asserted iff the L2 appears in slot 3 - * 2. Cannot appear in another page table's L3: - * a. alloc_l3_table() calls this function and this check will fail - * b. mod_l3_entry() disallows updates to slot 3 in an existing table - */ - page = l3e_get_page(l3e3); - BUG_ON(page->u.inuse.type_info & PGT_pinned); - BUG_ON((page->u.inuse.type_info & PGT_count_mask) == 0); - BUG_ON(!(page->u.inuse.type_info & PGT_pae_xen_l2)); - if ( (page->u.inuse.type_info & PGT_count_mask) != 1 ) - { - gdprintk(XENLOG_WARNING, "PAE L3 3rd slot is shared\n"); - return 0; - } - - return 1; -} - static int alloc_l2_table(struct page_info *page, unsigned long type, int preemptible) { @@ -1573,8 +1533,6 @@ static int alloc_l3_table(struct page_info *page) adjust_guest_l3e(pl3e[i], d); } - if ( rc >= 0 && !create_pae_xen_mappings(d, pl3e) ) - rc = -EINVAL; if ( rc < 0 && rc != -ERESTART && rc != -EINTR ) { gdprintk(XENLOG_WARNING, "Failure in alloc_l3_table: slot %#x\n", i); @@ -2120,10 +2078,6 @@ static int mod_l3_entry(l3_pgentry_t *pl3e, return -EFAULT; } - if ( likely(rc == 0) ) - if ( !create_pae_xen_mappings(d, pl3e) ) - BUG(); - put_page_from_l3e(ol3e, pfn, 0, 1); return rc; }
This is leftovers from the 32bit hypervisor days. The only Xen content in this virtual range for 32bit PV guests is the compat M2P. It is not critical that the mapping is present, nor is it critical that the slot is unshared. 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 | 46 ---------------------------------------------- 1 file changed, 46 deletions(-)