>> 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