[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] xen/arm: vgic to ignore GICD ICPENDRn registers access


  • To: Hongda Deng <Hongda.Deng@xxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 20 Oct 2021 13:24:07 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=lMnlSk7ryK/CuINV4bvRZ+yHVMIhjzh1KukZkqwxgg0=; b=l85m5bUe6FE7mE2CpJZmw+Yq/uE2bW21OjliZfqWXTNNsY43rydgAnfV2lfFfvehbWBYdWLeqRUfACGGmjKZDAxVe0gt2YFNGV9MtAEh0y0xatcNk9LRUqpdXA21HnXyicVAn2EbpsMJahUbIqGPEf9iCTddkIcFmEj64ci7VnMrIlK6BHClKAlmT2ZbbIJt7ZnQnzkwoASvpJLGj3ij9Ka2eVfKCsm4M8tNbfH38t2kGQSYoui8O2rOrlm+Y7+nKLD9jVFEyte1lo3YT8JqKv5dnXlgo3MQPVsZzLYxCEBBBl69hLm7sefYQToWRvM+cWFFKOEsc8CZWcoZ34QCXw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=k3knO7q7f0szjbcg7/7n4rChTPPeBDIGdfJ+AeW3Mdqers+sXZyqZstA3IF0Xnv0H/d+5XxqD3Zcz054ffLWwwDE3d6BY46sVYe6NlFc+Ny2zXRjYn59KbMHLoPdZszwOYCJ69u3eJOr/6Lch83wVUf0uZp4qVpJk5TTaShZnhXlk+0MSCQI7XMKXtazFtTHPrd8FgQt/dH8mY0zudwg00Mdkr95owakwqu18KuuHYfjhPbhuCCe+ZeGeTfrgGjdiS1BXbzoJpF8tVyk1xsAXvYbSmx6GRdcczOqQLZqwSMfBpBbwKJjVU1bZAzls6J/CeorFPdB+V+lufNOuJuj5A==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>
  • Delivery-date: Wed, 20 Oct 2021 13:24:45 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHXxZrAxvKJmM1p6EelRjLGqFxZEKvb4E+A
  • Thread-topic: [PATCH v3] xen/arm: vgic to ignore GICD ICPENDRn registers access

Hi Hongda,

[+Ian]

> On 20 Oct 2021, at 11:10, Hongda Deng <Hongda.Deng@xxxxxxx> wrote:
> 
> Currently, Xen will return IO unhandled when guests access GICD ICPENRn
> registers. This will raise a data abort inside guest. For Linux Guest,
> these virtual registers will not be accessed. But for Zephyr, in its
> GIC initialization code, these virtual registers will be accessed. And
> zephyr guest will get an IO data abort in initilization stage and enter
> fatal error. Emulating ICPENDR is not easy with the existing vGIC, so
> we currently ignore these virtual registers access and print a message
> about whether they are already pending instead of returning unhandled.
> More details can be found at [1].
> 
> [1] https://github.com/zephyrproject-rtos/zephyr/blob/eaf6cf745df3807e6e
> cc941c3a30de6c179ae359/drivers/interrupt_controller/intc_gicv3.c#L274
> 
> Signed-off-by: Hongda Deng <hongda.deng@xxxxxxx>
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Ian this is fixing a bug in the vgic implementation which is preventing to run
Zephyr as a guest on top of Xen. Xen support has now been merged in Zephyr
so this is required to use it.

Could we consider adding this patch into the 4.16 release ?

Thanks
Bertrand

> ---
> Changes since v2:
> *  Avoid to print messages when there is no pending interrupt
> *  Add helper vgic_check_inflight_irqs_pending to check pending status
> *  Print a message for each interrupt separately
> Changes since v1:
> *  Check pending states by going through vcpu->arch.vgic.inflight_irqs
>    instead of checking hardware registers
> ---
> xen/arch/arm/vgic-v2.c     | 10 ++++++----
> xen/arch/arm/vgic-v3.c     | 16 ++++++++--------
> xen/arch/arm/vgic.c        | 36 ++++++++++++++++++++++++++++++++++++
> xen/include/asm-arm/vgic.h |  3 ++-
> 4 files changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index b2da886adc..7c30da327c 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -481,10 +481,12 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
> mmio_info_t *info,
> 
>     case VRANGE32(GICD_ICPENDR, GICD_ICPENDRN):
>         if ( dabt.size != DABT_WORD ) goto bad_width;
> -        printk(XENLOG_G_ERR
> -               "%pv: vGICD: unhandled word write %#"PRIregister" to 
> ICPENDR%d\n",
> -               v, r, gicd_reg - GICD_ICPENDR);
> -        return 0;
> +        rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICPENDR, DABT_WORD);
> +        if ( rank == NULL ) goto write_ignore;
> +
> +        vgic_check_inflight_irqs_pending(v->domain, v, rank->index, r);
> +
> +        goto write_ignore_32;
> 
>     case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
>         if ( dabt.size != DABT_WORD ) goto bad_width;
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index cb5a70c42e..4913301d22 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -817,10 +817,12 @@ static int __vgic_v3_distr_common_mmio_write(const char 
> *name, struct vcpu *v,
> 
>     case VRANGE32(GICD_ICPENDR, GICD_ICPENDRN):
>         if ( dabt.size != DABT_WORD ) goto bad_width;
> -        printk(XENLOG_G_ERR
> -               "%pv: %s: unhandled word write %#"PRIregister" to 
> ICPENDR%d\n",
> -               v, name, r, reg - GICD_ICPENDR);
> -        return 0;
> +        rank = vgic_rank_offset(v, 1, reg - GICD_ICPENDR, DABT_WORD);
> +        if ( rank == NULL ) goto write_ignore;
> +
> +        vgic_check_inflight_irqs_pending(v->domain, v, rank->index, r);
> +
> +        goto write_ignore_32;
> 
>     case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
>         if ( dabt.size != DABT_WORD ) goto bad_width;
> @@ -987,10 +989,8 @@ static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, 
> mmio_info_t *info,
> 
>     case VREG32(GICR_ICPENDR0):
>         if ( dabt.size != DABT_WORD ) goto bad_width;
> -        printk(XENLOG_G_ERR
> -               "%pv: vGICR: SGI: unhandled word write %#"PRIregister" to 
> ICPENDR0\n",
> -               v, r);
> -        return 0;
> +        return __vgic_v3_distr_common_mmio_write("vGICR: SGI", v,
> +                                                 info, gicr_reg, r);
> 
>     case VREG32(GICR_IGRPMODR0):
>         /* We do not implement security extensions for guests, write ignore */
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 8f9400a519..0565557814 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -726,6 +726,42 @@ unsigned int vgic_max_vcpus(unsigned int 
> domctl_vgic_version)
>     }
> }
> 
> +void vgic_check_inflight_irqs_pending(struct domain *d, struct vcpu *v,
> +                                      unsigned int rank, uint32_t r)
> +{
> +    const unsigned long mask = r;
> +    unsigned int i;
> +    unsigned long flags;
> +    struct pending_irq *p;
> +    bool private = rank == 0;
> +    struct vcpu *v_target;
> +
> +    for_each_set_bit( i, &mask, 32 )
> +    {
> +        unsigned int irq = i + 32 * rank;
> +
> +        if ( private )
> +            v_target = vgic_get_target_vcpu(v, irq);
> +        else
> +            v_target = vgic_get_target_vcpu(d->vcpu[0], irq);
> +
> +        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
> +
> +        p = irq_to_pending(v_target, irq);
> +
> +        if ( unlikely(!p) )
> +        {
> +            spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
> +            continue;
> +        }
> +
> +        if ( !list_empty(&p->inflight) )
> +            printk("%pv trying to clear pending interrupt %u.\n", v, irq);
> +
> +        spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
> +    }
> +}
> +
> /*
>  * Local variables:
>  * mode: C
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index 62c2ae538d..abcaae2969 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -298,7 +298,8 @@ extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
>                         enum gic_sgi_mode irqmode, int virq,
>                         const struct sgi_target *target);
> extern bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int 
> irq);
> -
> +extern void vgic_check_inflight_irqs_pending(struct domain *d, struct vcpu 
> *v,
> +                                             unsigned int rank, uint32_t r);
> #endif /* !CONFIG_NEW_VGIC */
> 
> /*** Common VGIC functions used by Xen arch code ****/
> -- 
> 2.17.1
> 




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.