Hi, attached patch fixes S3 resume on AMD CPUs. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 05/06/2009 09:00, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> attached patch fixes S3 resume on AMD CPUs. > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>Please make a minimal patch and explain each individual change. This one has bizarre additional alignment, nops, double short jumps, longjmp->shortjmp but keeps a comment which is then incorrect. Why are they needed? Is the need documented? I don''t see anything similar in Linux 2.6.27 wakeup routines (2.6.27 is what I happen to have to hand). I won''t take your random permutations on this file of all files especially, since it is a pain in the arse to debug when it gets broken. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2009-Jun-15 15:33 UTC
Re: [Xen-devel] [PATCH] xen: fix s3 resume on AMD CPUs
On Friday 05 June 2009 10:21:26 Keir Fraser wrote:> On 05/06/2009 09:00, "Christoph Egger" <Christoph.Egger@amd.com> wrote: > > attached patch fixes S3 resume on AMD CPUs. > > > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> > > Please make a minimal patch and explain each individual change. This one > has bizarre additional alignment, nops, double short jumps, > longjmp->shortjmp but keeps a comment which is then incorrect. Why are they > needed? Is the need documented? I don''t see anything similar in Linux > 2.6.27 wakeup routines (2.6.27 is what I happen to have to hand). > > I won''t take your random permutations on this file of all files especially, > since it is a pain in the arse to debug when it gets broken.Attached patch adds a few comments, turns a long jump into a short jump to avoid a #GP. Reload %cs via lretq right after stack pointer has been restored. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Christoph Egger <Christoph.Egger@amd.com> 15.06.09 17:33 >>> >@@ -174,8 +177,11 @@ wakeup_64: > mov $(__HYPERVISOR_DS64), %eax > mov %eax, %ds > >- # long jump to return point, with cs reload >- rex64 ljmp *ret_point(%rip) >+ /* Continue with wakeup in the high-level wakeup code. >+ * Reload cs there. >+ */ >+ movq ret_point(%rip), %rbx >+ jmp *%rbx > > .align 8 > ret_point:Why do you add a comment ("Reload cs here") here that is not in sync with the changed code? Also, if the sole reference ro ret_point is now a near jump, why don''t you remove the selector part of ret_point itself? Further more, why does this need to be an indirect jump now that it''s not a far jump anymore? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2009-Jun-16 08:52 UTC
Re: [Xen-devel] [PATCH] xen: fix s3 resume on AMD CPUs
On Tuesday 16 June 2009 10:30:57 Jan Beulich wrote:> >>> Christoph Egger <Christoph.Egger@amd.com> 15.06.09 17:33 >>> > > > >@@ -174,8 +177,11 @@ wakeup_64: > > mov $(__HYPERVISOR_DS64), %eax > > mov %eax, %ds > > > >- # long jump to return point, with cs reload > >- rex64 ljmp *ret_point(%rip) > >+ /* Continue with wakeup in the high-level wakeup code. > >+ * Reload cs there. > >+ */ > >+ movq ret_point(%rip), %rbx > >+ jmp *%rbx > > > > .align 8 > > ret_point: > > Why do you add a comment ("Reload cs here") here that is not in sync with > the changed code?Re-read: You missed the "t" :) Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 16/06/2009 09:30, "Jan Beulich" <JBeulich@novell.com> wrote:> Also, if the sole reference ro ret_point is now a near jump, why don''t you > remove the selector part of ret_point itself? Further more, why does this > need to be an indirect jump now that it''s not a far jump anymore?I''ll be cleaning this patch up a little bit before applying it. - Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel