Re: [PATCH 05/21] MIPS: KVM: Add CP0_EPC KVM register access

David Daney
2014-04-25T21:45:09Z

On 04/25/2014 01:29 PM, James Hogan wrote:
> Hi David,
>
> On Friday 25 April 2014 09:44:17 David Daney wrote:
>> On 04/25/2014 08:19 AM, James Hogan wrote:
>>> Contrary to the comment, the guest CP0_EPC register cannot be set via
>>> kvm_regs, since it is distinct from the guest PC. Add the EPC register
>>> to the KVM_{GET,SET}_ONE_REG ioctl interface.
>>>
>>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Gleb Natapov <gleb@kernel.org>
>>> Cc: kvm@vger.kernel.org
>>> Cc: Ralf Baechle <ralf@linux-mips.org>
>>> Cc: linux-mips@linux-mips.org
>>> Cc: David Daney <david.daney@cavium.com>
>>> Cc: Sanjay Lal <sanjayl@kymasys.com>
>>
>> NACK...
>>
>>> ---
>>>
>>> arch/mips/kvm/kvm_mips.c | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
>>> index 46cea0bad518..db41876cbac5 100644
>>> --- a/arch/mips/kvm/kvm_mips.c
>>> +++ b/arch/mips/kvm/kvm_mips.c
>>> @@ -512,6 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>>>
>>> #define KVM_REG_MIPS_CP0_COMPARE MIPS_CP0_32(11, 0)
>>> #define KVM_REG_MIPS_CP0_STATUS MIPS_CP0_32(12, 0)
>>> #define KVM_REG_MIPS_CP0_CAUSE MIPS_CP0_32(13, 0)
>>>
>>> +#define KVM_REG_MIPS_CP0_EPC MIPS_CP0_64(14, 0)
>>
>> This is already called KVM_REG_MIPS_PC, you cannot change that.
>
> KVM_REG_MIPS_PC gets you vcpu->arch.pc, i.e. the next address of guest
> execution.
>
> KVM_REG_MIPS_CP0_EPC gets you the guest's CP0 EPC register which is the PC of
> the last guest exception.
>
> They are quite distinct state, even though vcpu->arch.pc is read from the
> *root context*'s CP0 EPC register after an exception or interrupt.
>

Sorry, my mistake. I was confusing Root/Host and Guest state.

I remove my objection.

David Daney

> Cheers
> James
>
>>
>>> #define KVM_REG_MIPS_CP0_EBASE MIPS_CP0_32(15, 1)
>>> #define KVM_REG_MIPS_CP0_CONFIG MIPS_CP0_32(16, 0)
>>> #define KVM_REG_MIPS_CP0_CONFIG1 MIPS_CP0_32(16, 1)
>>>
>>> @@ -567,7 +568,7 @@ static u64 kvm_mips_get_one_regs[] = {
>>>
>>> KVM_REG_MIPS_CP0_ENTRYHI,
>>> KVM_REG_MIPS_CP0_STATUS,
>>> KVM_REG_MIPS_CP0_CAUSE,
>>>
>>> - /* EPC set via kvm_regs, et al. */
>>> + KVM_REG_MIPS_CP0_EPC,
>>>
>>> KVM_REG_MIPS_CP0_CONFIG,
>>> KVM_REG_MIPS_CP0_CONFIG1,
>>> KVM_REG_MIPS_CP0_CONFIG2,
>>>
>>> @@ -620,6 +621,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
>>>
>>> case KVM_REG_MIPS_CP0_CAUSE:
>>> v = (long)kvm_read_c0_guest_cause(cop0);
>>> break;
>>>
>>> + case KVM_REG_MIPS_CP0_EPC:
>>> + v = (long)kvm_read_c0_guest_epc(cop0);
>>> + break;
>>>
>>> case KVM_REG_MIPS_CP0_ERROREPC:
>>> v = (long)kvm_read_c0_guest_errorepc(cop0);
>>> break;
>>>
>>> @@ -716,6 +720,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
>>>
>>> case KVM_REG_MIPS_CP0_CAUSE:
>>> kvm_write_c0_guest_cause(cop0, v);
>>> break;
>>>
>>> + case KVM_REG_MIPS_CP0_EPC:
>>> + kvm_write_c0_guest_epc(cop0, v);
>>> + break;
>>>
>>> case KVM_REG_MIPS_CP0_ERROREPC:
>>> kvm_write_c0_guest_errorepc(cop0, v);
>>> break;