On 22/03/2019 16:15, Suraj Jitindar Singh wrote:
> On Tue, 2019-03-19 at 17:53 +1100, Alexey Kardashevskiy wrote:
>>
>> On 19/03/2019 15:04, Suraj Jitindar Singh wrote:
>>> Implement a real mode handler for the H_CALL H_PAGE_INIT which can
>>> be
>>> used to zero or copy a guest page. The page is defined to be 4k and
>>> must
>>> be 4k aligned.
>>>
>>> The in-kernel real mode handler halves the time to handle this
>>> H_CALL
>>> compared to handling it in userspace for a hash guest.
>>>
>>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@xxxxxxxxx>
>>> ---
>>> arch/powerpc/include/asm/kvm_ppc.h | 2 +
>>> arch/powerpc/kvm/book3s_hv_rm_mmu.c | 144
>>> ++++++++++++++++++++++++++++++++
>>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 +-
>>> 3 files changed, 147 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
>>> b/arch/powerpc/include/asm/kvm_ppc.h
>>> index 8e8bb1299a0e..f34f290463aa 100644
>>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>>> @@ -653,6 +653,8 @@ long kvmppc_h_clear_ref(struct kvm_vcpu *vcpu,
>>> unsigned long flags,
>>> unsigned long pte_index);
>>> long kvmppc_h_clear_mod(struct kvm_vcpu *vcpu, unsigned long
>>> flags,
>>> unsigned long pte_index);
>>> +long kvmppc_rm_h_page_init(struct kvm_vcpu *vcpu, unsigned long
>>> flags,
>>> + unsigned long dest, unsigned long src);
>>> long kvmppc_hpte_hv_fault(struct kvm_vcpu *vcpu, unsigned long
>>> addr,
>>> unsigned long slb_v, unsigned int
>>> status, bool data);
>>> unsigned long kvmppc_rm_h_xirr(struct kvm_vcpu *vcpu);
>>> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>>> b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>>> index 3b3791ed74a6..26cfe1480ff5 100644
>>> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>>> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
>>> @@ -867,6 +867,150 @@ long kvmppc_h_clear_mod(struct kvm_vcpu
>>> *vcpu, unsigned long flags,
>>> return ret;
>>> }
>>>
>>> +static int kvmppc_get_hpa(struct kvm_vcpu *vcpu, unsigned long
>>> gpa,
>>> + int writing, unsigned long *hpa,
>>> + struct kvm_memory_slot **memslot_p)
>>> +{
>>> + struct kvm *kvm = vcpu->kvm;
>>> + struct kvm_memory_slot *memslot;
>>> + unsigned long gfn, hva, pa, psize = PAGE_SHIFT;
>>> + unsigned int shift;
>>> + pte_t *ptep, pte;
>>> +
>>> + /* Find the memslot for this address */
>>> + gfn = gpa >> PAGE_SHIFT;
>>> + memslot = __gfn_to_memslot(kvm_memslots_raw(kvm), gfn);
>>> + if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID))
>>> + return H_PARAMETER;
>>> +
>>> + /* Translate to host virtual address */
>>> + hva = __gfn_to_hva_memslot(memslot, gfn);
>>> +
>>> + /* Try to find the host pte for that virtual address */
>>> + ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL,
>>> &shift);
>>> + if (!ptep)
>>> + return H_TOO_HARD;
>>> + pte = kvmppc_read_update_linux_pte(ptep, writing);
>>> + if (!pte_present(pte))
>>> + return H_TOO_HARD;
>>> +
>>> + /* Convert to a physical address */
>>> + if (shift)
>>> + psize = 1UL << shift;
>>> + pa = pte_pfn(pte) << PAGE_SHIFT;
>>> + pa |= hva & (psize - 1);
>>> + pa |= gpa & ~PAGE_MASK;
>>> +
>>> + if (hpa)
>>> + *hpa = pa;
>>
>>
>> hpa is always not null.
>
> For now that is the case. I feel like it's better to check incase
> someone reuses the function in future.
Hard to imagine such case though. kvmppc_rm_ua_to_hpa() has never been
reused, for example.
>>
>>
>>> + if (memslot_p)
>>> + *memslot_p = memslot;
>>
>> memslot_p!=NULL only when writing==1, you can safely drop the writing
>> parameter.
>
> As above.
I still suggest adding more parameters only when you need them.
--
Alexey
