From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44605) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLBgz-0004JB-Gv for qemu-devel@nongnu.org; Wed, 14 Jun 2017 13:02:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLBgy-00030P-K6 for qemu-devel@nongnu.org; Wed, 14 Jun 2017 13:02:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57800) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dLBgy-00030A-EK for qemu-devel@nongnu.org; Wed, 14 Jun 2017 13:02:52 -0400 References: <20170613214736.19963-1-david@redhat.com> <20170613214736.19963-3-david@redhat.com> <3c564230-7879-45ff-ffa1-c4e7bea17902@twiddle.net> <52401112-57c8-201f-d091-afb3a196412f@redhat.com> From: David Hildenbrand Message-ID: <0959e60a-7894-d7d7-0bfe-dbb19b162875@redhat.com> Date: Wed, 14 Jun 2017 19:02:48 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instruction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org Cc: agraf@suse.de, Aurelien Jarno , thuth@redhat.com, Miroslav Benes >> Would it makes sense to >> >> a) move cpu_restore_state() into program_interrupt() >> b) make all callers forward ra from GETPC() (problem with kvm code that >> share handlers?) >> c) fixup callers that already do the cpu_restore_state() >> d) drop potential_page_fault() completely > > Yes, that makes sense. For B, kvm can pass 0 for RA and nothing will happen. > For C, that project is on-going but not complete; D is indeed the ultimate goal. > >> Two questions: >> a) Could we avoid having to forward the ra by doing GETPC directly in >> program_interrupt()? In mem_helper, I can see that we do GETPC on >> several places and pass it around, so I assume GETPC() has to be called >> in the first handler? > > You must use GETPC in the first handler. We're looking for the address of the > TCG generated code from where we were called. So, no, you can't use GETPC from > program_interrupt. > >> b) With cpu_restore_state(), there is no need for update_psw_addr() + >> update_cc_op(), correct? > > Correct. Thanks for the clarification! > >>>> + potential_page_fault(s); >>>> + gen_helper_mvcos(cc_op, cpu_env, o->addr1, o->in2, regs[r3]); >>> >>> ... the potential_page_fault. >> >> I would suggest to leave it in this patch as it and then clean it up all >> together in one shot (adding 5 cpu_restore_state() vs. one >> potential_page_fault() temporarily looks better to me). > > I would say the opposite, since the code generated by potential_page_fault is > always executed, whereas the cpu_restore_state is on an error path which for a > well-behaved guest will never be executed. By temporary I meant: I will be looking into cleaning this all up and getting rid of potential_page_fault() soon :) However, in v2 I avoided potential_page_fault(). > > > r~ > Thanks! -- Thanks, David