On Tue, 14 Jun 2011, Alexander Graf wrote: > On 03.06.2011, at 17:56, <stefano.stabell...@eu.citrix.com> > <stefano.stabell...@eu.citrix.com> wrote: > > > From: Stefano Stabellini <stefano.stabell...@eu.citrix.com> > > > > Xen can only do dirty bit tracking for one memory region, so we should > > explicitly avoid trying to track the legacy VGA region between 0xa0000 > > and 0xbffff, rather than trying and failing. > > > > Signed-off-by: Stefano Stabellini <stefano.stabell...@eu.citrix.com> > > --- > > xen-all.c | 4 ++++ > > 1 files changed, 4 insertions(+), 0 deletions(-) > > > > diff --git a/xen-all.c b/xen-all.c > > index 9a5c3ec..1fdc2e8 100644 > > --- a/xen-all.c > > +++ b/xen-all.c > > @@ -218,6 +218,10 @@ static int xen_add_to_physmap(XenIOState *state, > > if (get_physmapping(state, start_addr, size)) { > > return 0; > > } > > + /* do not try to map legacy VGA memory */ > > + if (start_addr >= 0xa0000 && start_addr + size <= 0xbffff) { > > I don't quite like the hardcoded range here. What exactly is the issue? The > fact that you can only map a single region? Then do a counter and fail when > it's > 1.
That is what we were doing before: succeeding the first time and failing from the second time on. By "coincidence" the second time was the range 0xa0000-0xbffff so everything worked as expected, but it wasn't obvious why. I am just trying to make sure that one year from now it will be clear just looking at the code why it works. > If you don't want to map the VGA region as memory slot, why not change the > actual mapping code in the cirrus adapter? Because I didn't want to introduce any ugly if (xen_enable()) in generic code, if it is that simple to catch the issue from xen specific code.