On Sat, Sep 21, 2024 at 6:43 PM Nikita Shushura <m...@nikitashushura.com> wrote:
>
> Signed-off-by: Nikita Shushura <m...@nikitashushura.com>
Additionally to inlined comments:
- there are a few "extended (not supported)" you can now remove.
- I think the extended part in "grlib_irqmp_write" is still wrong,
the extended register being read-only.
> ---
> hw/intc/grlib_irqmp.c | 69 +++++++++++++++++++++++++++++++------------
> 1 file changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c
> index 37ac63fd80..e9414c054a 100644
> --- a/hw/intc/grlib_irqmp.c
> +++ b/hw/intc/grlib_irqmp.c
> @@ -1,8 +1,6 @@
> /*
> * QEMU GRLIB IRQMP Emulator
> *
> - * (Extended interrupt not supported)
> - *
> * SPDX-License-Identifier: MIT
> *
> * Copyright (c) 2010-2024 AdaCore
> @@ -38,25 +36,29 @@
> #include "qemu/module.h"
> #include "qom/object.h"
>
> -#define IRQMP_MAX_CPU 16
> -#define IRQMP_REG_SIZE 256 /* Size of memory mapped registers */
> +#define IRQMP_MAX_CPU (16)
> +#define IRQMP_REG_SIZE (256) /* Size of memory mapped registers */
>
> /* Memory mapped register offsets */
> -#define LEVEL_OFFSET 0x00
> -#define PENDING_OFFSET 0x04
> -#define FORCE0_OFFSET 0x08
> -#define CLEAR_OFFSET 0x0C
> -#define MP_STATUS_OFFSET 0x10
> -#define BROADCAST_OFFSET 0x14
> -#define MASK_OFFSET 0x40
> -#define FORCE_OFFSET 0x80
> -#define EXTENDED_OFFSET 0xC0
> +#define LEVEL_OFFSET (0x00)
> +#define PENDING_OFFSET (0x04)
> +#define FORCE0_OFFSET (0x08)
> +#define CLEAR_OFFSET (0x0C)
> +#define MP_STATUS_OFFSET (0x10)
> +#define BROADCAST_OFFSET (0x14)
> +#define MASK_OFFSET (0x40)
> +#define FORCE_OFFSET (0x80)
> +#define EXTENDED_OFFSET (0xC0)
>
> /* Multiprocessor Status Register */
> #define MP_STATUS_CPU_STATUS_MASK ((1 << IRQMP_MAX_CPU)-2)
> -#define MP_STATUS_NCPU_SHIFT 28
> +#define MP_STATUS_NCPU_SHIFT (28)
> +#define MP_STATUS_EIRQ_OFFSET (16)
> +
> +#define MAX_PILS_STD (16)
> +#define MAX_PILS_EXT (32)
>
> -#define MAX_PILS 16
> +#define DEFAULT_EIRQ (12)
>
> OBJECT_DECLARE_SIMPLE_TYPE(IRQMP, GRLIB_IRQMP)
>
> @@ -68,6 +70,7 @@ struct IRQMP {
> MemoryRegion iomem;
>
> unsigned int ncpus;
> + unsigned int eirq;
> IRQMPState *state;
> qemu_irq start_signal[IRQMP_MAX_CPU];
> qemu_irq irq[IRQMP_MAX_CPU];
> @@ -89,13 +92,26 @@ struct IRQMPState {
>
> static void grlib_irqmp_check_irqs(IRQMPState *state)
> {
> - int i;
> + int i, j;
>
> assert(state != NULL);
> assert(state->parent != NULL);
>
> for (i = 0; i < state->parent->ncpus; i++) {
> uint32_t pend = (state->pending | state->force[i]) & state->mask[i];
> +
> + /*
> + * Checks is pending interrupt extended.
s/is/if
> + * If so, sets pending to EIRQ and acknowledges
> + * extended interrupt
> + */
> + for (j = MAX_PILS_STD; j < MAX_PILS_EXT; j++) {
> + if ((pend & (1 << j)) != 0) {
> + pend = (1 << state->parent->eirq);
> + state->extended[i] = (j & 0xffff);
You're writing 1 bit too far. It should be ((j>>1) & 0xf).
Moreover, what's happening when two extended interrupts are both
pending ? Here, only the last one is taken into account
> + }
> + }
> +
> uint32_t level0 = pend & ~state->level;
> uint32_t level1 = pend & state->level;
>
> @@ -110,6 +126,10 @@ static void grlib_irqmp_check_irqs(IRQMPState *state)
> static void grlib_irqmp_ack_mask(IRQMPState *state, unsigned int cpu,
> uint32_t mask)
> {
> + if ((mask & (1 << state->parent->eirq)) != 0) {
> + mask |= (1 << state->extended[cpu]);
> + }
> +
> /* Clear registers */
> state->pending &= ~mask;
> state->force[cpu] &= ~mask;
> @@ -144,7 +164,6 @@ static void grlib_irqmp_set_irq(void *opaque, int irq,
> int level)
> assert(s != NULL);
> assert(s->parent != NULL);
>
> -
> if (level) {
> trace_grlib_irqmp_set_irq(irq);
>
> @@ -278,6 +297,9 @@ static void grlib_irqmp_write(void *opaque, hwaddr addr,
> state->mpstatus &= ~(1 << i);
> }
> }
> +
> + /* Writing EIRQ number */
> + state->mpstatus |= (state->parent->eirq << MP_STATUS_EIRQ_OFFSET);
> return;
>
> case BROADCAST_OFFSET:
> @@ -345,7 +367,8 @@ static void grlib_irqmp_reset(DeviceState *d)
> memset(irqmp->state, 0, sizeof *irqmp->state);
> irqmp->state->parent = irqmp;
> irqmp->state->mpstatus = ((irqmp->ncpus - 1) << MP_STATUS_NCPU_SHIFT) |
> - ((1 << irqmp->ncpus) - 2);
> + ((1 << irqmp->ncpus) - 2) |
> + (irqmp->eirq << MP_STATUS_EIRQ_OFFSET);
> }
>
> static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
> @@ -359,7 +382,14 @@ static void grlib_irqmp_realize(DeviceState *dev, Error
> **errp)
> return;
> }
>
> - qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS);
> + if ((!irqmp->eirq) || (irqmp->eirq >= MAX_PILS_STD)) {
> + error_setg(errp, "Invalid eirq properties: "
> + "%u, must be 0 < eirq < %u.", irqmp->eirq,
> + MAX_PILS_STD);
> + return;
> + }
> +
> + qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS_EXT);
>
> /*
> * Transitionning from 0 to 1 starts the CPUs. The opposite can't
> @@ -378,6 +408,7 @@ static void grlib_irqmp_realize(DeviceState *dev, Error
> **errp)
>
> static Property grlib_irqmp_properties[] = {
> DEFINE_PROP_UINT32("ncpus", IRQMP, ncpus, 1),
> + DEFINE_PROP_UINT32("eirq", IRQMP, eirq, DEFAULT_EIRQ),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> --
> 2.46.1
>
>
