From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36389) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzlQz-0008Q9-6n for qemu-devel@nongnu.org; Wed, 25 Jun 2014 07:32:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzlQt-0006UC-T1 for qemu-devel@nongnu.org; Wed, 25 Jun 2014 07:32:13 -0400 Message-ID: <53AAB335.1070608@suse.de> Date: Wed, 25 Jun 2014 13:32:05 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1402574971-26672-2-git-send-email-nikunj@linux.vnet.ibm.com> <53A006A9.90108@suse.de> <871tunc288.fsf@linux.vnet.ibm.com> <53A01006.6080800@suse.de> <87y4wvamb3.fsf@linux.vnet.ibm.com> <53A01220.90009@suse.de> <87vbrzale9.fsf@linux.vnet.ibm.com> <87a991ppv7.fsf@abhimanyu.in.ibm.com> <53AAAC97.6030409@suse.de> <87zjh1ns91.fsf@abhimanyu.in.ibm.com> In-Reply-To: <87zjh1ns91.fsf@abhimanyu.in.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] ppc: spapr-rtas - implement os-term rtas call List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania , qemu-devel@nongnu.org, Anton Blanchard Cc: qemu-ppc@nongnu.org, Benjamin Herrenschmidt , aik@au1.ibm.com On 25.06.14 13:27, Nikunj A Dadhania wrote: > Alexander Graf writes: > >> On 25.06.14 06:36, Nikunj A Dadhania wrote: >>> Nikunj A Dadhania writes: >>> >>>> Alexander Graf writes: >>>> >>>>> On 17.06.14 11:59, Nikunj A Dadhania wrote: >>>>>> Alexander Graf writes: >>>>>>> On 17.06.14 11:30, Nikunj A Dadhania wrote: >>>>>>>> Alexander Graf writes: >>>>>>>>>> + spapr_rtas_register("ibm,os-term", rtas_ibm_os_term); >>>>>>>>>> + spapr_rtas_register("ibm,extended-os-term", rtas_ibm_ext_os_term); >>>>>>>>> Why do we need the extended-os-term if we don't do anything with it? >>>>>>>> Linux kernel checks for both of them because of legacy: >>>>>>>> >>>>>>>> arch/powerpc/kernel/rtas.c: >>>>>>>> >>>>>>>> void rtas_os_term(char *str) >>>>>>>> { >>>>>>>> [...] >>>>>>>> /* >>>>>>>> * Firmware with the ibm,extended-os-term property is guaranteed >>>>>>>> * to always return from an ibm,os-term call. Earlier versions without >>>>>>>> * this property may terminate the partition which we want to avoid >>>>>>>> * since it interferes with panic_timeout. >>>>>>> But we do not return from the RTAS call, so we don't adhere to the >>>>>>> extended semantics? >>>>>> But you would return without calling os-term call if >>>>>> ibm,extended-os-term isnt registered. For that reason I h ave defined a >>>>>> stub. >>>>> I appreciate the hacker mentality, but Linux explicitly checks on >>>>> ibm,extended-os-term to ensure that the hypervisor does not stop the VM >>>>> when it calls ibm,os-term. However, the implementation above does stop >>>>> the VM when the guest calls ibm,os-term. >>>> Seems to be added to do just that: >>>> >>>> commit e9bbc8cde0e3c33b42ddbe1b02108cb5c97275eb >>>> Author: Anton Blanchard >>>> Date: Thu Feb 18 12:11:51 2010 +0000 >>>> >>>> powerpc/pseries: Call ibm,os-term if the ibm,extended-os-term is present >>>> >>>> We have had issues in the past with ibm,os-term initiating shutdown of a >>>> partition. This is confusing to the user, especially if panic_timeout is >>>> non zero. >>>> >>>> The temporary fix was to avoid calling ibm,os-term if a panic_timeout was set >>>> and since we set it on every boot we basically never call ibm,os-term. >>>> >>>> An extended version of ibm,os-term has since been implemented which gives us >>>> the behaviour we want: >>>> >>>> "When the platform supports extended ibm,os-term behavior, the return to the >>>> RTAS will always occur unless there is a kernel assisted dump active as >>>> initiated by an ibm,configure-kernel-dump call." >>>> >>>> This patch checks for the ibm,extended-os-term property and calls ibm,os-term >>>> if it exists. >>>> >>>> Signed-off-by: Anton Blanchard >>>> Signed-off-by: Benjamin Herrenschmidt >>> I was thinking of the following: >>> >>> 1) Return the RTAS unsupported for extended-os-term >>> 2) A comment in the beginning of the function to suggest that this is a >>> stub need for legacy of PowerVM >>> >>> Please let me know your thoughts. >> I think we need to clarify what bug Anton was trying to fix. The >> implementation you're proposing for os-term may "initiate a shutdown >> of a partition", albeit a hard stop usually. Is this what Linux is >> trying to avoid? > Let me put down my understanding: > > There are two possible way to handle kernel panic: > 1) Kdump service running in guest - already working > 2) Pass the kernel panic information to hypervisor - not there in Qemu > pseries > > So without kdump service running, if linux kernel hits a panic, its going > to check os-term and extended-os-term, only then its going to call > os-term. It's checking both for a reason. Find that reason. Alex