From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wzkzh-0005ug-9Q for qemu-devel@nongnu.org; Wed, 25 Jun 2014 07:04:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wzkzb-00045d-VY for qemu-devel@nongnu.org; Wed, 25 Jun 2014 07:04:01 -0400 Message-ID: <53AAAC97.6030409@suse.de> Date: Wed, 25 Jun 2014 13:03:51 +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> In-Reply-To: <87a991ppv7.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 Cc: qemu-ppc@nongnu.org, Benjamin Herrenschmidt , aik@au1.ibm.com, Anton Blanchard 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? Did PowerVM try to inject a soft shutdown signal into the VM and this check is to make sure we don't get that? I think we should make 100% sure we understand what situation the fix above actually tried to avoid and make sure QEMU doesn't do it. Alex