[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instructio
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instruction |
Date: |
Wed, 14 Jun 2017 19:02:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
>> 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
Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instruction, Thomas Huth, 2017/06/14
Re: [Qemu-devel] [PATCH v1 0/3] target/s390x: implement MVCOS and allow to enable it, no-reply, 2017/06/13