[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 06/20] hw/arm/smmuv3: Wired IRQ and GERROR he
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v7 06/20] hw/arm/smmuv3: Wired IRQ and GERROR helpers |
Date: |
Mon, 9 Oct 2017 18:01:49 +0100 |
On 1 September 2017 at 18:21, Eric Auger <address@hidden> wrote:
> We introduce some helpers to handle wired IRQs and especially
> GERROR interrupt. SMMU writes GERROR register on GERROR event
> and SW acks GERROR interrupts by setting GERRORn.
>
> The Wired interrupts are edge sensitive.
>
> Signed-off-by: Eric Auger <address@hidden>
>
> ---
>
> Is CMD_SYNC interrupt enabled somewhere?
> ---
> hw/arm/smmuv3-internal.h | 20 ++++++++++++++++++
> hw/arm/smmuv3.c | 55
> ++++++++++++++++++++++++++++++++++++++++++++++++
> hw/arm/trace-events | 2 ++
> 3 files changed, 77 insertions(+)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 488acc8..2b44ee2 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -198,4 +198,24 @@ static inline int smmu_enabled(SMMUV3State *s)
> return smmu_read32_reg(s, SMMU_REG_CR0) & SMMU_CR0_SMMU_ENABLE;
> }
>
> +/*****************************
> + * Interrupts
> + *****************************/
> +
> +#define smmu_evt_irq_enabled(s) \
> + (smmu_read64_reg(s, SMMU_REG_IRQ_CTRL) & SMMU_IRQ_CTRL_EVENT_EN)
> +#define smmu_gerror_irq_enabled(s) \
> + (smmu_read64_reg(s, SMMU_REG_IRQ_CTRL) & SMMU_IRQ_CTRL_GERROR_EN)
> +#define smmu_pri_irq_enabled(s) \
> + (smmu_read64_reg(s, SMMU_REG_IRQ_CTRL) & SMMU_IRQ_CTRL_PRI_EN)
> +
> +#define SMMU_PENDING_GERRORS(s) \
> + (smmu_read32_reg(s, SMMU_REG_GERROR) ^ \
> + smmu_read32_reg(s, SMMU_REG_GERRORN))
Hiding this XOR inside a macro is very confusing.
> +
> +#define SMMU_CMDQ_ERR(s) (SMMU_PENDING_GERRORS(s) & SMMU_GERROR_CMDQ)
> +
> +void smmuv3_irq_trigger(SMMUV3State *s, SMMUIrq irq, uint32_t gerror_val);
> +void smmuv3_write_gerrorn(SMMUV3State *s, uint32_t gerrorn);
> +
> #endif
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 0a7cd1c..468134f 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -29,6 +29,61 @@
> #include "hw/arm/smmuv3.h"
> #include "smmuv3-internal.h"
>
> +/**
> + * smmuv3_irq_trigger - pulse @irq if enabled and update
> + * GERROR register in case of GERROR interrupt
> + *
> + * @irq: irq type
> + * @gerror: gerror new value, only relevant if @irq is GERROR
> + */
> +void smmuv3_irq_trigger(SMMUV3State *s, SMMUIrq irq, uint32_t gerror_val)
> +{
> + uint32_t pending_gerrors = SMMU_PENDING_GERRORS(s);
> + bool pulse = false;
> +
> + switch (irq) {
> + case SMMU_IRQ_EVTQ:
> + pulse = smmu_evt_irq_enabled(s);
> + break;
> + case SMMU_IRQ_PRIQ:
> + pulse = smmu_pri_irq_enabled(s);
> + break;
> + case SMMU_IRQ_CMD_SYNC:
> + pulse = true;
> + break;
> + case SMMU_IRQ_GERROR:
> + {
> + /* don't toggle an already pending error */
> + bool new_gerrors = ~pending_gerrors & gerror_val;
> + uint32_t gerror = smmu_read32_reg(s, SMMU_REG_GERROR);
> +
> + smmu_write32_reg(s, SMMU_REG_GERROR, gerror | new_gerrors);
The SMMU toggles GERROR bits to indicate that they have been
set, it doesn't just set them to 1.
> +
> + /* pulse the GERROR irq only if all fields were acked */
> + pulse = smmu_gerror_irq_enabled(s) && !pending_gerrors;
> + break;
> + }
> + }
> + if (pulse) {
> + trace_smmuv3_irq_trigger(irq,
> + smmu_read32_reg(s, SMMU_REG_GERROR),
> + SMMU_PENDING_GERRORS(s));
> + qemu_irq_pulse(s->irq[irq]);
qemu_irq_pulse() is very rarely the right thing for an interrupt
line, but it looks like it's right here (per spec 3.18.2).
> + }
> +}
> +
> +void smmuv3_write_gerrorn(SMMUV3State *s, uint32_t gerrorn)
> +{
> + uint32_t pending_gerrors = SMMU_PENDING_GERRORS(s);
> + uint32_t sanitized;
> +
> + /* Make sure SW does not toggle irqs that are not active */
If it does, this is CONSTRAINED UNPREDICTABLE. That is worth
remarking in either a comment or a LOG_GUEST_ERROR warning.
> + sanitized = gerrorn & pending_gerrors;
This isn't right -- if you want to sanitize the result
being written then you need to make it only change bits
that are in pending_errors, but this expression allows
the guest to write a 0 to a field that is for a non-pending
error. (Or you could just not sanitize at all, that's allowed
by the CONSTRAINED UNPREDICTABLE.)
> +
> + smmu_write32_reg(s, SMMU_REG_GERRORN, sanitized);
> + trace_smmuv3_write_gerrorn(gerrorn, sanitized, SMMU_PENDING_GERRORS(s));
> +}
> +
> static void smmuv3_init_regs(SMMUV3State *s)
> {
> uint32_t data =
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 8affbf7..c1ce8eb 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -17,3 +17,5 @@ smmu_set_translated_address(hwaddr iova, hwaddr pa) "iova =
> 0x%"PRIx64" -> pa =
>
> #hw/arm/smmuv3.c
> smmuv3_read_mmio(hwaddr addr, uint64_t val, unsigned size) "addr:
> 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x"
> +smmuv3_irq_trigger(int irq, uint32_t gerror, uint32_t pending) "irq=%d
> gerror=0x%x pending gerrors=0x%x"
> +smmuv3_write_gerrorn(uint32_t gerrorn, uint32_t sanitized, uint32_t pending)
> "gerrorn=0x%x sanitized=0x%x pending=0x%x"
> --
> 2.5.5
thanks
-- PMM
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v7 06/20] hw/arm/smmuv3: Wired IRQ and GERROR helpers,
Peter Maydell <=