* [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem
@ 2013-05-24 13:36 Konrad Rzeszutek Wilk
2013-05-24 13:58 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-24 13:36 UTC (permalink / raw)
To: Ian.Campbell, david.vrabel, xen-devel; +Cc: christian, Konrad Rzeszutek Wilk
The code had an obvious bug where it would assume that the balloon
amount would always be _something_ and add an E820_RAM entry at the
end of the E820 array. The added E820_RAM would contain the balloon amount
plus the delta of memory that had to be subtracted b/c of the various
E820 entries. That assumption is certainly true when maxmem != mem,
but if guest config has maxmem = memory that is incorrect (as balloon
value is zero). The end result is that the E820 that is constructed
is missing a swath of "delta" memory and in most cases ends up with
only one E820_RAM entry that is of 512MB size on many Intel systems.
Reported-by: Christian Holpert <christian@holpert.de>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxl/libxl_x86.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index a17f6ae..a78c91d 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -182,7 +182,7 @@ static int e820_sanitize(libxl_ctx *ctx, struct e820entry src[],
idx++;
}
/* At this point we have the mapped RAM + E820 entries from src. */
- if (balloon_kb) {
+ if (balloon_kb || delta_kb) {
/* and if we truncated the RAM region, then add it to the end. */
e820[idx].type = E820_RAM;
e820[idx].addr = (uint64_t)(1ULL << 32) > last ?
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem
2013-05-24 13:36 [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem Konrad Rzeszutek Wilk
@ 2013-05-24 13:58 ` Ian Campbell
2013-05-24 14:51 ` Konrad Rzeszutek Wilk
2013-05-30 9:01 ` Ian Campbell
0 siblings, 2 replies; 6+ messages in thread
From: Ian Campbell @ 2013-05-24 13:58 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, david.vrabel, christian, Konrad Rzeszutek Wilk
On Fri, 2013-05-24 at 09:36 -0400, Konrad Rzeszutek Wilk wrote:
> The code had an obvious bug where it would assume that the balloon
> amount would always be _something_ and add an E820_RAM entry at the
> end of the E820 array. The added E820_RAM would contain the balloon amount
> plus the delta of memory that had to be subtracted b/c of the various
> E820 entries. That assumption is certainly true when maxmem != mem,
> but if guest config has maxmem = memory that is incorrect (as balloon
> value is zero). The end result is that the E820 that is constructed
> is missing a swath of "delta" memory and in most cases ends up with
> only one E820_RAM entry that is of 512MB size on many Intel systems.
>
> Reported-by: Christian Holpert <christian@holpert.de>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> tools/libxl/libxl_x86.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index a17f6ae..a78c91d 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -182,7 +182,7 @@ static int e820_sanitize(libxl_ctx *ctx, struct e820entry src[],
> idx++;
> }
> /* At this point we have the mapped RAM + E820 entries from src. */
> - if (balloon_kb) {
> + if (balloon_kb || delta_kb) {
> /* and if we truncated the RAM region, then add it to the end. */
> e820[idx].type = E820_RAM;
> e820[idx].addr = (uint64_t)(1ULL << 32) > last ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem
2013-05-24 13:58 ` Ian Campbell
@ 2013-05-24 14:51 ` Konrad Rzeszutek Wilk
2013-05-24 14:55 ` Ian Campbell
2013-05-30 9:01 ` Ian Campbell
1 sibling, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-24 14:51 UTC (permalink / raw)
To: Ian Campbell, george.dunlap
Cc: Konrad Rzeszutek Wilk, xen-devel, david.vrabel, christian
On Fri, May 24, 2013 at 02:58:24PM +0100, Ian Campbell wrote:
> On Fri, 2013-05-24 at 09:36 -0400, Konrad Rzeszutek Wilk wrote:
> > The code had an obvious bug where it would assume that the balloon
> > amount would always be _something_ and add an E820_RAM entry at the
> > end of the E820 array. The added E820_RAM would contain the balloon amount
> > plus the delta of memory that had to be subtracted b/c of the various
> > E820 entries. That assumption is certainly true when maxmem != mem,
> > but if guest config has maxmem = memory that is incorrect (as balloon
> > value is zero). The end result is that the E820 that is constructed
> > is missing a swath of "delta" memory and in most cases ends up with
> > only one E820_RAM entry that is of 512MB size on many Intel systems.
> >
> > Reported-by: Christian Holpert <christian@holpert.de>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
George,
Would you be OK putting this in Xen 4.3? It is only exposed if a user
1) launches a PV guest
2). Has in the guest config: pci=['blabla']\ne820_host=1
3). Is not that freq used as only a couple of people seem to use it
>
> > ---
> > tools/libxl/libxl_x86.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> > index a17f6ae..a78c91d 100644
> > --- a/tools/libxl/libxl_x86.c
> > +++ b/tools/libxl/libxl_x86.c
> > @@ -182,7 +182,7 @@ static int e820_sanitize(libxl_ctx *ctx, struct e820entry src[],
> > idx++;
> > }
> > /* At this point we have the mapped RAM + E820 entries from src. */
> > - if (balloon_kb) {
> > + if (balloon_kb || delta_kb) {
> > /* and if we truncated the RAM region, then add it to the end. */
> > e820[idx].type = E820_RAM;
> > e820[idx].addr = (uint64_t)(1ULL << 32) > last ?
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem
2013-05-24 14:51 ` Konrad Rzeszutek Wilk
@ 2013-05-24 14:55 ` Ian Campbell
2013-05-24 14:57 ` George Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-05-24 14:55 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: george.dunlap, Konrad Rzeszutek Wilk, xen-devel, david.vrabel, christian
On Fri, 2013-05-24 at 10:51 -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, May 24, 2013 at 02:58:24PM +0100, Ian Campbell wrote:
> > On Fri, 2013-05-24 at 09:36 -0400, Konrad Rzeszutek Wilk wrote:
> > > The code had an obvious bug where it would assume that the balloon
> > > amount would always be _something_ and add an E820_RAM entry at the
> > > end of the E820 array. The added E820_RAM would contain the balloon amount
> > > plus the delta of memory that had to be subtracted b/c of the various
> > > E820 entries. That assumption is certainly true when maxmem != mem,
> > > but if guest config has maxmem = memory that is incorrect (as balloon
> > > value is zero). The end result is that the E820 that is constructed
> > > is missing a swath of "delta" memory and in most cases ends up with
> > > only one E820_RAM entry that is of 512MB size on many Intel systems.
> > >
> > > Reported-by: Christian Holpert <christian@holpert.de>
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> George,
> Would you be OK putting this in Xen 4.3? It is only exposed if a user
> 1) launches a PV guest
> 2). Has in the guest config: pci=['blabla']\ne820_host=1
> 3). Is not that freq used as only a couple of people seem to use it
TBH I was just going to commit it as an obvious bug fix in my next sweep
of my queue (probably on Tuesday at this rate...)
I guess George has until then to object ;-)
Ian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem
2013-05-24 14:55 ` Ian Campbell
@ 2013-05-24 14:57 ` George Dunlap
0 siblings, 0 replies; 6+ messages in thread
From: George Dunlap @ 2013-05-24 14:57 UTC (permalink / raw)
To: Ian Campbell
Cc: Konrad Rzeszutek Wilk, xen-devel, david.vrabel, christian,
Konrad Rzeszutek Wilk
On 24/05/13 15:55, Ian Campbell wrote:
> On Fri, 2013-05-24 at 10:51 -0400, Konrad Rzeszutek Wilk wrote:
>> On Fri, May 24, 2013 at 02:58:24PM +0100, Ian Campbell wrote:
>>> On Fri, 2013-05-24 at 09:36 -0400, Konrad Rzeszutek Wilk wrote:
>>>> The code had an obvious bug where it would assume that the balloon
>>>> amount would always be _something_ and add an E820_RAM entry at the
>>>> end of the E820 array. The added E820_RAM would contain the balloon amount
>>>> plus the delta of memory that had to be subtracted b/c of the various
>>>> E820 entries. That assumption is certainly true when maxmem != mem,
>>>> but if guest config has maxmem = memory that is incorrect (as balloon
>>>> value is zero). The end result is that the E820 that is constructed
>>>> is missing a swath of "delta" memory and in most cases ends up with
>>>> only one E820_RAM entry that is of 512MB size on many Intel systems.
>>>>
>>>> Reported-by: Christian Holpert <christian@holpert.de>
>>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>> George,
>> Would you be OK putting this in Xen 4.3? It is only exposed if a user
>> 1) launches a PV guest
>> 2). Has in the guest config: pci=['blabla']\ne820_host=1
>> 3). Is not that freq used as only a couple of people seem to use it
> TBH I was just going to commit it as an obvious bug fix in my next sweep
> of my queue (probably on Tuesday at this rate...)
>
> I guess George has until then to object ;-)
Sounds like a bug fix -- no objections. :-)
-George
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem
2013-05-24 13:58 ` Ian Campbell
2013-05-24 14:51 ` Konrad Rzeszutek Wilk
@ 2013-05-30 9:01 ` Ian Campbell
1 sibling, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2013-05-30 9:01 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, david.vrabel, christian, Konrad Rzeszutek Wilk
On Fri, 2013-05-24 at 14:58 +0100, Ian Campbell wrote:
> On Fri, 2013-05-24 at 09:36 -0400, Konrad Rzeszutek Wilk wrote:
> > The code had an obvious bug where it would assume that the balloon
> > amount would always be _something_ and add an E820_RAM entry at the
> > end of the E820 array. The added E820_RAM would contain the balloon amount
> > plus the delta of memory that had to be subtracted b/c of the various
> > E820 entries. That assumption is certainly true when maxmem != mem,
> > but if guest config has maxmem = memory that is incorrect (as balloon
> > value is zero). The end result is that the E820 that is constructed
> > is missing a swath of "delta" memory and in most cases ends up with
> > only one E820_RAM entry that is of 512MB size on many Intel systems.
> >
> > Reported-by: Christian Holpert <christian@holpert.de>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
And applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-30 9:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-24 13:36 [PATCH] xl, e820_host, PV passthrough: Fix guests crashing when memory == maxmem Konrad Rzeszutek Wilk
2013-05-24 13:58 ` Ian Campbell
2013-05-24 14:51 ` Konrad Rzeszutek Wilk
2013-05-24 14:55 ` Ian Campbell
2013-05-24 14:57 ` George Dunlap
2013-05-30 9:01 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.