[PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[Julien Grall]

The commit 6ae2df9 "mem_access: Add helper API to setup ring and enable
mem_access¨ break libxc compilation for ARM.

This is because xc_map_foreign_map and xc_domain_decrease_reservation_exact
is taking an xen_pfn_t in parameters. On ARM, xen_pfn_t is always an uin64_t.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Aravindh Puthiyaparambil <aravindp@cisco.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxc/xc_mem_event.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_mem_event.c b/tools/libxc/xc_mem_event.c
index be7c63d..0b2eecb 100644
--- a/tools/libxc/xc_mem_event.c
+++ b/tools/libxc/xc_mem_event.c
@@ -60,7 +60,8 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
                           uint32_t *port)
 {
     void *ring_page = NULL;
-    unsigned long ring_pfn, mmap_pfn;
+    unsigned long pfn;
+    xen_pfn_t ring_pfn, mmap_pfn;
     unsigned int op, mode;
     int rc1, rc2, saved_errno;
 
@@ -79,14 +80,15 @@ void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
     }
 
     /* Get the pfn of the ring page */
-    rc1 = xc_get_hvm_param(xch, domain_id, param, &ring_pfn);
+    rc1 = xc_get_hvm_param(xch, domain_id, param, &pfn);
     if ( rc1 != 0 )
     {
         PERROR("Failed to get pfn of ring page\n");
         goto out;
     }
 
-    mmap_pfn = ring_pfn;
+    ring_pfn = pfn;
+    mmap_pfn = pfn;
     ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ | PROT_WRITE,
                                      &mmap_pfn, 1);
     if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
-- 
1.7.10.4


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

Re: [PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[Ian Jackson]

Julien Grall writes ("[PATCH] libxc: Fix xc_mem_event.c compilation for ARM"):
> The commit 6ae2df9 "mem_access: Add helper API to setup ring and enable
> mem_access¨ break libxc compilation for ARM.
> 
> This is because xc_map_foreign_map and xc_domain_decrease_reservation_exact
> is taking an xen_pfn_t in parameters. On ARM, xen_pfn_t is always an uin64_t.

I have applied this, mostly because we don't want to have a broken
build for any length of time, but:

> -    unsigned long ring_pfn, mmap_pfn;
> +    unsigned long pfn;
> +    xen_pfn_t ring_pfn, mmap_pfn;
...
> -    rc1 = xc_get_hvm_param(xch, domain_id, param, &ring_pfn);
> +    rc1 = xc_get_hvm_param(xch, domain_id, param, &pfn);
...
> -    mmap_pfn = ring_pfn;
> +    ring_pfn = pfn;
> +    mmap_pfn = pfn;

I asked on IRC:

16:05 <@Diziet> julieng: If xen_pfn_t is uint64_t and on 32-bit ARM unsigned 
                long is uint32_t, how can the xc_get_hvm_param in 
                xc_mem_event_enable DTRT ?

I don't know whether that's a real problem.  (Related seems to be the
question of whether we support a 32-bit dom0 with 64-bit guests on ARM
and if not what stops the user from setting up such a thing.)

Thanks,
Ian.

Re: [PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[Ian Campbell]

On Mon, 2014-06-23 at 16:34 +0100, Ian Jackson wrote:
> Julien Grall writes ("[PATCH] libxc: Fix xc_mem_event.c compilation for ARM"):
> > The commit 6ae2df9 "mem_access: Add helper API to setup ring and enable
> > mem_access¨ break libxc compilation for ARM.
> > 
> > This is because xc_map_foreign_map and xc_domain_decrease_reservation_exact
> > is taking an xen_pfn_t in parameters. On ARM, xen_pfn_t is always an uin64_t.
> 
> I have applied this, mostly because we don't want to have a broken
> build for any length of time, but:
> 
> > -    unsigned long ring_pfn, mmap_pfn;
> > +    unsigned long pfn;
> > +    xen_pfn_t ring_pfn, mmap_pfn;
> ...
> > -    rc1 = xc_get_hvm_param(xch, domain_id, param, &ring_pfn);
> > +    rc1 = xc_get_hvm_param(xch, domain_id, param, &pfn);
> ...
> > -    mmap_pfn = ring_pfn;
> > +    ring_pfn = pfn;
> > +    mmap_pfn = pfn;
> 
> I asked on IRC:
> 
> 16:05 <@Diziet> julieng: If xen_pfn_t is uint64_t and on 32-bit ARM unsigned 
>                 long is uint32_t, how can the xc_get_hvm_param in 
>                 xc_mem_event_enable DTRT ?
> 
> I don't know whether that's a real problem.  (Related seems to be the
> question of whether we support a 32-bit dom0 with 64-bit guests on ARM
> and if not what stops the user from setting up such a thing.)

It's supported, although it's not clear that it is such a useful thing
to be doing on ARM (on x86 32-bit PAE was quite a nice trade off between
memory size vs performance due to the 64-bit syscall overhead, which
doesn't apply to ARM).

I'm _pretty_ sure that the only things which we pass as a pfn to HVM
params are the "magic pfns", which are at a hardcoded guest location
which needs to be under 4GB anyway so that 32-bit non-LPAE guests can
map them, so no danger of an actual overflow. We never see the actual
MFN.

FWIW I think the same issue with HVM parameters would impact 32-bit dom0
on x86 too, but I suppose the magic PFNs have the same property there
too. Luckily I think David's patches sorts it out at the root.

> 
> Thanks,
> Ian.



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

Re: [PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[Julien Grall]

On 06/24/2014 02:16 PM, Ian Campbell wrote:
> On Mon, 2014-06-23 at 16:34 +0100, Ian Jackson wrote:
> FWIW I think the same issue with HVM parameters would impact 32-bit dom0
> on x86 too, but I suppose the magic PFNs have the same property there
> too. Luckily I think David's patches sorts it out at the root.

On IRC, I pointed that the HVM param hypercall structure is using
uint64_t for the value rather than the helpers in libxc use unsigned long.

Even tho, it's not an issue for the magic PFN right now, a developer may
want to add a new HVM param which requires a 64 bit value.

Does David's series sort it out this problem?

-- 
Julien Grall

Re: [PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[David Vrabel]

On 24/06/14 14:46, Julien Grall wrote:
> On 06/24/2014 02:16 PM, Ian Campbell wrote:
>> On Mon, 2014-06-23 at 16:34 +0100, Ian Jackson wrote:
>> FWIW I think the same issue with HVM parameters would impact 32-bit dom0
>> on x86 too, but I suppose the magic PFNs have the same property there
>> too. Luckily I think David's patches sorts it out at the root.
> 
> On IRC, I pointed that the HVM param hypercall structure is using
> uint64_t for the value rather than the helpers in libxc use unsigned long.
> 
> Even tho, it's not an issue for the magic PFN right now, a developer may
> want to add a new HVM param which requires a 64 bit value.
> 
> Does David's series sort it out this problem?

Yes. It adds new xc_hvm_param_get() and xc_hvm_param_set() functions
that take uint64_t values and changes all callers (in tools/) to use them.

David

Re: [PATCH] libxc: Fix xc_mem_event.c compilation for ARM

[Aravindh Puthiyaparambil (aravindp)]

>The commit 6ae2df9 "mem_access: Add helper API to setup ring and enable
>mem_access¨ break libxc compilation for ARM.

Sorry about breaking the ARM build and taking so long to respond. I was on vacation and just got back today. I will make a habit of cross compiling ARM from here on before submitting patches.

Thanks,
Aravindh

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