* Re: Change of max-ram-below-4g initial value breaks Xen
@ 2016-06-23 16:18 ` Anthony PERARD
0 siblings, 0 replies; 12+ messages in thread
From: Anthony PERARD @ 2016-06-23 16:18 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Paolo Bonzini, Stefano Stabellini, qemu-devel, xen-devel
On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote:
> Hi,
>
> > How could xen_ram_init() find out if the value of max-ram-below-4g is
> > the default or if a user have set it? Is there another way we could fix
> > this?
>
> Attached patch should fix it. Patch survived a quick smoke test on kvm
> so far, need to do some more testing tomorrow. Can you give it a spin
> on xen?
Thanks. Unfortunately, it does not work :(.
In this patch, max_ram_below_4g is set before the call to xen_ram_init()
and xen_ram_init read it back (via object_property_get_int()). So, in
xen_ram_init, user_lowmem is not 0.
Thanks,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Change of max-ram-below-4g initial value breaks Xen
2016-06-23 16:18 ` Anthony PERARD
(?)
@ 2016-06-24 5:46 ` Gerd Hoffmann
-1 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-06-24 5:46 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Paolo Bonzini, Stefano Stabellini, qemu-devel, xen-devel
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]
On Do, 2016-06-23 at 17:18 +0100, Anthony PERARD wrote:
> On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote:
> > Hi,
> >
> > > How could xen_ram_init() find out if the value of max-ram-below-4g is
> > > the default or if a user have set it? Is there another way we could fix
> > > this?
> >
> > Attached patch should fix it. Patch survived a quick smoke test on kvm
> > so far, need to do some more testing tomorrow. Can you give it a spin
> > on xen?
>
> Thanks. Unfortunately, it does not work :(.
>
> In this patch, max_ram_below_4g is set before the call to xen_ram_init()
> and xen_ram_init read it back (via object_property_get_int()). So, in
> xen_ram_init, user_lowmem is not 0.
Ah, I see. We do the split calculation twice on xen. That is pretty
pointless. New patch attached.
cheers,
Gerd
[-- Attachment #2: 0001-xen-fix-ram-init-regression.patch --]
[-- Type: text/x-patch, Size: 5459 bytes --]
From a1bb0d4f7a94e97102e7ea72d0a65de2a17b1160 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 23 Jun 2016 16:49:03 +0200
Subject: [PATCH] xen: fix ram init regression
Commit "8156d48 pc: allow raising low memory via max-ram-below-4g
option" causes a regression on xen, because it uses a different
memory split.
This patch initializes max-ram-below-4g to zero and leaves the
initialization to the memory initialization functions. That way
they can pick different default values (max-ram-below-4g is zero
still) or use the user supplied value (max-ram-below-4g is non-zero).
Also skip the whole ram split calculation on Xen. xen_ram_init()
does its own split calculation anyway so it is superfluous, also
this way xen_ram_init can actually see whenever max-ram-below-4g
is zero or not.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/pc.c | 2 +-
hw/i386/pc_piix.c | 52 +++++++++++++++++++++++++++++-----------------------
hw/i386/pc_q35.c | 3 +++
xen-hvm.c | 3 +++
4 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7198ed5..66e1dae 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1886,7 +1886,7 @@ static void pc_machine_initfn(Object *obj)
pc_machine_get_hotplug_memory_region_size,
NULL, NULL, NULL, &error_abort);
- pcms->max_ram_below_4g = 0xe0000000; /* 3.5G */
+ pcms->max_ram_below_4g = 0; /* use default */
object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
pc_machine_get_max_ram_below_4g,
pc_machine_set_max_ram_below_4g,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 53bc968..f51fa77 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -108,37 +108,43 @@ static void pc_init1(MachineState *machine,
* so legacy non-PAE guests can get as much memory as possible in
* the 32bit address space below 4G.
*
+ * - Note that Xen has its own ram setp code in xen_ram_init(),
+ * called via xen_hvm_init().
+ *
* Examples:
* qemu -M pc-1.7 -m 4G (old default) -> 3584M low, 512M high
* qemu -M pc -m 4G (new default) -> 3072M low, 1024M high
* qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high
* qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M)
*/
- lowmem = pcms->max_ram_below_4g;
- if (machine->ram_size >= pcms->max_ram_below_4g) {
- if (pcmc->gigabyte_align) {
- if (lowmem > 0xc0000000) {
- lowmem = 0xc0000000;
- }
- if (lowmem & ((1ULL << 30) - 1)) {
- error_report("Warning: Large machine and max_ram_below_4g "
- "(%" PRIu64 ") not a multiple of 1G; "
- "possible bad performance.",
- pcms->max_ram_below_4g);
- }
- }
- }
-
- if (machine->ram_size >= lowmem) {
- pcms->above_4g_mem_size = machine->ram_size - lowmem;
- pcms->below_4g_mem_size = lowmem;
- } else {
- pcms->above_4g_mem_size = 0;
- pcms->below_4g_mem_size = machine->ram_size;
- }
-
if (xen_enabled()) {
xen_hvm_init(pcms, &ram_memory);
+ } else {
+ if (!pcms->max_ram_below_4g) {
+ pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+ }
+ lowmem = pcms->max_ram_below_4g;
+ if (machine->ram_size >= pcms->max_ram_below_4g) {
+ if (pcmc->gigabyte_align) {
+ if (lowmem > 0xc0000000) {
+ lowmem = 0xc0000000;
+ }
+ if (lowmem & ((1ULL << 30) - 1)) {
+ error_report("Warning: Large machine and max_ram_below_4g "
+ "(%" PRIu64 ") not a multiple of 1G; "
+ "possible bad performance.",
+ pcms->max_ram_below_4g);
+ }
+ }
+ }
+
+ if (machine->ram_size >= lowmem) {
+ pcms->above_4g_mem_size = machine->ram_size - lowmem;
+ pcms->below_4g_mem_size = lowmem;
+ } else {
+ pcms->above_4g_mem_size = 0;
+ pcms->below_4g_mem_size = machine->ram_size;
+ }
}
pc_cpus_init(pcms);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e4b541f..1b653e2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -93,6 +93,9 @@ static void pc_q35_init(MachineState *machine)
/* Handle the machine opt max-ram-below-4g. It is basically doing
* min(qemu limit, user limit).
*/
+ if (!pcms->max_ram_below_4g) {
+ pcms->max_ram_below_4g = 1ULL << 32; /* default: 4G */;
+ }
if (lowmem > pcms->max_ram_below_4g) {
lowmem = pcms->max_ram_below_4g;
if (machine->ram_size - lowmem > lowmem &&
diff --git a/xen-hvm.c b/xen-hvm.c
index 98ea44f..eb57792 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -190,6 +190,9 @@ static void xen_ram_init(PCMachineState *pcms,
/* Handle the machine opt max-ram-below-4g. It is basically doing
* min(xen limit, user limit).
*/
+ if (!user_lowmem) {
+ user_lowmem = HVM_BELOW_4G_RAM_END; /* default */
+ }
if (HVM_BELOW_4G_RAM_END <= user_lowmem) {
user_lowmem = HVM_BELOW_4G_RAM_END;
}
--
1.8.3.1
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] Change of max-ram-below-4g initial value breaks Xen
2016-06-23 16:18 ` Anthony PERARD
(?)
(?)
@ 2016-06-24 5:46 ` Gerd Hoffmann
2016-06-24 11:19 ` Anthony PERARD
2016-06-24 11:19 ` [Qemu-devel] " Anthony PERARD
-1 siblings, 2 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-06-24 5:46 UTC (permalink / raw)
To: Anthony PERARD; +Cc: qemu-devel, xen-devel, Stefano Stabellini, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]
On Do, 2016-06-23 at 17:18 +0100, Anthony PERARD wrote:
> On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote:
> > Hi,
> >
> > > How could xen_ram_init() find out if the value of max-ram-below-4g is
> > > the default or if a user have set it? Is there another way we could fix
> > > this?
> >
> > Attached patch should fix it. Patch survived a quick smoke test on kvm
> > so far, need to do some more testing tomorrow. Can you give it a spin
> > on xen?
>
> Thanks. Unfortunately, it does not work :(.
>
> In this patch, max_ram_below_4g is set before the call to xen_ram_init()
> and xen_ram_init read it back (via object_property_get_int()). So, in
> xen_ram_init, user_lowmem is not 0.
Ah, I see. We do the split calculation twice on xen. That is pretty
pointless. New patch attached.
cheers,
Gerd
[-- Attachment #2: 0001-xen-fix-ram-init-regression.patch --]
[-- Type: text/x-patch, Size: 5459 bytes --]
From a1bb0d4f7a94e97102e7ea72d0a65de2a17b1160 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 23 Jun 2016 16:49:03 +0200
Subject: [PATCH] xen: fix ram init regression
Commit "8156d48 pc: allow raising low memory via max-ram-below-4g
option" causes a regression on xen, because it uses a different
memory split.
This patch initializes max-ram-below-4g to zero and leaves the
initialization to the memory initialization functions. That way
they can pick different default values (max-ram-below-4g is zero
still) or use the user supplied value (max-ram-below-4g is non-zero).
Also skip the whole ram split calculation on Xen. xen_ram_init()
does its own split calculation anyway so it is superfluous, also
this way xen_ram_init can actually see whenever max-ram-below-4g
is zero or not.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/pc.c | 2 +-
hw/i386/pc_piix.c | 52 +++++++++++++++++++++++++++++-----------------------
hw/i386/pc_q35.c | 3 +++
xen-hvm.c | 3 +++
4 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7198ed5..66e1dae 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1886,7 +1886,7 @@ static void pc_machine_initfn(Object *obj)
pc_machine_get_hotplug_memory_region_size,
NULL, NULL, NULL, &error_abort);
- pcms->max_ram_below_4g = 0xe0000000; /* 3.5G */
+ pcms->max_ram_below_4g = 0; /* use default */
object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
pc_machine_get_max_ram_below_4g,
pc_machine_set_max_ram_below_4g,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 53bc968..f51fa77 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -108,37 +108,43 @@ static void pc_init1(MachineState *machine,
* so legacy non-PAE guests can get as much memory as possible in
* the 32bit address space below 4G.
*
+ * - Note that Xen has its own ram setp code in xen_ram_init(),
+ * called via xen_hvm_init().
+ *
* Examples:
* qemu -M pc-1.7 -m 4G (old default) -> 3584M low, 512M high
* qemu -M pc -m 4G (new default) -> 3072M low, 1024M high
* qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high
* qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M)
*/
- lowmem = pcms->max_ram_below_4g;
- if (machine->ram_size >= pcms->max_ram_below_4g) {
- if (pcmc->gigabyte_align) {
- if (lowmem > 0xc0000000) {
- lowmem = 0xc0000000;
- }
- if (lowmem & ((1ULL << 30) - 1)) {
- error_report("Warning: Large machine and max_ram_below_4g "
- "(%" PRIu64 ") not a multiple of 1G; "
- "possible bad performance.",
- pcms->max_ram_below_4g);
- }
- }
- }
-
- if (machine->ram_size >= lowmem) {
- pcms->above_4g_mem_size = machine->ram_size - lowmem;
- pcms->below_4g_mem_size = lowmem;
- } else {
- pcms->above_4g_mem_size = 0;
- pcms->below_4g_mem_size = machine->ram_size;
- }
-
if (xen_enabled()) {
xen_hvm_init(pcms, &ram_memory);
+ } else {
+ if (!pcms->max_ram_below_4g) {
+ pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+ }
+ lowmem = pcms->max_ram_below_4g;
+ if (machine->ram_size >= pcms->max_ram_below_4g) {
+ if (pcmc->gigabyte_align) {
+ if (lowmem > 0xc0000000) {
+ lowmem = 0xc0000000;
+ }
+ if (lowmem & ((1ULL << 30) - 1)) {
+ error_report("Warning: Large machine and max_ram_below_4g "
+ "(%" PRIu64 ") not a multiple of 1G; "
+ "possible bad performance.",
+ pcms->max_ram_below_4g);
+ }
+ }
+ }
+
+ if (machine->ram_size >= lowmem) {
+ pcms->above_4g_mem_size = machine->ram_size - lowmem;
+ pcms->below_4g_mem_size = lowmem;
+ } else {
+ pcms->above_4g_mem_size = 0;
+ pcms->below_4g_mem_size = machine->ram_size;
+ }
}
pc_cpus_init(pcms);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e4b541f..1b653e2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -93,6 +93,9 @@ static void pc_q35_init(MachineState *machine)
/* Handle the machine opt max-ram-below-4g. It is basically doing
* min(qemu limit, user limit).
*/
+ if (!pcms->max_ram_below_4g) {
+ pcms->max_ram_below_4g = 1ULL << 32; /* default: 4G */;
+ }
if (lowmem > pcms->max_ram_below_4g) {
lowmem = pcms->max_ram_below_4g;
if (machine->ram_size - lowmem > lowmem &&
diff --git a/xen-hvm.c b/xen-hvm.c
index 98ea44f..eb57792 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -190,6 +190,9 @@ static void xen_ram_init(PCMachineState *pcms,
/* Handle the machine opt max-ram-below-4g. It is basically doing
* min(xen limit, user limit).
*/
+ if (!user_lowmem) {
+ user_lowmem = HVM_BELOW_4G_RAM_END; /* default */
+ }
if (HVM_BELOW_4G_RAM_END <= user_lowmem) {
user_lowmem = HVM_BELOW_4G_RAM_END;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: Change of max-ram-below-4g initial value breaks Xen
2016-06-24 5:46 ` [Qemu-devel] " Gerd Hoffmann
@ 2016-06-24 11:19 ` Anthony PERARD
2016-06-24 11:19 ` [Qemu-devel] " Anthony PERARD
1 sibling, 0 replies; 12+ messages in thread
From: Anthony PERARD @ 2016-06-24 11:19 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Paolo Bonzini, Stefano Stabellini, qemu-devel, xen-devel
On Fri, Jun 24, 2016 at 07:46:23AM +0200, Gerd Hoffmann wrote:
> On Do, 2016-06-23 at 17:18 +0100, Anthony PERARD wrote:
> > On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote:
> > > Hi,
> > >
> > > > How could xen_ram_init() find out if the value of max-ram-below-4g is
> > > > the default or if a user have set it? Is there another way we could fix
> > > > this?
> > >
> > > Attached patch should fix it. Patch survived a quick smoke test on kvm
> > > so far, need to do some more testing tomorrow. Can you give it a spin
> > > on xen?
> >
> > Thanks. Unfortunately, it does not work :(.
> >
> > In this patch, max_ram_below_4g is set before the call to xen_ram_init()
> > and xen_ram_init read it back (via object_property_get_int()). So, in
> > xen_ram_init, user_lowmem is not 0.
>
> Ah, I see. We do the split calculation twice on xen. That is pretty
> pointless. New patch attached.
I've tested on Xen, it works fine. Thanks. Also, the patch look good.
Cheers,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] Change of max-ram-below-4g initial value breaks Xen
2016-06-24 5:46 ` [Qemu-devel] " Gerd Hoffmann
2016-06-24 11:19 ` Anthony PERARD
@ 2016-06-24 11:19 ` Anthony PERARD
1 sibling, 0 replies; 12+ messages in thread
From: Anthony PERARD @ 2016-06-24 11:19 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, xen-devel, Stefano Stabellini, Paolo Bonzini
On Fri, Jun 24, 2016 at 07:46:23AM +0200, Gerd Hoffmann wrote:
> On Do, 2016-06-23 at 17:18 +0100, Anthony PERARD wrote:
> > On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote:
> > > Hi,
> > >
> > > > How could xen_ram_init() find out if the value of max-ram-below-4g is
> > > > the default or if a user have set it? Is there another way we could fix
> > > > this?
> > >
> > > Attached patch should fix it. Patch survived a quick smoke test on kvm
> > > so far, need to do some more testing tomorrow. Can you give it a spin
> > > on xen?
> >
> > Thanks. Unfortunately, it does not work :(.
> >
> > In this patch, max_ram_below_4g is set before the call to xen_ram_init()
> > and xen_ram_init read it back (via object_property_get_int()). So, in
> > xen_ram_init, user_lowmem is not 0.
>
> Ah, I see. We do the split calculation twice on xen. That is pretty
> pointless. New patch attached.
I've tested on Xen, it works fine. Thanks. Also, the patch look good.
Cheers,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 12+ messages in thread