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

[Andrii Tseglytskyi <andrii.tseglytskyi@xxxxxxxxxxxxxxx>]

Hi Ian,

On Wed, Jul 16, 2014 at 6:42 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Fri, 2014-07-04 at 15:05 +0100, Stefano Stabellini wrote:
>> > 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.
>
> Or physdev op perhaps?
>
>>  Otherwise you'll need to provide an empty stub implementation
>> for x86.
>
> All unused hypercalls -ENOSYS automatically without stubs I think.

Taking this in account - do I need to redefine this hypercall in other
place? Or leave it as is?

>
> Are there any security concerns with exposing machine addresses to
> guests? There are certainly "breaking the abstraction" concerns, but
> that's unavoidable here I think.

Right. This is not good for security. But unfortunately on of OMAP5
remoteprocs (GPU) uses very specific pagetable allocations.
I described this in previous mails - it allocates pagetable and sends
it physical address directly for GPU, using messaging mechanism.
This address is not written to any MMU device iomem, and therefore I
can't avoid exposing of pagetable mfn.

>
>> >  /* 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;
>> > +   u32 paddr;
>> > +   u32 maddr;
>>
>> Could you please comment what these fields are for exactly?
>
> And mark them as IN/OUT as appropriate please.
>

OK

> Also the physical and machine addresses should always be 64 bits for
> compatibility with future larger chips.

OK. Will update.

>
> Having done that you then need padding after reg or to reorder things to
> avoid holes on 64-bit. Or maybe reg should be 64-bit, depends what it
> is. Either way the 32-bit and 64-bit layout of this struct should be
> identical.

I think reg will be 64 bit in this case. I would prefer this way.


Regards,
Andrii

>
> Ian.
>
>



-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

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