Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Wed, Dec 18, 2013 at 06:25:15PM +, Stefano Stabellini wrote:
> On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote:
> > From: Mukesh Rathor
> >
> > In xen_add_extra_mem() we can skip updating P2M as it's managed
> > by Xen. PVH maps the entire IO space, but only RAM pages need
> > to be repopulated.
> >
> > Signed-off-by: Mukesh Rathor
> > Signed-off-by: Konrad Rzeszutek Wilk
> > ---
> > arch/x86/xen/setup.c | 19 +--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> > index 2137c51..f93bca1 100644
> > --- a/arch/x86/xen/setup.c
> > +++ b/arch/x86/xen/setup.c
> > @@ -27,6 +27,7 @@
> > #include
> > #include
> > #include
> > +#include "mmu.h"
> > #include "xen-ops.h"
> > #include "vdso.h"
> >
> > @@ -81,6 +82,9 @@ static void __init xen_add_extra_mem(u64 start, u64 size)
> >
> > memblock_reserve(start, size);
> >
> > + if (xen_feature(XENFEAT_auto_translated_physmap))
> > + return;
> > +
> > xen_max_p2m_pfn = PFN_DOWN(start + size);
> > for (pfn = PFN_DOWN(start); pfn < xen_max_p2m_pfn; pfn++) {
> > unsigned long mfn = pfn_to_mfn(pfn);
> > @@ -103,6 +107,7 @@ static unsigned long __init xen_do_chunk(unsigned long
> > start,
> > .domid= DOMID_SELF
> > };
> > unsigned long len = 0;
> > + int xlated_phys = xen_feature(XENFEAT_auto_translated_physmap);
> > unsigned long pfn;
> > int ret;
> >
> > @@ -116,7 +121,7 @@ static unsigned long __init xen_do_chunk(unsigned long
> > start,
> > continue;
> > frame = mfn;
> > } else {
> > - if (mfn != INVALID_P2M_ENTRY)
> > + if (!xlated_phys && mfn != INVALID_P2M_ENTRY)
> > continue;
> > frame = pfn;
> > }
> > @@ -154,6 +159,13 @@ static unsigned long __init xen_do_chunk(unsigned long
> > start,
> > static unsigned long __init xen_release_chunk(unsigned long start,
> > unsigned long end)
> > {
> > + /*
> > +* Xen already ballooned out the E820 non RAM regions for us
> > +* and set them up properly in EPT.
> > +*/
> > + if (xen_feature(XENFEAT_auto_translated_physmap))
> > + return end - start;
> > +
> > return xen_do_chunk(start, end, true);
> > }
> >
> > @@ -222,6 +234,9 @@ static void __init xen_set_identity_and_release_chunk(
> > * (except for the ISA region which must be 1:1 mapped) to
> > * release the refcounts (in Xen) on the original frames.
> > */
> > + if (xen_feature(XENFEAT_auto_translated_physmap))
> > + goto skip;
> > +
> > for (pfn = start_pfn; pfn <= max_pfn_mapped && pfn < end_pfn; pfn++) {
> > pte_t pte = __pte_ma(0);
> >
> > @@ -231,7 +246,7 @@ static void __init xen_set_identity_and_release_chunk(
> > (void)HYPERVISOR_update_va_mapping(
> > (unsigned long)__va(pfn << PAGE_SHIFT), pte, 0);
> > }
> > -
> > +skip:
> > if (start_pfn < nr_pages)
> > *released += xen_release_chunk(
> > start_pfn, min(end_pfn, nr_pages));
>
> A goto? Really? What's wrong with an if?
Moves too much code.
> Also considering that you are turning xen_release_chunk into a nop, the
> only purpose of this function on PVH is to call set_phys_range_identity.
> Can't we just do that?
And set_phys_range_identity ends up just doing:
769 if (unlikely(xen_feature(XENFEAT_auto_translated_physmap)))
770 return pfn_e - pfn_s;
So what we really could do (which is what Mukesh's patch had):
if (xen_feature(XENFEAT..) {
if (start_pfn < nr_pages)
*released += min(end_pfn, nr_pages) - start_pfn;
*identity += end_pfn - start_pfn;
}
in its own function. We could do that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Wed, 18 Dec 2013, Mukesh Rathor wrote: > On Wed, 18 Dec 2013 18:25:15 + > Stefano Stabellini wrote: > > > On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote: > > > From: Mukesh Rathor > > > > > > In xen_add_extra_mem() we can skip updating P2M as it's managed > > > by Xen. PVH maps the entire IO space, but only RAM pages need > > > to be repopulated. > > > > > > Signed-off-by: Mukesh Rathor > > > Signed-off-by: Konrad Rzeszutek Wilk > > > --- > > > arch/x86/xen/setup.c | 19 +-- > > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c > > > > @@ -231,7 +246,7 @@ static void __init > > > xen_set_identity_and_release_chunk( (void)HYPERVISOR_update_va_mapping( > > > (unsigned long)__va(pfn << PAGE_SHIFT), > > > pte, 0); } > > > - > > > +skip: > > > if (start_pfn < nr_pages) > > > *released += xen_release_chunk( > > > start_pfn, min(end_pfn, nr_pages)); > ... > > Also considering that you are turning xen_release_chunk into a nop, > > the only purpose of this function on PVH is to call > > set_phys_range_identity. Can't we just do that? > > xen_release_chunk() is called for PVH to give us the count of released, > altho we don't need to release anything for pvh as it was already done in > xen. The released count is then used later to add memory. > > I had separate function to just adjust the stats, which is all we need > to do for pvh, konrad just merged it with pv functions. I see. I'll let Konrad decide which way is nicer. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Wed, 18 Dec 2013, Mukesh Rathor wrote: On Wed, 18 Dec 2013 18:25:15 + Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote: From: Mukesh Rathor mukesh.rat...@oracle.com In xen_add_extra_mem() we can skip updating P2M as it's managed by Xen. PVH maps the entire IO space, but only RAM pages need to be repopulated. Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com --- arch/x86/xen/setup.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c @@ -231,7 +246,7 @@ static void __init xen_set_identity_and_release_chunk( (void)HYPERVISOR_update_va_mapping( (unsigned long)__va(pfn PAGE_SHIFT), pte, 0); } - +skip: if (start_pfn nr_pages) *released += xen_release_chunk( start_pfn, min(end_pfn, nr_pages)); ... Also considering that you are turning xen_release_chunk into a nop, the only purpose of this function on PVH is to call set_phys_range_identity. Can't we just do that? xen_release_chunk() is called for PVH to give us the count of released, altho we don't need to release anything for pvh as it was already done in xen. The released count is then used later to add memory. I had separate function to just adjust the stats, which is all we need to do for pvh, konrad just merged it with pv functions. I see. I'll let Konrad decide which way is nicer. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Wed, Dec 18, 2013 at 06:25:15PM +, Stefano Stabellini wrote:
On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote:
From: Mukesh Rathor mukesh.rat...@oracle.com
In xen_add_extra_mem() we can skip updating P2M as it's managed
by Xen. PVH maps the entire IO space, but only RAM pages need
to be repopulated.
Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
arch/x86/xen/setup.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 2137c51..f93bca1 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -27,6 +27,7 @@
#include xen/interface/memory.h
#include xen/interface/physdev.h
#include xen/features.h
+#include mmu.h
#include xen-ops.h
#include vdso.h
@@ -81,6 +82,9 @@ static void __init xen_add_extra_mem(u64 start, u64 size)
memblock_reserve(start, size);
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return;
+
xen_max_p2m_pfn = PFN_DOWN(start + size);
for (pfn = PFN_DOWN(start); pfn xen_max_p2m_pfn; pfn++) {
unsigned long mfn = pfn_to_mfn(pfn);
@@ -103,6 +107,7 @@ static unsigned long __init xen_do_chunk(unsigned long
start,
.domid= DOMID_SELF
};
unsigned long len = 0;
+ int xlated_phys = xen_feature(XENFEAT_auto_translated_physmap);
unsigned long pfn;
int ret;
@@ -116,7 +121,7 @@ static unsigned long __init xen_do_chunk(unsigned long
start,
continue;
frame = mfn;
} else {
- if (mfn != INVALID_P2M_ENTRY)
+ if (!xlated_phys mfn != INVALID_P2M_ENTRY)
continue;
frame = pfn;
}
@@ -154,6 +159,13 @@ static unsigned long __init xen_do_chunk(unsigned long
start,
static unsigned long __init xen_release_chunk(unsigned long start,
unsigned long end)
{
+ /*
+* Xen already ballooned out the E820 non RAM regions for us
+* and set them up properly in EPT.
+*/
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return end - start;
+
return xen_do_chunk(start, end, true);
}
@@ -222,6 +234,9 @@ static void __init xen_set_identity_and_release_chunk(
* (except for the ISA region which must be 1:1 mapped) to
* release the refcounts (in Xen) on the original frames.
*/
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ goto skip;
+
for (pfn = start_pfn; pfn = max_pfn_mapped pfn end_pfn; pfn++) {
pte_t pte = __pte_ma(0);
@@ -231,7 +246,7 @@ static void __init xen_set_identity_and_release_chunk(
(void)HYPERVISOR_update_va_mapping(
(unsigned long)__va(pfn PAGE_SHIFT), pte, 0);
}
-
+skip:
if (start_pfn nr_pages)
*released += xen_release_chunk(
start_pfn, min(end_pfn, nr_pages));
A goto? Really? What's wrong with an if?
Moves too much code.
Also considering that you are turning xen_release_chunk into a nop, the
only purpose of this function on PVH is to call set_phys_range_identity.
Can't we just do that?
And set_phys_range_identity ends up just doing:
769 if (unlikely(xen_feature(XENFEAT_auto_translated_physmap)))
770 return pfn_e - pfn_s;
So what we really could do (which is what Mukesh's patch had):
if (xen_feature(XENFEAT..) {
if (start_pfn nr_pages)
*released += min(end_pfn, nr_pages) - start_pfn;
*identity += end_pfn - start_pfn;
}
in its own function. We could do that.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Wed, 18 Dec 2013 18:25:15 + Stefano Stabellini wrote: > On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote: > > From: Mukesh Rathor > > > > In xen_add_extra_mem() we can skip updating P2M as it's managed > > by Xen. PVH maps the entire IO space, but only RAM pages need > > to be repopulated. > > > > Signed-off-by: Mukesh Rathor > > Signed-off-by: Konrad Rzeszutek Wilk > > --- > > arch/x86/xen/setup.c | 19 +-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c > > @@ -231,7 +246,7 @@ static void __init > > xen_set_identity_and_release_chunk( (void)HYPERVISOR_update_va_mapping( > > (unsigned long)__va(pfn << PAGE_SHIFT), > > pte, 0); } > > - > > +skip: > > if (start_pfn < nr_pages) > > *released += xen_release_chunk( > > start_pfn, min(end_pfn, nr_pages)); ... > Also considering that you are turning xen_release_chunk into a nop, > the only purpose of this function on PVH is to call > set_phys_range_identity. Can't we just do that? xen_release_chunk() is called for PVH to give us the count of released, altho we don't need to release anything for pvh as it was already done in xen. The released count is then used later to add memory. I had separate function to just adjust the stats, which is all we need to do for pvh, konrad just merged it with pv functions. thanks mukesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote:
> From: Mukesh Rathor
>
> In xen_add_extra_mem() we can skip updating P2M as it's managed
> by Xen. PVH maps the entire IO space, but only RAM pages need
> to be repopulated.
>
> Signed-off-by: Mukesh Rathor
> Signed-off-by: Konrad Rzeszutek Wilk
> ---
> arch/x86/xen/setup.c | 19 +--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 2137c51..f93bca1 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include "mmu.h"
> #include "xen-ops.h"
> #include "vdso.h"
>
> @@ -81,6 +82,9 @@ static void __init xen_add_extra_mem(u64 start, u64 size)
>
> memblock_reserve(start, size);
>
> + if (xen_feature(XENFEAT_auto_translated_physmap))
> + return;
> +
> xen_max_p2m_pfn = PFN_DOWN(start + size);
> for (pfn = PFN_DOWN(start); pfn < xen_max_p2m_pfn; pfn++) {
> unsigned long mfn = pfn_to_mfn(pfn);
> @@ -103,6 +107,7 @@ static unsigned long __init xen_do_chunk(unsigned long
> start,
> .domid= DOMID_SELF
> };
> unsigned long len = 0;
> + int xlated_phys = xen_feature(XENFEAT_auto_translated_physmap);
> unsigned long pfn;
> int ret;
>
> @@ -116,7 +121,7 @@ static unsigned long __init xen_do_chunk(unsigned long
> start,
> continue;
> frame = mfn;
> } else {
> - if (mfn != INVALID_P2M_ENTRY)
> + if (!xlated_phys && mfn != INVALID_P2M_ENTRY)
> continue;
> frame = pfn;
> }
> @@ -154,6 +159,13 @@ static unsigned long __init xen_do_chunk(unsigned long
> start,
> static unsigned long __init xen_release_chunk(unsigned long start,
> unsigned long end)
> {
> + /*
> + * Xen already ballooned out the E820 non RAM regions for us
> + * and set them up properly in EPT.
> + */
> + if (xen_feature(XENFEAT_auto_translated_physmap))
> + return end - start;
> +
> return xen_do_chunk(start, end, true);
> }
>
> @@ -222,6 +234,9 @@ static void __init xen_set_identity_and_release_chunk(
>* (except for the ISA region which must be 1:1 mapped) to
>* release the refcounts (in Xen) on the original frames.
>*/
> + if (xen_feature(XENFEAT_auto_translated_physmap))
> + goto skip;
> +
> for (pfn = start_pfn; pfn <= max_pfn_mapped && pfn < end_pfn; pfn++) {
> pte_t pte = __pte_ma(0);
>
> @@ -231,7 +246,7 @@ static void __init xen_set_identity_and_release_chunk(
> (void)HYPERVISOR_update_va_mapping(
> (unsigned long)__va(pfn << PAGE_SHIFT), pte, 0);
> }
> -
> +skip:
> if (start_pfn < nr_pages)
> *released += xen_release_chunk(
> start_pfn, min(end_pfn, nr_pages));
A goto? Really? What's wrong with an if?
Also considering that you are turning xen_release_chunk into a nop, the
only purpose of this function on PVH is to call set_phys_range_identity.
Can't we just do that?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote:
From: Mukesh Rathor mukesh.rat...@oracle.com
In xen_add_extra_mem() we can skip updating P2M as it's managed
by Xen. PVH maps the entire IO space, but only RAM pages need
to be repopulated.
Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
arch/x86/xen/setup.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 2137c51..f93bca1 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -27,6 +27,7 @@
#include xen/interface/memory.h
#include xen/interface/physdev.h
#include xen/features.h
+#include mmu.h
#include xen-ops.h
#include vdso.h
@@ -81,6 +82,9 @@ static void __init xen_add_extra_mem(u64 start, u64 size)
memblock_reserve(start, size);
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return;
+
xen_max_p2m_pfn = PFN_DOWN(start + size);
for (pfn = PFN_DOWN(start); pfn xen_max_p2m_pfn; pfn++) {
unsigned long mfn = pfn_to_mfn(pfn);
@@ -103,6 +107,7 @@ static unsigned long __init xen_do_chunk(unsigned long
start,
.domid= DOMID_SELF
};
unsigned long len = 0;
+ int xlated_phys = xen_feature(XENFEAT_auto_translated_physmap);
unsigned long pfn;
int ret;
@@ -116,7 +121,7 @@ static unsigned long __init xen_do_chunk(unsigned long
start,
continue;
frame = mfn;
} else {
- if (mfn != INVALID_P2M_ENTRY)
+ if (!xlated_phys mfn != INVALID_P2M_ENTRY)
continue;
frame = pfn;
}
@@ -154,6 +159,13 @@ static unsigned long __init xen_do_chunk(unsigned long
start,
static unsigned long __init xen_release_chunk(unsigned long start,
unsigned long end)
{
+ /*
+ * Xen already ballooned out the E820 non RAM regions for us
+ * and set them up properly in EPT.
+ */
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return end - start;
+
return xen_do_chunk(start, end, true);
}
@@ -222,6 +234,9 @@ static void __init xen_set_identity_and_release_chunk(
* (except for the ISA region which must be 1:1 mapped) to
* release the refcounts (in Xen) on the original frames.
*/
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ goto skip;
+
for (pfn = start_pfn; pfn = max_pfn_mapped pfn end_pfn; pfn++) {
pte_t pte = __pte_ma(0);
@@ -231,7 +246,7 @@ static void __init xen_set_identity_and_release_chunk(
(void)HYPERVISOR_update_va_mapping(
(unsigned long)__va(pfn PAGE_SHIFT), pte, 0);
}
-
+skip:
if (start_pfn nr_pages)
*released += xen_release_chunk(
start_pfn, min(end_pfn, nr_pages));
A goto? Really? What's wrong with an if?
Also considering that you are turning xen_release_chunk into a nop, the
only purpose of this function on PVH is to call set_phys_range_identity.
Can't we just do that?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v11 05/12] xen/pvh: Update E820 to work with PVH
On Wed, 18 Dec 2013 18:25:15 + Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: On Tue, 17 Dec 2013, Konrad Rzeszutek Wilk wrote: From: Mukesh Rathor mukesh.rat...@oracle.com In xen_add_extra_mem() we can skip updating P2M as it's managed by Xen. PVH maps the entire IO space, but only RAM pages need to be repopulated. Signed-off-by: Mukesh Rathor mukesh.rat...@oracle.com Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com --- arch/x86/xen/setup.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c @@ -231,7 +246,7 @@ static void __init xen_set_identity_and_release_chunk( (void)HYPERVISOR_update_va_mapping( (unsigned long)__va(pfn PAGE_SHIFT), pte, 0); } - +skip: if (start_pfn nr_pages) *released += xen_release_chunk( start_pfn, min(end_pfn, nr_pages)); ... Also considering that you are turning xen_release_chunk into a nop, the only purpose of this function on PVH is to call set_phys_range_identity. Can't we just do that? xen_release_chunk() is called for PVH to give us the count of released, altho we don't need to release anything for pvh as it was already done in xen. The released count is then used later to add memory. I had separate function to just adjust the stats, which is all we need to do for pvh, konrad just merged it with pv functions. thanks mukesh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
