|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8 05/27] ARM: GICv3: forward pending LPIs to guests
Hi,
On 12/04/17 11:44, Julien Grall wrote:
> Hi Andre,
>
> On 12/04/17 01:44, Andre Przywara wrote:
>> Upon receiving an LPI on the host, we need to find the right VCPU and
>> virtual IRQ number to get this IRQ injected.
>> Iterate our two-level LPI table to find this information quickly when
>> the host takes an LPI. Call the existing injection function to let the
>> GIC emulation deal with this interrupt.
>> Also we enhance struct pending_irq to cache the pending bit and the
>> priority information for LPIs. Reading the information from there is
>> faster than accessing the property table from guest memory. Also it
>> use some padding area, so does not require more memory.
>> This introduces a do_LPI() as a hardware gic_ops and a function to
>> retrieve the (cached) priority value of an LPI and a vgic_ops.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ---
>> xen/arch/arm/gic-v2.c | 7 ++++
>> xen/arch/arm/gic-v3-lpi.c | 71
>> ++++++++++++++++++++++++++++++++++++++++
>> xen/arch/arm/gic-v3.c | 1 +
>> xen/arch/arm/gic.c | 8 ++++-
>> xen/arch/arm/vgic-v2.c | 7 ++++
>> xen/arch/arm/vgic-v3.c | 12 +++++++
>> xen/arch/arm/vgic.c | 7 +++-
>> xen/include/asm-arm/domain.h | 3 +-
>> xen/include/asm-arm/gic.h | 2 ++
>> xen/include/asm-arm/gic_v3_its.h | 8 +++++
>> xen/include/asm-arm/vgic.h | 2 ++
>> 11 files changed, 125 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
>> index 270a136..ffbe47c 100644
>> --- a/xen/arch/arm/gic-v2.c
>> +++ b/xen/arch/arm/gic-v2.c
>> @@ -1217,6 +1217,12 @@ static int __init gicv2_init(void)
>> return 0;
>> }
>>
>> +static void gicv2_do_LPI(unsigned int lpi)
>> +{
>> + /* No LPIs in a GICv2 */
>> + BUG();
>> +}
>> +
>> const static struct gic_hw_operations gicv2_ops = {
>> .info = &gicv2_info,
>> .init = gicv2_init,
>> @@ -1244,6 +1250,7 @@ const static struct gic_hw_operations gicv2_ops = {
>> .make_hwdom_madt = gicv2_make_hwdom_madt,
>> .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings,
>> .iomem_deny_access = gicv2_iomem_deny_access,
>> + .do_LPI = gicv2_do_LPI,
>> };
>>
>> /* Set up the GIC */
>> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
>> index 292f2d0..44f6315 100644
>> --- a/xen/arch/arm/gic-v3-lpi.c
>> +++ b/xen/arch/arm/gic-v3-lpi.c
>> @@ -136,6 +136,77 @@ uint64_t gicv3_get_redist_address(unsigned int
>> cpu, bool use_pta)
>> return per_cpu(lpi_redist, cpu).redist_id << 16;
>> }
>>
>> +/*
>> + * Handle incoming LPIs, which are a bit special, because they are
>> potentially
>> + * numerous and also only get injected into guests. Treat them
>> specially here,
>> + * by just looking up their target vCPU and virtual LPI number and
>> hand it
>> + * over to the injection function.
>> + * Please note that LPIs are edge-triggered only, also have no active
>> state,
>> + * so spurious interrupts on the host side are no issue (we can just
>> ignore
>> + * them).
>> + * Also a guest cannot expect that firing interrupts that haven't been
>> + * fully configured yet will reach the CPU, so we don't need to care
>> about
>> + * this special case.
>> + */
>> +void gicv3_do_LPI(unsigned int lpi)
>> +{
>> + struct domain *d;
>> + union host_lpi *hlpip, hlpi;
>> + struct vcpu *vcpu;
>> +
>> + irq_enter();
>> +
>> + /* EOI the LPI already. */
>> + WRITE_SYSREG32(lpi, ICC_EOIR1_EL1);
>> +
>> + /* Find out if a guest mapped something to this physical LPI. */
>> + hlpip = gic_get_host_lpi(lpi);
>> + if ( !hlpip )
>> + goto out;
>> +
>> + hlpi.data = read_u64_atomic(&hlpip->data);
>> +
>> + /*
>> + * Unmapped events are marked with an invalid LPI ID. We can safely
>> + * ignore them, as they have no further state and no-one can expect
>> + * to see them if they have not been mapped.
>> + */
>> + if ( hlpi.virt_lpi == INVALID_LPI )
>> + goto out;
>> +
>> + d = rcu_lock_domain_by_id(hlpi.dom_id);
>> + if ( !d )
>> + goto out;
>> +
>> + /* Make sure we don't step beyond the vcpu array. */
>> + if ( hlpi.vcpu_id >= d->max_vcpus )
>> + {
>> + rcu_unlock_domain(d);
>> + goto out;
>> + }
>> +
>> + vcpu = d->vcpu[hlpi.vcpu_id];
>> +
>> + /* Check if the VCPU is ready to receive LPIs. */
>> + if ( vcpu->arch.vgic.flags & VGIC_V3_LPIS_ENABLED )
>> + /*
>> + * TODO: Investigate what to do here for potential interrupt
>> storms.
>> + * As we keep all host LPIs enabled, for disabling LPIs we
>> would need
>> + * to queue a ITS host command, which we avoid so far during
>> a guest's
>> + * runtime. Also re-enabling would trigger a host command
>> upon the
>> + * guest sending a command, which could be an attack vector for
>> + * hogging the host command queue.
>> + * See the thread around here for some background:
>> + *
>> https://lists.xen.org/archives/html/xen-devel/2016-12/msg00003.html
>> + */
>
> This TODO should have been listed in the cover letter.
Done.
>> + vgic_vcpu_inject_irq(vcpu, hlpi.virt_lpi);
>> +
>> + rcu_unlock_domain(d);
>> +
>> +out:
>> + irq_exit();
>> +}
>> +
>> static int gicv3_lpi_allocate_pendtable(uint64_t *reg)
>> {
>> uint64_t val;
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 29c8964..8140c5f 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1674,6 +1674,7 @@ static const struct gic_hw_operations gicv3_ops = {
>> .make_hwdom_dt_node = gicv3_make_hwdom_dt_node,
>> .make_hwdom_madt = gicv3_make_hwdom_madt,
>> .iomem_deny_access = gicv3_iomem_deny_access,
>> + .do_LPI = gicv3_do_LPI,
>> };
>>
>> static int __init gicv3_dt_preinit(struct dt_device_node *node, const
>> void *data)
>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>> index 62ae3b8..d752352 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -735,7 +735,13 @@ void gic_interrupt(struct cpu_user_regs *regs,
>> int is_fiq)
>> do_IRQ(regs, irq, is_fiq);
>> local_irq_disable();
>> }
>> - else if (unlikely(irq < 16))
>> + else if ( is_lpi(irq) )
>> + {
>> + local_irq_enable();
>> + gic_hw_ops->do_LPI(irq);
>> + local_irq_disable();
>> + }
>> + else if ( unlikely(irq < 16) )
>> {
>> do_sgi(regs, irq);
>> }
>> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
>> index 0587569..df91940 100644
>> --- a/xen/arch/arm/vgic-v2.c
>> +++ b/xen/arch/arm/vgic-v2.c
>> @@ -709,11 +709,18 @@ static struct pending_irq
>> *vgic_v2_lpi_to_pending(struct domain *d,
>> BUG();
>> }
>>
>> +static int vgic_v2_lpi_get_priority(struct domain *d, unsigned int vlpi)
>> +{
>> + /* Dummy function, no LPIs on a VGICv2. */
>> + BUG();
>> +}
>> +
>> static const struct vgic_ops vgic_v2_ops = {
>> .vcpu_init = vgic_v2_vcpu_init,
>> .domain_init = vgic_v2_domain_init,
>> .domain_free = vgic_v2_domain_free,
>> .lpi_to_pending = vgic_v2_lpi_to_pending,
>> + .lpi_get_priority = vgic_v2_lpi_get_priority,
>> .max_vcpus = 8,
>> };
>>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index f462610..c059dbd 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -1558,12 +1558,24 @@ static struct pending_irq
>> *vgic_v3_lpi_to_pending(struct domain *d,
>> return pirq;
>> }
>>
>> +/* Retrieve the priority of an LPI from its struct pending_irq. */
>> +static int vgic_v3_lpi_get_priority(struct domain *d, uint32_t vlpi)
>> +{
>> + struct pending_irq *p = vgic_v3_lpi_to_pending(d, vlpi);
>> +
>> + if ( !p )
>> + return GIC_PRI_IRQ;
>
> Why the check here? And why returning GIC_PRI_IRQ?
>
> AFAICT, vgic_v3_lpi_to_pending should never return NULL when reading the
> priority. Or else, you are in big trouble.
Now (with one change I made after your comment on 02/27) we call
vgic_get_virq_priority() as the very first thing in
vgic_vcpu_inject_irq(). So lpi_to_pending() could very well returning
NULL, which we would handle properly later.
As we can't move the vgic_get_virq_priority() call due to the locking
order rules, we just deal with this, returning *some* value here.
Shall I make the return value zero or 0xff in this case?
Cheers,
Andre.
>> +
>> + return p->lpi_priority;
>> +}
>> +
>> static const struct vgic_ops v3_ops = {
>> .vcpu_init = vgic_v3_vcpu_init,
>> .domain_init = vgic_v3_domain_init,
>> .domain_free = vgic_v3_domain_free,
>> .emulate_reg = vgic_v3_emulate_reg,
>> .lpi_to_pending = vgic_v3_lpi_to_pending,
>> + .lpi_get_priority = vgic_v3_lpi_get_priority,
>> /*
>> * We use both AFF1 and AFF0 in (v)MPIDR. Thus, the max number of
>> CPU
>> * that can be supported is up to 4096(==256*16) in theory.
>> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
>> index c2bfdb1..b6fe34f 100644
>> --- a/xen/arch/arm/vgic.c
>> +++ b/xen/arch/arm/vgic.c
>> @@ -226,10 +226,15 @@ struct vcpu *vgic_get_target_vcpu(struct vcpu
>> *v, unsigned int virq)
>>
>> static int vgic_get_virq_priority(struct vcpu *v, unsigned int virq)
>> {
>> - struct vgic_irq_rank *rank = vgic_rank_irq(v, virq);
>> + struct vgic_irq_rank *rank;
>> unsigned long flags;
>> int priority;
>>
>> + /* LPIs don't have a rank, also store their priority separately. */
>> + if ( is_lpi(virq) )
>> + return
>> v->domain->arch.vgic.handler->lpi_get_priority(v->domain, virq);
>> +
>> + rank = vgic_rank_irq(v, virq);
>> vgic_lock_rank(v, rank, flags);
>> priority = rank->priority[virq & INTERRUPT_RANK_MASK];
>> vgic_unlock_rank(v, rank, flags);
>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>> index 3d8e84c..ebaea35 100644
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -260,7 +260,8 @@ struct arch_vcpu
>>
>> /* GICv3: redistributor base and flags for this vCPU */
>> paddr_t rdist_base;
>> -#define VGIC_V3_RDIST_LAST (1 << 0) /* last vCPU of the rdist */
>> +#define VGIC_V3_RDIST_LAST (1 << 0) /* last vCPU of the
>> rdist */
>> +#define VGIC_V3_LPIS_ENABLED (1 << 1)
>> uint8_t flags;
>> } vgic;
>>
>> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
>> index 836a103..42963c0 100644
>> --- a/xen/include/asm-arm/gic.h
>> +++ b/xen/include/asm-arm/gic.h
>> @@ -366,6 +366,8 @@ struct gic_hw_operations {
>> int (*map_hwdom_extra_mappings)(struct domain *d);
>> /* Deny access to GIC regions */
>> int (*iomem_deny_access)(const struct domain *d);
>> + /* Handle LPIs, which require special handling */
>> + void (*do_LPI)(unsigned int lpi);
>> };
>>
>> void register_gic_ops(const struct gic_hw_operations *ops);
>> diff --git a/xen/include/asm-arm/gic_v3_its.h
>> b/xen/include/asm-arm/gic_v3_its.h
>> index 29559a3..7470779 100644
>> --- a/xen/include/asm-arm/gic_v3_its.h
>> +++ b/xen/include/asm-arm/gic_v3_its.h
>> @@ -134,6 +134,8 @@ void gicv3_its_dt_init(const struct dt_device_node
>> *node);
>>
>> bool gicv3_its_host_has_its(void);
>>
>> +void gicv3_do_LPI(unsigned int lpi);
>> +
>> int gicv3_lpi_init_rdist(void __iomem * rdist_base);
>>
>> /* Initialize the host structures for LPIs and the host ITSes. */
>> @@ -175,6 +177,12 @@ static inline bool gicv3_its_host_has_its(void)
>> return false;
>> }
>>
>> +static inline void gicv3_do_LPI(unsigned int lpi)
>> +{
>> + /* We don't enable LPIs without an ITS. */
>> + BUG();
>> +}
>> +
>> static inline int gicv3_lpi_init_rdist(void __iomem * rdist_base)
>> {
>> return -ENODEV;
>> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
>> index c9075a9..7efa164 100644
>> --- a/xen/include/asm-arm/vgic.h
>> +++ b/xen/include/asm-arm/vgic.h
>> @@ -72,6 +72,7 @@ struct pending_irq
>> #define GIC_INVALID_LR (uint8_t)~0
>> uint8_t lr;
>> uint8_t priority;
>> + uint8_t lpi_priority; /* Caches the priority if this is an
>> LPI. */
>> /* inflight is used to append instances of pending_irq to
>> * vgic.inflight_irqs */
>> struct list_head inflight;
>> @@ -136,6 +137,7 @@ struct vgic_ops {
>> bool (*emulate_reg)(struct cpu_user_regs *regs, union hsr hsr);
>> /* lookup the struct pending_irq for a given LPI interrupt */
>> struct pending_irq *(*lpi_to_pending)(struct domain *d, unsigned
>> int vlpi);
>> + int (*lpi_get_priority)(struct domain *d, uint32_t vlpi);
>> /* Maximum number of vCPU supported */
>> const unsigned int max_vcpus;
>> };
>>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |