Mailing List Archive

new domain builder breaks compatibility
To quote xen.h:

495 * 9. There is guaranteed to be at least 512kB padding after the final
496 * bootstrap element. If necessary, the bootstrap virtual region is
497 * extended by an extra 4MB to ensure this.

The new domain builder forgot this:

763 if (dom->alloc_bootstack)
764 dom->bootstack_pfn = xc_dom_alloc_page(dom, "boot stack");
765 xc_dom_printf("%-20s: virt_alloc_end : 0x%" PRIx64 "\n",
766 __FUNCTION__, dom->virt_alloc_end);

which, predictably enough, breaks Solaris, which tries to use this area as
ecratch memory during early boot.

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
John Levon wrote:
> To quote xen.h:
>
> 495 * 9. There is guaranteed to be at least 512kB padding after the final
> 496 * bootstrap element. If necessary, the bootstrap virtual region is
> 497 * extended by an extra 4MB to ensure this.

> which, predictably enough, breaks Solaris, which tries to use this area as
> ecratch memory during early boot.

The attached patch should fix this.

cheers,
Gerd

--
Gerd Hoffmann <kraxel@suse.de>
Re: new domain builder breaks compatibility [ In reply to ]
On Thu, Feb 01, 2007 at 10:04:43AM +0100, Gerd Hoffmann wrote:

> John Levon wrote:
> > To quote xen.h:
> >
> > 495 * 9. There is guaranteed to be at least 512kB padding after the final
> > 496 * bootstrap element. If necessary, the bootstrap virtual region is
> > 497 * extended by an extra 4MB to ensure this.
>
> > which, predictably enough, breaks Solaris, which tries to use this area as
> > ecratch memory during early boot.
>
> The attached patch should fix this.

Does that ensure the 4Mb alignment too? We're expecting:

scratch_end = RNDUP((paddr_t)scratch_start + 512 * 1024, FOUR_MEG);

BTW what is dom->extra_pages intended for?

thanks,
john


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
John Levon wrote:
> On Thu, Feb 01, 2007 at 10:04:43AM +0100, Gerd Hoffmann wrote:
>
>> John Levon wrote:
>>> To quote xen.h:
>>>
>>> 495 * 9. There is guaranteed to be at least 512kB padding after the final
>>> 496 * bootstrap element. If necessary, the bootstrap virtual region is
>>> 497 * extended by an extra 4MB to ensure this.
>>> which, predictably enough, breaks Solaris, which tries to use this area as
>>> ecratch memory during early boot.
>> The attached patch should fix this.
>
> Does that ensure the 4Mb alignment too?

Yes. Uhm, no, depends. It completely fills the topmost L1 pagetable
with entries, so you end up with an 4MB alignment for 32bit and 2MB
alignment for PAE and 64bit. I think the old builder had the same
behaviour, so the 4MB are actually a documentation bug ;)

> BTW what is dom->extra_pages intended for?

Allows allocation of some extra space. Used by the kexec bits I have in
the queue, it places some code which finishes the memory layout before
jumping to the new kernel entry point above the boot stack, so it needs
some way to reserve memory there.

cheers,
Gerd

--
Gerd Hoffmann <kraxel@suse.de>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
On Thu, Feb 01, 2007 at 04:15:16PM +0100, Gerd Hoffmann wrote:

> > Does that ensure the 4Mb alignment too?
>
> Yes. Uhm, no, depends. It completely fills the topmost L1 pagetable
> with entries, so you end up with an 4MB alignment for 32bit and 2MB
> alignment for PAE and 64bit. I think the old builder had the same
> behaviour, so the 4MB are actually a documentation bug ;)

Hmm... am I misunderstanding this code:

816 /* v_end = (vstack_end + (1UL<<22)-1) & ~((1UL<<22)-1); */
817 v_end = vstack_end;
818 if ( !increment_ulong(&v_end, (1UL<<22)-1) )
819 goto error_out;
820 v_end &= ~((1UL<<22)-1);
821
822 if ( (v_end - vstack_end) < (512UL << 10) )
823 {
824 /* Add extra 4MB to get >= 512kB padding. */
825 if ( !increment_ulong(&v_end, 1UL << 22) )
826 goto error_out;
827 }

Surely that guarantees we get at least 512Kb and it ends on a 4Mb
boundary?

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
John Levon wrote:
> On Thu, Feb 01, 2007 at 04:15:16PM +0100, Gerd Hoffmann wrote:
> Hmm... am I misunderstanding this code:

> 822 if ( (v_end - vstack_end) < (512UL << 10) )
> 823 {
> 824 /* Add extra 4MB to get >= 512kB padding. */
> 825 if ( !increment_ulong(&v_end, 1UL << 22) )
> 826 goto error_out;
> 827 }

No, you are right, it's 4MB unconditionally. Seems to have changed
though since I checked last time.

> Surely that guarantees we get at least 512Kb and it ends on a 4Mb
> boundary?

Yep. Is the 2MB alignment ok for Solaris? You can depend on the 512kb
only anyway, so the alignment shouldn't matter much ...

cheers,
Gerd

--
Gerd Hoffmann <kraxel@suse.de>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
On Thu, Feb 01, 2007 at 05:33:20PM +0100, Gerd Hoffmann wrote:

> > 822 if ( (v_end - vstack_end) < (512UL << 10) )
> > 823 {
> > 824 /* Add extra 4MB to get >= 512kB padding. */
> > 825 if ( !increment_ulong(&v_end, 1UL << 22) )
> > 826 goto error_out;
> > 827 }
>
> No, you are right, it's 4MB unconditionally. Seems to have changed
> though since I checked last time.
>
> > Surely that guarantees we get at least 512Kb and it ends on a 4Mb
> > boundary?
>
> Yep. Is the 2MB alignment ok for Solaris? You can depend on the 512kb
> only anyway, so the alignment shouldn't matter much ...

I think we could probably make it 2Mb. I'll test it out with your patch.
I don't see a reason why it would fail on 3.0.3 or 3.0.4 anyway.

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
On Thu, Feb 01, 2007 at 10:04:43AM +0100, Gerd Hoffmann wrote:

> --- tools/libxc/xc_dom_x86.c~ 2007-01-31 18:07:56.000000000 +0100
> +++ tools/libxc/xc_dom_x86.c 2007-02-01 10:02:08.000000000 +0100
> @@ -66,6 +66,7 @@
>
> extra_pages = dom->alloc_bootstack ? 1 : 0;
> extra_pages += dom->extra_pages;
> + extra_pages += 128; /* 512kB padding */

I've tested this along with a kernel change to only assume 2Mb
alignment, and it works on PAE and 64-bit now. Could you send a patch
upstream please? And update xen.h to clarify things?

thanks,
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
On 1/2/07 19:17, "John Levon" <levon@movementarian.org> wrote:

>> --- tools/libxc/xc_dom_x86.c~ 2007-01-31 18:07:56.000000000 +0100
>> +++ tools/libxc/xc_dom_x86.c 2007-02-01 10:02:08.000000000 +0100
>> @@ -66,6 +66,7 @@
>>
>> extra_pages = dom->alloc_bootstack ? 1 : 0;
>> extra_pages += dom->extra_pages;
>> + extra_pages += 128; /* 512kB padding */
>
> I've tested this along with a kernel change to only assume 2Mb
> alignment, and it works on PAE and 64-bit now. Could you send a patch
> upstream please? And update xen.h to clarify things?

I'll handle this and update xen.h.

Thanks,
Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
Keir Fraser wrote:
> On 1/2/07 19:17, "John Levon" <levon@movementarian.org> wrote:
>
>>> --- tools/libxc/xc_dom_x86.c~ 2007-01-31 18:07:56.000000000 +0100
>>> +++ tools/libxc/xc_dom_x86.c 2007-02-01 10:02:08.000000000 +0100
>>> @@ -66,6 +66,7 @@
>>>
>>> extra_pages = dom->alloc_bootstack ? 1 : 0;
>>> extra_pages += dom->extra_pages;
>>> + extra_pages += 128; /* 512kB padding */
>> I've tested this along with a kernel change to only assume 2Mb
>> alignment, and it works on PAE and 64-bit now. Could you send a patch
>> upstream please? And update xen.h to clarify things?
>
> I'll handle this and update xen.h.

Restoring the 4MB alignment is easy too, see attached patch (which also
fixes the 4MB alignment for virt_base).

cheers,
Gerd

--
Gerd Hoffmann <kraxel@suse.de>
Re: new domain builder breaks compatibility [ In reply to ]
John Levon wrote:
> I've tested this along with a kernel change to only assume 2Mb
> alignment, and it works on PAE and 64-bit now.

Oh, you change the kernel for that? Hmm, why don't you simply check how
many L2 entries are filled? Then you don't have to hard-code that
domain builder implementation detail into the solaris kernel ...

cheers,
Gerd

--
Gerd Hoffmann <kraxel@suse.de>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
On 2/2/07 07:59, "Gerd Hoffmann" <kraxel@suse.de> wrote:

>> I'll handle this and update xen.h.
>
> Restoring the 4MB alignment is easy too, see attached patch (which also
> fixes the 4MB alignment for virt_base).

I suppose it is nice to stick with the guarantees described in xen.h, since
it is so easy to do so.

-- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: new domain builder breaks compatibility [ In reply to ]
Keir Fraser wrote:
> On 2/2/07 07:59, "Gerd Hoffmann" <kraxel@suse.de> wrote:
>
>>> I'll handle this and update xen.h.
>> Restoring the 4MB alignment is easy too, see attached patch (which also
>> fixes the 4MB alignment for virt_base).
>
> I suppose it is nice to stick with the guarantees described in xen.h, since
> it is so easy to do so.

Slightly updated version of the patch, the previous one did fixup
virt_base too late. This one should work in theory. It's not really
tested though, the VIRT_BASE specified by linux kernels is at a 4MB
border anyway, so the fixup is a no-op ...

cheers,
Gerd

--
Gerd Hoffmann <kraxel@suse.de>
Re: new domain builder breaks compatibility [ In reply to ]
On Fri, Feb 02, 2007 at 04:13:54PM +0100, Gerd Hoffmann wrote:

> Slightly updated version of the patch, the previous one did fixup
> virt_base too late. This one should work in theory. It's not really
> tested though, the VIRT_BASE specified by linux kernels is at a 4MB
> border anyway, so the fixup is a no-op ...

Well, we're only expecting 2Mb now anyway...

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel