Hallo! On Mon, 9 May 2011 00:07:16 +0200, Samuel Thibault <samuel.thiba...@gnu.org> wrote: > I've started having a look at Zheng Da's user-level driver integration. > I've cleaned his tree a bit, and now considering adding patches to > the debian packages for wider testing.
Great! I'm not the most knowledgeable person to comment on these RPC interfaces, but anyway: > The patches however add a few > kernel RPCs, which we should probably agree on first, at the minimum > that their existence makes sense, so we can reserve slots in upstream > gnumach. Basically, it's about allocating physically-contiguous memory > for DMAs, and getting IRQ events: Of course, it doesn't matter at the moment, but are these for x86 only, or architecture independent? > * This routine is created for allocating DMA buffers. > * We are going to get a contiguous physical memory > * and its physical address in addition to the virtual address. > */ > routine vm_allocate_contiguous( > host_priv : host_priv_t; > target_task : vm_task_t; > out vaddr : vm_address_t; > out paddr : vm_address_t; > size : vm_size_t); Hmm, I guess we don't have anything that is better than using vm_address_t for physical addresses? At least not in include/mach/std_types.h, i386/include/mach/i386/vm_types.h. Should we? (phys_address_t based on natural_t?) At this time, would it make sense to make paddr inout (and/or vaddr, too?) and add an additional ``anywhere : boolean_t'' parameter as vm_allocate has? Or can't there be any need for such functionality? > /* Requesting IRQ events on a port */ > > routine device_intr_notify( > master_port : mach_port_t; > in irq : int; > in id : int; > in flags : int; > in receive_port : mach_port_send_t > ); Minor, but I would put the receive_port at the top of the ``in data'' list, after the master_port. Shouldn't all the other ``in data'' items be wrapped in an (architecured-dependent) structure similar to what we're doing for I/O ports? Compare io_perm_t in i386/include/mach/i386/mach_i386.defs, i386/include/mach/i386/mach_i386_types.h, i386/i386/io_perm.h. I believe this would help to separate implementation details (IRQ number being an integer; specific values of flags; etc.) from the RPC mechanism. Especially so if these are architecture independent calls. > The actual event: > > simpleroutine mach_notify_irq( > notify : notify_port_t; > name : int); I don't understand this ``name : int''. Isn't this rather a pointer to a struct mach_irq_notification_t? > And a way to mask/unmask irqs: > > /* > * enable/disable the specified irq. > */ > routine device_irq_enable( > master_port : mach_port_t; > irq : int; > status : char); > > Should this be rather split into device_irq_enable/disable and drop the > status paramter? What about using the I/O port scheme? That is, decide_intr_notify doesn't enable IRQ notifications, but instead just returns a handle (compare i386_io_perm_create) that is then passed to device_irq_enable to enable/disable IRQ notifications (compare i386_io_perm_modify). Does that make sense in this IRQ scenario? Grüße, Thomas
pgpKNGJJPeDT8.pgp
Description: PGP signature
