Damien Zammit, le sam. 02 mars 2024 10:31:50 +0000, a ecrit:
> +static error_t
> +get_acpi(void)
> +{
> + error_t err = 0;
> + mach_port_t tryacpi, device_master;
> +
> + acpidev = MACH_PORT_NULL;
This looks odd. If we had a previous port in acpidev, we want to
deallocate it, not forget it.
> + err = get_privileged_ports (0, &device_master);
> + if (!err)
> + {
> + err = device_open (device_master, D_READ, "acpi", &tryacpi);
> + mach_port_deallocate (mach_task_self (), device_master);
> + if (!err)
> + {
> + acpidev = tryacpi;
> + return 0;
> + }
> + }
> +
> + tryacpi = file_name_lookup (_SERVERS_ACPI, O_RDONLY, 0);
> + if (tryacpi == MACH_PORT_NULL)
> + return ENODEV;
> +
> + acpidev = tryacpi;
> + return 0;
> +}
> +
> +static error_t
> +get_irq(void)
> +{
> + error_t err = 0;
> + mach_port_t tryirq, device_master;
> +
> + irqdev = MACH_PORT_NULL;
Same here.
> + err = get_privileged_ports (0, &device_master);
> + if (err)
> + return err;
> +
> + err = device_open (device_master, D_READ|D_WRITE, "irq", &tryirq);
> + mach_port_deallocate (mach_task_self (), device_master);
> + if (err)
> + return err;
> +
> + irqdev = tryirq;
> + return err;
> +}
> +
> +error_t
> +irqhelp_init(void)
> +{
> + static bool inited = false;
> + error_t err;
> +
> + if (inited)
> + {
> + log_error("already inited\n");
> + return 0;
But we already have a guard against several initializations. So I don't
see the point of setting the ports to MACH_PORT_NULL? Or else you meant
to statically initalize them to MACH_PORT_NULL? (which will be much more
efficient).
> + }
> +
> + err = get_irq();
> + if (err)
> + {
> + log_error("cannot grab irq device\n");
> + return err;
> + }
> +
> + err = get_acpi();
> + if (err)
> + {
> + log_error("cannot grab acpi device\n");
> + return err;
> + }
> +
> + inited = true;
> + return 0;
> +}
> +
> +void *
> +irqhelp_server_loop(void *arg)
> +{
> + struct irq *irq = (struct irq *)arg;
> + mach_port_t master_host;
> + mach_port_t pset, psetcntl;
> + error_t err;
> +
> + if (!irq)
> + {
> + log_error("cannot start this irq thread\n");
> + return NULL;
> + }
> +
> + err = get_privileged_ports (&master_host, 0);
> + if (err)
> + {
> + log_error("cannot get master_host port\n");
> + return NULL;
> + }
> +
> + err = thread_get_assignment (mach_thread_self (), &pset);
> + if (err)
> + goto fail;
> +
> + err = host_processor_set_priv (master_host, pset, &psetcntl);
> + if (err)
> + goto fail;
> +
> + thread_max_priority (mach_thread_self (), psetcntl, 0);
> + err = thread_priority (mach_thread_self (), IRQ_THREAD_PRIORITY, 0);
> + if (err)
> + goto fail;
> +
> + mach_port_deallocate (mach_task_self (), master_host);
> +
> + int interrupt_demuxer (mach_msg_header_t *inp,
> + mach_msg_header_t *outp)
> + {
> + static bool printed0 = false;
> + static bool printed1 = false;
Better put them in their respective blocks.
> + device_intr_notification_t *n = (device_intr_notification_t *) inp;
> +
> + ((mig_reply_header_t *) outp)->RetCode = MIG_NO_REPLY;
> + if (n->intr_header.msgh_id != DEVICE_INTR_NOTIFY)
> + {
> + if (!printed0)
> + {
> + log_error("msg received is not an interrupt\n");
> + printed0 = true;
> + }
> + return 0;
> + }
> +
> + /* FIXME: id <-> gsi now has an indirection, assuming 1:1 */
> + if (n->id != irq->gsi)
> + {
> + if (!printed1)
> + {
> + log_error("interrupt expected on irq %d arrived on irq %d\n",
> irq->gsi, n->id);
> + printed1 = true;
> + }
> + return 0;
> + }
> +
> + /* wait if irq disabled */
> + pthread_mutex_lock (&irq->irqlock);
> + while (!irq->enabled)
> + pthread_cond_wait (&irq->irqcond, &irq->irqlock);
> + pthread_mutex_unlock (&irq->irqlock);
> +
> + /* call handler */
> + irq->handler(irq->context);
You probably want to add an if (!irq->shutdown) before this? Otherwise
after the application called irqhelp_remove_interrupt_handler, the
handler will still be called once.
> + /* ACK interrupt */
> + device_intr_ack (irqdev, irq->port, MACH_MSG_TYPE_MAKE_SEND);
This might be spurious as well in the irq->shutdown case?
> + if (irq->shutdown)
> + mach_port_deallocate (mach_task_self (), irq->port);
> +
> + return 1;
> + }
> +
> + /* Server loop */
> + mach_msg_server (interrupt_demuxer, 0, irq->port);
> + goto done;
> +
> +fail:
> + log_error("failed to register irq %d\n", irq->gsi);
> +
> +done:
> + pthread_cond_destroy(&irq->irqcond);
> + pthread_mutex_destroy(&irq->irqlock);
> + free(irq);
> + return NULL;
> +}
