Re: [PATCH 2/2] add rpc_versions for vm types

[Flavio Cruz]

On Tue, Dec 06, 2022 at 02:10:52AM +0100, Samuel Thibault wrote:
> Hello Flavio,
>
> What do you think of that?

I think we could just define vm_size_t as typedef of __mach_uintptr_t 
then it will be unsigned int on 32 bits and unsigned long on 64 bits.

Regarding mach_msg_type_number_t, do you know if there is a need to copy 
more than 4GB using RPCs? I guess it could be useful, but servers can just
paginate the data. For now, we could just change some of the signatures to
use mach_msg_type_number_t instead of vm_size_t to make the interface
explicit about the limits. If we think there is a need to go beyond 4GB 
we can make mach_msg_type_number_t an unsigned long on pure 64 bits.

> Samuel
>
> Samuel Thibault, le lun. 29 août 2022 01:21:23 +0200, a ecrit:
> > Hello,
> >
> > Luca Dariz, le dim. 03 avril 2022 16:59:55 +0200, a ecrit:
> > > @@ -88,13 +94,36 @@ typedef unsigned long long rpc_phys_addr_t;
> > >   * expressing the difference between two
> > >   * vm_offset_t entities.
> > >   */
> > > -#ifdef __x86_64__
> > >  typedef   unsigned long   vm_size_t;
> > > -#else
> > > -typedef   natural_t       vm_size_t;
> > > -#endif
> > >  typedef   vm_size_t *     vm_size_array_t;
> >
> > Mmm, this has triggered a lot of warnings & errors about mismatches
> > between vm_size_t and mach_msg_type_number_t in userland hurd&glibc.
> >
> > While a lot of them are real mistakes (passing mach_msg_type_number_t*
> > for a vm_size_t* parameter), others are questioning, for instance:
> >
> > routine io_read (
> >         io_object: io_t;
> >         RPT
> >         out data: data_t, dealloc;
> >         offset: loff_t;
> >         amount: vm_size_t);
> >
> > amount is a vm_size_t, thus unsigned long, but the data_t array will
> > have a mach_msg_type_number_t size. On a 64bit userland, vm_size_t being
> > 64bit would be useless if mach_msg_type_number_t is 32bit. Perhaps we
> > want to make mach_msg_type_number_t long? Or use another type for
> > the size of data_t?
> >
> > That's tricky questions ahead. Perhaps we can for now use
> >
> > #if defined(KERNEL) || defined(__x86_64__)
> > typedef unsigned long   vm_size_t;
> > #else
> > typedef natural_t       vm_size_t;
> > #endif
> >
> > to avoid exposing the new type to userland for now, so as to have the
> > time to fix such questions before exposing the switch?
> >
> > Samuel