On Wed, 2010-10-27 at 18:20 +0100, Jeremy Fitzhardinge wrote:
> On 10/27/2010 09:04 AM, Ian Campbell wrote:
> > On Wed, 2010-10-27 at 14:18 +0100, Ian Campbell wrote:
> >>
> >> IOW we are faulting on precisely one of the PFNs which we earlier
> >> released. We released these because of a hole in the e820 between
> >> 0xa0000-0x100000 which dom0 apparently manufactured.
> >>
> >> IIRC in a PV domU we reserve some of the legacy address space to stop
> >> old ISA drivers etc from getting at it. I suspect this is wrong for a
> >> dom0 where we want the e820 to be more or less unmolested. I'll track
> >> down the code in question and see if removing it for dom0 helps.
> > More than the issue with unnecessarily reserving memory we simply can't
> > trust the e820 to cover all the firmware tables here. I think it is
> > better to simply not give any memory under 1M back to Xen in the domain
> > 0 case.
> >
> > 8<-----------
> >
> > >From e25932748e354f5da3fbb5719fb17f9ca2e35f09 Mon Sep 17 00:00:00 2001
> > From: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Date: Wed, 27 Oct 2010 17:02:25 +0100
> > Subject: [PATCH] xen: do not release any memory under 1M in domain 0
> >
> > We already deliberately setup a 1-1 P2M for the region up to 1M in
> > order to allow code which assumes this region is already mapped to
> > work without having to convert everything to ioremap.
> >
>
> Looks good, but:
>
> > Domain 0 should not return any apparently unused memory regions
> > (reserved or otherwise) in this region to Xen since the e820 may not
> > accurately reflect what the BIOS has stashed in this region.
> >
> > Similarly do not reserve the ISA range if we are domain 0 since it is
> > not necessarily normal, usable memory in that case.
> >
> > Since we now assume that we have a (reasonably) sensible e820 map in
> > domain 0 make a failure to obtain a machine memory map from the
> > hypervisor fatal rather than struggling on with a made up map and
> > suffering all the potential fallout.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> > arch/x86/xen/setup.c | 32 +++++++++++++++++++++++++-------
> > 1 files changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> > index 915b0c3..baad88c 100644
> > --- a/arch/x86/xen/setup.c
> > +++ b/arch/x86/xen/setup.c
> > @@ -84,6 +84,22 @@ static unsigned long __init
> > xen_release_chunk(phys_addr_t start_addr,
> > start = PFN_UP(start_addr);
> > end = PFN_DOWN(end_addr);
> >
> > + /*
> > + * Domain 0 maintains a 1-1 P2M mapping for the first megabyte
> > + * so do not return such memory to the hypervisor.
> > + *
> > + * This region can contain various firmware tables and the
> > + * like which are often assumed to be always mapped and
> > + * available via phys_to_virt.
> > + */
> > + if (xen_initial_domain()) {
> > + if (end < PFN_DOWN(0x100000UL))
> > + return 0;
> > +
> > + if (start < PFN_DOWN(0x100000UL))
> > + start = PFN_DOWN(0x100000UL);
>
> Use ISA_END_ADDRESS I think.
Yes.
I notice that the 1-1 P2M only seems to extend from ISA_START_ADDRESS
(0xa0000) but the range missing from the e820 here starts at 0x9e000.
Should we be doing the 1-1 P2M over the whole lower 1M?
>
> > + }
> > +
> > if (end <= start)
> > return 0;
> >
> > @@ -163,6 +179,7 @@ char * __init xen_memory_setup(void)
> > XENMEM_memory_map;
> > rc = HYPERVISOR_memory_op(op, &memmap);
> > if (rc == -ENOSYS) {
> > + BUG_ON(xen_initial_domain());
> > memmap.nr_entries = 1;
> > map[0].addr = 0ULL;
> > map[0].size = mem_end;
> > @@ -200,15 +217,16 @@ char * __init xen_memory_setup(void)
> > }
> >
> > /*
> > - * Even though this is normal, usable memory under Xen, reserve
> > - * ISA memory anyway because too many things think they can poke
> > - * about in there.
> > + * Even though this is normal, usable memory in a Xen domU,
> > + * reserve ISA memory anyway because too many things think
> > + * they can poke about in there.
> > *
> > - * In a dom0 kernel, this region is identity mapped with the
> > - * hardware ISA area, so it really is out of bounds.
> > + * In dom0 we use the host e820 and therefore do not need to
> > + * specially reserved anything.
> > */
> > - e820_add_region(ISA_START_ADDRESS, ISA_END_ADDRESS -
> > ISA_START_ADDRESS,
> > - E820_RESERVED);
> > + if (!xen_initial_domain())
> > + e820_add_region(ISA_START_ADDRESS, ISA_END_ADDRESS -
> > ISA_START_ADDRESS,
> > + E820_RESERVED);
>
> What's the problem with making it unconditionally reserved even if the
> host E820 doesn't?
Perhaps nothing, but if it was necessary wouldn't it be needed on native
too?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home