irq: phytium: update phytium irq controller driver support to 6.6.0.4#1683
Open
wangchenlu2236 wants to merge 3 commits into
Open
irq: phytium: update phytium irq controller driver support to 6.6.0.4#1683wangchenlu2236 wants to merge 3 commits into
wangchenlu2236 wants to merge 3 commits into
Conversation
There are two hacks needed for IOMMU enablement on Phytium Ps17064 SoC. One is the MSI hack, the other is the SMMU hack. When using this enablement, we assumes that users would set CONIFIG_IOMMU_DEFAULT_PASSTHROUGH=y or pass 'iommu.passthrough=on' or 'iommu.pt' as the kernel command-line parameters. Therefore, we also force default iommu domain type to IOMMU_DOMAIN_IDENTITY on Ps17064 to avoid unnecessary troubles. Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> Signed-off-by:Wang Chenlu <wangchenlu2236@phytium.com.cn>
When compiling with the aarch32 compiler, the arm64-specific macros were not defined. Add restriction to bring these macros only into effect on arm64 architecture. Mainline: NA Signed-off-by: Li Mingzhe <limingzhe1839@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by:Wang Chenlu <wangchenlu2236@phytium.com.cn>
Fix the logic in its_irq_compose_msi() where CONFIG_ARCH_PHYTIUM filtering accidentally changed the original behavior. Now iommu_dma_compose_msi_msg() is skipped only on PS17064 SoCs, and executed on all other platforms as intended. Mainline: NA Signed-off-by: Li Mingzhe <limingzhe1839@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by:Wang Chenlu <wangchenlu2236@phytium.com.cn>
Reviewer's GuideUpdates IOMMU and GICv3 ITS handling for Phytium PS17064 SoCs by adding an SMMU firmware-ID workaround, forcing identity IOMMU domains, and bypassing generic MSI message composition on that CPU model. Sequence diagram for updated MSI composition on Phytium PS17064sequenceDiagram
actor Phytium_PS17064
participant GICv3_ITS as its_irq_compose_msi_msg
participant IOMMU_DMA as iommu_dma_compose_msi_msg
Phytium_PS17064 ->> GICv3_ITS: its_irq_compose_msi_msg(irq_data, msg)
GICv3_ITS ->> GICv3_ITS: its_get_event_id(irq_data)
GICv3_ITS ->> GICv3_ITS: set msg.address_lo, msg.address_hi, msg.data
alt [CPU is Phytium PS17064]
GICv3_ITS -->> Phytium_PS17064: return
else [other CPUs]
GICv3_ITS ->> IOMMU_DMA: iommu_dma_compose_msi_msg(msi_desc, msg)
IOMMU_DMA -->> GICv3_ITS: return
GICv3_ITS -->> Phytium_PS17064: return
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @wangchenlu2236. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The Phytium PS17064 MIDR check is open‑coded in multiple places; consider introducing a small helper (e.g.
cpu_is_ps17064()) to centralize the model detection logic and reduce duplication. - In
arm_smmu_probe_device(), you append additional FWIDs tofwspecwhile later iterating overfwspec->num_ids; double‑check whether this can cause the ID list to grow unexpectedly or be processed twice and, if so, consider using a separate buffer or normalizing the IDs before probing. - The new
drivers/irqchip/irq-gic-v3-its.c.origfile appears to be an editor backup and should be removed from the commit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Phytium PS17064 MIDR check is open‑coded in multiple places; consider introducing a small helper (e.g. `cpu_is_ps17064()`) to centralize the model detection logic and reduce duplication.
- In `arm_smmu_probe_device()`, you append additional FWIDs to `fwspec` while later iterating over `fwspec->num_ids`; double‑check whether this can cause the ID list to grow unexpectedly or be processed twice and, if so, consider using a separate buffer or normalizing the IDs before probing.
- The new `drivers/irqchip/irq-gic-v3-its.c.orig` file appears to be an editor backup and should be removed from the commit.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR updates kernel IRQ/MSI and IOMMU/SMMU handling for Phytium PS17064 SoCs, aiming to apply platform-specific workarounds and avoid incorrect MSI programming and IOMMU issues on that platform.
Changes:
- Bypass
iommu_dma_compose_msi_msg()on PS17064 in the GICv3 ITS MSI compose path. - Force the IOMMU default domain to identity/passthrough on PS17064 (including the
iommu.passthrough=early param path). - Add a PS17064-specific SMMU probing workaround that rewrites/derives firmware IDs for stream IDs.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/irqchip/irq-gic-v3-its.c.orig | Adds a full duplicate copy of the ITS driver as a .orig file. |
| drivers/irqchip/irq-gic-v3-its.c | Skips iommu_dma_compose_msi_msg() on PS17064 when composing MSI messages. |
| drivers/iommu/iommu.c | Forces default IOMMU domain to identity/passthrough on PS17064. |
| drivers/iommu/arm/arm-smmu/arm-smmu.h | Adds Phytium-specific helper macro and includes for CPU model detection. |
| drivers/iommu/arm/arm-smmu/arm-smmu.c | Adds PS17064 workaround in SMMU device probing by adding derived firmware IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1382
to
+1386
| for (i = 0; i < num; i++) { | ||
| u32 fwid = FWID_READ(fwspec->ids[i]); | ||
|
|
||
| iommu_fwspec_add_ids(dev, &fwid, 1); | ||
| } |
Comment on lines
+1
to
+155
| * be 64kB aligned (one bit per LPI, plus 8192 bits for SPI/PPI/SGI). | ||
| */ | ||
| #define LPI_NRBITS lpi_id_bits | ||
| #define LPI_PROPBASE_SZ ALIGN(BIT(LPI_NRBITS), SZ_64K) | ||
| #define LPI_PENDBASE_SZ ALIGN(BIT(LPI_NRBITS) / 8, SZ_64K) | ||
|
|
||
| #define LPI_PROP_DEFAULT_PRIO GICD_INT_DEF_PRI | ||
|
|
||
| /* | ||
| * Collection structure - just an ID, and a redistributor address to | ||
| * ping. We use one per CPU as a bag of interrupts assigned to this | ||
| * CPU. | ||
| */ | ||
| struct its_collection { | ||
| u64 target_address; | ||
| u16 col_id; | ||
| }; | ||
|
|
||
| /* | ||
| * The ITS_BASER structure - contains memory information, cached | ||
| * value of BASER register configuration and ITS page size. | ||
| */ | ||
| struct its_baser { | ||
| void *base; | ||
| u64 val; | ||
| u32 order; | ||
| u32 psz; | ||
| }; | ||
|
|
||
| struct its_device; | ||
|
|
||
| /* | ||
| * The ITS structure - contains most of the infrastructure, with the | ||
| * top-level MSI domain, the command queue, the collections, and the | ||
| * list of devices writing to it. | ||
| * | ||
| * dev_alloc_lock has to be taken for device allocations, while the | ||
| * spinlock must be taken to parse data structures such as the device | ||
| * list. | ||
| */ | ||
| struct its_node { | ||
| raw_spinlock_t lock; | ||
| struct mutex dev_alloc_lock; | ||
| struct list_head entry; | ||
| void __iomem *base; | ||
| void __iomem *sgir_base; | ||
| phys_addr_t phys_base; | ||
| struct its_cmd_block *cmd_base; | ||
| struct its_cmd_block *cmd_write; | ||
| struct its_baser tables[GITS_BASER_NR_REGS]; | ||
| struct its_collection *collections; | ||
| struct fwnode_handle *fwnode_handle; | ||
| u64 (*get_msi_base)(struct its_device *its_dev); | ||
| u64 typer; | ||
| u64 cbaser_save; | ||
| u32 ctlr_save; | ||
| u32 mpidr; | ||
| struct list_head its_device_list; | ||
| u64 flags; | ||
| unsigned long list_nr; | ||
| int numa_node; | ||
| unsigned int msi_domain_flags; | ||
| u32 pre_its_base; /* for Socionext Synquacer */ | ||
| int vlpi_redist_offset; | ||
| }; | ||
|
|
||
| #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS)) | ||
| #define is_v4_1(its) (!!((its)->typer & GITS_TYPER_VMAPP)) | ||
| #define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1) | ||
|
|
||
| #define ITS_ITT_ALIGN SZ_256 | ||
|
|
||
| /* The maximum number of VPEID bits supported by VLPI commands */ | ||
| #define ITS_MAX_VPEID_BITS \ | ||
| ({ \ | ||
| int nvpeid = 16; \ | ||
| if (gic_rdists->has_rvpeid && \ | ||
| gic_rdists->gicd_typer2 & GICD_TYPER2_VIL) \ | ||
| nvpeid = 1 + (gic_rdists->gicd_typer2 & \ | ||
| GICD_TYPER2_VID); \ | ||
| \ | ||
| nvpeid; \ | ||
| }) | ||
| #define ITS_MAX_VPEID (1 << (ITS_MAX_VPEID_BITS)) | ||
|
|
||
| /* Convert page order to size in bytes */ | ||
| #define PAGE_ORDER_TO_SIZE(o) (PAGE_SIZE << (o)) | ||
|
|
||
| struct event_lpi_map { | ||
| unsigned long *lpi_map; | ||
| u16 *col_map; | ||
| irq_hw_number_t lpi_base; | ||
| int nr_lpis; | ||
| raw_spinlock_t vlpi_lock; | ||
| struct its_vm *vm; | ||
| struct its_vlpi_map *vlpi_maps; | ||
| int nr_vlpis; | ||
| }; | ||
|
|
||
| /* |
| * introduced by the SMMU workaround. | ||
| */ | ||
| if ((read_cpuid_id() & MIDR_CPU_MODEL_MASK) == MIDR_PHYTIUM_PS17064) | ||
| iommu_set_default_passthrough(true); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patches updates the support for phytium irq controller driver.
1.iommu/arm-smmu: Add SMMU workaround for Phytium Ps17064
2.arm64: Phytium: Slove the error on aarch32 compiler
3.arm64: Phytium: Fix incorrect MSI compose logic on Phytium PS17064 SoCs
Summary by Sourcery
Update IOMMU and IRQ handling for Phytium PS17064 SoCs and add a platform-specific SMMU workaround.
Bug Fixes:
Enhancements: