Re: [PATCH v6 06/15] emul/ns16x50: implement IER/IIR registers

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 5 Sep 2025, dmukhin@xxxxxxx wrote:
> +/*
> + * Get the interrupt identity reason.
> + *
> + * IIR is re-calculated once called, because ns16x50 always reports high
> + * priority events first.
> + * regs[NS16X50_REGS_NUM + UART_IIR] is used to store THR reason only.
> + */
> +static uint8_t ns16x50_iir_get(const struct vuart_ns16x50 *vdev)
> +{
> +    /*
> +     * Interrupt identity reasons by priority.
> +     * NB: high priority are at lower indexes below.
> +     */
> +    static const struct {
> +        bool (*check)(const struct vuart_ns16x50 *vdev);
> +        uint8_t ier;
> +        uint8_t iir;
> +    } iir_by_prio[] = {
> +        [0] = { ns16x50_iir_check_lsi, UART_IER_ELSI,   UART_IIR_LSI },
> +        [1] = { ns16x50_iir_check_rda, UART_IER_ERDAI,  UART_IIR_RDA },
> +        [2] = { ns16x50_iir_check_thr, UART_IER_ETHREI, UART_IIR_THR },
> +        [3] = { ns16x50_iir_check_msi, UART_IER_EMSI,   UART_IIR_MSI },
> +    };
> +    const uint8_t *regs = vdev->regs;
> +    uint8_t iir = 0;
> +    unsigned int i;
> +
> +    /*
> +     * NB: every interaction w/ ns16x50 registers (except DLAB=1) goes
> +     * through that call.
> +     */
> +    ASSERT(spin_is_locked(&vdev->lock));
> +
> +    for ( i = 0; i < ARRAY_SIZE(iir_by_prio); i++ )
> +    {
> +        if ( (regs[UART_IER] & iir_by_prio[i].ier) &&
> +             iir_by_prio[i].check(vdev) )
> +            break;
> +
> +    }
> +    if ( i == ARRAY_SIZE(iir_by_prio) )
> +        iir |= UART_IIR_NOINT;
> +    else
> +        iir |= iir_by_prio[i].iir;
> +
> +    if ( regs[UART_FCR] & UART_FCR_ENABLE )
> +        iir |= UART_IIR_FE;
> +
> +    return iir;
> +}
> +
> +static void ns16x50_irq_assert(const struct vuart_ns16x50 *vdev)
> +{
> +    struct domain *d = vdev->owner;
> +    const struct vuart_info *info = vdev->info;
> +    int vector;
> +
> +    if ( has_vpic(d) )
> +        vector = hvm_isa_irq_assert(d, info->irq, vioapic_get_vector);
> +    else
> +        ASSERT_UNREACHABLE();

This seems dangerous as there are guests without vpic


> +    ns16x50_debug(vdev, "IRQ#%d vector %d assert\n", info->irq, vector);
> +}
> +
> +static void ns16x50_irq_deassert(const struct vuart_ns16x50 *vdev)
> +{
> +    struct domain *d = vdev->owner;
> +    const struct vuart_info *info = vdev->info;
> +
> +    if ( has_vpic(d) )
> +        hvm_isa_irq_deassert(d, info->irq);
> +    else
> +        ASSERT_UNREACHABLE();

also here


> +    ns16x50_debug(vdev, "IRQ#%d deassert\n", info->irq);
> +}