On Sun, 7 Sep 2025, Leonid Komarianskyi wrote:
> Hello Stefano,
>
> Thank you for your comments and for providing the Eclair reports.
>
> On 06.09.25 03:17, Stefano Stabellini wrote:
> > Hi Leonid,
> >
> > I was about to commit this but unfortunately it is introducing MISRA
> > regressions. See:
> > https://gitlab.com/xen-project/people/sstabellini/xen/-/tree/ppp6?ref_type=heads
> >
> > https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/11265005118
> >
> > Compared this result:
> > https://eclair-analysis-logs.xenproject.org/fs/var/local/eclair/xen-project.ecdf/xen-project/people/sstabellini/xen/ECLAIR_normal/ppp6/ARM64/11265005118/PROJECT.ecd;/by_service.html#service&kind
> >
> > Against upstream staging:
> > https://eclair-analysis-logs.xenproject.org/fs/space/XEN.ecdf/xen-project/hardware/xen/ECLAIR_normal/staging/ARM64/11264772605/PROJECT.ecd;/by_service.html#service&kind
> >
> > It is introducing a couple of easy-to-fix 16.3 issues and also a couple
> > of new 16.4 issues. They should be all easy to fix. It is also
> > introducing three new 13.2 issues and one 18.1 but I haven't looked
> > closely into those. Please address them.
> >
>
> Regarding the MISRA 16.3/16.4 violations and 13.2 cautions - there are
> no questions from my side. I have fixed them and will send the updated
> V8 (with typo fixes, added acked/reviewed-by tags, etc.).
Thanks!
> However, I
> would like to clarify regarding the MISRA 18.1 caution regression and
> review process rules:
>
> MISRA 18.1 caution:
>
> xen/arch/arm/irq.c:105.13-105.39: [8] access of 'irq_desc' at an
> overflowing index, while it holds only 992 'struct irq_desc' elements
>
> Actually, there is no new real issue here, because the mainline
> irq_desc() currently does not have upper limit checks for IRQs:
>
> struct irq_desc *__irq_to_desc(unsigned int irq)
> {
> if ( irq < NR_LOCAL_IRQS )
> return &this_cpu(local_irq_desc)[irq];
>
> return &irq_desc[irq-NR_LOCAL_IRQS];
> }
>
> ... as a result Eclair does not spot any issues in this code according
> to the staging report. As I understand, it triggers on patches with eSPI
> because I introduced new checks for the eSPI INTID range, which should
> not be used without CONFIG_GICV3_ESPI=y.
>
> Also, a similar issue with invalid INTIDs was discussed in the thread:
>
> https://lists.xenproject.org/archives/html/xen-devel/2025-09/msg00401.html
>
> Long story short: the mainline Xen currently allows defining invalid
> INTIDs in the Xen DTS and crashes in __irq_to_desc() when attempting to
> operate on an invalid interrupt number.
>
> I have prepared a fix for the 18.1 caution, but I am not sure whether it
> is worth applying just to address the Eclair report (if it is not critical):
> https://github.com/LKomaryanskiy/xen/commit/af5d9a483302f7bcfaadbefb85f5e4ee35f6cb3b
>
> So, could you please clarify which option would be better:
> 1) Leave the code as it is for now, accepting the MISRA caution from
> Eclair and fix the issue later in the context of addressing Xen crashing
> with invalid INTIDs and implementing dynamic allocation of irq_desc_t array;
> 2) Apply the fix now (while also planning to address the invalid INTID
> issue in the future)
I am happy to apply the fix now as part of the series
> Regarding the review process:
> Should I remove the 'reviewed-by' tags from the patches where I added
> missing breaks (with the corresponding code updates) or introduced
> variables to fix MISRA issues? I am asking because these are code
> changes, and I am not sure if I should leave the RB tags in this case.
Please keep them
> >
> > Oleksii,
> >
> > Technically, the series is fully acked and ready to be committed. From a
> > risk perspective, I would be comfortable committing it now with the
> > outstanding MISRA regressions, leaving Leonid to fix them over the next
> > few days. However, I have not done so because it would make it harder to
> > spot the MISRA regressions due to the way the scanner works (it
> > compares against the previous version).
> >
> > I suggest we allow this series to be committed in the next couple of
> > days, once Leonid addresses the regressions, even though it would
> > technically be past the feature freeze.
> >
> > Cheers,
> >
> > Stefano
> >
> > P.S.
> >
> > Leonid, you might want to check my commits because I fixed a couple of
> > things on commit, in addition to adding the various acked-by tags.
> >
> >
> > On Thu, 4 Sep 2025, Leonid Komarianskyi wrote:
> >> Hello everyone!
> >>
> >> V6 contains an issue for debug builds with CONFIG_GICV3_ESPI=n due to a
> >> mistake in the ASSERT() condition in the is_espi() function. This patch
> >> series fixes the issue and also includes minor fixes according to the
> >> review of V6.
> >>
> >> Summarized description:
> >> This patch series adds support for the extended shared peripheral
> >> interrupt (eSPI) range (INTIDs 4096-5119 [2](ranges of INTIDs)) for Xen
> >> and guest domains. The implementation uses a generic approach to handle
> >> eSPIs, similar to regular SPIs, while maintaining compatibility with the
> >> existing SPI range. Functionality remains unchanged for setups that do
> >> not require eSPIs.
> >>
> >> The series includes:
> >> 1) General refactoring of common IRQ operations with GIC registers to
> >> improve code readability, simplify further maintenance and prepare the
> >> key functions for eSPI implementation.
> >> 2) Introducing a new Kconfig option (default n) to enable or disable
> >> eSPI support. Disabling this option prevents unnecessary resource
> >> allocation for setups that do not require eSPIs.
> >> 3) Adding additional resources to store required information and operate
> >> with up to 1024 interrupts from eSPI range.
> >> 4) Adjusting assertions and checks to pass verification for INTIDs in
> >> the eSPI range.
> >> 5) Configuration of eSPI-specific registers during GIC initialization
> >> for systems with GICv3.1+ hardware.
> >> 6) Enables eSPI MMIO emulation for vGIC, allowing guest domains to
> >> access and operate within the eSPI's INTIDs.
> >> 7) Updating documentation and CHANGELOG to reflect the changes made for
> >> eSPI
> >> support.
> >>
> >> Also, to simplify reviewing, please find below link to unsquashed patches,
> >> that
> >> are on top of every patch, that is changed in the series, compared to V6:
> >> https://github.com/LKomaryanskiy/xen/commits/espi-support-master-upstream-v7-unsquashed/
> >>
> >> Github branch with patch series:
> >> https://github.com/LKomaryanskiy/xen/commits/espi-support-master-upstream-v7/
> >>
> >> Changes in V7:
> >> - individual changes in patches
> >>
> >> Link on V6:
> >> -
> >> https://lists.xenproject.org/archives/html/xen-devel/2025-09/msg00296.html
> >>
> >> Changes in V6:
> >> - individual changes in patches
> >>
> >> Link on V5:
> >> -
> >> https://lists.xenproject.org/archives/html/xen-devel/2025-08/msg02086.html
> >>
> >> Changes in V5:
> >> - individual changes in patches
> >>
> >> Link on V4:
> >> -
> >> https://lists.xenproject.org/archives/html/xen-devel/2025-08/msg01767.html
> >>
> >> Changes in V4:
> >> - added a patch for documentation
> >> - individual changes in patches
> >>
> >> Link on V3:
> >> -
> >> https://lists.xenproject.org/archives/html/xen-devel/2025-08/msg01628.html
> >>
> >> Changes in V3:
> >> - added a patch to update CHANGELOG.md
> >> - individual changes in patches
> >>
> >> Link on V2:
> >> -
> >> https://lists.xenproject.org/archives/html/xen-devel/2025-08/msg00372.html
> >>
> >> Changes in V2:
> >> - added 2 more patches to implement helper
> >> functions for gic/vgic:
> >> xen/arm: gic: implement helper functions for INTID checks
> >> xen/arm: vgic: implement helper functions for virq checks
> >> - removed 2 patches:
> >> xen/arm/irq: allow assignment/releasing of eSPI interrupts
> >> xen/arm: gic/irq: permit routing of eSPI interrupts to Xen and domains
> >> since their functionality can be moved to appropriate patches after
> >> introducing patches with helper functions
> >> - individual changes in patches
> >>
> >> Link on V1:
> >> -
> >> https://lists.xenproject.org/archives/html/xen-devel/2025-07/msg01809.html
> >>
> >> Leonid Komarianskyi (12):
> >> xen/arm: gicv3: refactor obtaining GIC addresses for common operations
> >> xen/arm: gic: implement helper functions for INTID checks
> >> xen/arm: vgic: implement helper functions for virq checks
> >> xen/arm/irq: add handling for IRQs in the eSPI range
> >> xen/arm: gicv3: implement handling of GICv3.1 eSPI
> >> xen/arm/irq: allow eSPI processing in the gic_interrupt function
> >> xen/arm: gicv3: modify ICH_LR_PHYSICAL_MASK to allow eSPI processing
> >> xen/arm: vgic: add resource management for extended SPIs
> >> xen/arm: domain_build/dom0less-build: adjust domains config to support
> >> eSPIs
> >> xen/arm: vgic-v3: add emulation of GICv3.1 eSPI registers
> >> doc/man: update description for nr_spis with eSPI
> >> CHANGELOG.md: add mention of GICv3.1 eSPI support
> >>
> >> CHANGELOG.md | 2 +
> >> docs/man/xl.cfg.5.pod.in | 13 +-
> >> xen/arch/arm/Kconfig | 8 +
> >> xen/arch/arm/dom0less-build.c | 2 +-
> >> xen/arch/arm/domain_build.c | 2 +-
> >> xen/arch/arm/gic-v3.c | 195 +++++++++++++++++++----
> >> xen/arch/arm/gic.c | 8 +-
> >> xen/arch/arm/include/asm/gic.h | 28 ++++
> >> xen/arch/arm/include/asm/gic_v3_defs.h | 40 ++++-
> >> xen/arch/arm/include/asm/irq.h | 38 +++++
> >> xen/arch/arm/include/asm/vgic.h | 56 ++++++-
> >> xen/arch/arm/irq.c | 62 +++++++-
> >> xen/arch/arm/vgic-v3.c | 203 ++++++++++++++++++-----
> >> xen/arch/arm/vgic.c | 212 +++++++++++++++++++++++--
> >> xen/arch/arm/vgic/vgic.c | 5 +
> >> 15 files changed, 762 insertions(+), 112 deletions(-)
> >>
> >> --
> >> 2.34.1
> >>
>
> Best regards,
> Leonid
>
