Re: [Xen-devel] [PATCH 3/5] xen: few more xen_ulong_t substitutions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.08.12 at 16:12, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 
>>> wrote:
> There are still few unsigned long in the xen public interface: replace
> them with xen_ulong_t.
> 
> Also typedef xen_ulong_t to uint64_t on ARM.

While this change by itself already looks suspicious to me, I don't
follow what the global replacement is going to be good for, in
particular when done in places that ARM should be completely
ignorant of, e.g.

> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -124,7 +124,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
>  #define PHYSDEVOP_apic_write             9
>  struct physdev_apic {
>      /* IN */
> -    unsigned long apic_physbase;
> +    xen_ulong_t apic_physbase;
>      uint32_t reg;
>      /* IN or OUT */
>      uint32_t value;
>...
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -518,8 +518,8 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
>   * NB. The fields are natural register size for this architecture.
>   */
>  struct multicall_entry {
> -    unsigned long op, result;
> -    unsigned long args[6];
> +    xen_ulong_t op, result;
> +    xen_ulong_t args[6];

And here I really start to wonder - what use is it to put all 64-bit
values here on a 32-bit arch? You certainly know a lot more about
ARM than me, but this looks pretty inefficient, the more that
you'll have to deal with checking the full values when converting
to e.g. pointers anyway, in order to avoid behavioral differences
between running on a 32- or 64-bit host. Zero-extending from
32-bits when in a 64-bit hypervisor wouldn't have this problem.

Jan

>  };
>  typedef struct multicall_entry multicall_entry_t;
>  DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);



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