Re: [Xen-devel] [PATCH v02 6/7] arm: introduce do_translate_pagetable hypercall

[Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>]

Hi Stefano,


On Fri, Jul 4, 2014 at 5:05 PM, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Thu, 26 Jun 2014, Andrii Tseglytskyi wrote:
>> The reason of the patch is the following - some remoteprocs
>> are quite complicated, and their MMUs can handle several
>> pagetables. Good example is a OMAP5 GPU, which allocates
>> several pagetables during it work. Additional requirement
>> is that not all pagetable physical addresses are stored
>> in MMU registers. Some pagetables may be allocated and
>> then their physical addresses are sent to GPU using private
>> message loop between GPU kernel driver and GPU remoteproc.
>>
>> Patch is developed to handle this. At any moment of time
>> kernel can perform translation of such pagetables, before
>> sending their addresses to GPU remoteproc.
>>
>> Change-Id: Ie84012163205c3a2f920dc4cf07327a244647c93
>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>
>> ---
>>  xen/arch/arm/remoteproc_iommu.c |   22 ++++++++++++++++++++++
>>  xen/arch/arm/traps.c            |    1 +
>>  xen/include/public/xen.h        |    1 +
>>  xen/include/xen/hypercall.h     |   12 ++++++++++++
>>  4 files changed, 36 insertions(+)
>>
>> diff --git a/xen/arch/arm/remoteproc_iommu.c 
>> b/xen/arch/arm/remoteproc_iommu.c
>> index 3b3da3b..316ceb4 100644
>> --- a/xen/arch/arm/remoteproc_iommu.c
>> +++ b/xen/arch/arm/remoteproc_iommu.c
>> @@ -23,6 +23,8 @@
>>  #include <xen/init.h>
>>  #include <xen/sched.h>
>>  #include <xen/stdbool.h>
>> +#include <xen/hypercall.h>
>> +#include <xen/guest_access.h>
>>  #include <asm/system.h>
>>  #include <asm/current.h>
>>  #include <asm/io.h>
>> @@ -382,6 +384,26 @@ static int mmu_init(struct mmu_info *mmu, u32 data)
>>      return 0;
>>  }
>>
>> +long do_translate_pagetable(int cmd, 
>> XEN_GUEST_HANDLE_PARAM(xen_pagetable_addr_t) pgt_addr)
>> +{
>> +    struct xen_pagetable_addr pgt;
>> +    struct mmu_info *mmu = NULL;
>> +
>> +    if ( copy_from_guest(&pgt, pgt_addr, 1) )
>> +        return -EFAULT;
>> +
>> +    mmu = mmu_lookup(pgt.reg);
>> +    if ( !mmu )
>> +    {
>> +        pr_mmu("can't get mmu for addr 0x%08x", pgt.reg);
>> +        return -EINVAL;
>> +    }
>> +
>> +    pgt.maddr = mmu_translate_pagetable(mmu, pgt.paddr);
>> +
>> +    return copy_to_guest(pgt_addr, &pgt, 1);
>> +}
>> +
>>  static int mmu_init_all(void)
>>  {
>>      int res;
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 21c7b26..05b5184 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -1012,6 +1012,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
>>      HYPERCALL(hvm_op, 2),
>>      HYPERCALL(grant_table_op, 3),
>>      HYPERCALL_ARM(vcpu_op, 3),
>> +    HYPERCALL(translate_pagetable, 2),
>>  };
>>
>>  typedef int (*arm_psci_fn_t)(uint32_t, register_t);
>> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
>> index 8c5697e..91ca6a1 100644
>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -101,6 +101,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>>  #define __HYPERVISOR_kexec_op             37
>>  #define __HYPERVISOR_tmem_op              38
>>  #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
>> +#define __HYPERVISOR_translate_pagetable  40
>
> It might be best to introduce this hypercall as an arch memory op
> (xen/arch/arm/mm.c:arch_memory_op), rather than a full blown new
> hypercall. Otherwise you'll need to provide an empty stub implementation
> for x86.
>

I would prefer not to touch x86 code. arch_memory_op may be the point
here. Will try this.

>
>>  /* Architecture-specific hypercall definitions. */
>>  #define __HYPERVISOR_arch_0               48
>> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
>> index a9e5229..a025435 100644
>> --- a/xen/include/xen/hypercall.h
>> +++ b/xen/include/xen/hypercall.h
>> @@ -136,6 +136,18 @@ extern long
>>  do_tmem_op(
>>      XEN_GUEST_HANDLE_PARAM(tmem_op_t) uops);
>>
>> +struct xen_pagetable_addr {
>> +     u32 reg;

Base address of iomem, associated with corresponding MMU. Used as a search key

>> +     u32 paddr;

Physical address of MMU pagetable, which is allocated in kernel.

>> +     u32 maddr;

Output parameter. Machine address of translated pagetable.

>
> Could you please comment what these fields are for exactly?
>

Regards,
Andrii



-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel