[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 07.08.12 at 14:08, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 
>>> wrote:
> On Mon, 6 Aug 2012, Jan Beulich wrote:
>> >>> On 06.08.12 at 16:12, Stefano Stabellini 
>> >>> <stefano.stabellini@xxxxxxxxxxxxx>  wrote:
>> > --- 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;
>> >...
> 
> This change is actually not required, considering that ARM doesn't have
> an APIC. I changed apic_physbase to xen_ulong_t only for consistency,
> but it wouldn't make any difference for ARM (or x86).
> If you think that it is better to keep it unsigned long, I'll remove
> this chunk for the patch.

I'd prefer that. Also any others that you may not actually need
to be converted.

Also, seems I was misremembering how PPC dealt with this - they
indeed had xen_ulong_t defined to be a 64-bit value too.

> Considering that each field of a multicall_entry is usually passed as an
> hypercall parameter, they should all remain unsigned long.

That'll give you subtle bugs I'm afraid: do_memory_op()'s
encoding of a continuation start extent (into the 'cmd' value),
for example, depends on being able to store the full value into
the command field of the multicall structure. The limit checking
of the permitted number of extents therefore is different
between native (ULONG_MAX >> MEMOP_EXTENT_SHIFT) and
compat (UINT_MAX >> MEMOP_EXTENT_SHIFT). I would
neither find it very appealing to have do_memory_op() adjusted
for dealing with this new special case, nor am I sure that's the
only place your approach would cause problems if you excluded
the multicall structure from the model change.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.