[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v4 12/20] intc/arm_gic: Implement virtualization e
From: |
Peter Maydell |
Subject: |
Re: [Qemu-arm] [PATCH v4 12/20] intc/arm_gic: Implement virtualization extensions in gic_(deactivate|complete_irq) |
Date: |
Tue, 17 Jul 2018 14:32:57 +0100 |
On 14 July 2018 at 18:15, Luc Michel <address@hidden> wrote:
> Implement virtualization extensions in the gic_deactivate_irq() and
> gic_complete_irq() functions. When a guest tries to deactivat or end an
"deactivate"
> IRQ that does not exist in the LRs, the EOICount field of the virtual
> interface HCR register is incremented by one, and the request is
> ignored.
>
> Signed-off-by: Luc Michel <address@hidden>
> ---
> hw/intc/arm_gic.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index be9e2594d9..50cbbfbe24 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -590,6 +590,15 @@ static void gic_deactivate_irq(GICState *s, int cpu, int
> irq, MemTxAttrs attrs)
> return;
> }
>
> + if (gic_is_vcpu(cpu) && !gic_virq_is_valid(s, irq, cpu)) {
> + /* This vIRQ does not have an LR entry which is either active or
> + * pending and active. Increment EOICount and ignore the write.
> + */
> + int rcpu = gic_get_vcpu_real_id(cpu);
> + s->h_hcr[rcpu] += 1 << R_GICH_HCR_EOICount_SHIFT;
> + return;
> + }
It's a bit hard to tell from the amount of context in the diff,
but I think this check is being done too late in this function.
It needs to happen before we do any operations on the irq we're
passed (eg checking which group it is).
> +
> if (gic_cpu_ns_access(s, cpu, attrs) && !group) {
> DPRINTF("Non-secure DI for Group0 interrupt %d ignored\n", irq);
> return;
> @@ -604,6 +613,15 @@ static void gic_complete_irq(GICState *s, int cpu, int
> irq, MemTxAttrs attrs)
> int group;
>
> DPRINTF("EOI %d\n", irq);
> + if (gic_is_vcpu(cpu) && !gic_virq_is_valid(s, irq, cpu)) {
> + /* This vIRQ does not have an LR entry which is either active or
> + * pending and active. Increment EOICount and ignore the write.
> + */
> + int rcpu = gic_get_vcpu_real_id(cpu);
> + s->h_hcr[rcpu] += 1 << R_GICH_HCR_EOICount_SHIFT;
> + return;
> + }
This isn't consistent with the deactivate code about whether we
do this check before the "irq >= s->num_irq" check or after.
I think the correct answer is "before", because the number of
physical interrupts in the GIC shouldn't affect the valid
range of virtual interrupts.
There is also an edge case here: if the VIRQ written to the
EOI or DIR registers is a special interrupt number (1020..1023),
then should we increment the EOI count or not? The GICv2 spec
is not entirely clear on this point, but it does say in the
GICH_HCR.EOICount docs that "EOIs that do not clear a bit in
the Active Priorities register GICH_APR do not cause an increment",
and the GICv3 spec is clear so my recommendation is that for
1020..1023 we should ignore the write and not increment EOICount.
That bit about "EOIs that do not clear a bit in GICH_APR do
not increment EOICount" also suggests that our logic for DIR
and EOI needs to be something like:
if (vcpu) {
if (irq > 1020) {
return;
}
clear GICH_HCR bit;
if (no bit cleared) {
return;
}
if (!gic_virq_is_valid()) {
increment EOICount;
return;
}
clear active bit in LR;
}
?
> +
> if (irq >= s->num_irq) {
> /* This handles two cases:
> * 1. If software writes the ID of a spurious interrupt [ie 1023]
> --
> 2.18.0
>
thanks
-- PMM
- [Qemu-arm] [PATCH v4 00/20] arm_gic: add virtualization extensions support, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 04/20] vmstate.h: Provide VMSTATE_UINT16_SUB_ARRAY, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 06/20] intc/arm_gic: Add virtual interface register definitions, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 02/20] intc/arm_gic: Implement GICD_ISACTIVERn and GICD_ICACTIVERn registers, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 08/20] intc/arm_gic: Refactor secure/ns access check in the CPU interface, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 12/20] intc/arm_gic: Implement virtualization extensions in gic_(deactivate|complete_irq), Luc Michel, 2018/07/14
- Re: [Qemu-arm] [PATCH v4 12/20] intc/arm_gic: Implement virtualization extensions in gic_(deactivate|complete_irq),
Peter Maydell <=
- [Qemu-arm] [PATCH v4 03/20] intc/arm_gic: Remove some dead code and put some functions static, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 10/20] intc/arm_gic: Implement virtualization extensions in gic_(activate_irq|drop_prio), Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 13/20] intc/arm_gic: Implement virtualization extensions in gic_cpu_(read|write), Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 09/20] intc/arm_gic: Add virtualization enabled IRQ helper functions, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 11/20] intc/arm_gic: Implement virtualization extensions in gic_acknowledge_irq, Luc Michel, 2018/07/14
- [Qemu-arm] [PATCH v4 01/20] intc/arm_gic: Refactor operations on the distributor, Luc Michel, 2018/07/14