From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Smith Subject: Re: [PATCH] Mini-OS to use evtchn_port_t for ports and other improvements Date: Tue, 25 Jul 2006 11:27:15 +0100 Message-ID: <20060725102715.GA4351@cam.ac.uk> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0348616103==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Mime-version: 1.0 Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "John D. Ramsdell" Cc: xen-devel@lists.xensource.com, Steven Smith , Grzegorz Milos , sos22@srcf.ucam.org List-Id: xen-devel@lists.xenproject.org --===============0348616103== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="W/nzBZO5zC0uMSeA" Content-Disposition: inline --W/nzBZO5zC0uMSeA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Sorry about the delay responding, I've been busy with other things. This mostly looks like a pretty reasonable bit of cleanup, with just a few minor niggles. > @@ -86,8 +86,7 @@ > ev_actions[port].data =3D NULL; > } > =20 > -int bind_virq( u32 virq, void (*handler)(int, struct pt_regs *, void *da= ta), > - void *data) > +int bind_virq(evtchn_port_t virq, evtchn_handler_t handler, void *data) Did you mean this? A virq isn't the same as an evtchn_port_t, and is usually uint32_t in the Xen header files. > { > evtchn_op_t op; > =20 > @@ -105,7 +104,7 @@ > return 0; > } > =20 > -void unbind_virq( u32 port ) > +void unbind_virq(evtchn_port_t port) > { > unbind_evtchn(port); > } Hmm... not your fault, but unbinding from virqs is broken. You need to know the port number, but there's no way of getting it from bind_virq. Given that noone in the tree currently uses either unbind_virq, I'd be inclined to just kill it. > +static inline evtchn_port_t > +maybe_bind_evtchn(evtchn_port_t port, evtchn_handler_t handler, void *da= ta) > +{ > + if (handler) > + return bind_evtchn(port, handler, data); > + else > + return port; > +} > + > +int evtchn_alloc_unbound(domid_t pal, evtchn_handler_t handler, > + void *data, evtchn_port_t *port) > +{ > + evtchn_op_t op; > + op.cmd =3D EVTCHNOP_alloc_unbound; > + op.u.alloc_unbound.dom =3D DOMID_SELF; > + op.u.alloc_unbound.remote_dom =3D pal; > + int err =3D HYPERVISOR_event_channel_op(&op); > + if (err) > + return err; > + *port =3D maybe_bind_evtchn(op.u.alloc_unbound.port, handler, data); Why maybe_bind? Do you ever expect to need to allocate an unbound event channel before you know what handler to use for it? > + return err; > +} > + > +/* Connect to a port so as to allow the exchange of notifications with > + the pal. Returns the result of the hypervisor call. */ > + > +int evtchn_bind_interdomain(domid_t pal, evtchn_port_t remote_port, > + evtchn_handler_t handler, void *data, > + evtchn_port_t *local_port) > +{ > + evtchn_op_t op; > + op.cmd =3D EVTCHNOP_bind_interdomain; > + op.u.bind_interdomain.remote_dom =3D pal; > + op.u.bind_interdomain.remote_port =3D remote_port; > + int err =3D HYPERVISOR_event_channel_op(&op); > + if (err) > + return err; > + evtchn_port_t port =3D op.u.bind_interdomain.local_port; > + clear_evtchn(port); /* Without, handler gets invoked now! */ Invoking the handler as soon as you bind the interdomain channel is a mostly-deliberate part of the interface. If the other end makes notifications before you get around to binding they can get lost, and forcing the channel to fire as soon as you bind to it avoids some potential lost wakeups. > + *local_port =3D maybe_bind_evtchn(port, handler, data); Again, why maybe_bind? > @@ -137,6 +137,24 @@ > return gref; > } > =20 > +static char *gnttabop_error_msgs[] =3D GNTTABOP_error_msgs; Maybe static const char *const gnttabop_error_msgs[] ? > + > +static const size_t gnttabop_error_size =3D > + sizeof(gnttabop_error_msgs)/sizeof(gnttabop_error_msgs[0]); Use ARRAY_SIZE here. Other than that, this looks pretty good to me. Steven. --W/nzBZO5zC0uMSeA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.3 (GNU/Linux) iD8DBQFExfIDO4S8/gLNrjcRAsLIAJ9+NWPlbWcyLwQI4veKzCafHh/A/ACdHgtX 8tjBbXSBVYa+Lwq+9sDqgOQ= =RFPN -----END PGP SIGNATURE----- --W/nzBZO5zC0uMSeA-- --===============0348616103== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --===============0348616103==--