Subject: Re: [PATCH -V3 3/4] powerpc/kvm: Contiguous memory allocator based RMA allocation

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Tue, 02 Jul 2013 21:58:45 +0530

Alexander Graf <[email-masked]> writes:

> On 07/02/2013 05:29 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf<[email-masked]>  writes:
>>
>>> On 07/02/2013 07:45 AM, Aneesh Kumar K.V wrote:
>>>> From: "Aneesh Kumar K.V"<[email-masked]>
>>>>
>>>> Older version of power architecture use Real Mode Offset register and Real Mode Limit
>>>> Selector for mapping guest Real Mode Area. The guest RMA should be physically
>>>> contigous since we use the range when address translation is not enabled.
>>>>
>>>> This patch switch RMA allocation code to use contigous memory allocator. The patch
>>>> also remove the the linear allocator which not used any more
>>>>
>>>> Acked-by: Paul Mackerras<[email-masked]>
>>>> Signed-off-by: Aneesh Kumar K.V<[email-masked]>
>>>> ---
>> .... snip ....
>>
>>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>>> index 550f592..55c8519 100644
>>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>>> @@ -1511,10 +1511,10 @@ static inline int lpcr_rmls(unsigned long rma_size)
>>>>
>>>>    static int kvm_rma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>>>>    {
>>>> -	struct kvmppc_linear_info *ri = vma->vm_file->private_data;
>>>>    	struct page *page;
>>>> +	struct kvm_rma_info *ri = vma->vm_file->private_data;
>>>>
>>>> -	if (vmf->pgoff>= ri->npages)
>>>> +	if (vmf->pgoff>= kvm_rma_pages)
>>>>    		return VM_FAULT_SIGBUS;
>>>>
>>>>    	page = pfn_to_page(ri->base_pfn + vmf->pgoff);
>>>> @@ -1536,7 +1536,7 @@ static int kvm_rma_mmap(struct file *file, struct vm_area_struct *vma)
>>>>
>>>>    static int kvm_rma_release(struct inode *inode, struct file *filp)
>>>>    {
>>>> -	struct kvmppc_linear_info *ri = filp->private_data;
>>>> +	struct kvm_rma_info *ri = filp->private_data;
>>>>
>>>>    	kvm_release_rma(ri);
>>>>    	return 0;
>>>> @@ -1549,8 +1549,17 @@ static const struct file_operations kvm_rma_fops = {
>>>>
>>>>    long kvm_vm_ioctl_allocate_rma(struct kvm *kvm, struct kvm_allocate_rma *ret)
>>>>    {
>>>> -	struct kvmppc_linear_info *ri;
>>>>    	long fd;
>>>> +	struct kvm_rma_info *ri;
>>>> +	/*
>>>> +	 * Only do this on PPC970 in HV mode
>>>> +	 */
>>>> +	if (!cpu_has_feature(CPU_FTR_HVMODE) ||
>>>> +	    !cpu_has_feature(CPU_FTR_ARCH_201))
>>>> +		return -EINVAL;
>>> Is this really what we want? User space may want to use an RMA on POWER7
>>> systems, no?
>> IIUC they will use virtual real mode area (VRMA) and not RMA
>
> Then I suppose we should at least update the comment a bit further down 
> the patch that indicates that on POWER7 systems we do support a real 
> RMA. I can't really think of any reason why user space would want to use 
> RMA over VRMA.
>

where ? We have comments like

/* On POWER7, use VRMA; on PPC970, give up */

-aneesh