[PATCH] Software Suspend 2 for ppc64 (1/2 - core)

[Santiago Leon]

Ben... thanks for the comments...

>>+	if (try_to_freeze()) {
>>+		signr = 0;
>>+		if (!signal_pending(current))
>>+			goto no_signal;
>>+	}
>>+
>>+	if (freezing(current)) {
>>+		try_to_freeze();
>>+		signr = 0;
>>+		recalc_sigpending();
>>+		if (!signal_pending(current))
>>+			goto no_signal;
>>+	}
> 
> 
> The above looks a bit weird & redundant ... But then, it might be some
> subtelty with Nigel's updated refrigerator.

This was copied from the ppc version and I couldn't find a reason to
change it...

>>+	/* Save MSR & SDR1 */
>>+	asm volatile ("mfmsr 4; std 4,%0" : "=m" (s->msr));
>>+	asm volatile ("mfsdr1 4; std 4,%0": "=m" (s->sdr1));
>>+}
> 
> The above should be in an assembly .S file. 

again, I copied this from the ppc version, but you're right, it doesn't
make sense to have a function with all assembly code in a .c file...

> Generally, I don't like the
> way the processor state saving is done separately from the actual low
> level asm suspend call. 

Do you mean that you don't like the separation between
__save_processor_state() and __smp_suspend_lowlevel()?...

> It makes little sense. You assume gcc won't play
> with registers behind your back, fairly unsafe.

yeah... gcc *did* play with the registers on me and I fixed that by 
adding a constraint to one of the asm calls... pretty silly of me...

>>+		__restore_processor_state(suspend2_saved_contexts + 
>>+					 _smp_processor_id());
>>+		local_flush_tlb();
> 
> 
> I'm not sure calling local_flush_tlb() here makes much sense. You need
> to do more than just flushing the current batch here. 

Yeah, I'm aware... it's just that the thing worked before I started 
playing with this function (which I copied from i386), so I went ahead 
and released it to get people to play with it...

> You actually need
> to invalidate the entire TLB which is not necessarily easy,

why is it "not necessarily easy"?... isn't executing a tlbia enough?

> and you need to take care of the SLB as well.

yep...

>>+		/* 
>>+		 *Save context and go back to idling.
>>+		 * Note that we cannot leave the processor
>>+		 * here. It must be able to receive IPIs if
>>+		 * the LZF compression driver (eg) does a
>>+		 * vfree after compressing the kernel etc
>>+		 */
> Gack ? Can you explain the above comment a bit more ? Something is
> playing with kernel virtual space while CPUs are locked into IPIs and/or
> that kind of horror ? Doesn't seem like a very sane thing to do.

Sorry, I can't :)... This was copied from i386 with comments and all...

>>+static inline void move_stack_to_nonconflicing_area(void)
>>+{
>>+	unsigned long old_stack, src;
...
> In what context is the above called ? You are likely to die an horrible
> death when moving the stack around if you take an SLB miss at the wrong
> time. The kernel is careful about always locking the current kernel
> stack segment in SLB, various bits make assumption that remains true at
> all time.

Now that I realize, this function is not called by the latest ssusp2
core (I did the development on an earlier version)... so i guess it's a
moot point...

> In general, you seem to completely ignore the hash table and SLB. That
> might work by luck, because your "loader" kernel is the same as your
> "saved" kernel, you'll eventually end up with proper bolted down hash
> entries, but it's very dodgy. 

yeah I know it is luck that it works without messing with the htab and 
slb... I'll fix that...

> You are also ignoring the iommu,
Well... when the drivers suspend, they should unmap all their dmaable 
buffers and structs... when they resume, they have to map them, so I'm 
not sure if it is necessary to do anything else the iommu...

> , and RTAs,
yeah, I'll make sure that RTAS gets instantiated again...