* [PATCH] arm64: Fix early handling of FEAT_E2H0 not being implemented
From: Marc Zyngier @ 2024-03-21 11:54 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm
Cc: Catalin Marinas, Will Deacon, James Morse, Suzuki K Poulose,
Oliver Upton, Zenghui Yu
Commit 3944382fa6f2 introduced checks for the FEAT_E2H0 not being
implemented. However, the check is absolutely wrong and makes a
point it testing a bit that is guaranteed to be zero.
On top of that, the detection happens way too late, after the
init_el2_state has done its job.
This went undetected because the HW this was tested on has E2H being
RAO/WI, and not RES1. However, the bug shows up when run as a nested
guest, where HCR_EL2.E2H is not necessarily set to 1. As a result,
booting the kernel in hVHE mode fails with timer accesses being
cought in a trap loop (which was fun to debug).
Fix the check for ID_AA64MMFR4_EL1.E2H0, and set the HCR_EL2.E2H bit
early so that it can be checked by the rest of the init sequence.
With this, hVHE works again in a NV environment that doesn't have
FEAT_E2H0.
Fixes: 3944382fa6f2 ("arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/kernel/head.S | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index ce08b744aaab..06234c3a15f3 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -291,6 +291,21 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
blr x2
0:
mov_q x0, HCR_HOST_NVHE_FLAGS
+
+ /*
+ * Compliant CPUs advertise their VHE-onlyness with
+ * ID_AA64MMFR4_EL1.E2H0 < 0. HCR_EL2.E2H can be
+ * RES1 in that case. Publish the E2H bit early so that
+ * it can be picked up by the init_el2_state macro.
+ *
+ * Fruity CPUs seem to have HCR_EL2.E2H set to RAO/WI, but
+ * don't advertise it (they predate this relaxation).
+ */
+ mrs_s x1, SYS_ID_AA64MMFR4_EL1
+ tbz x1, #(ID_AA64MMFR4_EL1_E2H0_SHIFT + ID_AA64MMFR4_EL1_E2H0_WIDTH - 1), 1f
+
+ orr x0, x0, #HCR_E2H
+1:
msr hcr_el2, x0
isb
@@ -303,22 +318,10 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
mov_q x1, INIT_SCTLR_EL1_MMU_OFF
- /*
- * Compliant CPUs advertise their VHE-onlyness with
- * ID_AA64MMFR4_EL1.E2H0 < 0. HCR_EL2.E2H can be
- * RES1 in that case.
- *
- * Fruity CPUs seem to have HCR_EL2.E2H set to RES1, but
- * don't advertise it (they predate this relaxation).
- */
- mrs_s x0, SYS_ID_AA64MMFR4_EL1
- ubfx x0, x0, #ID_AA64MMFR4_EL1_E2H0_SHIFT, #ID_AA64MMFR4_EL1_E2H0_WIDTH
- tbnz x0, #(ID_AA64MMFR4_EL1_E2H0_SHIFT + ID_AA64MMFR4_EL1_E2H0_WIDTH - 1), 1f
-
mrs x0, hcr_el2
and x0, x0, #HCR_E2H
cbz x0, 2f
-1:
+
/* Set a sane SCTLR_EL1, the VHE way */
pre_disable_mmu_workaround
msr_s SYS_SCTLR_EL12, x1
--
2.39.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v2] ARM: unwind: improve unwinders for noreturn case
From: Russell King (Oracle) @ 2024-03-21 11:23 UTC (permalink / raw)
To: David Laight, Ard Biesheuvel
Cc: 'Jiangfeng Xiao', arnd@arndb.de, keescook@chromium.org,
haibo.li@mediatek.com, angelogioacchino.delregno@collabora.com,
amergnat@baylibre.com, akpm@linux-foundation.org,
dave.hansen@linux.intel.com, douzhaolei@huawei.com,
gustavoars@kernel.org, jpoimboe@kernel.org,
kepler.chenxin@huawei.com, kirill.shutemov@linux.intel.com,
linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org,
nixiaoming@huawei.com, peterz@infradead.org, wangbing6@huawei.com,
wangfangpeng1@huawei.com, jannh@google.com, willy@infradead.org
In-Reply-To: <bbcca1e205384cf0b42236e17f3969f7@AcuMS.aculab.com>
On Thu, Mar 21, 2024 at 10:22:30AM +0000, David Laight wrote:
> How aggressively does the compiler optimise 'noreturn' functions?
I've seen cases where the compiler emits a BL instruction as the very
last thing in the function, and nothing after it.
This is where the problem lies - because the link register value
created by the BL instruction will point to the instruction after the
BL which will _not_ part of the function that invoked the BL. That
will probably cause issues for the ELF unwinder, which means this
issue probably goes beyond _just_ printing the function name.
I have vague memories that Ard has been involved in the unwinder,
maybe he could comment on this problem? Maybe we need the unwinder
itself to do the correction? I also wonder whether we should only
do the correction if we detect that we're pointing at the first
instruction of a function, and the previous instruction in the
text segment was a BL.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net v2 0/2] MT7530 DSA subdriver fix VLAN egress and handling of all link-local frames
From: patchwork-bot+netdevbpf @ 2024-03-21 11:20 UTC (permalink / raw)
To: =?utf-8?b?QXLEsW7DpyDDnE5BTCB2aWEgQjQgUmVsYXkgPGRldm51bGwrYXJpbmMudW5hbC5h?=,
=?utf-8?b?cmluYzkuY29tQGtlcm5lbC5vcmc+?=
Cc: daniel, dqfext, sean.wang, andrew, f.fainelli, olteanv, davem,
edumazet, kuba, pabeni, matthias.bgg, angelogioacchino.delregno,
frank-w, bartel.eerdekens, mithat.guner, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek, arinc.unal
In-Reply-To: <20240314-b4-for-net-mt7530-fix-link-local-vlan-v2-0-7dbcf6429ba0@arinc9.com>
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Thu, 14 Mar 2024 12:33:40 +0300 you wrote:
> Hi.
>
> This patch series fixes the VLAN tag egress procedure for link-local
> frames, and fixes handling of all link-local frames.
>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>
> [...]
Here is the summary with links:
- [net,v2,1/2] net: dsa: mt7530: fix link-local frames that ingress vlan filtering ports
https://git.kernel.org/netdev/net/c/e8bf353577f3
- [net,v2,2/2] net: dsa: mt7530: fix handling of all link-local frames
https://git.kernel.org/netdev/net/c/69ddba9d170b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net v2 0/2] MT7530 DSA subdriver fix VLAN egress and handling of all link-local frames
From: Paolo Abeni @ 2024-03-21 11:18 UTC (permalink / raw)
To: Arınç ÜNAL, Daniel Golle, DENG Qingfang, Sean Wang,
Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: Frank Wunderlich, Bartel Eerdekens, mithat.guner, erkin.bozoglu,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <f6f064b8-efb2-4ab0-94f1-468d5d273d6e@arinc9.com>
On Wed, 2024-03-20 at 19:41 +0300, Arınç ÜNAL wrote:
> On 14.03.2024 12:33, Arınç ÜNAL via B4 Relay wrote:
> > Hi.
> >
> > This patch series fixes the VLAN tag egress procedure for link-local
> > frames, and fixes handling of all link-local frames.
> >
> > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
For future memory: I think the SoB in the cover letter is not required,
and is, at least to me, a bit confusing.
No action needed here, I'll keep this unmodified, but I suggest to omit
it in future submission.
Thanks!
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 5/5] mm: support large folios swapin as a whole
From: Ryan Roberts @ 2024-03-21 11:13 UTC (permalink / raw)
To: Barry Song
Cc: Huang, Ying, Matthew Wilcox, akpm, linux-mm, chengming.zhou,
chrisl, david, hannes, kasong, linux-arm-kernel, linux-kernel,
mhocko, nphamcs, shy828301, steven.price, surenb, wangkefeng.wang,
xiang, yosryahmed, yuzhao, Chuanhua Han, Barry Song
In-Reply-To: <CAGsJ_4x5UMM9XFJWBERZssBtJosEKZrP5SxygKJ+fq=BG0Hh0g@mail.gmail.com>
On 21/03/2024 09:22, Barry Song wrote:
> On Tue, Mar 19, 2024 at 10:05 PM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 19/03/2024 06:27, Barry Song wrote:
>>> On Tue, Mar 19, 2024 at 5:45 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>>
>>>>>>> I agree phones are not the only platform. But Rome wasn't built in a
>>>>>>> day. I can only get
>>>>>>> started on a hardware which I can easily reach and have enough hardware/test
>>>>>>> resources on it. So we may take the first step which can be applied on
>>>>>>> a real product
>>>>>>> and improve its performance, and step by step, we broaden it and make it
>>>>>>> widely useful to various areas in which I can't reach :-)
>>>>>>
>>>>>> We must guarantee the normal swap path runs correctly and has no
>>>>>> performance regression when developing SWP_SYNCHRONOUS_IO optimization.
>>>>>> So we have to put some effort on the normal path test anyway.
>>>>>>
>>>>>>> so probably we can have a sysfs "enable" entry with default "n" or
>>>>>>> have a maximum
>>>>>>> swap-in order as Ryan's suggestion [1] at the beginning,
>>>>>>>
>>>>>>> "
>>>>>>> So in the common case, swap-in will pull in the same size of folio as was
>>>>>>> swapped-out. Is that definitely the right policy for all folio sizes? Certainly
>>>>>>> it makes sense for "small" large folios (e.g. up to 64K IMHO). But I'm not sure
>>>>>>> it makes sense for 2M THP; As the size increases the chances of actually needing
>>>>>>> all of the folio reduces so chances are we are wasting IO. There are similar
>>>>>>> arguments for CoW, where we currently copy 1 page per fault - it probably makes
>>>>>>> sense to copy the whole folio up to a certain size.
>>>>>>> "
>>>>
>>>> I thought about this a bit more. No clear conclusions, but hoped this might help
>>>> the discussion around policy:
>>>>
>>>> The decision about the size of the THP is made at first fault, with some help
>>>> from user space and in future we might make decisions to split based on
>>>> munmap/mremap/etc hints. In an ideal world, the fact that we have had to swap
>>>> the THP out at some point in its lifetime should not impact on its size. It's
>>>> just being moved around in the system and the reason for our original decision
>>>> should still hold.
>>>
>>> Indeed, this is an ideal framework for smartphones and likely for
>>> widely embedded
>>> Linux systems utilizing zRAM. We set the mTHP size to 64KiB to
>>> leverage CONT-PTE,
>>> given that more than half of the memory on phones may frequently swap out and
>>> swap in (for instance, when opening and switching between apps). The
>>> ideal approach
>>> would involve adhering to the decision made in do_anonymous_page().
>>>
>>>>
>>>> So from that PoV, it would be good to swap-in to the same size that was
>>>> swapped-out. But we only kind-of keep that information around, via the swap
>>>> entry contiguity and alignment. With that scheme it is possible that multiple
>>>> virtually adjacent but not physically contiguous folios get swapped-out to
>>>> adjacent swap slot ranges and then they would be swapped-in to a single, larger
>>>> folio. This is not ideal, and I think it would be valuable to try to maintain
>>>> the original folio size information with the swap slot. One way to do this would
>>>> be to store the original order for which the cluster was allocated in the
>>>> cluster. Then we at least know that a given swap slot is either for a folio of
>>>> that order or an order-0 folio (due to cluster exhaustion/scanning). Can we
>>>> steal a bit from swap_map to determine which case it is? Or are there better
>>>> approaches?
>>>
>>> In the case of non-SWP_SYNCHRONOUS_IO, users will invariably invoke
>>> swap_readahead()
>>> even when __swap_count(entry) equals 1. This leads to two scenarios:
>>> swap_vma_readahead
>>> and swap_cluster_readahead.
>>>
>>> In swap_vma_readahead, when blk_queue_nonrot, physical contiguity
>>> doesn't appear to be a
>>> critical concern. However, for swap_cluster_readahead, the focus
>>> shifts towards the potential
>>> impact of physical discontiguity.
>>
>> When you talk about "physical [dis]contiguity" I think you are talking about
>> contiguity of the swap entries in the swap device? Both paths currently allocate
>> order-0 folios to swap into, so neither have a concept of physical contiguity in
>> memory at the moment.
>>
>> As I understand it, roughly the aim is to readahead by cluster for rotating
>> disks to reduce seek time, and readahead by virtual address for non-rotating
>> devices since there is no seek time cost. Correct?
>
> From the code comment, I agree with this.
>
> * It's a main entry function for swap readahead. By the configuration,
> * it will read ahead blocks by cluster-based(ie, physical disk based)
> * or vma-based(ie, virtual address based on faulty address) readahead.
>
>>
>> Note that today, swap-out on supports (2M) THP if the swap device is
>> non-rotating. If it is rotating, the THP is first split. My swap-out series
>> maintains this policy for mTHP. So I think we only really care about
>> swap_vma_readahead() here; we want to teach it to figure out the order of the
>> swap entries and swap them into folios of the same order (with a fallback to
>> order-0 if allocation fails).
>
> I agree we don't need to care about devices which rotate.
>
>>
>>>
>>> struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
>>> struct vm_fault *vmf)
>>> {
>>> struct mempolicy *mpol;
>>> pgoff_t ilx;
>>> struct folio *folio;
>>>
>>> mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
>>> folio = swap_use_vma_readahead() ?
>>> swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
>>> swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
>>> mpol_cond_put(mpol);
>>>
>>> if (!folio)
>>> return NULL;
>>> return folio_file_page(folio, swp_offset(entry));
>>> }
>>>
>>> In Android and embedded systems, SWP_SYNCHRONOUS_IO is consistently utilized,
>>> rendering physical contiguity less of a concern. Moreover, instances where
>>> swap_readahead() is accessed are rare, typically occurring only in scenarios
>>> involving forked but non-CoWed memory.
>>
>> Yes understood. What I'm hearing is that for Android at least, stealing a bit
>> from swap_map to remember if a swap entry is the order marked in the cluster or
>> order-0 won't be noticed because almost all entries have swap count == 1. From
>> memory, I think swap_map is 8 bits, and 2 bits are currently stolen, leaving 6
>> bits (count = 64) before having to move to the swap map continuation stuff. Does
>> anyone know what workloads provoke this overflow? What are the consequences of
>> reducing that count to 32?
>
> I'm not entirely clear on why you need bits to record this
> information. Could you
> provide more details?
For norot media, the swap device is carved up into "clusters", each the same
size as the pmd-size. Empty clusters are kept on a free list. Each CPU maintains
a "current cluster" that it fills sequentially with order-0 entries. To swap out
a THP, a whole cluster is allocated from the free list and used. (My swap-out
series expands this so that each CPU maintains a current cluster per-order to
fill sequentially with swap entries of that order). Once a cluster has been
filled, the CPU allocates a new current cluster. If no clusters are available,
only order-0 swap out will succeed, and in that case it will scan through the
entire swap device looking for a free slot. Once swapped-out, there is no
maintained information to tell you, for a given swap slot, what the originating
folio size was. We always swap-in in order-0 units, so it doesn't matter.
A side effect of this is that a page from what was a PMD-sized THP could be
swapped back in, meaning there is a free order-0 swap slot in that cluster. That
could then be allocated by the scanner, so the cluster now holds part of what
was a THP and an order-0 page. With my swap-out series, that mixing of orders
within a cluster becomes even more likely; (e.g.) we could have a current
cluster (2M) for order-4 (64K) and have swapped-out 2 folios, using up the first
128K). Then we try to swap out an order-0 and have to fall back to the scanner
because swap is fragmented. The scanner then puts the order-0 in the swap entry
after the second order-4 folio; the cluster now has a mix of order-4 and order-0
folios.
If we want to store the original folio size in the swap device, I was suggesting
that one approach would be to store the "primary" order of the cluster in the
cluster info struct. But that is not sufficient on its own, because there can
also be order-0 entries in the same cluster (due to either of the 2 mechanisms I
described above - either a page from a large order entry was freed, meaning all
the other pages in the large swap entry become order-0, or an order-0 was added
to the cluster by the scanner). So to discriminate these 2 possibilities on a
per-swap entry basis (either the swap entry is large and of the order indicated
by the cluster, or it is small), we need a per-swap entry bit.
swap_map is the the per-swap entry state that is currently maintained, and it is
(in the common case) 8 bits. 1 bit is used to remember if the swap entry has a
folio in the swap cache. And another bit is used for something relating to shmem
IIRC). The rest of the bits are used to count references to the swap entry (its
a bit more complicated but you get the idea). I was wondering if we could steal
a bit from the reference count to use for discrimanation between "swap entry is
size indicated by cluster's primary order" and "swap entry is order-0".
>
>>
>>>
>>> So I think large folios swap-in will at least need three steps
>>>
>>> 1. on SWP_SYNCHRONOUS_IO (Android and embedded Linux), this has a very
>>> clear model and has no complex I/O issue.
>>> 2. on nonrot block device(bdev_nonrot == true), it cares less about
>>> I/O contiguity.
>>> 3. on rot block devices which care about I/O contiguity.
>>
>> I don't think we care about (3); if the device rotates, we will have split the
>> folio at swap-out, so we are only concerned with swapping-in order-0 folios.
>>
>>>
>>> This patchset primarily addresses the systems utilizing
>>> SWP_SYNCHRONOUS_IO(type1),
>>> such as Android and embedded Linux, a straightforward model is established,
>>> with minimal complexity regarding I/O issues.
>>
>> Understood. But your implication is that making swap_vma_readahead() large folio
>> swap-in aware will be complex. I think we can remember the original order in the
>> swap device, then it shouldn't be too difficult - conceptually at least.
>
> Currently, I can scan PTE entries and determine the number of
> contiguous swap offsets.
That's an approximation; if the folio was mapped into multiple processes and
partially mapped in the one you are swapping in for. But perhaps that is
sufficiently uncommon that it doesn't matter? It certainly removes the need for
storing the precise information in the swap device as I described above. To be
honest, I hadn't considered the PTEs; I was thinking only about the swap slot
contiguity.
> The swap_vma_readahead code to support large folios already exists in
> OPPO's repository.
> I'm confident that it can be cleaned up and submitted to LKML.
> However, the issue lies with
> the readahead policy. We typically prefer using the same 64KiB size as in
> do_anonymous_page(), but clearly, this isn't the preference for Ying :-)
I haven't caught up on all the latest discssion (although I see a bunch of
unread emails in my inbox :) ). But my view at the moment is roughly;
- continue to calculate readahead as before
- always swap-in folios in their original size
- always swap-in the full faulting folio
- calculate which other folios to readahead by rounding the calculated
readahead endpoint to the nearest folio boundary.
- Or if the "round to nearest" policy is shown to be problematic, introduce a
"max swap-in folio size" tunable and round to that nearest boundary instead.
If we need the tunable, a default of 4K is same as current behaviour. 64K would
likely be a good setting for a system using contpte mappings. And 2M would
provide behaviour descibed above without the tubable.
>
>>
>>>
>>>>
>>>> Next we (I?) have concerns about wasting IO by swapping-in folios that are too
>>>> large (e.g. 2M). I'm not sure if this is a real problem or not - intuitively I'd
>>>> say yes but I have no data. But on the other hand, memory is aged and
>>>> swapped-out per-folio, so why shouldn't it be swapped-in per folio? If the
>>>> original allocation size policy is good (it currently isn't) then a folio should
>>>> be sized to cover temporally close memory and if we need to access some of it,
>>>> chances are we need all of it.
>>>>
>>>> If we think the IO concern is legitimate then we could define a threshold size
>>>> (sysfs?) for when we start swapping-in the folio in chunks. And how big should
>>>> those chunks be - one page, or the threshold size itself? Probably the latter?
>>>> And perhaps that threshold could also be used by zRAM to decide its upper limit
>>>> for compression chunk.
>>>
>>>
>>> Agreed. What about introducing a parameter like
>>> /sys/kernel/mm/transparent_hugepage/max_swapin_order
>>> giving users the opportunity to fine-tune it according to their needs. For type1
>>> users specifically, setting it to any value above 4 would be
>>> beneficial. If there's
>>> still a lack of tuning for desktop and server environments (type 2 and type 3),
>>> the default value could be set to 0.
>>
>> This sort of thing sounds sensible to me. But I have a history of proposing
>> crappy sysfs interfaces :) So I'd like to hear from others - I suspect it will
>> take a fair bit of discussion before we converge. Having data to show that this
>> threshold is needed would also help (i.e. demonstration that the intuition that
>> swapping in a 2M folio is often counter-productive to performance).
>>
>
> I understand. The ideal swap-in size is obviously a contentious topic :-)
> However, for my real use case, simplicity reigns: we consistently adhere
> to a single size - 64KiB.
>
>>>
>>>>
>>>> Perhaps we can learn from khugepaged here? I think it has programmable
>>>> thresholds for how many swapped-out pages can be swapped-in to aid collapse to a
>>>> THP? I guess that exists for the same concerns about increased IO pressure?
>>>>
>>>>
>>>> If we think we will ever be swapping-in folios in chunks less than their
>>>> original size, then we need a separate mechanism to re-foliate them. We have
>>>> discussed a khugepaged-like approach for doing this asynchronously in the
>>>> background. I know that scares the Android folks, but David has suggested that
>>>> this could well be very cheap compared with khugepaged, because it would be
>>>> entirely limited to a single pgtable, so we only need the PTL. If we need this
>>>> mechanism anyway, perhaps we should develop it and see how it performs if
>>>> swap-in remains order-0? Although I guess that would imply not being able to
>>>> benefit from compressing THPs for the zRAM case.
>>>
>>> The effectiveness of collapse operation relies on the stability of
>>> forming large folios
>>> to ensure optimal performance. In embedded systems, where more than half of the
>>> memory may be allocated to zRAM, folios might undergo swapping out before
>>> collapsing or immediately after the collapse operation. It seems a
>>> TAO-like optimization
>>> to decrease fallback and latency is more effective.
>>
>> Sorry, I'm not sure I've understood what you are saying here.
>
> I'm not entirely clear on the specifics of the khugepaged-like
> approach. However,a major
> distinction for Android is that its folios may not remain in memory
> for extended periods.
> If we incur the cost of compaction and page migration to form a large
> folio, it might soon
> be swapped out. Therefore, a potentially more efficient approach could
> involve a TAO-like
> pool, where we obtain large folios at a low cost.
>
>>
>>>
>>>>
>>>> I see all this as orthogonal to synchronous vs asynchronous swap devices. I
>>>> think the latter just implies that you might want to do some readahead to try to
>>>> cover up the latency? If swap is moving towards being folio-orientated, then
>>>> readahead also surely needs to be folio-orientated, but I think that should be
>>>> the only major difference.
>>>>
>>>> Anyway, just some thoughts!
>>>
>>> Thank you very much for your valuable and insightful deliberations.
>>>
>>>>
>>>> Thanks,
>>>> Ryan
>>>>
>>>
>
> Thanks
> Barry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 10/11] pinctrl: pinctrl-tps6594: Add TPS65224 PMIC pinctrl and GPIO
From: Julien Panis @ 2024-03-21 11:10 UTC (permalink / raw)
To: Bhargav Raviprakash, linux-kernel
Cc: m.nirmaladevi, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
devicetree, arnd, gregkh, lgirdwood, broonie, linus.walleij,
linux-gpio, linux-arm-kernel, nm, vigneshr, kristo, eblanc
In-Reply-To: <20240320102559.464981-11-bhargav.r@ltts.com>
On 3/20/24 11:25, Bhargav Raviprakash wrote:
> From: Nirmala Devi Mal Nadar <m.nirmaladevi@ltts.com>
>
> Add support for TPS65224 pinctrl and GPIOs to TPS6594 driver as they have
> significant functional overlap.
> TPS65224 PMIC has 6 GPIOS which can be configured as GPIO or other
> dedicated device functions.
>
> Signed-off-by: Nirmala Devi Mal Nadar <m.nirmaladevi@ltts.com>
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
With this patch, an issue is observed on am62a:
root@am62axx-evm:~# dmesg | grep tps
...
[ 12.122631] tps6594-pinctrl tps6594-pinctrl.2.auto: error -EINVAL: Couldn't register gpio_regmap
driver
[ 12.133216] tps6594-pinctrl: probe of tps6594-pinctrl.2.auto failed with error -22
Without this patch, the issue disappears. Do you observe
the same result with your am62p ?
Julien Panis
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] irqchip/gic-v3-its: Fix double free on error
From: Guanrui Huang @ 2024-03-21 11:04 UTC (permalink / raw)
To: maz; +Cc: shannon.zhao, tglx, linux-arm-kernel, linux-kernel, Guanrui Huang
In its_vpe_irq_domain_alloc, when its_vpe_init() returns an error
with i > 0, its_vpe_irq_domain_free may free bitmap and vprop_page,
and then there is a double free in its_vpe_irq_domain_alloc.
Fix it by checking if bitmap is empty in its_vpe_irq_domain_alloc.
If bitmap is empty and i > 0, means that bitmap have been clear and free
in its_vpe_irq_domain_free.
Signed-off-by: Guanrui Huang <guanrui.huang@linux.alibaba.com>
---
drivers/irqchip/irq-gic-v3-its.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fca888b36680..98a1be90caef 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4562,9 +4562,14 @@ static int its_vpe_irq_domain_alloc(struct irq_domain *domain, unsigned int virq
}
if (err) {
- if (i > 0)
+ if (i > 0) {
its_vpe_irq_domain_free(domain, virq, i);
+ /* bitmap and vprop_page be freed in its_vpe_irq_domain_free */
+ if (bitmap_empty(bitmap, nr_ids))
+ return err;
+ }
+
its_lpi_free(bitmap, base, nr_ids);
its_free_prop_table(vprop_page);
}
--
2.36.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5 19/27] iommu/arm-smmu-v3: Keep track of arm_smmu_master_domain for SVA
From: Michael Shavit @ 2024-03-21 10:47 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: iommu, Joerg Roedel, linux-arm-kernel, Robin Murphy, Will Deacon,
Eric Auger, Jean-Philippe Brucker, Moritz Fischer, Nicolin Chen,
patches, Shameerali Kolothum Thodi, Daniel Mentz
In-Reply-To: <19-v5-9a37e0c884ce+31e3-smmuv3_newapi_p2_jgg@nvidia.com>
On Tue, Mar 5, 2024 at 7:44 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> Currently the smmu_domain->devices list is unused for SVA domains.
> Fill it in with the SSID and master of every arm_smmu_set_pasid()
> using the same logic as the RID attach.
>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 23 +++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 2db2b822292a87..6d15fe3a160acf 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2603,7 +2603,8 @@ to_smmu_domain_devices(struct iommu_domain *domain)
> /* The domain can be NULL only when processing the first attach */
> if (!domain)
> return NULL;
> - if (domain->type & __IOMMU_DOMAIN_PAGING)
> + if ((domain->type & __IOMMU_DOMAIN_PAGING) ||
> + domain->type == IOMMU_DOMAIN_SVA)
> return to_smmu_domain(domain);
> return NULL;
> }
> @@ -2813,7 +2814,9 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
> struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
> const struct arm_smmu_cd *cd)
> {
> + struct attach_state state = {.ssid = pasid};
> struct arm_smmu_cd *cdptr;
> + int ret;
>
> if (!arm_smmu_is_s1_domain(iommu_get_domain_for_dev(master->dev)))
> return -ENODEV;
> @@ -2821,14 +2824,30 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
> cdptr = arm_smmu_get_cd_ptr(master, pasid);
> if (!cdptr)
> return -ENOMEM;
> +
> + mutex_lock(&arm_smmu_asid_lock);
> + ret = arm_smmu_attach_prepare(master, &smmu_domain->domain, &state);
> + if (ret)
> + goto out_unlock;
> +
> arm_smmu_write_cd_entry(master, pasid, cdptr, cd);
> - return 0;
> +
> + arm_smmu_attach_commit(master, &state);
> +
> +out_unlock:
> + mutex_unlock(&arm_smmu_asid_lock);
> + return ret;
> }
arm_smmu_attach_commit tries to remove the master_domain entry from
the previous domain that this master was attached to. It gets a
pointer to the previous domain from the iommu framework with
iommu_get_domain_for_dev().
But in this path, arm_smmu_attach_prepare is creating a master_domain
entry for the pasid domain, which may be different from the one
returned by iommu_get_domain_for_dev() on the next attach.
I think this ended up being safe in the end because afaict the iommu
framework requires detaching the previous pasid domain before
attaching a new one. But nonetheless this is pretty fragile and
doesn't look intentional.
>
> void arm_smmu_remove_pasid(struct arm_smmu_master *master,
> struct arm_smmu_domain *smmu_domain, ioasid_t pasid)
> {
> + mutex_lock(&arm_smmu_asid_lock);
> arm_smmu_clear_cd(master, pasid);
> + if (master->ats_enabled)
> + arm_smmu_atc_inv_master(master, pasid);
> + arm_smmu_remove_master_domain(master, &smmu_domain->domain, pasid);
> + mutex_unlock(&arm_smmu_asid_lock);
> }
>
> static int arm_smmu_attach_dev_ste(struct iommu_domain *domain,
> --
> 2.43.2
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: Barry Song @ 2024-03-21 10:43 UTC (permalink / raw)
To: Ryan Roberts
Cc: akpm, linux-mm, chengming.zhou, chrisl, david, hannes, kasong,
linux-arm-kernel, linux-kernel, mhocko, nphamcs, shy828301,
steven.price, surenb, wangkefeng.wang, willy, xiang, ying.huang,
yosryahmed, yuzhao, Barry Song, Catalin Marinas, Will Deacon,
Mark Rutland, Kemeng Shi, Anshuman Khandual, Peter Collingbourne,
Peter Xu, Lorenzo Stoakes, Mike Rapoport (IBM), Hugh Dickins,
Aneesh Kumar K.V, Rick Edgecombe
In-Reply-To: <e77c18df-2796-4fd1-b589-57f0569587e8@arm.com>
On Thu, Mar 21, 2024 at 6:31 PM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 21/03/2024 08:42, Barry Song wrote:
> > Hi Ryan,
> > Sorry for the late reply.
>
> No problem!
>
> >
> > On Tue, Mar 12, 2024 at 5:56 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
> >>
> >> On 04/03/2024 08:13, Barry Song wrote:
> >>> From: Barry Song <v-songbaohua@oppo.com>
> >>>
> >>> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
> >>> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
> >>> MTE as the MTE code works with the assumption tags save/restore is
> >>> always handling a folio with only one page.
> >>>
> >>> The limitation should be removed as more and more ARM64 SoCs have
> >>> this feature. Co-existence of MTE and THP_SWAP becomes more and
> >>> more important.
> >>>
> >>> This patch makes MTE tags saving support large folios, then we don't
> >>> need to split large folios into base pages for swapping out on ARM64
> >>> SoCs with MTE any more.
> >>>
> >>> arch_prepare_to_swap() should take folio rather than page as parameter
> >>> because we support THP swap-out as a whole. It saves tags for all
> >>> pages in a large folio.
> >>>
> >>> As now we are restoring tags based-on folio, in arch_swap_restore(),
> >>> we may increase some extra loops and early-exitings while refaulting
> >>> a large folio which is still in swapcache in do_swap_page(). In case
> >>> a large folio has nr pages, do_swap_page() will only set the PTE of
> >>> the particular page which is causing the page fault.
> >>> Thus do_swap_page() runs nr times, and each time, arch_swap_restore()
> >>> will loop nr times for those subpages in the folio. So right now the
> >>> algorithmic complexity becomes O(nr^2).
> >>>
> >>> Once we support mapping large folios in do_swap_page(), extra loops
> >>> and early-exitings will decrease while not being completely removed
> >>> as a large folio might get partially tagged in corner cases such as,
> >>> 1. a large folio in swapcache can be partially unmapped, thus, MTE
> >>> tags for the unmapped pages will be invalidated;
> >>> 2. users might use mprotect() to set MTEs on a part of a large folio.
> >>>
> >>> arch_thp_swp_supported() is dropped since ARM64 MTE was the only one
> >>> who needed it.
>
> I think we should decouple this patch from your swap-in series. I suspect this
> one could be ready and go in sooner than the swap-in series based on the current
> discussions :)
I concur, particularly given that nowadays, most modern and popular ARM64
SoCs are equipped with MTE. The absence of this patch also hinders the
effective functioning of mTHP swap-out.
>
> >>>
> >>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>> Cc: Will Deacon <will@kernel.org>
> >>> Cc: Ryan Roberts <ryan.roberts@arm.com>
> >>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>> Cc: David Hildenbrand <david@redhat.com>
> >>> Cc: Kemeng Shi <shikemeng@huaweicloud.com>
> >>> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> >>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> >>> Cc: Peter Collingbourne <pcc@google.com>
> >>> Cc: Steven Price <steven.price@arm.com>
> >>> Cc: Yosry Ahmed <yosryahmed@google.com>
> >>> Cc: Peter Xu <peterx@redhat.com>
> >>> Cc: Lorenzo Stoakes <lstoakes@gmail.com>
> >>> Cc: "Mike Rapoport (IBM)" <rppt@kernel.org>
> >>> Cc: Hugh Dickins <hughd@google.com>
> >>> CC: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> >>> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> >>> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> >>> Reviewed-by: Steven Price <steven.price@arm.com>
> >>> Acked-by: Chris Li <chrisl@kernel.org>
> >>> ---
> >>> arch/arm64/include/asm/pgtable.h | 19 ++------------
> >>> arch/arm64/mm/mteswap.c | 43 ++++++++++++++++++++++++++++++++
> >>> include/linux/huge_mm.h | 12 ---------
> >>> include/linux/pgtable.h | 2 +-
> >>> mm/page_io.c | 2 +-
> >>> mm/swap_slots.c | 2 +-
> >>> 6 files changed, 48 insertions(+), 32 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> >>> index 401087e8a43d..7a54750770b8 100644
> >>> --- a/arch/arm64/include/asm/pgtable.h
> >>> +++ b/arch/arm64/include/asm/pgtable.h
> >>> @@ -45,12 +45,6 @@
> >>> __flush_tlb_range(vma, addr, end, PUD_SIZE, false, 1)
> >>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> >>>
> >>> -static inline bool arch_thp_swp_supported(void)
> >>> -{
> >>> - return !system_supports_mte();
> >>> -}
> >>> -#define arch_thp_swp_supported arch_thp_swp_supported
> >>> -
> >>> /*
> >>> * Outside of a few very special situations (e.g. hibernation), we always
> >>> * use broadcast TLB invalidation instructions, therefore a spurious page
> >>> @@ -1095,12 +1089,7 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
> >>> #ifdef CONFIG_ARM64_MTE
> >>>
> >>> #define __HAVE_ARCH_PREPARE_TO_SWAP
> >>> -static inline int arch_prepare_to_swap(struct page *page)
> >>> -{
> >>> - if (system_supports_mte())
> >>> - return mte_save_tags(page);
> >>> - return 0;
> >>> -}
> >>> +extern int arch_prepare_to_swap(struct folio *folio);
> >>>
> >>> #define __HAVE_ARCH_SWAP_INVALIDATE
> >>> static inline void arch_swap_invalidate_page(int type, pgoff_t offset)
> >>> @@ -1116,11 +1105,7 @@ static inline void arch_swap_invalidate_area(int type)
> >>> }
> >>>
> >>> #define __HAVE_ARCH_SWAP_RESTORE
> >>> -static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
> >>> -{
> >>> - if (system_supports_mte())
> >>> - mte_restore_tags(entry, &folio->page);
> >>> -}
> >>> +extern void arch_swap_restore(swp_entry_t entry, struct folio *folio);
> >>>
> >>> #endif /* CONFIG_ARM64_MTE */
> >>>
> >>> diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
> >>> index a31833e3ddc5..295836fef620 100644
> >>> --- a/arch/arm64/mm/mteswap.c
> >>> +++ b/arch/arm64/mm/mteswap.c
> >>> @@ -68,6 +68,13 @@ void mte_invalidate_tags(int type, pgoff_t offset)
> >>> mte_free_tag_storage(tags);
> >>> }
> >>>
> >>> +static inline void __mte_invalidate_tags(struct page *page)
> >>> +{
> >>> + swp_entry_t entry = page_swap_entry(page);
> >>> +
> >>> + mte_invalidate_tags(swp_type(entry), swp_offset(entry));
> >>> +}
> >>> +
> >>> void mte_invalidate_tags_area(int type)
> >>> {
> >>> swp_entry_t entry = swp_entry(type, 0);
> >>> @@ -83,3 +90,39 @@ void mte_invalidate_tags_area(int type)
> >>> }
> >>> xa_unlock(&mte_pages);
> >>> }
> >>> +
> >>> +int arch_prepare_to_swap(struct folio *folio)
> >>> +{
> >>> + long i, nr;
> >>> + int err;
> >>> +
> >>> + if (!system_supports_mte())
> >>> + return 0;
> >>> +
> >>> + nr = folio_nr_pages(folio);
> >>> +
> >>> + for (i = 0; i < nr; i++) {
> >>> + err = mte_save_tags(folio_page(folio, i));
> >>> + if (err)
> >>> + goto out;
> >>> + }
> >>> + return 0;
> >>> +
> >>> +out:
> >>> + while (i--)
> >>> + __mte_invalidate_tags(folio_page(folio, i));
> >>> + return err;
> >>> +}
> >>> +
> >>> +void arch_swap_restore(swp_entry_t entry, struct folio *folio)
> >>
> >> I'm still not a fan of the fact that entry could be anywhere within folio.
> >>
> >>> +{
> >>> + if (system_supports_mte()) {
> >>
> >> nit: if you do:
> >>
> >> if (!system_supports_mte())
> >> return;
> >
> > Acked
> >
> >>
> >> It will be consistent with arch_prepare_to_swap() and reduce the indentation of
> >> the main body.
> >>
> >>> + long i, nr = folio_nr_pages(folio);
> >>> +
> >>> + entry.val -= swp_offset(entry) & (nr - 1);
> >>
> >> This assumes that folios are always stored in swap with natural alignment. Is
> >> that definitely a safe assumption? My swap-out series is currently ensuring that
> >> folios are swapped-out naturally aligned, but that is an implementation detail.
> >>
> >
> > I concur that this is an implementation detail. However, we should be
> > bold enough
> > to state that swap slots will be contiguous, considering we are
> > currently utilizing
> > folio->swap instead of subpage->swap ?
>
> Yes, I agree about contiguity. My objection is about assuming natural alignment
> though. It can still be contiguous while not naturally aligned in swap.
right.
>
> >
> >> Your cover note for swap-in says that you could technically swap in a large
> >> folio without it having been swapped-out large. If you chose to do that in
> >> future, this would break, right? I don't think it's good to couple the swap
> >
> > Right. technically I agree. Given that we still have many tasks involving even
> > swapping in contiguous swap slots, it's unlikely that swapping in large folios
> > for non-contiguous entries will occur in the foreseeable future :-)
> >
> >> storage layout to the folio order that you want to swap into. Perhaps that's an
> >> argument for passing each *page* to this function with its exact, corresponding
> >> swap entry?
> >
> > I recall Matthew Wilcox strongly objected to using "page" as the
> > parameter, so I've
> > discarded that approach. Alternatively, it appears I can consistently pass
> > folio->swap to this function and ensure the function always retrieves
> > the first entry?
>
> Yes, if we must pass a folio here, I'd prefer that entry always corresponds to
> the first entry for the folio. That will remove the need for this function to do
> the alignment above too. So win-win.
right.
>
> >
> >>
> >>> + for (i = 0; i < nr; i++) {
> >>> + mte_restore_tags(entry, folio_page(folio, i));
> >>> + entry.val++;
> >>> + }
> >>> + }
> >>> +}
> >>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> >>> index de0c89105076..e04b93c43965 100644
> >>> --- a/include/linux/huge_mm.h
> >>> +++ b/include/linux/huge_mm.h
> >>> @@ -535,16 +535,4 @@ static inline int split_folio_to_order(struct folio *folio, int new_order)
> >>> #define split_folio_to_list(f, l) split_folio_to_list_to_order(f, l, 0)
> >>> #define split_folio(f) split_folio_to_order(f, 0)
> >>>
> >>> -/*
> >>> - * archs that select ARCH_WANTS_THP_SWAP but don't support THP_SWP due to
> >>> - * limitations in the implementation like arm64 MTE can override this to
> >>> - * false
> >>> - */
> >>> -#ifndef arch_thp_swp_supported
> >>> -static inline bool arch_thp_swp_supported(void)
> >>> -{
> >>> - return true;
> >>> -}
> >>> -#endif
> >>> -
> >>> #endif /* _LINUX_HUGE_MM_H */
> >>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> >>> index e1b22903f709..bfcfe3386934 100644
> >>> --- a/include/linux/pgtable.h
> >>> +++ b/include/linux/pgtable.h
> >>> @@ -1106,7 +1106,7 @@ static inline int arch_unmap_one(struct mm_struct *mm,
> >>> * prototypes must be defined in the arch-specific asm/pgtable.h file.
> >>> */
> >>> #ifndef __HAVE_ARCH_PREPARE_TO_SWAP
> >>> -static inline int arch_prepare_to_swap(struct page *page)
> >>> +static inline int arch_prepare_to_swap(struct folio *folio)
> >>> {
> >>> return 0;
> >>> }
> >>> diff --git a/mm/page_io.c b/mm/page_io.c
> >>> index ae2b49055e43..a9a7c236aecc 100644
> >>> --- a/mm/page_io.c
> >>> +++ b/mm/page_io.c
> >>> @@ -189,7 +189,7 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
> >>> * Arch code may have to preserve more data than just the page
> >>> * contents, e.g. memory tags.
> >>> */
> >>> - ret = arch_prepare_to_swap(&folio->page);
> >>> + ret = arch_prepare_to_swap(folio);
> >>> if (ret) {
> >>> folio_mark_dirty(folio);
> >>> folio_unlock(folio);
> >>> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> >>> index 90973ce7881d..53abeaf1371d 100644
> >>> --- a/mm/swap_slots.c
> >>> +++ b/mm/swap_slots.c
> >>> @@ -310,7 +310,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio)
> >>> entry.val = 0;
> >>>
> >>> if (folio_test_large(folio)) {
> >>> - if (IS_ENABLED(CONFIG_THP_SWAP) && arch_thp_swp_supported())
> >>> + if (IS_ENABLED(CONFIG_THP_SWAP))
> >>> get_swap_pages(1, &entry, folio_nr_pages(folio));
> >>> goto out;
> >>> }
> >>
> >
Thanks
Barry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: Ryan Roberts @ 2024-03-21 10:31 UTC (permalink / raw)
To: Barry Song
Cc: akpm, linux-mm, chengming.zhou, chrisl, david, hannes, kasong,
linux-arm-kernel, linux-kernel, mhocko, nphamcs, shy828301,
steven.price, surenb, wangkefeng.wang, willy, xiang, ying.huang,
yosryahmed, yuzhao, Barry Song, Catalin Marinas, Will Deacon,
Mark Rutland, Kemeng Shi, Anshuman Khandual, Peter Collingbourne,
Peter Xu, Lorenzo Stoakes, Mike Rapoport (IBM), Hugh Dickins,
Aneesh Kumar K.V, Rick Edgecombe
In-Reply-To: <CAGsJ_4z5xDtBj-nyvPXfgv+ZVmzPQzvptyBrf=kB49Q3=Dmq-Q@mail.gmail.com>
On 21/03/2024 08:42, Barry Song wrote:
> Hi Ryan,
> Sorry for the late reply.
No problem!
>
> On Tue, Mar 12, 2024 at 5:56 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 04/03/2024 08:13, Barry Song wrote:
>>> From: Barry Song <v-songbaohua@oppo.com>
>>>
>>> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
>>> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
>>> MTE as the MTE code works with the assumption tags save/restore is
>>> always handling a folio with only one page.
>>>
>>> The limitation should be removed as more and more ARM64 SoCs have
>>> this feature. Co-existence of MTE and THP_SWAP becomes more and
>>> more important.
>>>
>>> This patch makes MTE tags saving support large folios, then we don't
>>> need to split large folios into base pages for swapping out on ARM64
>>> SoCs with MTE any more.
>>>
>>> arch_prepare_to_swap() should take folio rather than page as parameter
>>> because we support THP swap-out as a whole. It saves tags for all
>>> pages in a large folio.
>>>
>>> As now we are restoring tags based-on folio, in arch_swap_restore(),
>>> we may increase some extra loops and early-exitings while refaulting
>>> a large folio which is still in swapcache in do_swap_page(). In case
>>> a large folio has nr pages, do_swap_page() will only set the PTE of
>>> the particular page which is causing the page fault.
>>> Thus do_swap_page() runs nr times, and each time, arch_swap_restore()
>>> will loop nr times for those subpages in the folio. So right now the
>>> algorithmic complexity becomes O(nr^2).
>>>
>>> Once we support mapping large folios in do_swap_page(), extra loops
>>> and early-exitings will decrease while not being completely removed
>>> as a large folio might get partially tagged in corner cases such as,
>>> 1. a large folio in swapcache can be partially unmapped, thus, MTE
>>> tags for the unmapped pages will be invalidated;
>>> 2. users might use mprotect() to set MTEs on a part of a large folio.
>>>
>>> arch_thp_swp_supported() is dropped since ARM64 MTE was the only one
>>> who needed it.
I think we should decouple this patch from your swap-in series. I suspect this
one could be ready and go in sooner than the swap-in series based on the current
discussions :)
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Kemeng Shi <shikemeng@huaweicloud.com>
>>> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>>> Cc: Peter Collingbourne <pcc@google.com>
>>> Cc: Steven Price <steven.price@arm.com>
>>> Cc: Yosry Ahmed <yosryahmed@google.com>
>>> Cc: Peter Xu <peterx@redhat.com>
>>> Cc: Lorenzo Stoakes <lstoakes@gmail.com>
>>> Cc: "Mike Rapoport (IBM)" <rppt@kernel.org>
>>> Cc: Hugh Dickins <hughd@google.com>
>>> CC: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
>>> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
>>> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
>>> Reviewed-by: Steven Price <steven.price@arm.com>
>>> Acked-by: Chris Li <chrisl@kernel.org>
>>> ---
>>> arch/arm64/include/asm/pgtable.h | 19 ++------------
>>> arch/arm64/mm/mteswap.c | 43 ++++++++++++++++++++++++++++++++
>>> include/linux/huge_mm.h | 12 ---------
>>> include/linux/pgtable.h | 2 +-
>>> mm/page_io.c | 2 +-
>>> mm/swap_slots.c | 2 +-
>>> 6 files changed, 48 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>>> index 401087e8a43d..7a54750770b8 100644
>>> --- a/arch/arm64/include/asm/pgtable.h
>>> +++ b/arch/arm64/include/asm/pgtable.h
>>> @@ -45,12 +45,6 @@
>>> __flush_tlb_range(vma, addr, end, PUD_SIZE, false, 1)
>>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>>>
>>> -static inline bool arch_thp_swp_supported(void)
>>> -{
>>> - return !system_supports_mte();
>>> -}
>>> -#define arch_thp_swp_supported arch_thp_swp_supported
>>> -
>>> /*
>>> * Outside of a few very special situations (e.g. hibernation), we always
>>> * use broadcast TLB invalidation instructions, therefore a spurious page
>>> @@ -1095,12 +1089,7 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
>>> #ifdef CONFIG_ARM64_MTE
>>>
>>> #define __HAVE_ARCH_PREPARE_TO_SWAP
>>> -static inline int arch_prepare_to_swap(struct page *page)
>>> -{
>>> - if (system_supports_mte())
>>> - return mte_save_tags(page);
>>> - return 0;
>>> -}
>>> +extern int arch_prepare_to_swap(struct folio *folio);
>>>
>>> #define __HAVE_ARCH_SWAP_INVALIDATE
>>> static inline void arch_swap_invalidate_page(int type, pgoff_t offset)
>>> @@ -1116,11 +1105,7 @@ static inline void arch_swap_invalidate_area(int type)
>>> }
>>>
>>> #define __HAVE_ARCH_SWAP_RESTORE
>>> -static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
>>> -{
>>> - if (system_supports_mte())
>>> - mte_restore_tags(entry, &folio->page);
>>> -}
>>> +extern void arch_swap_restore(swp_entry_t entry, struct folio *folio);
>>>
>>> #endif /* CONFIG_ARM64_MTE */
>>>
>>> diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
>>> index a31833e3ddc5..295836fef620 100644
>>> --- a/arch/arm64/mm/mteswap.c
>>> +++ b/arch/arm64/mm/mteswap.c
>>> @@ -68,6 +68,13 @@ void mte_invalidate_tags(int type, pgoff_t offset)
>>> mte_free_tag_storage(tags);
>>> }
>>>
>>> +static inline void __mte_invalidate_tags(struct page *page)
>>> +{
>>> + swp_entry_t entry = page_swap_entry(page);
>>> +
>>> + mte_invalidate_tags(swp_type(entry), swp_offset(entry));
>>> +}
>>> +
>>> void mte_invalidate_tags_area(int type)
>>> {
>>> swp_entry_t entry = swp_entry(type, 0);
>>> @@ -83,3 +90,39 @@ void mte_invalidate_tags_area(int type)
>>> }
>>> xa_unlock(&mte_pages);
>>> }
>>> +
>>> +int arch_prepare_to_swap(struct folio *folio)
>>> +{
>>> + long i, nr;
>>> + int err;
>>> +
>>> + if (!system_supports_mte())
>>> + return 0;
>>> +
>>> + nr = folio_nr_pages(folio);
>>> +
>>> + for (i = 0; i < nr; i++) {
>>> + err = mte_save_tags(folio_page(folio, i));
>>> + if (err)
>>> + goto out;
>>> + }
>>> + return 0;
>>> +
>>> +out:
>>> + while (i--)
>>> + __mte_invalidate_tags(folio_page(folio, i));
>>> + return err;
>>> +}
>>> +
>>> +void arch_swap_restore(swp_entry_t entry, struct folio *folio)
>>
>> I'm still not a fan of the fact that entry could be anywhere within folio.
>>
>>> +{
>>> + if (system_supports_mte()) {
>>
>> nit: if you do:
>>
>> if (!system_supports_mte())
>> return;
>
> Acked
>
>>
>> It will be consistent with arch_prepare_to_swap() and reduce the indentation of
>> the main body.
>>
>>> + long i, nr = folio_nr_pages(folio);
>>> +
>>> + entry.val -= swp_offset(entry) & (nr - 1);
>>
>> This assumes that folios are always stored in swap with natural alignment. Is
>> that definitely a safe assumption? My swap-out series is currently ensuring that
>> folios are swapped-out naturally aligned, but that is an implementation detail.
>>
>
> I concur that this is an implementation detail. However, we should be
> bold enough
> to state that swap slots will be contiguous, considering we are
> currently utilizing
> folio->swap instead of subpage->swap ?
Yes, I agree about contiguity. My objection is about assuming natural alignment
though. It can still be contiguous while not naturally aligned in swap.
>
>> Your cover note for swap-in says that you could technically swap in a large
>> folio without it having been swapped-out large. If you chose to do that in
>> future, this would break, right? I don't think it's good to couple the swap
>
> Right. technically I agree. Given that we still have many tasks involving even
> swapping in contiguous swap slots, it's unlikely that swapping in large folios
> for non-contiguous entries will occur in the foreseeable future :-)
>
>> storage layout to the folio order that you want to swap into. Perhaps that's an
>> argument for passing each *page* to this function with its exact, corresponding
>> swap entry?
>
> I recall Matthew Wilcox strongly objected to using "page" as the
> parameter, so I've
> discarded that approach. Alternatively, it appears I can consistently pass
> folio->swap to this function and ensure the function always retrieves
> the first entry?
Yes, if we must pass a folio here, I'd prefer that entry always corresponds to
the first entry for the folio. That will remove the need for this function to do
the alignment above too. So win-win.
>
>>
>>> + for (i = 0; i < nr; i++) {
>>> + mte_restore_tags(entry, folio_page(folio, i));
>>> + entry.val++;
>>> + }
>>> + }
>>> +}
>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>>> index de0c89105076..e04b93c43965 100644
>>> --- a/include/linux/huge_mm.h
>>> +++ b/include/linux/huge_mm.h
>>> @@ -535,16 +535,4 @@ static inline int split_folio_to_order(struct folio *folio, int new_order)
>>> #define split_folio_to_list(f, l) split_folio_to_list_to_order(f, l, 0)
>>> #define split_folio(f) split_folio_to_order(f, 0)
>>>
>>> -/*
>>> - * archs that select ARCH_WANTS_THP_SWAP but don't support THP_SWP due to
>>> - * limitations in the implementation like arm64 MTE can override this to
>>> - * false
>>> - */
>>> -#ifndef arch_thp_swp_supported
>>> -static inline bool arch_thp_swp_supported(void)
>>> -{
>>> - return true;
>>> -}
>>> -#endif
>>> -
>>> #endif /* _LINUX_HUGE_MM_H */
>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
>>> index e1b22903f709..bfcfe3386934 100644
>>> --- a/include/linux/pgtable.h
>>> +++ b/include/linux/pgtable.h
>>> @@ -1106,7 +1106,7 @@ static inline int arch_unmap_one(struct mm_struct *mm,
>>> * prototypes must be defined in the arch-specific asm/pgtable.h file.
>>> */
>>> #ifndef __HAVE_ARCH_PREPARE_TO_SWAP
>>> -static inline int arch_prepare_to_swap(struct page *page)
>>> +static inline int arch_prepare_to_swap(struct folio *folio)
>>> {
>>> return 0;
>>> }
>>> diff --git a/mm/page_io.c b/mm/page_io.c
>>> index ae2b49055e43..a9a7c236aecc 100644
>>> --- a/mm/page_io.c
>>> +++ b/mm/page_io.c
>>> @@ -189,7 +189,7 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
>>> * Arch code may have to preserve more data than just the page
>>> * contents, e.g. memory tags.
>>> */
>>> - ret = arch_prepare_to_swap(&folio->page);
>>> + ret = arch_prepare_to_swap(folio);
>>> if (ret) {
>>> folio_mark_dirty(folio);
>>> folio_unlock(folio);
>>> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
>>> index 90973ce7881d..53abeaf1371d 100644
>>> --- a/mm/swap_slots.c
>>> +++ b/mm/swap_slots.c
>>> @@ -310,7 +310,7 @@ swp_entry_t folio_alloc_swap(struct folio *folio)
>>> entry.val = 0;
>>>
>>> if (folio_test_large(folio)) {
>>> - if (IS_ENABLED(CONFIG_THP_SWAP) && arch_thp_swp_supported())
>>> + if (IS_ENABLED(CONFIG_THP_SWAP))
>>> get_swap_pages(1, &entry, folio_nr_pages(folio));
>>> goto out;
>>> }
>>
>
> Thanks
> Barry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH v2] ARM: unwind: improve unwinders for noreturn case
From: David Laight @ 2024-03-21 10:22 UTC (permalink / raw)
To: 'Jiangfeng Xiao', Russell King (Oracle)
Cc: arnd@arndb.de, keescook@chromium.org, haibo.li@mediatek.com,
angelogioacchino.delregno@collabora.com, amergnat@baylibre.com,
akpm@linux-foundation.org, dave.hansen@linux.intel.com,
douzhaolei@huawei.com, gustavoars@kernel.org, jpoimboe@kernel.org,
kepler.chenxin@huawei.com, kirill.shutemov@linux.intel.com,
linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org,
nixiaoming@huawei.com, peterz@infradead.org, wangbing6@huawei.com,
wangfangpeng1@huawei.com, jannh@google.com, willy@infradead.org
In-Reply-To: <bad25937-fc98-8e11-4279-ab6b3a727c1f@huawei.com>
How aggressively does the compiler optimise 'noreturn' functions?
Consider:
void f(...)
{
...
if () {
...
noreturn(...);
}
}
Without the noreturn() call it is a leaf function.
So the compiler doesn't need to save 'lr' on stack
(or the save could be deferred to inside the conditional).
Since noreturn() doesn't return it can be jumped to.
Additionally 'lr' can be used as a scratch register prior to
the noreturn() call.
So it is likely that inside noreturn() (and anything it
might call) you don't have a valid 'lr' chain at all.
No amount of picking between 'pc' and 'pc-1' is going to fix that.
The only way you can find a return address is by searching the
stack and hoping to find something that works.
So you need the compiler to 'not believe' the 'noreturn' attribute.
Setup a normal call frame and put a faulting instruction after the
call in case it returns.
That would give you half a chance of generating a backtrace.
Without that I suspect you are playing whack-a-mole.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 5/5] mm: support large folios swapin as a whole
From: Barry Song @ 2024-03-21 10:20 UTC (permalink / raw)
To: Ryan Roberts
Cc: Huang, Ying, Matthew Wilcox, akpm, linux-mm, chengming.zhou,
chrisl, david, hannes, kasong, linux-arm-kernel, linux-kernel,
mhocko, nphamcs, shy828301, steven.price, surenb, wangkefeng.wang,
xiang, yosryahmed, yuzhao, Chuanhua Han, Barry Song
In-Reply-To: <f918354d-12ee-4349-9356-fc02d2457a26@arm.com>
On Wed, Mar 20, 2024 at 1:19 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 19/03/2024 09:20, Huang, Ying wrote:
> > Ryan Roberts <ryan.roberts@arm.com> writes:
> >
> >>>>> I agree phones are not the only platform. But Rome wasn't built in a
> >>>>> day. I can only get
> >>>>> started on a hardware which I can easily reach and have enough hardware/test
> >>>>> resources on it. So we may take the first step which can be applied on
> >>>>> a real product
> >>>>> and improve its performance, and step by step, we broaden it and make it
> >>>>> widely useful to various areas in which I can't reach :-)
> >>>>
> >>>> We must guarantee the normal swap path runs correctly and has no
> >>>> performance regression when developing SWP_SYNCHRONOUS_IO optimization.
> >>>> So we have to put some effort on the normal path test anyway.
> >>>>
> >>>>> so probably we can have a sysfs "enable" entry with default "n" or
> >>>>> have a maximum
> >>>>> swap-in order as Ryan's suggestion [1] at the beginning,
> >>>>>
> >>>>> "
> >>>>> So in the common case, swap-in will pull in the same size of folio as was
> >>>>> swapped-out. Is that definitely the right policy for all folio sizes? Certainly
> >>>>> it makes sense for "small" large folios (e.g. up to 64K IMHO). But I'm not sure
> >>>>> it makes sense for 2M THP; As the size increases the chances of actually needing
> >>>>> all of the folio reduces so chances are we are wasting IO. There are similar
> >>>>> arguments for CoW, where we currently copy 1 page per fault - it probably makes
> >>>>> sense to copy the whole folio up to a certain size.
> >>>>> "
> >>
> >> I thought about this a bit more. No clear conclusions, but hoped this might help
> >> the discussion around policy:
> >>
> >> The decision about the size of the THP is made at first fault, with some help
> >> from user space and in future we might make decisions to split based on
> >> munmap/mremap/etc hints. In an ideal world, the fact that we have had to swap
> >> the THP out at some point in its lifetime should not impact on its size. It's
> >> just being moved around in the system and the reason for our original decision
> >> should still hold.
> >>
> >> So from that PoV, it would be good to swap-in to the same size that was
> >> swapped-out.
> >
> > Sorry, I don't agree with this. It's better to swap-in and swap-out in
> > smallest size if the page is only accessed seldom to avoid to waste
> > memory.
>
> If we want to optimize only for memory consumption, I'm sure there are many
> things we would do differently. We need to find a balance between memory and
> performance. The benefits of folios are well documented and the kernel is
> heading in the direction of managing memory in variable-sized blocks. So I don't
> think it's as simple as saying we should always swap-in the smallest possible
> amount of memory.
Absolutely agreed. With 64KiB large folios implemented, there may have been
a slight uptick in memory usage due to fragmentation. Nevertheless, through the
optimization of zRAM and zsmalloc to compress entire large folios, we found that
the compressed data could be up to 1GiB smaller compared to compressing them
in 4KiB increments on a typical phone with 12~16GiB memory. Consequently, we
not only reclaimed our memory loss entirely but also gained the benefits of
CONT-PTE , reduced TLB misses etc.
>
> You also said we should swap *out* in smallest size possible. Have I
> misunderstood you? I thought the case for swapping-out a whole folio without
> splitting was well established and non-controversial?
>
> >
> >> But we only kind-of keep that information around, via the swap
> >> entry contiguity and alignment. With that scheme it is possible that multiple
> >> virtually adjacent but not physically contiguous folios get swapped-out to
> >> adjacent swap slot ranges and then they would be swapped-in to a single, larger
> >> folio. This is not ideal, and I think it would be valuable to try to maintain
> >> the original folio size information with the swap slot. One way to do this would
> >> be to store the original order for which the cluster was allocated in the
> >> cluster. Then we at least know that a given swap slot is either for a folio of
> >> that order or an order-0 folio (due to cluster exhaustion/scanning). Can we
> >> steal a bit from swap_map to determine which case it is? Or are there better
> >> approaches?
> >
> > [snip]
> >
> > --
> > Best Regards,
> > Huang, Ying
Thanks
Barry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/3] ARM: dts: renesas: Add more TMU support
From: Geert Uytterhoeven @ 2024-03-21 10:17 UTC (permalink / raw)
To: Magnus Damm, Niklas Söderlund; +Cc: linux-renesas-soc, linux-arm-kernel
In-Reply-To: <cover.1710864964.git.geert+renesas@glider.be>
On Tue, Mar 19, 2024 at 5:29 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> This patch series adds device nodes for the Timer Units (TMU) on the
> R-Mobile APE6 SoC, and on various SoCs from the RZ/G1 and R-Car Gen2
> family.
In case you are wondering why I did not include SH-Mobile AG5: on that
SoC, the TMU interrupts are not wired to the interrupt controller for
the AP-System Core (INTCAx), only to the interrupt controller for the
AP-Realtime Core (INTCS).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] ARM: unwind: improve unwinders for noreturn case
From: Jiangfeng Xiao @ 2024-03-21 9:44 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: arnd, keescook, haibo.li, angelogioacchino.delregno, amergnat,
akpm, dave.hansen, douzhaolei, gustavoars, jpoimboe,
kepler.chenxin, kirill.shutemov, linux-hardening, linux-kernel,
linux-mm, linux-arm-kernel, nixiaoming, peterz, wangbing6,
wangfangpeng1, jannh, willy, David.Laight
In-Reply-To: <Zfs7sT6Pxy5yjnPC@shell.armlinux.org.uk>
On 2024/3/21 3:40, Russell King (Oracle) wrote:
> On Wed, Mar 20, 2024 at 11:30:05PM +0800, Jiangfeng Xiao wrote:
>>
>>
>> On 2024/3/20 16:45, Russell King (Oracle) wrote:
>>> On Wed, Mar 20, 2024 at 11:44:38AM +0800, Jiangfeng Xiao wrote:
>>>> This is an off-by-one bug which is common in unwinders,
>>>> due to the fact that the address on the stack points
>>>> to the return address rather than the call address.
>>>>
>>>> So, for example, when the last instruction of a function
>>>> is a function call (e.g., to a noreturn function), it can
>>>> cause the unwinder to incorrectly try to unwind from
>>>> the function after the callee.
>>>>
>>>> foo:
>>>> ...
>>>> bl bar
>>>> ... end of function and thus next function ...
>>>>
>>>> which results in LR pointing into the next function.
>>>>
>>>> Fixed this by subtracting 1 from frmae->pc in the call frame
>>>> (but not exception frames) like ORC on x86 does.
>>>
>>> The reason that I'm not accepting this patch is because the above says
>>> that it fixes it by subtracting 1 from the PC value, but the patch is
>>> *way* more complicated than that and there's no explanation why.
>>>
>>> For example, the following are unexplained:
>>>
>>> - Why do we always need ex_frame
>>
>> ```
>> bar:
>> ...
>> ... end of function bar ...
>>
>> foo:
>> BUG();
>> ... end of function foo ...
>> ```
>>
>> For example, when the first instruction of function 'foo'
>> is a undefined instruction, after function 'foo' is executed
>> to trigger an exception, 'arm_get_current_stackframe' assigns
>> 'regs->ARM_pc' to 'frame.pc'.
>>
>> If we always decrement frame.pc by 1, unwinder will incorrectly
>> try to unwind from the function 'bar' before the function 'foo'.
>>
>> So we need to 'ext_frame' to distinguish this case
>> where we don't need to subtract 1.
>
> This just sounds wrong to me. We have two different cases:
>
> 1) we're unwinding a frame where PC points at the offending instruction.
> This may or may not be in the exception text.
> 2) we're unwinding a frame that has been created because of a branch,
> where the PC points at the next instruction _after_ that callsite.
>
> While we're unwinding, we will mostly hit the second type of frames, but
> we'll only hit the first type on the initial frame. Some exception
> entries will have the PC pointing at the next instruction to be
> executed, others will have it pointing at the offending instruction
> (e.g. if it needs to be retried.)
Thank you for your summary.
Now we try to enumerate all cases:
1) When we hit the first type of frames
1.1) If the pc pointing at the next instruction
of the offending instruction
1.1.1) If the offending instruction is the first instruction
of the function
pc: cause to unwind from current function
pc-1: casue to unwind from current function
1.1.2) If the offending instruction is neither the first
instruction nor the last instruction of the function
pc: cause to unwind from current function
pc-1: casue to unwind from current function
1.1.3) If the offending instruction is the last instruction
of the function
pc: cause to unwind from next function
pc-1: casue to unwind from current function
1.2) If the pc pointing at the offending instruction
1.2.1) If the offending instruction is the first instruction
of the function
pc: cause to unwind from current function
pc-1: casue to unwind from previous function
1.2.2) If the offending instruction is neither the first
instruction nor the last instruction of the function
pc: cause to unwind from current function
pc-1: casue to unwind from current function
1.2.3) If the offending instruction is the last instruction
of the function
pc: cause to unwind from current function
pc-1: casue to unwind from current function
2) When we hit the second type of frames
2.1) pc always pointing at the next instruction after that callsite
2.1.1) If the callsite is the first instruction
pc: cause to unwind from current function
pc-1: casue to unwind from current function
2.1.2) If the callsite is neither the first nor last instruction
pc: cause to unwind from current function
pc-1: casue to unwind from current function
2.1.3) If the callsite is the last instruction
pc: cause to unwind from next function
pc-1: casue to unwind from current function
All in all, I think you are right.
In case 2), We can always unwind by 'pc-1'.
In case 1), If we unwind by 'pc', case 1.1.3) is problematic.
If we unwind by 'pc-1', 1.2.1) is problematic.
>
> So, I don't see what being in the exception/entry text really has much
> to do with any decision making here. I think you've found that it works
> for your case, but it won't _always_ work and you're just shifting the
> "bug" with how these traces work from one issue to a different but
> similar issue.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector
From: Michael Walle @ 2024-03-21 9:38 UTC (permalink / raw)
To: Vaishnav Achath, Andrew Lunn
Cc: Russell King (Oracle), Ayush Singh, open list, jkridner,
robertcnelson, lorforlinux, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
Mark Brown, Johan Hovold, Alex Elder,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE,
open list:SPI SUBSYSTEM, moderated list:GREYBUS SUBSYSTEM,
Vaishnav M A
In-Reply-To: <ded6c350-4c70-4a26-8b18-6605dcc6e084@ti.com>
Hi,
> > Is that because the current software support is too limited? Are there
> > manufactures who want to create more complex designed, but are limited
> > by what can be described in the manifest?
> >
>
> most mikroBUS add-on boards in production lies in the category of
> sensors, displays, connectivity, mixed signal (ADC/DAC .etc) and if you
> look at the existing bindings under bindings/iio/ , most devices need
> only simple descriptions and the properties are mainly standard bus
> properties (SPI/I2C properties), IRQ, named-gpios, named properties,
> regulators, clocks the extension to manifest was made taking this into
> account and the named property description interface just maps the
> manifest entries to the unified device property interface under
> include/linux/property.h
How will the ethernet boards ([1], [2]) work? Where do they get
their MAC address from, for example. The DT has some nice properties
for that, but I doubt that will be possible with the manifest files.
I've looked at the manifest file for the w5500 board [3] and to me
it looks like that board will come up with a random MAC address on
each start. Thus, even today, you have some boards which require
a more complex description.
Apart from the discussion whether the manifest is a suitable or
sufficient mechanism to describe the hardware, I think the main
problem with the proposed binding, is that it doesn't follow the
binding Rob was proposing for a socket. If I want to use DT
overlays, how would you describe an add-on board?
The proposal was that the base board has something like
mikrobus: socket {
compatible = "mikrobus-socket";
i2c-parent = <&i2c0>;
spi-parent = <&spi0>;
i2c {};
spi {};
};
an add-on board can then have a DT snippet/overlay like the
following:
&mikrobus {
i2c {
eeprom@52: {
reg = <52>;
compatible = <atmel,at24..>;
}
};
spi {
sensor@0: {
reg = <0>;
compatible = <foobar>;
};
};
};
That should be possible with a binding for the mikrobus, which
in fact it is just a pin header with a standard pinout. Also as
Russell pointed out in v3, the EEPROM/manifest is not part of the
mikrobus standard. So maybe that deserves an own compatible, like
compatible = "mikroe,click", "mikrobus-socket";
Or maybe click-eeprom? Although click seems to be the brand name of
MikroElektronika.
-michael
[1] https://www.mikroe.com/eth-3-click
[2] https://www.mikroe.com/eth-wiz-click
[3] https://github.com/MikroElektronika/click_id/blob/main/manifests/ETH-WIZ-CLICK.mnfs
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
From: Heiko Stübner @ 2024-03-21 9:32 UTC (permalink / raw)
To: Emmanuel Gil Peyrot, linux-kernel, Krzysztof Kozlowski
Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joerg Roedel,
Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
linux-arm-kernel, iommu
In-Reply-To: <70439a01-7949-46bf-a701-c82ba961171a@linaro.org>
Am Donnerstag, 21. März 2024, 10:19:54 CET schrieb Krzysztof Kozlowski:
> On 21/03/2024 09:47, Heiko Stübner wrote:
> >>> enum:
> >>> - rockchip,rk3568-vepu
> >>> + - rockchip,rk3588-vepu121
> >>
> >> What is 121?
> >
> > That is the strange naming of the ip block inside the soc.
> >
> > I.e. the rk3588 TRM lists a number of different video encoders and decoders:
> > - VDPU121 is decoding h.263 and mpeg1,2,4
> > - VDPU381 is decoding h.265, h.264 and some more
> > - VDPU720 is decoding jpeg
> > - VDPU981 decodes AV1
> > - VEPU121 is the jpeg encoder above
> > - VEPU580 encodes h.264 and h.265
> >
> > Each of those are separate IP blocks with their own io-memory, their own
> > interrupts and their own iommus, etc.
>
> Thanks for explanation. Short introduction in commit msg would be nice
> (e.g. VEPU121, one of two VEPU encoders). OTOH, why not documenting all
> of them? Bindings are supposed to be as complete as possible.
We have a concurrent series for the vdpu121 running at
https://lore.kernel.org/all/20240316071100.2419369-1-liujianfeng1994@gmail.com
I think not all of those encoders/decoders are based on the Hantro IP
or at least at the moment we don't know this yet.
Hence people adding compatibles for the blocks they have actually
managed to run on their hardware.
Bindings are supposed to be as complete as possible, but revising a wrong
binding later is very hard. And the whole media part is full of binary libraries
in the vendor kernel and has not the best documentation.
So I guess people are just cautious ;-)
Heiko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net v2 0/2] MT7530 DSA subdriver fix VLAN egress and handling of all link-local frames
From: Paolo Abeni @ 2024-03-21 9:29 UTC (permalink / raw)
To: Arınç ÜNAL, Daniel Golle, DENG Qingfang, Sean Wang,
Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: Frank Wunderlich, Bartel Eerdekens, mithat.guner, erkin.bozoglu,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <f6f064b8-efb2-4ab0-94f1-468d5d273d6e@arinc9.com>
On Wed, 2024-03-20 at 19:41 +0300, Arınç ÜNAL wrote:
> On 14.03.2024 12:33, Arınç ÜNAL via B4 Relay wrote:
> > Hi.
> >
> > This patch series fixes the VLAN tag egress procedure for link-local
> > frames, and fixes handling of all link-local frames.
> >
> > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> > ---
> > Changes in v2:
> > - Add Fixes: tag to both patches.
> > - Link to v1: https://lore.kernel.org/r/20240311-b4-for-net-mt7530-fix-link-local-vlan-v1-0-d67e6cc31af2@arinc9.com
> >
> > ---
> > Arınç ÜNAL (2):
> > net: dsa: mt7530: fix link-local frames that ingress vlan filtering ports
> > net: dsa: mt7530: fix handling of all link-local frames
> >
> > drivers/net/dsa/mt7530.c | 52 ++++++++++++++++++++++++++++++++++++++++--------
> > drivers/net/dsa/mt7530.h | 22 +++++++++++++++++++-
> > 2 files changed, 65 insertions(+), 9 deletions(-)
> > ---
> > base-commit: d7d75124965aee23e5e4421d78376545cf070b0a
> > change-id: 20240208-b4-for-net-mt7530-fix-link-local-vlan-af6e9928ad8d
> >
> > Best regards,
>
> Reminder this patch series is waiting to be applied.
I hoped to get some feedback from the DSA crew, so it waited a bit in
patchwork. Anyway it looks like it staged long enough and I'll go
through it soon.
Cheers,
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/2] perf/hx_arm_ni: Support uncore NI-700 PMU
From: Yang Jialong 杨佳龙 @ 2024-03-21 9:28 UTC (permalink / raw)
To: Will Deacon, Mark Rutland; +Cc: shenghui.qu, linux-kernel, linux-arm-kernel
In-Reply-To: <6869702989ee61281e574d0c3de913d1d5dbb602.1709694173.git.jialong.yang@shingroup.cn>
Gentle remainder
在 2024/3/6 14:16, JiaLong.Yang 写道:
> This code is based on uncore PMUs arm_smmuv3_pmu and arm-cmn.
> One ni-700 can have many clock domains. Each of them has only one PMU.
> Here one PMU corresponds to one 'struct ni_pmu' instance.
> PMU name will be ni_pmu_N_M, which N means different NI-700s and M means
> different PMU in one NI-700. If only one NI-700 found in NI-700, name will
> be ni_pmu_N.
> Node interface event name will be xxni_N_eventname, such as
> asni_0_rdreq_any. There are many kinds of type of nodes in one clock
> domain. Also means that there are many kinds of that in one PMU. So we
> distinguish them by xxni string. Besides, maybe there are many nodes
> have same type. So we have number N in event name.
> By ni_pmu_0_0/asni_0_rdreq_any/, we can pinpoint accurate bus traffic.
> Example1: perf stat -a -e ni_pmu_0_0/asni_0_rdreq_any/,ni_pmu_0_0/cycles/
> EXample2: perf stat -a -e ni_pmu_0_0/asni,id=0,event=0x0/
>
> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn>
> ---
> v1 --> v2:
> 1. Submit MAINTANER Documentation/ files seperately.
> 2. Delete some useless info printing.
> 3. Change print from pr_xxx to dev_xxx.
> 4. Fix more than 75 length log info.
> 5. Fix dts attribute pccs-id.
> 6. Fix generic name according to DT specification.
> 7. Some indentation.
> 8. Del of_match_ptr macro.
>
> v2 --> v3:
> 1. Delete not existed arch macro PPC_HX_C2000.
> 2. Support 32bit and 64bit.
> 3. Add help content in Kconfig.
> 4. Use lo_hi_writeq/lo_hi_readq instead of writel and readl.
> 5. Place declarations to the top the file.
> 6. Clear redundant blank lines.
> 7. Use DEFINE_FREE() macro instead of kmalloc solely.
> 8. return dev_err_probe directly.
> 9. Use __ffs64 instead of ffs and fls64 instead of fls.
> 10. Delete BUILD_BUG_ON() macro function to support multiple platforms.
> 11. Use alias-id instead of customized property to index NIs.
>
> drivers/perf/Kconfig | 11 +
> drivers/perf/Makefile | 1 +
> drivers/perf/hx_arm_ni.c | 1241 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1253 insertions(+)
> create mode 100644 drivers/perf/hx_arm_ni.c
>
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index ec6e0d9194a1..0c50edc4172c 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -241,4 +241,15 @@ config CXL_PMU
>
> If unsure say 'm'.
>
> +source "drivers/perf/hisilicon/Kconfig"
> +
> +config HX_ARM_NI_PMU
> + tristate "Enable HX NI-700 Performance Monitoring Unit"
> + help
> + Support for NI-700(Network-on-chip Interconnect) PMUs, which
> + provide monitoring of transactions passing through between
> + CMN and other buses or periapherals.
> +
> + Say 'y/m' to enable this.
> +
> endmenu
> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> index a06338e3401c..ec8b9c08577d 100644
> --- a/drivers/perf/Makefile
> +++ b/drivers/perf/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_DWC_PCIE_PMU) += dwc_pcie_pmu.o
> obj-$(CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU) += arm_cspmu/
> obj-$(CONFIG_MESON_DDR_PMU) += amlogic/
> obj-$(CONFIG_CXL_PMU) += cxl_pmu.o
> +obj-$(CONFIG_HX_ARM_NI_PMU) += hx_arm_ni.o
> diff --git a/drivers/perf/hx_arm_ni.c b/drivers/perf/hx_arm_ni.c
> new file mode 100644
> index 000000000000..a7a4fade1281
> --- /dev/null
> +++ b/drivers/perf/hx_arm_ni.c
> @@ -0,0 +1,1241 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HX ARM-NI-700 uncore PMU support
> + *
> + * This code is based on uncore PMUs arm_smmuv3_pmu and arm-cmn.
> + *
> + * One ni-700 can have many clock domains. Each of them has only one PMU.
> + * Here one PMU corresponds to one 'struct ni_pmu' instance.
> + *
> + * PMU name will be ni_pmu_N_M, which N means different NI-700s and M means
> + * different PMU in one NI-700. If only one NI-700 found in NI-700, name
> + * will be ni_pmu_N.
> + *
> + * Node interface event name will be xxni_N_eventname, such as
> + * asni_0_rdreq_any. There are many kinds of type of nodes in one clock
> + * domain. Also means that there are many kinds of that in one PMU. So we
> + * distinguish them by xxni string. Besides, maybe there are many nodes
> + * have same type. So we have number N in event name.
> + * By ni_pmu_0_0/asni_0_rdreq_any/, we can pinpoint accurate bus traffic.
> + *
> + * Example1: perf stat -a -e ni_pmu_0_0/asni_0_rdreq_any/,ni_pmu_0_0/cycles/
> + * Example2: perf stat -a -e ni_pmu_0_0/asni,id=0,event=0x0/
> + *
> + * TODO: Secure or non-secure attribute in all event omitted now.
> + *
> + */
> +
> +#define dev_fmt(fmt) "hx-ni-700 pmu: " fmt
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cpuhotplug.h>
> +#include <linux/cpumask.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/perf_event.h>
> +#include <linux/platform_device.h>
> +#include <linux/smp.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +#include <linux/build_bug.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +
> +/* number of counters in one ni pmu */
> +#define NI_PMU_COUNTER_NUM 8
> +
> +/* node type values */
> +enum ni_node_type {
> + NI_BASE = 0x0,
> + NI_VD,
> + NI_PD,
> + NI_CD,
> + NI_ASNI = 0x4,
> + NI_AMNI,
> + NI_PMU,
> + NI_HSNI,
> + NI_HMNI,
> + NI_PMNI = 0x9,
> +};
> +
> +/*
> + * config:
> + * 0-5 31 32-47 48-63
> + * event cycles node_type node_id
> + *
> + */
> +#define NI_EVENT_FORMAT_EVENT GENMASK_ULL(5, 0)
> +#define NI_EVENT_FORMAT_CYCLES BIT(31)
> +#define NI_EVENT_FORMAT_NODETYPE GENMASK_ULL(32 + NI_PMNI, 32)
> +#define NI_EVENT_FORMAT_ASNI BIT(32 + NI_ASNI)
> +#define NI_EVENT_FORMAT_AMNI BIT(32 + NI_AMNI)
> +#define NI_EVENT_FORMAT_HSNI BIT(32 + NI_HSNI)
> +#define NI_EVENT_FORMAT_HMNI BIT(32 + NI_HMNI)
> +#define NI_EVENT_FORMAT_PMNI BIT(32 + NI_PMNI)
> +#define NI_EVENT_FORMAT_NODEID GENMASK_ULL(63, 48)
> +
> +#define NI_EVENT_FORMAT_NODE_TYPE GENMASK_ULL(63, 32)
> +
> +#define ni_event_config_eventid(_config) FIELD_GET(NI_EVENT_FORMAT_EVENT, _config)
> +#define ni_event_config_cc(_config) FIELD_GET(NI_EVENT_FORMAT_CYCLES, _config)
> +#define _ni_event_config_nodetype(_config) FIELD_GET(NI_EVENT_FORMAT_NODETYPE, _config)
> +#define ni_event_config_nodeid(_config) FIELD_GET(NI_EVENT_FORMAT_NODEID, _config)
> +
> +#define NI_NODE_TYPE_MASK GENMASK(15, 0)
> +#define NI_NODE_ID_MASK GENMASK(31, 16)
> +
> +#define NI_PMU_PMCR_RST_CYC_CNTR BIT(2)
> +#define NI_PMU_PMCR_RST_EV_CNTR BIT(1)
> +#define NI_PMU_PMCR_ENABLE BIT(0)
> +
> +static enum cpuhp_state hp_state;
> +
> +static const char *const ni_node_name[] = {
> + [NI_ASNI] = "asni",
> + [NI_AMNI] = "amni",
> + [NI_PMU] = "pmu",
> + [NI_HSNI] = "hsni",
> + [NI_HMNI] = "hmni",
> + [NI_PMNI] = "pmni",
> +};
> +
> +/* one instance for one node */
> +struct ni_node {
> + void __iomem *base;
> + union {
> + struct {
> + u32 type:16;
> + u32 id:16;
> + };
> + u32 node_type;
> + };
> +};
> +
> +/* xxx_reg_map only used to provide offset by using offsetof(). */
> +struct ni_node_reg_map {
> + union {
> + struct {
> + u32 type:16;
> + u32 id:16;
> + };
> + u32 node_type;
> + };
> +
> + union {
> + u32 child_num;
> + u32 node_info;
> + };
> +
> + union {
> + struct {
> + u32 secr_acc;
> + u32 pmusela;
> + u32 pmuselb;
> + };
> + DECLARE_FLEX_ARRAY(u32, child_offset);
> + };
> +};
> +
> +#define ni_node_offsetof(member) \
> + offsetof(struct ni_node_reg_map, member)
> +
> +#define ni_node_pmuevsel(node, config) \
> + lo_hi_writeq(config, node->base + ni_node_offsetof(pmusela))
> +
> +#define ni_node_read(base, member, readx) \
> + readx((void __iomem *)base + ni_node_offsetof(member))
> +
> +#define ni_node_type(base) \
> + FIELD_GET(NI_NODE_TYPE_MASK, ni_node_read(base, node_type, readl_relaxed))
> +
> +#define ni_node_id(base) \
> + FIELD_GET(NI_NODE_ID_MASK, ni_node_read(base, node_type, readl_relaxed))
> +
> +#define ni_node_node_type(base) \
> + ni_node_read(base, node_type, readl_relaxed)
> +
> +#define ni_child_number(base) \
> + (ni_node_type(base) < NI_ASNI ? ni_node_read(base, child_num, readl_relaxed) : 0)
> +
> +#define ni_child_pointer(periphbase, base, idx) \
> + ((void __iomem *)periphbase + ni_node_read(base, child_offset[idx], readl_relaxed))
> +
> +struct ni_pmu;
> +struct ni_pmu_reg_map {
> + u32 node_type; /* offset: 0x000 */
> + u32 secr_acc; /* offset: 0x004 */
> + struct {
> + u32 counter;
> + u32 reserved;
> + } pmevcntr[8]; /* offset: 0x008 */
> + u8 reserved_1[0xF8 - 0x48]; /* offset: 0x048 */
> + u32 pmccntr_lower; /* offset: 0x0F8 */
> + u32 pmccntr_upper; /* offset: 0x0FC */
> + u8 reserved_2[0x400 - 0x100]; /* offset: 0x100 */
> + u32 pmevtyper[8]; /* offset: 0x400 */
> + u8 reserved_3[0x610 - 0x420]; /* offset: 0x420 */
> + u32 pmssr; /* offset: 0x610 */
> + u32 pmovssr; /* offset: 0x614 */
> + u32 pmccntsr_lower; /* offset: 0x618 */
> + u32 pmccntsr_upper; /* offset: 0x61C */
> + u32 pmevcntsr[8]; /* offset: 0x620 */
> + u8 reserved_4[0x6F0 - 0x640]; /* offset: 0x640 */
> + u32 pmsscr; /* offset: 0x6F0 */
> + u8 reserved_5[0xC00 - 0x6F4]; /* offset: 0x6F4 */
> + u32 pmcntenset; /* offset: 0xC00 */
> + u8 reserved_6[0xC20 - 0xC04]; /* offset: 0xC04 */
> + u32 pmcntenclr; /* offset: 0xC20 */
> + u8 reserved_7[0xC40 - 0xC24]; /* offset: 0xC24 */
> + u32 pmintenset; /* offset: 0xC40 */
> + u8 reserved_8[0xC60 - 0xC44]; /* offset: 0xC44 */
> + u32 pmintenclr; /* offset: 0xC60 */
> + u8 reserved_9[0xC80 - 0xC64]; /* offset: 0xC64 */
> + u32 pmovsclr; /* offset: 0xC80 */
> + u8 reserved_10[0xCC0 - 0xC84]; /* offset: 0xC84 */
> + u32 pmovsset; /* offset: 0xCC0 */
> + u8 reserved_11[0xD80 - 0xCC4]; /* offset: 0xCC4 */
> + u32 pmcccgr; /* offset: 0xD80 */
> + u8 reserved_12[0xE00 - 0xD84]; /* offset: 0xD84 */
> + u32 pmcfgr; /* offset: 0xE00 */
> + u32 pmcr; /* offset: 0xE04 */
> +};
> +
> +/* Not read or write registers directly. */
> +#define ni_pmu_offset(ni_pmu, member) \
> + ((void __iomem *)ni_pmu->pmu_node.base + offsetof(struct ni_pmu_reg_map, member))
> +
> +#define ni_pmu_interrupt_enable(ni_pmu, en_bit) \
> + writel(en_bit, ni_pmu_offset(ni_pmu, pmintenset))
> +
> +#define ni_pmu_interrupt_disable(ni_pmu, en_bit) \
> + writel(en_bit, ni_pmu_offset(ni_pmu, pmintenclr))
> +
> +#define ni_pmu_counter_enable(ni_pmu, en_bit) \
> + writel(en_bit, ni_pmu_offset(ni_pmu, pmcntenset))
> +
> +#define ni_pmu_counter_disable(ni_pmu, en_bit) \
> + writel(en_bit, ni_pmu_offset(ni_pmu, pmcntenclr))
> +
> +#define ni_pmu_pmevtyper_sel_node(ni_pmu, ev_typer, cnt_idx) \
> + writel(ev_typer, ni_pmu_offset(ni_pmu, pmevtyper[cnt_idx]))
> +
> +struct global_ni {
> + void __iomem *base;
> + struct hlist_node node;
> + struct device *dev;
> + union {
> + unsigned int pmu_num;
> + unsigned int cd_num;
> + };
> + unsigned int on_cpu;
> + int irq_num;
> + struct ni_pmu *ni_pmus[];
> +};
> +
> +struct ni_pmu {
> + struct ni_node pmu_node;
> + struct perf_event *events[NI_PMU_COUNTER_NUM + 1];
> + struct pmu pmu;
> + struct device *dev;
> + unsigned int irq;
> + struct global_ni *ni;
> + int ev_src_num;
> + struct ni_node ev_src_nodes[];
> +};
> +
> +#define to_ni_pmu(_pmu) container_of(_pmu, struct ni_pmu, pmu)
> +
> +#define NI_PMU_CC_EN_BIT 31
> +#define is_cc_event(hwc) (!(hwc->config))
> +#define cnt_init_val(hwc) (is_cc_event(hwc) ? (0x1ULL << 63) : (0x1ULL << 31))
> +#define cnt_en_bit(hwc) (is_cc_event(hwc) ? BIT(NI_PMU_CC_EN_BIT) : BIT(hwc->idx))
> +
> +struct ni_event_desc {
> + u64 eventid;
> + const char *name;
> +};
> +
> +struct ni_event_attr {
> + struct device_attribute attr;
> + struct ni_event_desc *ev_desc;
> + struct ni_node *node;
> +};
> +
> +#define to_ni_event_attr(p) \
> + container_of(p, struct ni_event_attr, attr)
> +
> +#define NI_EVENT_DESC(_eventid, _name) \
> + (&((struct ni_event_desc[]) { \
> + { .name = __stringify(_name), \
> + .eventid = _eventid,} \
> + })[0])
> +
> +struct ni_format_attr {
> + struct device_attribute attr;
> + u64 field;
> +};
> +
> +static struct ni_event_desc *ni_asni_event_descs[] = {
> + NI_EVENT_DESC(0x00, rdreq_any),
> + NI_EVENT_DESC(0x01, rdreq_dev_arcache),
> + NI_EVENT_DESC(0x02, rdreq_rns),
> + NI_EVENT_DESC(0x03, rdreq_ro),
> + NI_EVENT_DESC(0x04, req_cache_clr),
> + NI_EVENT_DESC(0x05, rdreq_beat_any),
> + NI_EVENT_DESC(0x06, rdreq_handshake_rlast),
> + NI_EVENT_DESC(0x07, wtreq_any),
> + NI_EVENT_DESC(0x08, wtreq_dev),
> + NI_EVENT_DESC(0x09, wtreq_wns),
> + NI_EVENT_DESC(0x0a, wtreq_wlu),
> + NI_EVENT_DESC(0x0b, wtreq_wu),
> + NI_EVENT_DESC(0x0c, wtreq_atomic),
> + NI_EVENT_DESC(0x0d, wtreq_beat_any),
> + NI_EVENT_DESC(0x0e, rdreq_stall),
> + NI_EVENT_DESC(0x0f, rddata_stall),
> + NI_EVENT_DESC(0x10, wtreq_stall),
> + NI_EVENT_DESC(0x11, wtdata_stall),
> + NI_EVENT_DESC(0x12, wtresp_stall),
> + NI_EVENT_DESC(0x13, wtreq_cst),
> + NI_EVENT_DESC(0x14, wtchann_nopersist),
> + NI_EVENT_DESC(0x15, wtchann_persist),
> + NI_EVENT_DESC(0x16, rdreq_nzero_mem_ops),
> + NI_EVENT_DESC(0x17, wtreq_nzero_mem_ops),
> + NI_EVENT_DESC(0x20, req_stall_cc_ot_limit),
> + NI_EVENT_DESC(0x21, req_stall_cc_tspec_limit),
> + NI_EVENT_DESC(0x22, req_stall_arbit),
> + NI_EVENT_DESC(0x23, req_stall_rd_tracker),
> + NI_EVENT_DESC(0x24, req_stall_wt_tracker),
> + NI_EVENT_DESC(0x25, aw_stall_wdatafifo_full),
> + NI_EVENT_DESC(0x26, ar_stall_reorderbuf_full),
> + NI_EVENT_DESC(0x27, aw_cdas_stall),
> + NI_EVENT_DESC(0x28, ar_cdas_stall),
> + NI_EVENT_DESC(0x29, atomic_rd_stall),
> + NI_EVENT_DESC(0x2a, wtchann_wtreq_stall),
> + NI_EVENT_DESC(0x2b, rdchann_rdreq_stall),
> + NI_EVENT_DESC(0x2c, aw_stall_ot),
> + NI_EVENT_DESC(0x2d, ar_stall_ot),
> + NI_EVENT_DESC(0x2e, aw_stall_tspec),
> + NI_EVENT_DESC(0x2f, ar_stall_tspec),
> + NI_EVENT_DESC(0x30, lwmd_arbit_stall_wchann),
> + NI_EVENT_DESC(0x31, lwmd_arbit_stall_rchann),
> +};
> +
> +static struct ni_event_desc *ni_amni_event_descs[] = {
> + NI_EVENT_DESC(0x00, rdreq_any),
> + NI_EVENT_DESC(0x01, rdreq_dev_arcache),
> + NI_EVENT_DESC(0x02, rdreq_rns),
> + NI_EVENT_DESC(0x03, rdreq_ro),
> + NI_EVENT_DESC(0x04, req_cache_clr),
> + NI_EVENT_DESC(0x05, rdreq_beat_any),
> + NI_EVENT_DESC(0x06, rdreq_handshake_rlast),
> + NI_EVENT_DESC(0x07, wtreq_any),
> + NI_EVENT_DESC(0x08, wtreq_dev),
> + NI_EVENT_DESC(0x09, wtreq_wns),
> + NI_EVENT_DESC(0x0a, wtreq_wlu),
> + NI_EVENT_DESC(0x0b, wtreq_wu),
> + NI_EVENT_DESC(0x0c, wtreq_atomic),
> + NI_EVENT_DESC(0x0d, wtreq_beat_any),
> + NI_EVENT_DESC(0x0e, rdreq_stall),
> + NI_EVENT_DESC(0x0f, rddata_stall),
> + NI_EVENT_DESC(0x10, wtreq_stall),
> + NI_EVENT_DESC(0x11, wtdata_stall),
> + NI_EVENT_DESC(0x12, wtresp_stall),
> + NI_EVENT_DESC(0x13, wtreq_cst),
> + NI_EVENT_DESC(0x14, wtchann_nopersist),
> + NI_EVENT_DESC(0x15, wtchann_persist),
> + NI_EVENT_DESC(0x16, rdreq_nzero_mem_ops),
> + NI_EVENT_DESC(0x17, wtreq_nzero_mem_ops),
> + NI_EVENT_DESC(0x20, req_stall_rd_tracker),
> + NI_EVENT_DESC(0x21, req_stall_wt_tracker),
> + NI_EVENT_DESC(0x22, wtchann_b_resp),
> + NI_EVENT_DESC(0x23, rdchann_rd_resp),
> + NI_EVENT_DESC(0x24, lwmd_arbit_stall_wchann),
> + NI_EVENT_DESC(0x25, lwmd_arbit_stall_rchann),
> +};
> +
> +static struct ni_event_desc *ni_hsni_event_descs[] = {
> + NI_EVENT_DESC(0x00, rdreq_any),
> + NI_EVENT_DESC(0x01, rdreq_dev),
> + NI_EVENT_DESC(0x02, rdreq_noshare),
> + NI_EVENT_DESC(0x03, rdreq_share),
> + NI_EVENT_DESC(0x04, rdreq_share_nonormal),
> + NI_EVENT_DESC(0x05, rdreq_beat_any),
> + NI_EVENT_DESC(0x07, wtreq_any),
> + NI_EVENT_DESC(0x08, wtreq_dev),
> + NI_EVENT_DESC(0x09, wtreq_noshare),
> + NI_EVENT_DESC(0x0a, wtreq_all),
> + NI_EVENT_DESC(0x0b, wtreq_share),
> + NI_EVENT_DESC(0x0c, wtreq_share_nonormal),
> + NI_EVENT_DESC(0x0d, wtreq_beat_any),
> + NI_EVENT_DESC(0x0f, rddata_stall),
> + NI_EVENT_DESC(0x11, wtdata_stall),
> + NI_EVENT_DESC(0x20, req_stall_cc_ot_limit),
> + NI_EVENT_DESC(0x21, req_stall_cc_tspec_limit),
> + NI_EVENT_DESC(0x22, rdreq_stall_cc_ely_wtresp),
> + NI_EVENT_DESC(0x24, req_stall_nzero_wtcnt),
> + NI_EVENT_DESC(0x25, w_stall_wdatafifo_full),
> + NI_EVENT_DESC(0x2a, wtreq_stall_lack_gt),
> + NI_EVENT_DESC(0x2b, rdreq_stall_lack_gt),
> +};
> +
> +static struct ni_event_desc *ni_hmni_event_descs[] = {
> + NI_EVENT_DESC(0x00, rdreq_any),
> + NI_EVENT_DESC(0x01, rdreq_dev),
> + NI_EVENT_DESC(0x02, rdreq_noshare),
> + NI_EVENT_DESC(0x03, rdreq_share),
> + NI_EVENT_DESC(0x04, rdreq_share_nonormal),
> + NI_EVENT_DESC(0x05, rdreq_beat_any),
> + NI_EVENT_DESC(0x07, wtreq_any),
> + NI_EVENT_DESC(0x08, wtreq_dev),
> + NI_EVENT_DESC(0x09, wtreq_noshare),
> + NI_EVENT_DESC(0x0a, wtreq_all),
> + NI_EVENT_DESC(0x0b, wtreq_share),
> + NI_EVENT_DESC(0x0c, wtreq_share_nonormal),
> + NI_EVENT_DESC(0x0d, wtreq_beat_any),
> + NI_EVENT_DESC(0x0e, rd_addr_phase_stall),
> + NI_EVENT_DESC(0x0f, rd_data_phase_stall),
> + NI_EVENT_DESC(0x10, wt_addr_phase_stall),
> + NI_EVENT_DESC(0x11, wt_data_phase_stall),
> + NI_EVENT_DESC(0x22, wtresp_stall_lack_gt),
> + NI_EVENT_DESC(0x23, rdresp_stall_lack_gt),
> +};
> +
> +static struct ni_event_desc *ni_pmni_event_descs[] = {
> + NI_EVENT_DESC(0x00, rdreq_any),
> + NI_EVENT_DESC(0x01, rdreq_dev_arcache),
> + NI_EVENT_DESC(0x02, rdreq_noshared),
> + NI_EVENT_DESC(0x05, rd_prdata_any),
> + NI_EVENT_DESC(0x07, wtreq_any),
> + NI_EVENT_DESC(0x08, wtreq_dev),
> + NI_EVENT_DESC(0x09, wtreq_noshared),
> + NI_EVENT_DESC(0x0d, wtdata_beat_any),
> + NI_EVENT_DESC(0x0e, rdreq_stall),
> + NI_EVENT_DESC(0x0f, rddata_stall),
> + NI_EVENT_DESC(0x10, wtreq_stall),
> + NI_EVENT_DESC(0x11, wtdata_stall),
> + NI_EVENT_DESC(0x22, wtresp_stall_lack_gt),
> + NI_EVENT_DESC(0x23, rdresp_stall_lack_gt),
> +};
> +
> +static int ni_ev_desc_array_size(enum ni_node_type type,
> + struct ni_event_desc ***descs)
> +{
> + switch (type) {
> + case NI_ASNI:
> + if (descs)
> + *descs = ni_asni_event_descs;
> + return ARRAY_SIZE(ni_asni_event_descs);
> + case NI_AMNI:
> + if (descs)
> + *descs = ni_amni_event_descs;
> + return ARRAY_SIZE(ni_amni_event_descs);
> + case NI_HSNI:
> + if (descs)
> + *descs = ni_hsni_event_descs;
> + return ARRAY_SIZE(ni_hsni_event_descs);
> + case NI_HMNI:
> + if (descs)
> + *descs = ni_hmni_event_descs;
> + return ARRAY_SIZE(ni_hmni_event_descs);
> + case NI_PMNI:
> + if (descs)
> + *descs = ni_pmni_event_descs;
> + return ARRAY_SIZE(ni_pmni_event_descs);
> + default:
> + return 0;
> + }
> +}
> +
> +static ssize_t ni_event_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ni_event_attr *eattr;
> +
> + eattr = to_ni_event_attr(attr);
> +
> + if (eattr->ev_desc)
> + return sysfs_emit(buf,
> + "%s,id=0x%x,event=0x%llx\n",
> + ni_node_name[eattr->node->type],
> + eattr->node->id,
> + eattr->ev_desc->eventid);
> +
> + return sysfs_emit(buf, "cycles\n");
> +}
> +
> +static ssize_t ni_format_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ni_format_attr *fmt = container_of(attr, struct ni_format_attr, attr);
> + int lo = __ffs64(fmt->field), hi = fls64(fmt->field);
> +
> + if (lo == hi)
> + return sysfs_emit(buf, "config:%d\n", lo);
> +
> + return sysfs_emit(buf, "config:%d-%d\n", lo, hi);
> +}
> +
> +
> +#define NI_FORMAT_ATTR(_name, _fld) \
> + (&((struct ni_format_attr[]) {{ \
> + .attr = __ATTR(_name, 0444, ni_format_show, NULL), \
> + .field = _fld, \
> + }})[0].attr.attr)
> +
> +static struct attribute *ni_format_attrs[] = {
> + NI_FORMAT_ATTR(event, NI_EVENT_FORMAT_EVENT),
> + NI_FORMAT_ATTR(cycles, NI_EVENT_FORMAT_CYCLES),
> + NI_FORMAT_ATTR(asni, NI_EVENT_FORMAT_ASNI),
> + NI_FORMAT_ATTR(amni, NI_EVENT_FORMAT_AMNI),
> + NI_FORMAT_ATTR(hsni, NI_EVENT_FORMAT_HSNI),
> + NI_FORMAT_ATTR(hmni, NI_EVENT_FORMAT_HMNI),
> + NI_FORMAT_ATTR(pmni, NI_EVENT_FORMAT_PMNI),
> + NI_FORMAT_ATTR(id, NI_EVENT_FORMAT_NODEID),
> + NULL
> +};
> +
> +static const struct attribute_group ni_format_attrs_group = {
> + .name = "format",
> + .attrs = ni_format_attrs,
> +};
> +
> +static ssize_t ni_cpumask_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ni_pmu *ni_pmu = to_ni_pmu(dev_get_drvdata(dev));
> +
> + return cpumap_print_to_pagebuf(true, buf, cpumask_of(ni_pmu->ni->on_cpu));
> +}
> +
> +static struct device_attribute ni_cpumask_attr =
> + __ATTR(cpumask, 0444, ni_cpumask_show, NULL);
> +
> +static struct attribute *ni_addition_attrs[] = {
> + &ni_cpumask_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group ni_addition_attrs_group = {
> + .attrs = ni_addition_attrs,
> +};
> +
> +static u64 ni_cntr_get_and_init_optionally(struct perf_event *event, bool init)
> +{
> + u64 old_val, new_val;
> + struct ni_pmu *ni_pmu = to_ni_pmu(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> +
> + if (!is_cc_event(hwc))
> + old_val = readl(ni_pmu_offset(ni_pmu, pmevcntr[hwc->idx].counter));
> + else
> + old_val = lo_hi_readq(ni_pmu_offset(ni_pmu, pmccntr_lower));
> +
> + if (!init)
> + return old_val;
> +
> + new_val = cnt_init_val(hwc);
> + if (!is_cc_event(hwc))
> + writel(new_val, ni_pmu_offset(ni_pmu, pmevcntr[hwc->idx].counter));
> + else
> + lo_hi_writeq(new_val, ni_pmu_offset(ni_pmu, pmccntr_lower));
> +
> + return old_val;
> +}
> +
> +static void ni_pmu_event_update(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + u64 delta, prev, now;
> +
> + do {
> + prev = local64_read(&event->hw.prev_count);
> + now = ni_cntr_get_and_init_optionally(event, false);
> + } while (local64_cmpxchg(&event->hw.prev_count, prev, now) != prev);
> +
> + delta = now - prev;
> +
> + if (!is_cc_event(hwc))
> + delta &= 0xFFFFFFFFULL;
> +
> + local64_add(delta, &event->count);
> +}
> +
> +static void ni_pmu_set_period(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> +
> + ni_cntr_get_and_init_optionally(event, true);
> +
> + local64_set(&event->hw.prev_count, cnt_init_val(hwc));
> +}
> +
> +static void ni_pmu_enable(struct pmu *pmu)
> +{
> + struct ni_pmu *ni_pmu = to_ni_pmu(pmu);
> +
> + writel(NI_PMU_PMCR_ENABLE, ni_pmu_offset(ni_pmu, pmcr));
> +}
> +
> +static inline void ni_pmu_disable(struct pmu *pmu)
> +{
> + struct ni_pmu *ni_pmu = to_ni_pmu(pmu);
> +
> + writel(0, ni_pmu_offset(ni_pmu, pmcr));
> +}
> +
> +static struct ni_node *ni_pmu_find_ev_src(struct ni_pmu *ni_pmu, u32 node_type)
> +{
> + int idx;
> +
> + for (idx = 0; idx < ni_pmu->ev_src_num; idx++)
> + if (ni_pmu->ev_src_nodes[idx].node_type == node_type)
> + return &ni_pmu->ev_src_nodes[idx];
> +
> + return NULL;
> +}
> +
> +static bool is_event_supported(u64 eventid, enum ni_node_type type)
> +{
> + int num;
> + int idx;
> + struct ni_event_desc **descs;
> +
> + num = ni_ev_desc_array_size(type, &descs);
> +
> + for (idx = 0; idx < num; idx++)
> + if (eventid == descs[idx]->eventid)
> + break;
> +
> + return idx == num ? false : true;
> +}
> +
> +static enum ni_node_type ni_event_config_nodetype(u64 config)
> +{
> + unsigned long nodetype = _ni_event_config_nodetype(config);
> + unsigned long lo = __ffs(nodetype), hi = __fls(nodetype);
> +
> + if (!nodetype || lo != hi)
> + return 0;
> +
> + return lo;
> +}
> +
> +static int ni_pmu_event_init(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct ni_pmu *ni_pmu = to_ni_pmu(event->pmu);
> + u64 config;
> + u32 id, type, node_type;
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + if (is_sampling_event(event))
> + return -EINVAL;
> +
> + event->cpu = ni_pmu->ni->on_cpu;
> +
> + config = event->attr.config;
> +
> + hwc->config_base = config;
> + hwc->config = ni_event_config_eventid(config);
> +
> + if (ni_event_config_cc(config)) {
> + if (hwc->config)
> + return -EINVAL;
> + else
> + return 0; /* cycle event */
> + }
> +
> + type = ni_event_config_nodetype(config);
> + if (!type)
> + return -EINVAL;
> +
> + id = ni_event_config_nodeid(config);
> + node_type = type << 16 | id;
> +
> + if (!ni_pmu_find_ev_src(ni_pmu, node_type))
> + return -EINVAL;
> +
> + if (!is_event_supported(hwc->config, type))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static void ni_pmu_event_start(struct perf_event *event, int flags)
> +{
> + struct ni_pmu *ni_pmu = to_ni_pmu(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + u32 id, type, node_type, ev_typer;
> +
> + hwc->state = 0;
> +
> + ni_pmu_set_period(event);
> +
> + type = ni_event_config_nodetype(hwc->config_base);
> + id = ni_event_config_nodeid(hwc->config_base);
> +
> + node_type = type << 16 | id;
> + ev_typer = type << 8 | id;
> +
> + if (!is_cc_event(hwc)) {
> + ni_node_pmuevsel(ni_pmu_find_ev_src(ni_pmu, node_type), hwc->config);
> + ni_pmu_pmevtyper_sel_node(ni_pmu, ev_typer, hwc->idx);
> + }
> +
> + ni_pmu_counter_enable(ni_pmu, cnt_en_bit(hwc));
> +}
> +
> +static void ni_pmu_event_stop(struct perf_event *event, int flags)
> +{
> + struct ni_pmu *ni_pmu = to_ni_pmu(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> +
> + if (hwc->state & PERF_HES_STOPPED)
> + return;
> +
> + ni_pmu_counter_disable(ni_pmu, cnt_en_bit(hwc));
> +
> + ni_pmu_event_update(event);
> +
> + hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +}
> +
> +static void ni_pmu_event_read(struct perf_event *event)
> +{
> + ni_pmu_event_update(event);
> +}
> +
> +static int ni_pmu_event_add(struct perf_event *event, int flags)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct ni_pmu *ni_pmu = to_ni_pmu(event->pmu);
> + int idx;
> +
> + idx = 0;
> + if (is_cc_event(hwc) && ni_pmu->events[NI_PMU_COUNTER_NUM])
> + return -EAGAIN; /* The cycle counter is in use. */
> +
> + idx = 0;
> + if (is_cc_event(hwc))
> + idx = NI_PMU_COUNTER_NUM;
> + else
> + while ((idx < NI_PMU_COUNTER_NUM) && ni_pmu->events[idx])
> + idx++;
> +
> + if (!is_cc_event(hwc) && idx == NI_PMU_COUNTER_NUM)
> + return -EAGAIN; /* All general counter is in use. */
> +
> + hwc->idx = idx;
> +
> + hwc->config = hwc->config << idx * 8; /* including is_cc */
> + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +
> + local64_set(&event->hw.prev_count, 0);
> +
> + ni_pmu->events[idx] = event;
> +
> + ni_pmu_interrupt_enable(ni_pmu, cnt_en_bit(hwc));
> +
> + if (flags & PERF_EF_START)
> + ni_pmu_event_start(event, flags);
> +
> + return 0;
> +}
> +
> +static void ni_pmu_event_del(struct perf_event *event, int flags)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct ni_pmu *ni_pmu = to_ni_pmu(event->pmu);
> +
> + ni_pmu_event_stop(event, flags);
> + ni_pmu_interrupt_disable(ni_pmu, cnt_en_bit(hwc));
> + ni_pmu->events[hwc->idx] = NULL;
> +}
> +
> +static irqreturn_t _ni_pmu_handle_irq(struct ni_pmu *ni_pmu)
> +{
> + unsigned long ovsr;
> + int idx;
> + struct perf_event *event;
> +
> + ovsr = readl(ni_pmu_offset(ni_pmu, pmovsclr));
> + if (!ovsr)
> + return IRQ_NONE;
> +
> + writel(ovsr, ni_pmu_offset(ni_pmu, pmovsclr));
> +
> + for_each_set_bit(idx, (unsigned long *)&ovsr, 32) {
> + if (idx >= NI_PMU_COUNTER_NUM)
> + idx = NI_PMU_COUNTER_NUM;
> +
> + event = ni_pmu->events[idx];
> + if (WARN_ON_ONCE(!event))
> + continue;
> +
> + ni_pmu_event_update(event);
> + ni_pmu_set_period(event);
> + if (idx == NI_PMU_COUNTER_NUM)
> + break;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t ni_pmu_handle_irq(int irq_num, void *data)
> +{
> + struct ni_pmu *ni_pmu = data;
> + int idx, ret = IRQ_NONE;
> +
> + if (ni_pmu->ni->irq_num != 1)
> + return _ni_pmu_handle_irq(ni_pmu);
> +
> + for (idx = 0; idx < ni_pmu->ni->pmu_num; idx++)
> + ret |= _ni_pmu_handle_irq(ni_pmu->ni->ni_pmus[idx]);
> +
> + return ret;
> +}
> +
> +static int ni_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct global_ni *ni;
> + unsigned int target;
> + int idx;
> +
> +
> + ni = hlist_entry_safe(node, struct global_ni, node);
> + if (cpu != ni->on_cpu)
> + return 0;
> +
> +
> + target = cpumask_any_but(cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> + return 0;
> +
> +
> + for (idx = 0; idx < ni->pmu_num; idx++) {
> + perf_pmu_migrate_context(&ni->ni_pmus[idx]->pmu, cpu, target);
> + WARN_ON(irq_set_affinity(ni->ni_pmus[idx]->irq, cpumask_of(target)));
> + }
> +
> + ni->on_cpu = target;
> +
> + return 0;
> +}
> +
> +static u32 ni_child_number_total(void __iomem *periphbase,
> + void __iomem *from, enum ni_node_type type)
> +{
> + enum ni_node_type node_type;
> + int total, idx;
> + void __iomem *child_base;
> +
> + node_type = ni_node_type(from);
> +
> + if (node_type == type)
> + return 1;
> +
> + if (node_type >= NI_ASNI)
> + return 0;
> +
> + total = 0;
> + for (idx = 0; idx < ni_child_number(from); idx++) {
> + child_base = ni_child_pointer(periphbase, from, idx);
> + total += ni_child_number_total(periphbase, child_base, type);
> + }
> +
> + return total;
> +}
> +
> +static void ni_pmu_reset(struct ni_pmu *ni_pmu)
> +{
> + ni_pmu_disable(&ni_pmu->pmu);
> +
> +#define clear_reg(name) \
> + writel(readl(ni_pmu_offset(ni_pmu, name)), ni_pmu_offset(ni_pmu, name))
> +
> + clear_reg(pmcntenclr);
> + clear_reg(pmintenclr);
> + clear_reg(pmovsclr);
> +
> + writel_relaxed(NI_PMU_PMCR_RST_CYC_CNTR & NI_PMU_PMCR_RST_EV_CNTR,
> + ni_pmu_offset(ni_pmu, pmcr));
> +}
> +
> +static int ni_pmu_irq_setup(struct ni_pmu *ni_pmu, int irq_idx)
> +{
> + int err;
> + unsigned long flags = IRQF_NOBALANCING | IRQF_SHARED | IRQF_NO_THREAD;
> +
> + ni_pmu->irq = platform_get_irq(to_platform_device(ni_pmu->dev), irq_idx);
> + if (ni_pmu->irq < 0)
> + return ni_pmu->irq;
> +
> + err = devm_request_irq(ni_pmu->dev, ni_pmu->irq, ni_pmu_handle_irq,
> + flags, dev_name(ni_pmu->dev), ni_pmu);
> + if (err)
> + return err;
> +
> + err = irq_set_affinity(ni_pmu->irq, cpumask_of(ni_pmu->ni->on_cpu));
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static int ni_event_attr_init(struct device *dev,
> + struct ni_event_attr *eattr,
> + struct ni_node *node,
> + struct ni_event_desc *desc)
> +{
> + const char *name;
> +
> + sysfs_attr_init(&eattr->attr.attr);
> +
> + eattr->ev_desc = desc;
> + eattr->node = node;
> +
> + if (desc && node)
> + name = devm_kasprintf(dev,
> + GFP_KERNEL,
> + "%s_%d_%s",
> + ni_node_name[node->type],
> + node->id,
> + desc->name);
> + else if (!desc && !node)
> + name = "cycles";
> + else {
> + return -EINVAL;
> + }
> +
> + if (!name)
> + return -ENOMEM;
> +
> + eattr->attr = (struct device_attribute){
> + .attr = {
> + .name = name,
> + .mode = VERIFY_OCTAL_PERMISSIONS(0444)
> + },
> + .show = ni_event_show,
> + .store = NULL,
> + };
> +
> + return 0;
> +}
> +
> +static int ni_pmu_init_attr_groups(struct ni_pmu *ni_pmu)
> +{
> + int idx, ev_idx, ev_num, ret, ev_num_tmp;
> + struct ni_node *node;
> + struct ni_event_desc **descs;
> + struct attribute **eattrs;
> + struct ni_event_attr *ni_eattrs;
> + struct device *dev;
> + struct attribute_group *eattr_group;
> + const struct attribute_group **attr_groups;
> + const struct attribute_group *ni_attr_groups_template[4];
> +
> + dev = ni_pmu->dev;
> +
> + eattr_group = devm_kzalloc(dev, sizeof(*eattr_group), GFP_KERNEL);
> +
> + ev_num = 0;
> + for (idx = 0; idx < ni_pmu->ev_src_num; idx++) {
> + node = &ni_pmu->ev_src_nodes[idx];
> +
> + ev_num += ni_ev_desc_array_size(node->type, NULL);
> + }
> +
> + ev_num++;
> +
> + eattrs = devm_kmalloc(dev, sizeof(eattrs[0]) * (ev_num + 1), GFP_KERNEL);
> + if (!eattrs)
> + return -ENOMEM;
> +
> + ni_eattrs = devm_kzalloc(dev, sizeof(ni_eattrs[0]) * ev_num, GFP_KERNEL);
> + if (!ni_eattrs)
> + return -ENOMEM;
> +
> + ev_num = 0;
> + ret = ni_event_attr_init(dev, &ni_eattrs[ev_num++], NULL, NULL);
> + if (ret)
> + return ret;
> +
> + for (idx = 0; idx < ni_pmu->ev_src_num; idx++) {
> + node = &ni_pmu->ev_src_nodes[idx];
> +
> + ev_num_tmp = ni_ev_desc_array_size(node->type, &descs);
> + for (ev_idx = 0; ev_idx < ev_num_tmp; ev_idx++) {
> + struct ni_event_desc *desc;
> +
> + desc = descs[ev_idx];
> +
> + ret = ni_event_attr_init(dev, &ni_eattrs[ev_num++], node, desc);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + for (idx = 0; idx < ev_num; idx++)
> + eattrs[idx] = &ni_eattrs[idx].attr.attr;
> +
> + eattrs[idx] = NULL;
> +
> + eattr_group->name = "events";
> + eattr_group->attrs = eattrs;
> +
> + ni_attr_groups_template[0] = eattr_group;
> + ni_attr_groups_template[1] = &ni_format_attrs_group;
> + ni_attr_groups_template[2] = &ni_addition_attrs_group;
> + ni_attr_groups_template[3] = NULL;
> +
> + attr_groups = devm_kmemdup(dev,
> + ni_attr_groups_template,
> + sizeof(ni_attr_groups_template),
> + GFP_KERNEL);
> + if (!attr_groups)
> + return -ENOMEM;
> +
> + ni_pmu->pmu.attr_groups = attr_groups;
> +
> + return 0;
> +}
> +
> +DEFINE_FREE(cd_arrays, void __iomem **, if (_T) kfree(_T))
> +
> +static int ni_discovery(struct global_ni *ni)
> +{
> + u32 vd_idx, pd_idx, cd_idx, nd_idx, num_idx = 0;
> + void __iomem *vd, *pd, *cd, *nd;
> + int num;
> + struct ni_pmu *ni_pmu;
> + struct ni_node node;
> + void __iomem *pbase;
> + struct device *dev = ni->dev;
> +
> + void __iomem **cd_arrays __free(cd_arrays) = NULL;
> +
> + pbase = ni->base;
> +
> + cd_arrays = kmalloc_array(ni->cd_num, sizeof(typeof(cd)), GFP_KERNEL);
> +
> + /* Step1: Get all clock domains. */
> + for (vd_idx = 0; vd_idx < ni_child_number(ni->base); vd_idx++) {
> + vd = ni_child_pointer(pbase, ni->base, vd_idx);
> +
> + for (pd_idx = 0; pd_idx < ni_child_number(vd); pd_idx++) {
> + pd = ni_child_pointer(pbase, vd, pd_idx);
> +
> + dev_dbg(dev, "The %dth power domain has %d clock domain",
> + pd_idx,
> + ni_child_number(pd));
> +
> + for (cd_idx = 0; cd_idx < ni_child_number(pd); cd_idx++) {
> + cd_arrays[num_idx++] =
> + ni_child_pointer(pbase, pd, cd_idx);
> + }
> + }
> + }
> +
> + /* Step2: Traverse all clock domains. */
> + for (cd_idx = 0; cd_idx < ni->cd_num; cd_idx++) {
> + cd = cd_arrays[cd_idx];
> +
> + num = ni_child_number(cd);
> + dev_dbg(dev, "The %dth clock domain has %d child nodes:", cd_idx, num);
> +
> + /* Omit pmu node */
> + ni_pmu = devm_kzalloc(dev, struct_size(ni_pmu, ev_src_nodes, num - 1),
> + GFP_KERNEL);
> + ni_pmu->ev_src_num = num - 1;
> +
> + if (!ni_pmu)
> + return -ENOMEM;
> +
> + num_idx = 0;
> + for (nd_idx = 0; nd_idx < num; nd_idx++) {
> + nd = ni_child_pointer(pbase, cd, nd_idx);
> +
> + node.base = nd;
> + node.node_type = ni_node_node_type(nd);
> +
> + if (unlikely(ni_node_type(nd) == NI_PMU))
> + ni_pmu->pmu_node = node;
> + else
> + ni_pmu->ev_src_nodes[num_idx++] = node;
> + dev_dbg(dev, " name: %s id: %d", ni_node_name[node.type], node.id);
> + }
> +
> + ni_pmu->dev = dev;
> + ni_pmu->ni = ni;
> + ni->ni_pmus[cd_idx] = ni_pmu;
> + }
> +
> + return 0;
> +}
> +
> +static int ni_pmu_probe(struct platform_device *pdev)
> +{
> + int ret, cd_num, idx, irq_num, irq_idx;
> + void __iomem *periphbase;
> + struct global_ni *ni;
> + struct device *dev = &pdev->dev;
> + char *name;
> + static int id;
> + struct ni_pmu *ni_pmu;
> +
> + periphbase = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(periphbase))
> + return dev_err_probe(dev, PTR_ERR(periphbase), "Couldn't get ioremap\n");
> +
> + cd_num = ni_child_number_total(periphbase, periphbase, NI_CD);
> +
> + /* Each clock domain contains one PMU. So cd_num == pmu_num. */
> + ni = devm_kzalloc(dev,
> + struct_size(ni, ni_pmus, cd_num),
> + GFP_KERNEL);
> + if (!ni)
> + return -ENOMEM;
> +
> + ni->cd_num = cd_num;
> + ni->base = periphbase;
> + ni->dev = dev;
> + ni->on_cpu = raw_smp_processor_id();
> + platform_set_drvdata(pdev, ni);
> +
> + ret = ni_discovery(ni);
> + if (ret)
> + return ret;
> +
> + irq_num = platform_irq_count(pdev);
> + /* Support that one NI with one irq or one clock domain with one irq. */
> + if (irq_num < 0 || (irq_num != 1 && irq_num != ni->cd_num)) {
> + dev_err(dev, "Error in irq number: %d.", irq_num);
> + return -EINVAL;
> + }
> +
> + if (irq_num != cd_num) {
> + dev_warn(dev, "Only one IRQ found for all PMU.");
> + ret = ni_pmu_irq_setup(ni->ni_pmus[0], 0);
> + if (ret)
> + return ret;
> + }
> +
> + ni->irq_num = irq_num;
> +
> + for (idx = 0, irq_idx = 0; idx < ni->pmu_num; idx++) {
> + ni_pmu = ni->ni_pmus[idx];
> + ret = ni_pmu_init_attr_groups(ni_pmu);
> + if (ret)
> + return ret;
> +
> + if (irq_num == cd_num) {
> + ret = ni_pmu_irq_setup(ni_pmu, irq_idx++);
> + if (ret)
> + return ret;
> + }
> +
> + ni_pmu_reset(ni_pmu);
> +
> + ni_pmu->pmu = (struct pmu) {
> + .module = THIS_MODULE,
> + .task_ctx_nr = perf_invalid_context,
> + .pmu_enable = ni_pmu_enable,
> + .pmu_disable = ni_pmu_disable,
> + .event_init = ni_pmu_event_init,
> + .add = ni_pmu_event_add,
> + .del = ni_pmu_event_del,
> + .start = ni_pmu_event_start,
> + .stop = ni_pmu_event_stop,
> + .read = ni_pmu_event_read,
> + .attr_groups = ni_pmu->pmu.attr_groups,
> + .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
> + };
> +
> + ret = of_alias_get_id(pdev->dev.of_node, "ni-pmu");
> + if (ret >= 0)
> + id = ret;
> +
> + if (cd_num > 1)
> + name = devm_kasprintf(dev, GFP_KERNEL, "ni_pmu_%d_%d", id++, idx);
> + else
> + name = devm_kasprintf(dev, GFP_KERNEL, "ni_pmu_%d", id++);
> +
> + ret = perf_pmu_register(&ni_pmu->pmu, name, -1);
> + if (ret) {
> + dev_err(dev, "Error %d_%d registering PMU", id - 1, idx);
> + return ret;
> + }
> + }
> +
> + ret = cpuhp_state_add_instance_nocalls(hp_state,
> + &ni->node);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int ni_pmu_remove(struct platform_device *pdev)
> +{
> + struct global_ni *ni = platform_get_drvdata(pdev);
> + int idx;
> +
> + for (idx = 0; idx < ni->pmu_num; idx++)
> + perf_pmu_unregister(&ni->ni_pmus[idx]->pmu);
> +
> + cpuhp_remove_multi_state(hp_state);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ni_pmu_of_match[] = {
> + { .compatible = "hexin,c2000-arm-ni" },
> + {},
> +};
> +
> +static struct platform_driver ni_pmu_driver = {
> + .driver = {
> + .name = "ni-pmu",
> + .of_match_table = ni_pmu_of_match,
> + },
> + .remove = ni_pmu_remove,
> + .probe = ni_pmu_probe,
> +};
> +
> +static int __init ni_pmu_init(void)
> +{
> + int ret;
> +
> + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
> + "perf/arm/ni:online",
> + NULL,
> + ni_pmu_offline_cpu);
> + if (ret < 0)
> + return ret;
> +
> + hp_state = ret;
> +
> + ret = platform_driver_register(&ni_pmu_driver);
> + if (ret)
> + cpuhp_remove_multi_state(hp_state);
> +
> + return ret;
> +}
> +
> +static void __exit ni_pmu_exit(void)
> +{
> + cpuhp_remove_multi_state(hp_state);
> + platform_driver_unregister(&ni_pmu_driver);
> +}
> +
> +module_init(ni_pmu_init);
> +module_exit(ni_pmu_exit);
> +
> +MODULE_AUTHOR("Jialong Yang <jialong.yang@shingroup.cn>");
> +MODULE_DESCRIPTION("PMU driver for ARM NI-700 Performance Monitors Unit");
> +MODULE_LICENSE("GPL");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH v3 5/5] mm: support large folios swapin as a whole
From: Barry Song @ 2024-03-21 9:22 UTC (permalink / raw)
To: Ryan Roberts
Cc: Huang, Ying, Matthew Wilcox, akpm, linux-mm, chengming.zhou,
chrisl, david, hannes, kasong, linux-arm-kernel, linux-kernel,
mhocko, nphamcs, shy828301, steven.price, surenb, wangkefeng.wang,
xiang, yosryahmed, yuzhao, Chuanhua Han, Barry Song
In-Reply-To: <d678a127-f401-4bc2-94b5-723ce4e44270@arm.com>
On Tue, Mar 19, 2024 at 10:05 PM Ryan Roberts <ryan.roberts@arm.com> wrote:
>
> On 19/03/2024 06:27, Barry Song wrote:
> > On Tue, Mar 19, 2024 at 5:45 AM Ryan Roberts <ryan.roberts@arm.com> wrote:
> >>
> >>>>> I agree phones are not the only platform. But Rome wasn't built in a
> >>>>> day. I can only get
> >>>>> started on a hardware which I can easily reach and have enough hardware/test
> >>>>> resources on it. So we may take the first step which can be applied on
> >>>>> a real product
> >>>>> and improve its performance, and step by step, we broaden it and make it
> >>>>> widely useful to various areas in which I can't reach :-)
> >>>>
> >>>> We must guarantee the normal swap path runs correctly and has no
> >>>> performance regression when developing SWP_SYNCHRONOUS_IO optimization.
> >>>> So we have to put some effort on the normal path test anyway.
> >>>>
> >>>>> so probably we can have a sysfs "enable" entry with default "n" or
> >>>>> have a maximum
> >>>>> swap-in order as Ryan's suggestion [1] at the beginning,
> >>>>>
> >>>>> "
> >>>>> So in the common case, swap-in will pull in the same size of folio as was
> >>>>> swapped-out. Is that definitely the right policy for all folio sizes? Certainly
> >>>>> it makes sense for "small" large folios (e.g. up to 64K IMHO). But I'm not sure
> >>>>> it makes sense for 2M THP; As the size increases the chances of actually needing
> >>>>> all of the folio reduces so chances are we are wasting IO. There are similar
> >>>>> arguments for CoW, where we currently copy 1 page per fault - it probably makes
> >>>>> sense to copy the whole folio up to a certain size.
> >>>>> "
> >>
> >> I thought about this a bit more. No clear conclusions, but hoped this might help
> >> the discussion around policy:
> >>
> >> The decision about the size of the THP is made at first fault, with some help
> >> from user space and in future we might make decisions to split based on
> >> munmap/mremap/etc hints. In an ideal world, the fact that we have had to swap
> >> the THP out at some point in its lifetime should not impact on its size. It's
> >> just being moved around in the system and the reason for our original decision
> >> should still hold.
> >
> > Indeed, this is an ideal framework for smartphones and likely for
> > widely embedded
> > Linux systems utilizing zRAM. We set the mTHP size to 64KiB to
> > leverage CONT-PTE,
> > given that more than half of the memory on phones may frequently swap out and
> > swap in (for instance, when opening and switching between apps). The
> > ideal approach
> > would involve adhering to the decision made in do_anonymous_page().
> >
> >>
> >> So from that PoV, it would be good to swap-in to the same size that was
> >> swapped-out. But we only kind-of keep that information around, via the swap
> >> entry contiguity and alignment. With that scheme it is possible that multiple
> >> virtually adjacent but not physically contiguous folios get swapped-out to
> >> adjacent swap slot ranges and then they would be swapped-in to a single, larger
> >> folio. This is not ideal, and I think it would be valuable to try to maintain
> >> the original folio size information with the swap slot. One way to do this would
> >> be to store the original order for which the cluster was allocated in the
> >> cluster. Then we at least know that a given swap slot is either for a folio of
> >> that order or an order-0 folio (due to cluster exhaustion/scanning). Can we
> >> steal a bit from swap_map to determine which case it is? Or are there better
> >> approaches?
> >
> > In the case of non-SWP_SYNCHRONOUS_IO, users will invariably invoke
> > swap_readahead()
> > even when __swap_count(entry) equals 1. This leads to two scenarios:
> > swap_vma_readahead
> > and swap_cluster_readahead.
> >
> > In swap_vma_readahead, when blk_queue_nonrot, physical contiguity
> > doesn't appear to be a
> > critical concern. However, for swap_cluster_readahead, the focus
> > shifts towards the potential
> > impact of physical discontiguity.
>
> When you talk about "physical [dis]contiguity" I think you are talking about
> contiguity of the swap entries in the swap device? Both paths currently allocate
> order-0 folios to swap into, so neither have a concept of physical contiguity in
> memory at the moment.
>
> As I understand it, roughly the aim is to readahead by cluster for rotating
> disks to reduce seek time, and readahead by virtual address for non-rotating
> devices since there is no seek time cost. Correct?
From the code comment, I agree with this.
* It's a main entry function for swap readahead. By the configuration,
* it will read ahead blocks by cluster-based(ie, physical disk based)
* or vma-based(ie, virtual address based on faulty address) readahead.
>
> Note that today, swap-out on supports (2M) THP if the swap device is
> non-rotating. If it is rotating, the THP is first split. My swap-out series
> maintains this policy for mTHP. So I think we only really care about
> swap_vma_readahead() here; we want to teach it to figure out the order of the
> swap entries and swap them into folios of the same order (with a fallback to
> order-0 if allocation fails).
I agree we don't need to care about devices which rotate.
>
> >
> > struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
> > struct vm_fault *vmf)
> > {
> > struct mempolicy *mpol;
> > pgoff_t ilx;
> > struct folio *folio;
> >
> > mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> > folio = swap_use_vma_readahead() ?
> > swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
> > swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
> > mpol_cond_put(mpol);
> >
> > if (!folio)
> > return NULL;
> > return folio_file_page(folio, swp_offset(entry));
> > }
> >
> > In Android and embedded systems, SWP_SYNCHRONOUS_IO is consistently utilized,
> > rendering physical contiguity less of a concern. Moreover, instances where
> > swap_readahead() is accessed are rare, typically occurring only in scenarios
> > involving forked but non-CoWed memory.
>
> Yes understood. What I'm hearing is that for Android at least, stealing a bit
> from swap_map to remember if a swap entry is the order marked in the cluster or
> order-0 won't be noticed because almost all entries have swap count == 1. From
> memory, I think swap_map is 8 bits, and 2 bits are currently stolen, leaving 6
> bits (count = 64) before having to move to the swap map continuation stuff. Does
> anyone know what workloads provoke this overflow? What are the consequences of
> reducing that count to 32?
I'm not entirely clear on why you need bits to record this
information. Could you
provide more details?
>
> >
> > So I think large folios swap-in will at least need three steps
> >
> > 1. on SWP_SYNCHRONOUS_IO (Android and embedded Linux), this has a very
> > clear model and has no complex I/O issue.
> > 2. on nonrot block device(bdev_nonrot == true), it cares less about
> > I/O contiguity.
> > 3. on rot block devices which care about I/O contiguity.
>
> I don't think we care about (3); if the device rotates, we will have split the
> folio at swap-out, so we are only concerned with swapping-in order-0 folios.
>
> >
> > This patchset primarily addresses the systems utilizing
> > SWP_SYNCHRONOUS_IO(type1),
> > such as Android and embedded Linux, a straightforward model is established,
> > with minimal complexity regarding I/O issues.
>
> Understood. But your implication is that making swap_vma_readahead() large folio
> swap-in aware will be complex. I think we can remember the original order in the
> swap device, then it shouldn't be too difficult - conceptually at least.
Currently, I can scan PTE entries and determine the number of
contiguous swap offsets.
The swap_vma_readahead code to support large folios already exists in
OPPO's repository.
I'm confident that it can be cleaned up and submitted to LKML.
However, the issue lies with
the readahead policy. We typically prefer using the same 64KiB size as in
do_anonymous_page(), but clearly, this isn't the preference for Ying :-)
>
> >
> >>
> >> Next we (I?) have concerns about wasting IO by swapping-in folios that are too
> >> large (e.g. 2M). I'm not sure if this is a real problem or not - intuitively I'd
> >> say yes but I have no data. But on the other hand, memory is aged and
> >> swapped-out per-folio, so why shouldn't it be swapped-in per folio? If the
> >> original allocation size policy is good (it currently isn't) then a folio should
> >> be sized to cover temporally close memory and if we need to access some of it,
> >> chances are we need all of it.
> >>
> >> If we think the IO concern is legitimate then we could define a threshold size
> >> (sysfs?) for when we start swapping-in the folio in chunks. And how big should
> >> those chunks be - one page, or the threshold size itself? Probably the latter?
> >> And perhaps that threshold could also be used by zRAM to decide its upper limit
> >> for compression chunk.
> >
> >
> > Agreed. What about introducing a parameter like
> > /sys/kernel/mm/transparent_hugepage/max_swapin_order
> > giving users the opportunity to fine-tune it according to their needs. For type1
> > users specifically, setting it to any value above 4 would be
> > beneficial. If there's
> > still a lack of tuning for desktop and server environments (type 2 and type 3),
> > the default value could be set to 0.
>
> This sort of thing sounds sensible to me. But I have a history of proposing
> crappy sysfs interfaces :) So I'd like to hear from others - I suspect it will
> take a fair bit of discussion before we converge. Having data to show that this
> threshold is needed would also help (i.e. demonstration that the intuition that
> swapping in a 2M folio is often counter-productive to performance).
>
I understand. The ideal swap-in size is obviously a contentious topic :-)
However, for my real use case, simplicity reigns: we consistently adhere
to a single size - 64KiB.
> >
> >>
> >> Perhaps we can learn from khugepaged here? I think it has programmable
> >> thresholds for how many swapped-out pages can be swapped-in to aid collapse to a
> >> THP? I guess that exists for the same concerns about increased IO pressure?
> >>
> >>
> >> If we think we will ever be swapping-in folios in chunks less than their
> >> original size, then we need a separate mechanism to re-foliate them. We have
> >> discussed a khugepaged-like approach for doing this asynchronously in the
> >> background. I know that scares the Android folks, but David has suggested that
> >> this could well be very cheap compared with khugepaged, because it would be
> >> entirely limited to a single pgtable, so we only need the PTL. If we need this
> >> mechanism anyway, perhaps we should develop it and see how it performs if
> >> swap-in remains order-0? Although I guess that would imply not being able to
> >> benefit from compressing THPs for the zRAM case.
> >
> > The effectiveness of collapse operation relies on the stability of
> > forming large folios
> > to ensure optimal performance. In embedded systems, where more than half of the
> > memory may be allocated to zRAM, folios might undergo swapping out before
> > collapsing or immediately after the collapse operation. It seems a
> > TAO-like optimization
> > to decrease fallback and latency is more effective.
>
> Sorry, I'm not sure I've understood what you are saying here.
I'm not entirely clear on the specifics of the khugepaged-like
approach. However,a major
distinction for Android is that its folios may not remain in memory
for extended periods.
If we incur the cost of compaction and page migration to form a large
folio, it might soon
be swapped out. Therefore, a potentially more efficient approach could
involve a TAO-like
pool, where we obtain large folios at a low cost.
>
> >
> >>
> >> I see all this as orthogonal to synchronous vs asynchronous swap devices. I
> >> think the latter just implies that you might want to do some readahead to try to
> >> cover up the latency? If swap is moving towards being folio-orientated, then
> >> readahead also surely needs to be folio-orientated, but I think that should be
> >> the only major difference.
> >>
> >> Anyway, just some thoughts!
> >
> > Thank you very much for your valuable and insightful deliberations.
> >
> >>
> >> Thanks,
> >> Ryan
> >>
> >
Thanks
Barry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/4] media: dt-binding: media: Document rk3588’s vepu121
From: Krzysztof Kozlowski @ 2024-03-21 9:19 UTC (permalink / raw)
To: Heiko Stübner, Emmanuel Gil Peyrot, linux-kernel
Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joerg Roedel,
Will Deacon, Robin Murphy, Sebastian Reichel, Cristian Ciocaltea,
Dragan Simic, Shreeya Patel, Chris Morgan, Andy Yan,
Nicolas Frattaroli, linux-media, linux-rockchip, devicetree,
linux-arm-kernel, iommu
In-Reply-To: <855507987.0ifERbkFSE@diego>
On 21/03/2024 09:47, Heiko Stübner wrote:
>>> enum:
>>> - rockchip,rk3568-vepu
>>> + - rockchip,rk3588-vepu121
>>
>> What is 121?
>
> That is the strange naming of the ip block inside the soc.
>
> I.e. the rk3588 TRM lists a number of different video encoders and decoders:
> - VDPU121 is decoding h.263 and mpeg1,2,4
> - VDPU381 is decoding h.265, h.264 and some more
> - VDPU720 is decoding jpeg
> - VDPU981 decodes AV1
> - VEPU121 is the jpeg encoder above
> - VEPU580 encodes h.264 and h.265
>
> Each of those are separate IP blocks with their own io-memory, their own
> interrupts and their own iommus, etc.
Thanks for explanation. Short introduction in commit msg would be nice
(e.g. VEPU121, one of two VEPU encoders). OTOH, why not documenting all
of them? Bindings are supposed to be as complete as possible.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/3] drm/mediatek: Fixes for DDP component search/destroy
From: AngeloGioacchino Del Regno @ 2024-03-21 9:05 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, airlied, daniel, matthias.bgg, ck.hu, nancy.lin,
nathan.lu, dri-devel, linux-mediatek, linux-kernel,
linux-arm-kernel, kernel, wenst
In-Reply-To: <20240201125304.218467-1-angelogioacchino.delregno@collabora.com>
Il 01/02/24 13:53, AngeloGioacchino Del Regno ha scritto:
> This series performs some cleanups for DDP component CRTC search and
> correctly iounmaps the previously of_iomap() calls from drm_ddp_comp.
>
> Tested on MT8195 Cherry Tomato
>
Hello CK,
gentle ping for this series.
Cheers,
Angelo
> AngeloGioacchino Del Regno (3):
> drm/mediatek: drm_ddp_comp: Fix and cleanup DDP component CRTC search
> drm/mediatek: Perform iounmap on simple DDP component destruction
> drm/mediatek: drm_ddp_comp: Add mtk_ddp_is_simple_comp() internal
> helper
>
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 113 +++++++++++++-------
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +-
> 3 files changed, 80 insertions(+), 38 deletions(-)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3] clk: imx: imx8mp: Add pm_runtime support for power saving
From: Marc Kleine-Budde @ 2024-03-21 9:04 UTC (permalink / raw)
To: Shengjiu Wang
Cc: abelvesa, peng.fan, mturquette, sboyd, shawnguo, s.hauer, kernel,
festevam, linux-imx, shengjiu.wang, linux-clk, linux-arm-kernel,
linux-kernel
In-Reply-To: <1711010211-6825-1-git-send-email-shengjiu.wang@nxp.com>
[-- Attachment #1.1: Type: text/plain, Size: 2957 bytes --]
On 21.03.2024 16:36:51, Shengjiu Wang wrote:
> Add pm_runtime support for power saving. In pm runtime suspend
> state the registers will be reseted, so add registers save
> in pm runtime suspend and restore them in pm runtime resume.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
> changes in v3:
> - remove REGS_NUM, use the ARRAY_SIZE
> - merge clk_imx8mp_audiomix_drvdata and clk_hw_onecell_data together.
Look way nicer, but still room for improvement.
> changes in v2:
> - move pm_runtime_enable before the clk register
>
> drivers/clk/imx/clk-imx8mp-audiomix.c | 157 ++++++++++++++++++++++----
> 1 file changed, 136 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
> index 55ed211a5e0b..5ae33bce8ad8 100644
> --- a/drivers/clk/imx/clk-imx8mp-audiomix.c
> +++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
> @@ -7,10 +7,12 @@
>
> #include <linux/clk-provider.h>
> #include <linux/device.h>
> +#include <linux/io.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>
> #include <dt-bindings/clock/imx8mp-clock.h>
>
> @@ -18,6 +20,7 @@
>
> #define CLKEN0 0x000
> #define CLKEN1 0x004
> +#define EARC 0x200
> #define SAI1_MCLK_SEL 0x300
> #define SAI2_MCLK_SEL 0x304
> #define SAI3_MCLK_SEL 0x308
> @@ -26,6 +29,11 @@
> #define SAI7_MCLK_SEL 0x314
> #define PDM_SEL 0x318
> #define SAI_PLL_GNRL_CTL 0x400
> +#define SAI_PLL_FDIVL_CTL0 0x404
> +#define SAI_PLL_FDIVL_CTL1 0x408
> +#define SAI_PLL_SSCG_CTL 0x40C
> +#define SAI_PLL_MNIT_CTL 0x410
> +#define IPG_LP_CTRL 0x504
>
> #define SAIn_MCLK1_PARENT(n) \
> static const struct clk_parent_data \
> @@ -182,26 +190,82 @@ static struct clk_imx8mp_audiomix_sel sels[] = {
> CLK_SAIn(7)
> };
>
> +static const u16 audiomix_regs[] = {
> + CLKEN0,
> + CLKEN1,
> + EARC,
> + SAI1_MCLK_SEL,
> + SAI2_MCLK_SEL,
> + SAI3_MCLK_SEL,
> + SAI5_MCLK_SEL,
> + SAI6_MCLK_SEL,
> + SAI7_MCLK_SEL,
> + PDM_SEL,
> + SAI_PLL_GNRL_CTL,
> + SAI_PLL_FDIVL_CTL0,
> + SAI_PLL_FDIVL_CTL1,
> + SAI_PLL_SSCG_CTL,
> + SAI_PLL_MNIT_CTL,
> + IPG_LP_CTRL,
> +};
> +
> +struct clk_imx8mp_audiomix_priv {
> + void __iomem *base;
> + struct clk_hw_onecell_data *clk_hw_data;
> + u32 regs_save[ARRAY_SIZE(audiomix_regs)];
Put the "struct clk_hw_onecell_data clk_hw_data" at the end of the
struct. Then allocating should be easier and you don't need the
additional pointer.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
From: AngeloGioacchino Del Regno @ 2024-03-21 8:58 UTC (permalink / raw)
To: Fei Shao, Mark Brown
Cc: Daniel Kurtz, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
In-Reply-To: <20240321070942.1587146-2-fshao@chromium.org>
Il 21/03/24 08:08, Fei Shao ha scritto:
> The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
> handler may end up writing to the invalid memory and cause crashes.
>
> Add a check to trans->tx_buf before using it.
>
> Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
> Signed-off-by: Fei Shao <fshao@chromium.org>
Honestly, the code in the !host->can_dma conditional is probably a good candidate
for just going into its own function, as it is effectively an alternative flow for
the ISR (fifo vs dma xceiv) but whatever, as long as it doesn't get any "longer",
it's still fine I guess.... so:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
From: AngeloGioacchino Del Regno @ 2024-03-21 8:58 UTC (permalink / raw)
To: Fei Shao, Mark Brown
Cc: Matthias Brugger, linux-arm-kernel, linux-kernel, linux-mediatek,
linux-spi
In-Reply-To: <20240321070942.1587146-3-fshao@chromium.org>
Il 21/03/24 08:08, Fei Shao ha scritto:
> All the spi_transfer variables in this file use the name "xfer" except
> the one in mtk_spi_interrupt(). Align the naming for consistency and
> easier searching.
>
> While at it, reformat one memcpy() usage since the coding style allows
> 100 column lines today.
>
> This commit has no functional change.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3] clk: imx: imx8mp: Add pm_runtime support for power saving
From: Shengjiu Wang @ 2024-03-21 8:36 UTC (permalink / raw)
To: abelvesa, peng.fan, mturquette, sboyd, shawnguo, s.hauer, kernel,
festevam, linux-imx, shengjiu.wang
Cc: linux-clk, linux-arm-kernel, linux-kernel
Add pm_runtime support for power saving. In pm runtime suspend
state the registers will be reseted, so add registers save
in pm runtime suspend and restore them in pm runtime resume.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
changes in v3:
- remove REGS_NUM, use the ARRAY_SIZE
- merge clk_imx8mp_audiomix_drvdata and clk_hw_onecell_data together.
changes in v2:
- move pm_runtime_enable before the clk register
drivers/clk/imx/clk-imx8mp-audiomix.c | 157 ++++++++++++++++++++++----
1 file changed, 136 insertions(+), 21 deletions(-)
diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
index 55ed211a5e0b..5ae33bce8ad8 100644
--- a/drivers/clk/imx/clk-imx8mp-audiomix.c
+++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
@@ -7,10 +7,12 @@
#include <linux/clk-provider.h>
#include <linux/device.h>
+#include <linux/io.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <dt-bindings/clock/imx8mp-clock.h>
@@ -18,6 +20,7 @@
#define CLKEN0 0x000
#define CLKEN1 0x004
+#define EARC 0x200
#define SAI1_MCLK_SEL 0x300
#define SAI2_MCLK_SEL 0x304
#define SAI3_MCLK_SEL 0x308
@@ -26,6 +29,11 @@
#define SAI7_MCLK_SEL 0x314
#define PDM_SEL 0x318
#define SAI_PLL_GNRL_CTL 0x400
+#define SAI_PLL_FDIVL_CTL0 0x404
+#define SAI_PLL_FDIVL_CTL1 0x408
+#define SAI_PLL_SSCG_CTL 0x40C
+#define SAI_PLL_MNIT_CTL 0x410
+#define IPG_LP_CTRL 0x504
#define SAIn_MCLK1_PARENT(n) \
static const struct clk_parent_data \
@@ -182,26 +190,82 @@ static struct clk_imx8mp_audiomix_sel sels[] = {
CLK_SAIn(7)
};
+static const u16 audiomix_regs[] = {
+ CLKEN0,
+ CLKEN1,
+ EARC,
+ SAI1_MCLK_SEL,
+ SAI2_MCLK_SEL,
+ SAI3_MCLK_SEL,
+ SAI5_MCLK_SEL,
+ SAI6_MCLK_SEL,
+ SAI7_MCLK_SEL,
+ PDM_SEL,
+ SAI_PLL_GNRL_CTL,
+ SAI_PLL_FDIVL_CTL0,
+ SAI_PLL_FDIVL_CTL1,
+ SAI_PLL_SSCG_CTL,
+ SAI_PLL_MNIT_CTL,
+ IPG_LP_CTRL,
+};
+
+struct clk_imx8mp_audiomix_priv {
+ void __iomem *base;
+ struct clk_hw_onecell_data *clk_hw_data;
+ u32 regs_save[ARRAY_SIZE(audiomix_regs)];
+};
+
+static void clk_imx8mp_audiomix_save_restore(struct device *dev, bool save)
+{
+ struct clk_imx8mp_audiomix_priv *priv = dev_get_drvdata(dev);
+ void __iomem *base = priv->base;
+ int i;
+
+ if (save) {
+ for (i = 0; i < ARRAY_SIZE(audiomix_regs); i++)
+ priv->regs_save[i] = readl(base + audiomix_regs[i]);
+ } else {
+ for (i = 0; i < ARRAY_SIZE(audiomix_regs); i++)
+ writel(priv->regs_save[i], base + audiomix_regs[i]);
+ }
+}
+
static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
{
- struct clk_hw_onecell_data *priv;
+ struct clk_imx8mp_audiomix_priv *priv;
+ struct clk_hw_onecell_data *clk_hw_data;
struct device *dev = &pdev->dev;
void __iomem *base;
struct clk_hw *hw;
- int i;
+ int i, ret;
priv = devm_kzalloc(dev,
- struct_size(priv, hws, IMX8MP_CLK_AUDIOMIX_END),
+ sizeof(*priv) + struct_size(clk_hw_data, hws, IMX8MP_CLK_AUDIOMIX_END),
GFP_KERNEL);
if (!priv)
return -ENOMEM;
- priv->num = IMX8MP_CLK_AUDIOMIX_END;
+ priv->clk_hw_data = (struct clk_hw_onecell_data *)((void *)priv + sizeof(*priv));
+
+ clk_hw_data = priv->clk_hw_data;
+ clk_hw_data->num = IMX8MP_CLK_AUDIOMIX_END;
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
+ priv->base = base;
+ dev_set_drvdata(dev, priv);
+
+ /*
+ * pm_runtime_enable needs to be called before clk register.
+ * That is to make core->rpm_enabled to be true for clock
+ * usage.
+ */
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
for (i = 0; i < ARRAY_SIZE(sels); i++) {
if (sels[i].num_parents == 1) {
hw = devm_clk_hw_register_gate_parent_data(dev,
@@ -216,10 +280,12 @@ static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
0, NULL, NULL);
}
- if (IS_ERR(hw))
- return PTR_ERR(hw);
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
- priv->hws[sels[i].clkid] = hw;
+ clk_hw_data->hws[sels[i].clkid] = hw;
}
/* SAI PLL */
@@ -228,39 +294,86 @@ static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
ARRAY_SIZE(clk_imx8mp_audiomix_pll_parents),
CLK_SET_RATE_NO_REPARENT, base + SAI_PLL_GNRL_CTL,
0, 2, 0, NULL, NULL);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
hw = imx_dev_clk_hw_pll14xx(dev, "sai_pll", "sai_pll_ref_sel",
base + 0x400, &imx_1443x_pll);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
hw = devm_clk_hw_register_mux_parent_data_table(dev,
"sai_pll_bypass", clk_imx8mp_audiomix_pll_bypass_sels,
ARRAY_SIZE(clk_imx8mp_audiomix_pll_bypass_sels),
CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
base + SAI_PLL_GNRL_CTL, 16, 1, 0, NULL, NULL);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
hw = devm_clk_hw_register_gate(dev, "sai_pll_out", "sai_pll_bypass",
0, base + SAI_PLL_GNRL_CTL, 13,
0, NULL);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
hw = devm_clk_hw_register_fixed_factor(dev, "sai_pll_out_div2",
"sai_pll_out", 0, 1, 2);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+
+ ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
+ clk_hw_data);
+ if (ret)
+ goto err_clk_register;
+
+ pm_runtime_put_sync(dev);
+ return 0;
+
+err_clk_register:
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ return ret;
+}
+
+static int clk_imx8mp_audiomix_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static int clk_imx8mp_audiomix_runtime_suspend(struct device *dev)
+{
+ clk_imx8mp_audiomix_save_restore(dev, true);
- return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
- priv);
+ return 0;
}
+static int clk_imx8mp_audiomix_runtime_resume(struct device *dev)
+{
+ clk_imx8mp_audiomix_save_restore(dev, false);
+
+ return 0;
+}
+
+static const struct dev_pm_ops clk_imx8mp_audiomix_pm_ops = {
+ SET_RUNTIME_PM_OPS(clk_imx8mp_audiomix_runtime_suspend,
+ clk_imx8mp_audiomix_runtime_resume, NULL)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+};
+
static const struct of_device_id clk_imx8mp_audiomix_of_match[] = {
{ .compatible = "fsl,imx8mp-audio-blk-ctrl" },
{ /* sentinel */ }
@@ -269,9 +382,11 @@ MODULE_DEVICE_TABLE(of, clk_imx8mp_audiomix_of_match);
static struct platform_driver clk_imx8mp_audiomix_driver = {
.probe = clk_imx8mp_audiomix_probe,
+ .remove = clk_imx8mp_audiomix_remove,
.driver = {
.name = "imx8mp-audio-blk-ctrl",
.of_match_table = clk_imx8mp_audiomix_of_match,
+ .pm = &clk_imx8mp_audiomix_pm_ops,
},
};
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox