Re: pkgsrc: patch-ba

[Christoph Egger <Christoph_Egger%gmx.de@localhost>]

On Wednesday 28 November 2007 15:58:10 Manuel Bouyer wrote:
> Hi,
>
> On Wed, Nov 28, 2007 at 02:49:17PM +0100, Christoph Egger wrote:
> > Hi Manuel!
> >
> > I just noticed your xenkernel3 package update:
> >
> > RCS file: /cvsroot/pkgsrc/sysutils/xenkernel3/patches/patch-ba,v
> > Working file: patch-ba
> > head: 1.2
> > branch:
> > locks: strict
> > access list:
> > symbolic names:
> >         pkgsrc-2007Q3: 1.1.0.2
> >         pkgsrc-2007Q3-base: 1.1
> > keyword substitution: kv
> > total revisions: 2;     selected revisions: 2
> > description:
> > ----------------------------
> > revision 1.2
> > date: 2007/11/26 19:35:25;  author: bouyer;  state: Exp;  lines: +12 -3
> > Properly initialize physaddr_bitsize for native 64bit dom0. Makes
> > NetBSD/xenamd64 boot on systems with more than 4Gb RAM.
> > Bump pkgrevision.
> >
> >
> > Looking at the diff, you unconditionally initialize
> > d->arch.physaddr_bitsize. Are you sure this is correct for 32bit / 32bit
> > pae Dom0 ?
> >
> > Further, for 64bit xen-kernels this is already initialized some lines
> > further below and overrides your hardcoded value. So I don't understand
> > in which way your patch fixes the boot problem with more than 4GB RAM.
> >
> > Can you explain this, please?

Thank you for the explanation.

> I tracked down my problem with more than 4GB RAM to
> d->arch.physaddr_bitsize being 0 in domain_clamp_alloc_bitsize(). This
> cause all attemps to XENMEM_increase_reservation with bits > 0 to fail

More precisely, __alloc_domheap_pages() returns NULL. This impacts
Xen heap allocation in general and on featurewise this impacts ballooning and 
NUMA support in Xen.

It makes no difference if the Linux Dom0 has 1GB or 7GB.
I don't understand how this can show no effect on Linux Dom0.

> (well for bits == 32 at last, maybe this test is short-circuited for other
> values of bits).

In __alloc_domheap_pages(), bits range is limited to 0x0 ... 0xf.

> In construct_dom0(), physaddr_bitsize is initialised only if
> ifdef CONFIG_COMPAT and if elf_32bit(&elf) (which, I assume, means that
> dom0 is a 32bit kernel)

or a 32bit pae kernel.

> and if UNSET_ADDR != parms.virt_hv_start_low 
> (I didn't look at what this could mean).

This is needed for the case of having a 64bit hypervisor and a 32bit pae Dom0.

> So, if I got it right, my patch should cause physaddr_bitsize to be 64 by
> default, and set to some other value later if needed; and if "not needed"
> 64 is the right value. If the hypervisor is not 64bits, physaddr_bitsize
> isn't used at all anyway.

That's right.
I submitted the patch.

Christoph