lkml.org 
[lkml]   [2020]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 5/7] KVM: PPC: clean up redundant kvm_run parameters in assembly
From
Date


On 2020/5/26 13:59, Paul Mackerras wrote:
> On Mon, Apr 27, 2020 at 12:35:12PM +0800, Tianjia Zhang wrote:
>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>> structure. For historical reasons, many kvm-related function parameters
>> retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This
>> patch does a unified cleanup of these remaining redundant parameters.
>
> Some of these changes don't look completely correct to me, see below.
> If you're expecting these patches to go through my tree, I can fix up
> the patch and commit it (with you as author), noting the changes I
> made in the commit message. Do you want me to do that?
>

I am very glad for you to do so, although I have submitted a new version
of patch, I still prefer you to fix up and commit it.

Thanks and best,
Tianjia

>> diff --git a/arch/powerpc/kvm/book3s_interrupts.S b/arch/powerpc/kvm/book3s_interrupts.S
>> index f7ad99d972ce..0eff749d8027 100644
>> --- a/arch/powerpc/kvm/book3s_interrupts.S
>> +++ b/arch/powerpc/kvm/book3s_interrupts.S
>> @@ -55,8 +55,7 @@
>> ****************************************************************************/
>>
>> /* Registers:
>> - * r3: kvm_run pointer
>> - * r4: vcpu pointer
>> + * r3: vcpu pointer
>> */
>> _GLOBAL(__kvmppc_vcpu_run)
>>
>> @@ -68,8 +67,8 @@ kvm_start_entry:
>> /* Save host state to the stack */
>> PPC_STLU r1, -SWITCH_FRAME_SIZE(r1)
>>
>> - /* Save r3 (kvm_run) and r4 (vcpu) */
>> - SAVE_2GPRS(3, r1)
>> + /* Save r3 (vcpu) */
>> + SAVE_GPR(3, r1)
>>
>> /* Save non-volatile registers (r14 - r31) */
>> SAVE_NVGPRS(r1)
>> @@ -82,11 +81,11 @@ kvm_start_entry:
>> PPC_STL r0, _LINK(r1)
>>
>> /* Load non-volatile guest state from the vcpu */
>> - VCPU_LOAD_NVGPRS(r4)
>> + VCPU_LOAD_NVGPRS(r3)
>>
>> kvm_start_lightweight:
>> /* Copy registers into shadow vcpu so we can access them in real mode */
>> - mr r3, r4
>> + mr r4, r3
>
> This mr doesn't seem necessary.
>
>> bl FUNC(kvmppc_copy_to_svcpu)
>> nop
>> REST_GPR(4, r1)
>
> This should be loading r4 from GPR3(r1), not GPR4(r1) - which is what
> REST_GPR(4, r1) will do.
>
> Then, in the file but not in the patch context, there is this line:
>
> PPC_LL r3, GPR4(r1) /* vcpu pointer */
>
> where once again GPR4 needs to be GPR3.
>
>> @@ -191,10 +190,10 @@ after_sprg3_load:
>> PPC_STL r31, VCPU_GPR(R31)(r7)
>>
>> /* Pass the exit number as 3rd argument to kvmppc_handle_exit */
>
> The comment should be modified to say "2nd" instead of "3rd",
> otherwise it is confusing.
>
> The rest of the patch looks OK.
>
> Paul.
>

\
 
 \ /
  Last update: 2020-07-13 05:08    [W:0.082 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site