Re: [Xen-devel] [PATCH] x86/pv: Drop create_pae_xen_mappings()
>>> On 24.08.17 at 10:52, wrote: > On 08/24/2017 09:40 AM, Jan Beulich wrote: > On 23.08.17 at 20:11, 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 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Drop create_pae_xen_mappings()
On 08/24/2017 09:40 AM, Jan Beulich wrote: On 23.08.17 at 20:11, 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 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Drop create_pae_xen_mappings()
>>> On 23.08.17 at 20:11, 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
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/pv: Drop create_pae_xen_mappings()
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
---
CC: Jan Beulich
CC: Wei Liu
CC: Tim Deegan
CC: George Dunlap
---
xen/arch/x86/mm.c | 46 --
1 file changed, 46 deletions(-)
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;
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
