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

Re: [PATCH v4 08/12] xen/arm: vgic: add resource management for extended SPIs


  • To: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Leonid Komarianskyi <Leonid_Komarianskyi@xxxxxxxx>
  • Date: Fri, 29 Aug 2025 13:58:11 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=WP6UvMibGez4d1AXt7UeD+1nLSoEhGOy1agoladgPDo=; b=FRAWDnvhiajyPtBJ/4ww1jBBbN2dfLn5HrVbDPXuN/DyXMqIfzS3gctyOSHKMer9Xg4NjWPhSYfsTkZV85yJ4F2uI1bT+77pTC7hijeBGYQG4WQFO/HEc7/elnpbA6HyTFilu4X352ieeFqSRmYTPSjJ3Jblxlol0tbg+gvQRju2SorPRV0v8DKNFxj0CrGS33/LUdXdJV48nxAVFoZlBNN1wGVUJ3sK0/5tK0Teps1fCCVvPkt/5Erhx9o5v+3b+qM6QPi7bQL30M5sPLadBsJwZNd/HU9jrNLJ2V0mpRdB/CcWhj5dbINKpYH+6+i9skfOcA8fou1nFyxhcqQApw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=ps6UWHT/EazNlm/8kynhUn23qhAz9/sHOp1HsOHb3MVj8HaxSJkcAxcH0I5tQUTii+hGVHEGv6n9kiVZf0tXGVlzzTu5bS14Dvo2oKn0ipRgJ40gCmIzXcUtZqOpgXg/twvFGiWCfS1FViIJTudDXqvJZxE/sZEC+xt3tbMRnmVF2Wzx25Nq/v0DR5FPhmKEF5Ho28biSYeQ9k/wZheLxnFry5mJgrqZV6Z0DpSkOVM1aarQc9x4sNFpk68AvpiQkj3n/BnEdJaz1ygstQAueewrx+DkZYXsczFK5RWCTySNDH5dHkEXBV3s74HEAchOlKuG8F15oGa/z6XIAMr3Mw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Fri, 29 Aug 2025 13:58:21 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHcF3/NodAFMh424U2CquWmKsv0JLR4VPqAgAAai4CAAPWKAIAARdkA
  • Thread-topic: [PATCH v4 08/12] xen/arm: vgic: add resource management for extended SPIs

Hello Oleksandr,

Thank you for your comments and suggestions - they really help improve 
the code :)

On 29.08.25 12:48, Oleksandr Tyshchenko wrote:
> 
> 
> On 28.08.25 22:09, Leonid Komarianskyi wrote:
>> Hello Oleksandr,
> 
> Hello Leonid
> 
> 
>>
>> Thank you for your comment and suggestions.
>>
>> On 28.08.25 20:34, Oleksandr Tyshchenko wrote:
>>>
>>>
>>> On 27.08.25 21:24, Leonid Komarianskyi wrote:
>>>
>>> Hello Leonid
>>>
>>>> This change introduces resource management in the VGIC to handle
>>>> extended SPIs introduced in GICv3.1. The pending_irqs and
>>>> allocated_irqs arrays are resized to support the required
>>>> number of eSPIs, based on what is supported by the hardware and
>>>> requested by the guest. A new field, ext_shared_irqs, is added
>>>> to the VGIC structure to store information about eSPIs, similar
>>>> to how shared_irqs is used for regular SPIs.
>>>>
>>>> Since the eSPI range starts at INTID 4096 and INTIDs between 1025
>>>> and 4095 are reserved, helper macros are introduced to simplify the
>>>> transformation of indices and to enable easier access to eSPI-specific
>>>> resources. These changes prepare the VGIC for processing eSPIs as
>>>> required by future functionality.
>>>>
>>>> The initialization and deinitialization paths for vgic have been 
>>>> updated
>>>> to allocate and free these resources appropriately. Additionally,
>>>> updated handling of INTIDs greater than 1024, passed from the toolstack
>>>> during domain creation, and verification logic ensures only valid 
>>>> SPI or
>>>> eSPI INTIDs are used.
>>>>
>>>> The existing SPI behavior remains unaffected when guests do not request
>>>> eSPIs, GIC hardware does not support them, or the CONFIG_GICV3_ESPI
>>>> option is disabled.
>>>>
>>>> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@xxxxxxxx>
>>>>
>>>> ---
>>>> Changes in V4:
>>>> - added has_espi field to simplify determining whether a domain is able
>>>>     to operate with eSPI
>>>> - fixed formatting issues and misspellings
>>>>
>>>> Changes in V3:
>>>> - fixed formatting for lines with more than 80 symbols
>>>> - introduced helper functions to be able to use stubs in case of
>>>>     CONFIG_GICV3_ESPI disabled, and as a result, reduce the number of
>>>>     #ifdefs
>>>> - fixed checks for nr_spis in domain_vgic_init
>>>> - updated comment about nr_spis adjustments with dom0less mention
>>>> - moved comment with additional explanations before checks
>>>> - used unsigned int for indexes since they cannot be negative
>>>> - removed unnecessary parentheses
>>>> - move vgic_ext_rank_offset to the below ifdef guard, to reduce the
>>>>     number of ifdefs
>>>>
>>>> Changes in V2:
>>>> - change is_espi_rank to is_valid_espi_rank to verify whether the array
>>>>     element ext_shared_irqs exists. The previous version, is_espi_rank,
>>>>     only checked if the rank index was less than the maximum 
>>>> possible eSPI
>>>>     rank index, but this could potentially result in accessing a
>>>>     non-existing array element. To address this, is_valid_espi_rank was
>>>>     introduced, which ensures that the required eSPI rank exists
>>>> - move gic_number_espis to
>>>>     xen/arm: gicv3: implement handling of GICv3.1 eSPI
>>>> - update vgic_is_valid_irq checks to allow operating with eSPIs
>>>> - remove redundant newline in vgic_allocate_virq
>>>> ---
>>>>    xen/arch/arm/include/asm/vgic.h |  20 +++
>>>>    xen/arch/arm/vgic.c             | 213 +++++++++++++++++++++++++++ 
>>>> ++++-
>>>>    2 files changed, 230 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/
>>>> asm/vgic.h
>>>> index 3e7cbbb196..fb4cea73eb 100644
>>>> --- a/xen/arch/arm/include/asm/vgic.h
>>>> +++ b/xen/arch/arm/include/asm/vgic.h
>>>> @@ -146,6 +146,12 @@ struct vgic_dist {
>>>>        int nr_spis; /* Number of SPIs */
>>>>        unsigned long *allocated_irqs; /* bitmap of IRQs allocated */
>>>>        struct vgic_irq_rank *shared_irqs;
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +    struct vgic_irq_rank *ext_shared_irqs;
>>>> +    int nr_espis; /* Number of extended SPIs */
>>>
>>> Can nr_espis be negative?
>>>
>>
>> No, it cannot, so I will change it to unsigned int in V5.
>>
>>>> +    /* To simplify determining whether a domain is able to operate
>>>> with eSPI */
>>>> +    bool has_espi;
>>>
>>> I agree with the Volodymyr's comment provided in separate letter that we
>>> could avoid adding an extra var.
>>>
>>
>> Sure, as I mentioned to Volodymyr, I will remove it in V5.
>>
>>>> +#endif
>>>>        /*
>>>>         * SPIs are domain global, SGIs and PPIs are per-VCPU and 
>>>> stored in
>>>>         * struct arch_vcpu.
>>>> @@ -243,6 +249,14 @@ struct vgic_ops {
>>>>    /* Number of ranks of interrupt registers for a domain */
>>>>    #define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_spis+31)/32)
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +#define DOMAIN_NR_EXT_RANKS(d) (((d)->arch.vgic.nr_espis+31)/32)
>>>> +#define EXT_RANK_MIN (ESPI_BASE_INTID/32)
>>>> +#define EXT_RANK_MAX ((ESPI_MAX_INTID+31)/32)
>>>> +#define EXT_RANK_NUM2IDX(num) ((num)-EXT_RANK_MIN)
>>>> +#define EXT_RANK_IDX2NUM(idx) ((idx)+EXT_RANK_MIN)
>>>> +#endif
>>>> +
>>>>    #define vgic_lock(v)   spin_lock_irq(&(v)->domain->arch.vgic.lock)
>>>>    #define vgic_unlock(v) spin_unlock_irq(&(v)->domain->arch.vgic.lock)
>>>> @@ -302,6 +316,12 @@ extern struct vgic_irq_rank
>>>> *vgic_rank_offset(struct vcpu *v,
>>>>                                                  unsigned int b,
>>>>                                                  unsigned int n,
>>>>                                                  unsigned int s);
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +extern struct vgic_irq_rank *vgic_ext_rank_offset(struct vcpu *v,
>>>> +                                                  unsigned int b,
>>>> +                                                  unsigned int n,
>>>> +                                                  unsigned int s);
>>>> +#endif
>>>>    extern struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned
>>>> int irq);
>>>>    extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, unsigned
>>>> int n);
>>>>    extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, unsigned
>>>> int n);
>>>> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
>>>> index 2bbf4d99aa..f4b80cb05f 100644
>>>> --- a/xen/arch/arm/vgic.c
>>>> +++ b/xen/arch/arm/vgic.c
>>>> @@ -27,9 +27,82 @@
>>>>    bool vgic_is_valid_line(struct domain *d, unsigned int virq)
>>>>    {
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +    if ( virq >= ESPI_BASE_INTID &&
>>>> +         virq < ESPI_IDX2INTID(d->arch.vgic.nr_espis) )
>>>> +        return true;
>>>> +#endif
>>>> +
>>>>        return virq < vgic_num_irqs(d);
>>>>    }
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +/*
>>>> + * Since eSPI indexes start from 4096 and numbers from 1024 to
>>>> + * 4095 are forbidden, we need to check both lower and upper
>>>> + * limits for ranks.
>>>> + */
>>>> +static inline bool is_valid_espi_rank(struct domain *d, unsigned int
>>>> rank)
>>>> +{
>>>> +    return rank >= EXT_RANK_MIN &&
>>>> +           EXT_RANK_NUM2IDX(rank) < DOMAIN_NR_EXT_RANKS(d);
>>>> +}
>>>> +
>>>> +static inline struct vgic_irq_rank *vgic_get_espi_rank(struct vcpu *v,
>>>> +                                                       unsigned int
>>>> rank)
>>>> +{
>>>> +    return &v->domain-
>>>>> arch.vgic.ext_shared_irqs[EXT_RANK_NUM2IDX(rank)];
>>>> +}
>>>> +
>>>> +static inline bool vgic_reserve_espi_virq(struct domain *d, unsigned
>>>> int virq)
>>>> +{
>>>> +    return !test_and_set_bit(ESPI_INTID2IDX(virq) + vgic_num_irqs(d),
>>>> +                             d->arch.vgic.allocated_irqs);
>>>> +}
>>>> +
>>>> +static void arch_move_espis(struct vcpu *v)
>>>> +{
>>>> +    const cpumask_t *cpu_mask = cpumask_of(v->processor);
>>>> +    struct domain *d = v->domain;
>>>> +    struct pending_irq *p;
>>>> +    struct vcpu *v_target;
>>>> +    unsigned int i;
>>>> +
>>>> +    for ( i = ESPI_BASE_INTID;
>>>> +          i < EXT_RANK_IDX2NUM(d->arch.vgic.nr_espis); i++ )
>>>> +    {
>>>> +        v_target = vgic_get_target_vcpu(v, i);
>>>> +        p = irq_to_pending(v_target, i);
>>>> +
>>>> +        if ( v_target == v && !test_bit(GIC_IRQ_GUEST_MIGRATING, &p-
>>>>> status) )
>>>> +            irq_set_affinity(p->desc, cpu_mask);
>>>> +    }
>>>> +}
>>>> +#else
>>>> +static inline bool is_valid_espi_rank(struct domain *d, unsigned int
>>>> rank)
>>>> +{
>>>> +    return false;
>>>> +}
>>>> +
>>>> +/*
>>>> + * This function is stub and will not be called if 
>>>> CONFIG_GICV3_ESPI=n,
>>>> + * because in this case, is_valid_espi_rank will always return false.
>>>> + */
>>>> +static inline struct vgic_irq_rank *vgic_get_espi_rank(struct vcpu *v,
>>>> +                                                       unsigned int
>>>> rank)
>>>> +{
>>>> +    ASSERT_UNREACHABLE();
>>>> +    return NULL;
>>>> +}
>>>> +
>>>> +static inline bool vgic_reserve_espi_virq(struct domain *d, unsigned
>>>> int virq)
>>>> +{
>>>> +    return false;
>>>> +}
>>>> +
>>>> +static void arch_move_espis(struct vcpu *v) { }
>>>> +#endif
>>>> +
>>>>    static inline struct vgic_irq_rank *vgic_get_rank(struct vcpu *v,
>>>>                                                      unsigned int rank)
>>>>    {
>>>> @@ -37,6 +110,8 @@ static inline struct vgic_irq_rank
>>>> *vgic_get_rank(struct vcpu *v,
>>>>            return v->arch.vgic.private_irqs;
>>>>        else if ( rank <= DOMAIN_NR_RANKS(v->domain) )
>>>>            return &v->domain->arch.vgic.shared_irqs[rank - 1];
>>>> +    else if ( is_valid_espi_rank(v->domain, rank) )
>>>> +        return vgic_get_espi_rank(v, rank);
>>>>        else
>>>>            return NULL;
>>>>    }
>>>> @@ -117,6 +192,74 @@ int domain_vgic_register(struct domain *d,
>>>> unsigned int *mmio_count)
>>>>        return 0;
>>>>    }
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +/*
>>>> + * The function behavior is the same as for regular SPIs
>>>> (vgic_rank_offset),
>>>> + * but it operates with extended SPI ranks.
>>>> + */
>>>> +struct vgic_irq_rank *vgic_ext_rank_offset(struct vcpu *v, unsigned
>>>> int b,
>>>> +                                           unsigned int n, unsigned
>>>> int s)
>>>> +{
>>>> +    unsigned int rank = REG_RANK_NR(b, (n >> s));
>>>> +
>>>> +    return vgic_get_rank(v, rank + EXT_RANK_MIN);
>>>> +}
>>>
>>> So the difference between vgic_rank_offset() and vgic_ext_rank_offset()
>>> in using EXT_RANK_MIN in the latter. I am wondering, whether it makes
>>> sense to reuse the existing vgic_rank_offset() by moving #ifdef into it?
>>> Maybe it would be possible to optimize some code added in "[PATCH v4
>>> 10/12] xen/arm: vgic-v3: add emulation of GICv3.1 eSPI registers".
>>>
>>> e.g. __vgic_v3_distr_common_mmio_read:
>>>
>>> Code for the following cases looks very similar to the regular
>>> counterparts.
>>> case VRANGE32(GICD_ISENABLERnE, GICD_ISENABLERnEN):
>>> case VRANGE32(GICD_ICENABLERnE, GICD_ICENABLERnEN):
>>> case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
>>> case VRANGE32(GICD_ICFGRnE, GICD_ICFGRnEN):
>>> case VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN):
>>>
>>> The difference is in the said helper and the base calculated as
>>> "reg - GICD_XXX", where GICD_XXX is the start of the range for the eSPI-
>>> based reg (e.g. GICD_IPRIORITYRnE) or regular SPI-based reg
>>> (e.g.GICD_IPRIORITYR). The GICD_XXX could probably be chosen
>>> based on the "reg" value itself, for example:
>>>
>>> rank = vgic_rank_offset(v, 8, reg >= GICD_IPRIORITYRnE ? reg -
>>> GICD_IPRIORITYRnE : reg - GICD_IPRIORITYR, DABT_WORD);
>>>
>>> Please note, I am not requesting for any updates regarding to that, I am
>>> just thinking out loud.
>>>
>>
>> First of all, I have to say that I really tried to reuse the existing
>> code for regular SPIs whenever possible, but I could not come up with
>> something better than introducing a new function for eSPI ranks in this
>> case. This is due to several reasons:
>> 1) Currently, all eSPI-specific offsets are placed under #ifdef to avoid
>> or at least reduce the introduction of dead code or defines that will
>> not be used. This was requested by our teammates who are engaged in
>> MISRA fixes. Therefore, this is the first reason why eSPI-specific
>> elements are placed under the appropriate configuration. Without
>> CONFIG_GICV3_ESPI, we cannot use these offsets.
>>
>> 2) The proposed solution:
>>   > rank = vgic_rank_offset(v, 8, reg >= GICD_IPRIORITYRnE ? reg -
>>   > GICD_IPRIORITYRnE : reg - GICD_IPRIORITYR, DABT_WORD);
>>
>> will not work, unfortunately, because in this case, vgic_rank_offset
>> will always return ranks for regular SPIs.
>>
>> Long story short: we will encounter issues determining whether to use
>> shared_irqs (for regular SPIs) or ext_shared_irqs (for eSPIs).
>>
>> And here are the details about why this will not work:
>>
>> In the case of receiving an offset for a regular SPI, e.g., 0x6200
>> (GICD_IROUTER range [0x6100-0x7FD8]), we will get rank = 2 for regular 
>> SPIs:
>> vgic_rank_offset(v, 64, 0x6200-0x6000, DABT_DOUBLE_WORD) ->
>> DABT_DOUBLE_WORD = 3
>>
>> Thus:
>> rank = REG_RANK_NR(64, (0x200 >> 3) ->
>> REG_RANK_NR will return in this case:
>> 0x200 >> 3 >> 5 = 2
>>
>> On the other hand, if we receive an offset for an eSPI, e.g., 0x8200
>> (GICD_IROUTERnE range [0x8000-0x9FFC]), we will get the same rank = 2
>> when using the proposed solution:
>> vgic_rank_offset(v, 64, 0x8200-0x8000, DABT_DOUBLE_WORD) ->
>> DABT_DOUBLE_WORD = 3
>>
>> rank = REG_RANK_NR(64, (0x200 >> 3) ->
>> REG_RANK_NR will return also:
>> 0x200 >> 3 >> 5 = 2
>>
>> As a result, in both cases, vgic_get_rank(v, 2) will return the rank for
>> regular SPIs:
>>
>> /* This condition will apply because rank will be less than 32 */
>> else if ( rank <= DOMAIN_NR_RANKS(v->domain) )
>>       return &v->domain->arch.vgic.shared_irqs[rank - 1];
>>
>> Therefore, I decided to create a separate function to properly transform
>> ranks into the correct range.
>>
>> And, unfortunately, we cannot resolve the second issue with #ifdefs
>> inside vgic_rank_offset, because this function will receive the same
>> parameters for SPIs/eSPIs and we would not be able to determine whether
>> we should return shared_irqs or ext_shared_irqs.
> 
> Thanks for such a detailed analysis. I was thinking that somehow we could:
>   - reduce the number of #ifdef-s introduced by the current series as a 
> whole
>   - minimize the changes (optimize code) added in "[PATCH v4 10/12] xen/ 
> arm: vgic-v3: add emulation of GICv3.1 eSPI registers"
> 
> Maybe there is no valid concern at all, or I missed somethig. But, what 
> worries me a bit is that said patch adds the emulation code for eSPI 
> regs that are close for emulation code for regular SPI regs (e.g. please 
> refer to VRANGE64(GICD_IROUTERnE, GICD_IROUTERnEN) - 
> VRANGE64(GICD_IROUTER32, GICD_IROUTER1019) or 
> VRANGE32(GICD_IPRIORITYRnE, GICD_IPRIORITYRnEN) - 
> VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN)), besides the code 
> duplication, we would have to keep both regular and entended handlers
> in sync when doing possible updates/fixing the bugs.
> 
> While I completely agree with the dead code for eSPI-specific helpers/ 
> functions if we move them out of #ifdef CONFIG_GICV3_ESPI, what might be 
> wrong if we at least drop #ifdef CONFIG_GICV3_ESPI for simple #define-s 
> (non functional code), such as:
> 
>   #ifdef CONFIG_GICV3_ESPI
> /* Additional registers for GICv3.1 */
> #define GICD_IGROUPRnE               (0x1000)
> #define GICD_IGROUPRnEN              (0x107C)
> ...
> 
> or
> 
> #ifdef CONFIG_GICV3_ESPI
> #define ESPI_BASE_INTID 4096
> #define ESPI_MAX_INTID  5119
> #define NR_ESPI_IRQS    1024
> 
> #define ESPI_INTID2IDX(intid) ((intid) - ESPI_BASE_INTID)
> #define ESPI_IDX2INTID(idx)   ((idx) + ESPI_BASE_INTID)
> #endif
> 

Thank you again - after our discussion, I reviewed the entire patch 
series again and figured out how to significantly reduce the number of 
#ifdefs while also reusing the code implemented for regular SPIs. This 
allows the introduction of vGIC handlers and avoids dead code. I will 
send V5 soon and hope it will look much better.

>>
>>>
>>>> +
>>>> +static unsigned int vgic_num_spi_lines(struct domain *d)
>>>> +{
>>>> +    return d->arch.vgic.nr_spis + d->arch.vgic.nr_espis;
>>>> +}
>>>> +
>>>> +static int init_vgic_espi(struct domain *d)
>>>> +{
>>>> +    unsigned int i, idx;
>>>> +
>>>> +    if ( d->arch.vgic.nr_espis == 0 )
>>>> +        return 0;
>>>> +
>>>> +    d->arch.vgic.ext_shared_irqs =
>>>> +        xzalloc_array(struct vgic_irq_rank, DOMAIN_NR_EXT_RANKS(d));
>>>> +    if ( d->arch.vgic.ext_shared_irqs == NULL )
>>>> +        return -ENOMEM;
>>>> +
>>>> +    for ( i = d->arch.vgic.nr_spis, idx = 0;
>>>> +          i < vgic_num_spi_lines(d); i++, idx++ )
>>>> +        vgic_init_pending_irq(&d->arch.vgic.pending_irqs[i],
>>>> +                              ESPI_IDX2INTID(idx));
>>>> +
>>>> +    for ( i = 0; i < DOMAIN_NR_EXT_RANKS(d); i++ )
>>>> +        vgic_rank_init(&d->arch.vgic.ext_shared_irqs[i], i, 0);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +struct pending_irq *espi_to_pending(struct domain *d, unsigned int 
>>>> irq)
>>>> +{
>>>> +    irq = ESPI_INTID2IDX(irq) + d->arch.vgic.nr_spis;
>>>> +    return &d->arch.vgic.pending_irqs[irq];
>>>> +}
>>>> +#else
>>>> +static unsigned int init_vgic_espi(struct domain *d
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static unsigned int vgic_num_spi_lines(struct domain *d)
>>>> +{
>>>> +    return d->arch.vgic.nr_spis;
>>>> +}
>>>> +
>>>> +struct pending_irq *espi_to_pending(struct domain *d, unsigned int 
>>>> irq)
>>>> +{
>>>> +    return NULL;
>>>> +}
>>>> +#endif
>>>> +
>>>> +static unsigned int vgic_num_alloc_irqs(struct domain *d)
>>>> +{
>>>> +    return vgic_num_spi_lines(d) + NR_LOCAL_IRQS;
>>>> +}
>>>> +
>>>>    int domain_vgic_init(struct domain *d, unsigned int nr_spis)
>>>>    {
>>>>        int i;
>>>> @@ -131,6 +274,38 @@ int domain_vgic_init(struct domain *d, unsigned
>>>> int nr_spis)
>>>>         */
>>>>        nr_spis = ROUNDUP(nr_spis, 32);
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +    /*
>>>> +     * During domain creation, the dom0less DomUs code or toolstack
>>>> specifies
>>>> +     * the maximum INTID, which is defined in the domain config
>>>> subtracted by
>>>> +     * 32 to cover the local IRQs (please see the comment to
>>>> VGIC_DEF_NR_SPIS).
>>>> +     * To compute the actual number of eSPI that will be usable for,
>>>> +     * add back 32.
>>>> +     */
>>>> +    if ( nr_spis + 32 > ESPI_IDX2INTID(NR_ESPI_IRQS) )
>>>> +        return -EINVAL;
>>>> +
>>>> +    if ( nr_spis + 32 >= ESPI_BASE_INTID )
>>>> +    {
>>>> +        d->arch.vgic.nr_espis = min(nr_spis - ESPI_BASE_INTID + 32,
>>>> 1024U);
>>>> +        /* Verify if GIC HW can handle provided INTID */
>>>> +        if ( d->arch.vgic.nr_espis > gic_number_espis() )
>>>> +            return -EINVAL;
>>>> +        /*
>>>> +         * Set the maximum available number for regular
>>>> +         * SPI to pass the next check
>>>> +         */
>>>> +        nr_spis = VGIC_DEF_NR_SPIS;
>>>> +        d->arch.vgic.has_espi = true;
>>>> +    }
>>>> +    else
>>>> +    {
>>>> +        /* Domain will use the regular SPI range */
>>>> +        d->arch.vgic.nr_espis = 0;
>>>> +        d->arch.vgic.has_espi = false;
>>>> +    }
>>>> +#endif
>>>> +
>>>>        /* Limit the number of virtual SPIs supported to (1020 - 32) =
>>>> 988  */
>>>>        if ( nr_spis > (1020 - NR_LOCAL_IRQS) )
>>>>            return -EINVAL;
>>>> @@ -145,7 +320,7 @@ int domain_vgic_init(struct domain *d, unsigned
>>>> int nr_spis)
>>>>            return -ENOMEM;
>>>>        d->arch.vgic.pending_irqs =
>>>> -        xzalloc_array(struct pending_irq, d->arch.vgic.nr_spis);
>>>> +        xzalloc_array(struct pending_irq, vgic_num_spi_lines(d));
>>>>        if ( d->arch.vgic.pending_irqs == NULL )
>>>>            return -ENOMEM;
>>>> @@ -156,12 +331,16 @@ int domain_vgic_init(struct domain *d, unsigned
>>>> int nr_spis)
>>>>        for ( i = 0; i < DOMAIN_NR_RANKS(d); i++ )
>>>>            vgic_rank_init(&d->arch.vgic.shared_irqs[i], i + 1, 0);
>>>> +    ret = init_vgic_espi(d);
>>>> +    if ( ret )
>>>> +        return ret;
>>>> +
>>>>        ret = d->arch.vgic.handler->domain_init(d);
>>>>        if ( ret )
>>>>            return ret;
>>>>        d->arch.vgic.allocated_irqs =
>>>> -        xzalloc_array(unsigned long, BITS_TO_LONGS(vgic_num_irqs(d)));
>>>> +        xzalloc_array(unsigned long,
>>>> BITS_TO_LONGS(vgic_num_alloc_irqs(d)));
>>>>        if ( !d->arch.vgic.allocated_irqs )
>>>>            return -ENOMEM;
>>>> @@ -195,9 +374,27 @@ void domain_vgic_free(struct domain *d)
>>>>            }
>>>>        }
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +    for ( i = 0; i < d->arch.vgic.nr_espis; i++ )
>>>> +    {
>>>> +        struct pending_irq *p = spi_to_pending(d, ESPI_IDX2INTID(i));
>>>> +
>>>> +        if ( p->desc )
>>>> +        {
>>>> +            ret = release_guest_irq(d, p->irq);
>>>> +            if ( ret )
>>>> +                dprintk(XENLOG_G_WARNING, "d%u: Failed to release
>>>> virq %u ret = %d\n",
>>>
>>>
>>> NIT: you can use %pd for printing domain with its ID
>>>
>>>
>>
>> I will fix it in V5.
>>
>>>> +                        d->domain_id, p->irq, ret);
>>>> +        }
>>>> +    }
>>>> +#endif
>>>> +
>>>>        if ( d->arch.vgic.handler )
>>>>            d->arch.vgic.handler->domain_free(d);
>>>>        xfree(d->arch.vgic.shared_irqs);
>>>> +#ifdef CONFIG_GICV3_ESPI
>>>> +    xfree(d->arch.vgic.ext_shared_irqs);
>>>> +#endif
>>>>        xfree(d->arch.vgic.pending_irqs);
>>>>        xfree(d->arch.vgic.allocated_irqs);
>>>>    }
>>>> @@ -331,6 +528,8 @@ void arch_move_irqs(struct vcpu *v)
>>>>            if ( v_target == v && !test_bit(GIC_IRQ_GUEST_MIGRATING, &p-
>>>>> status) )
>>>>                irq_set_affinity(p->desc, cpu_mask);
>>>>        }
>>>> +
>>>> +    arch_move_espis(v);
>>>>    }
>>>>    void vgic_disable_irqs(struct vcpu *v, uint32_t r, unsigned int n)
>>>> @@ -538,6 +737,8 @@ struct pending_irq *irq_to_pending(struct vcpu *v,
>>>> unsigned int irq)
>>>>            n = &v->arch.vgic.pending_irqs[irq];
>>>>        else if ( is_lpi(irq) )
>>>>            n = v->domain->arch.vgic.handler->lpi_to_pending(v->domain,
>>>> irq);
>>>> +    else if ( is_espi(irq) )
>>>> +        n = espi_to_pending(v->domain, irq);
>>>>        else
>>>>            n = &v->domain->arch.vgic.pending_irqs[irq - 32];
>>>>        return n;
>>>> @@ -547,6 +748,9 @@ struct pending_irq *spi_to_pending(struct domain
>>>> *d, unsigned int irq)
>>>>    {
>>>>        ASSERT(irq >= NR_LOCAL_IRQS);
>>>> +    if ( is_espi(irq) )
>>>> +        return espi_to_pending(d, irq);
>>>> +
>>>>        return &d->arch.vgic.pending_irqs[irq - 32];
>>>>    }
>>>> @@ -668,6 +872,9 @@ bool vgic_reserve_virq(struct domain *d, unsigned
>>>> int virq)
>>>>        if ( !vgic_is_valid_line(d, virq) )
>>>>            return false;
>>>> +    if ( is_espi(virq) )
>>>> +        return vgic_reserve_espi_virq(d, virq);
>>>> +
>>>>        return !test_and_set_bit(virq, d->arch.vgic.allocated_irqs);
>>>>    }
>>>> @@ -685,7 +892,7 @@ int vgic_allocate_virq(struct domain *d, bool spi)
>>>>        else
>>>>        {
>>>>            first = 32;
>>>> -        end = vgic_num_irqs(d);
>>>> +        end = vgic_num_alloc_irqs(d);
>>>>        }
>>>>        /*
>>>
>>
>> Best regards,
>> Leonid
> 

Best regards,
Leonid

 


Rackspace

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