Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 00/30] KVM: arm64: Add support for protected guest memory with pKVM
From: Pavan Kondeti @ 2026-04-20  8:02 UTC (permalink / raw)
  To: Will Deacon
  Cc: kvmarm, linux-arm-kernel, Marc Zyngier, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Quentin Perret,
	Fuad Tabba, Vincent Donnefort, Mostafa Saleh
In-Reply-To: <20260105154939.11041-1-will@kernel.org>

Hi Will,

On Mon, Jan 05, 2026 at 03:49:08PM +0000, Will Deacon wrote:
> Hi folks,
> 
> Although pKVM has been shipping in Android kernels for a while now,
> protected guest (pVM) support has been somewhat languishing upstream.
> This has partly been because we've been waiting for guest_memfd() but
> also because it hasn't been clear how to expose pVMs to userspace (which
> is necessary for testing) without getting everything in place beforehand.
> This has led to frustration on both sides of the fence [1] and so this
> patch series attempts to get things moving again by exposing pVM
> features in an incremental fashion based on top of anonymous memory,
> which is what we have been using in Android. The big difference between
> this series and the Android implementation is the graceful handling of
> host stage-2 faults arising from accesses made using kernel mappings.
> The hope is that this will unblock pKVM upstreaming efforts while the
> guest_memfd() work continues to evolve.
> 
> Specifically, this patch series implements support for protected guest
> memory with pKVM, where pages are unmapped from the host as they are
> faulted into the guest and can be shared back from the guest using pKVM
> hypercalls. Protected guests are created using a new machine type
> identifier and can be booted to a shell using the kvmtool patches
> available at [2], which finally means that we are able to test the pVM
> logic in pKVM. Since this is an incremental step towards full isolation
> from the host (for example, the CPU register state and DMA accesses are
> not yet isolated), creating a pVM requires a developer Kconfig option to
> be enabled in addition to booting with 'kvm-arm.mode=protected' and
> results in a kernel taint.
> 

Good to see Protected VM support in upstream w/ pKVM.

We (Qualcomm) have been trying to resume Gunyah upstreaming [1] efforts 
for some time but the path to re-use guest_memfd is not straight forward as
guest_memfd is tightly coupled with KVM. While the efforts to use it for
pKVM is pending and refactoring to make it use outside KVM is not
happening anytime soon, we plan to send Gunyah series similar to how
this series is dealt with pages lent/donated to the Guest. Please let us
know if you have any suggestions/comments for us.

[1]
https://lore.kernel.org/all/20240222-gunyah-v17-0-1e9da6763d38@quicinc.com/
Thanks,
Pavan


^ permalink raw reply

* Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
From: Greg Kroah-Hartman @ 2026-04-20  8:05 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Mark Rutland, Guangshuo Li, Will Deacon, Anshuman Khandual,
	linux-arm-kernel, linux-perf-users, linux-kernel, stable
In-Reply-To: <aeXVr5enpjb3rfq7@hovoldconsulting.com>

On Mon, Apr 20, 2026 at 09:28:47AM +0200, Johan Hovold wrote:
> On Thu, Apr 16, 2026 at 10:30:23AM +0100, Mark Rutland wrote:
> > On Thu, Apr 16, 2026 at 09:23:33AM +0200, Johan Hovold wrote:
> 
> > > It's not just the platform code as this directly reflects the behaviour
> > > of device_register() as Mark pointed out.
> > > 
> > > It is indeed an unfortunate quirk of the driver model, but one can argue
> > > that having a registration function that frees its argument on errors
> > > would be even worse. And even more so when many (or most) users get this
> > > right.
> > 
> > Ah, sorry; I had missed that the _put() step would actually free the
> > object (and as you explain below, how that won't work for many callers).
> > 
> > > So if we want to change this, I think we would need to deprecate
> > > device_register() in favour of explicit device_initialize() and
> > > device_add().
> > 
> > Is is possible to have {platfom_,}device_uninitialize() functions that
> > does everything except the ->release() call? If we had that, then we'd
> > be able to have a flow along the lines of:
> > 
> > 	int some_init_function(void)
> > 	{
> > 		int err;
> > 	
> > 		platform_device_init(&static_pdev);
> > 	
> > 		err = platform_device_add(&static_pdev))
> > 		if (err)
> > 			goto out_uninit;
> > 	
> > 		return 0;
> > 	
> > 	out_uninit:
> > 		platform_device_uninit(&static_pdev);
> > 		return err;
> > 	}
> > 
> > ... which I think would align with what people generally expect to have
> > to do.
> 
> The issue here is that platform_device_add() allocates a device name and
> such resources are not released until the last reference is dropped.
> 
> It's been this way since 2008, but some of the static platform devices
> predates that and they both lack a release callback (explicitly required
> since 2003) and are not cleaned up on registration failure.
> 
> Since registration would essentially only fail during development (e.g.
> due to name collision or fault injection), this is hardly something to
> worry about, but we could consider moving towards dynamic objects to
> address both issues.

Agreed, this whole thing, including the error handling, is all just
theoretical as no real user ever hits this, which is why it has been
_way_ down my priority list.

> We have a few functions for allocating *and* registering platform
> devices that could be used in many of these cases (and they already
> clean up after themselves on errors):
> 
> 	platform_device_register_simple()
> 	platform_device_register_data()
> 	platform_device_register_resndata()
> 	platform_device_register_full()
> 
> and where those do not fit (and cannot be extended) we have the
> underlying:
> 
> 	platform_device_alloc()
> 	platform_device_add_resources()
> 	platform_device_add_data()
> 	plaform_device_add()
> 
> But there are some 800 static platform devices left, mostly in legacy
> platform code and board files that I assume few people care about.

Yes, I agree that we do have all of the needed apis here already, we
should just work at converting existing drivers to the new apis OR just
not caring at all as again, no one will ever hit these code paths :)

thanks,

greg k-h


^ permalink raw reply

* [PATCH net v2 0/2] net: airoha: Fix NULL pointer derefrences in airoha_qdma_cleanup()
From: Lorenzo Bianconi @ 2026-04-20  8:07 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lorenzo Bianconi
  Cc: Simon Horman, linux-arm-kernel, linux-mediatek, netdev

Fix two possible NULL pointer derefrences in airoha_qdma_cleanup routine
if airoha_qdma_init() fails.

---
Changes in v2:
- Move page_pool allocation after desc list allocation in
  airoha_qdma_init_rx_queue()
- Move netif_napi_add_tx() after irq desc queue allocation in
  airoha_qdma_tx_irq_init()
- Link to v1: https://lore.kernel.org/r/20260417-airoha_qdma_init_rx_queue-fix-v1-0-db9fa5e468e5@kernel.org

---
Lorenzo Bianconi (2):
      net: airoha: Move ndesc initialization at end of airoha_qdma_init_rx_queue()
      net: airoha: Add size check for TX NAPIs in airoha_qdma_cleanup()

 drivers/net/ethernet/airoha/airoha_eth.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
---
base-commit: 0cf004ffb61cd32d140531c3a84afe975f9fc7ea
change-id: 20260417-airoha_qdma_init_rx_queue-fix-b9bfada51671

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>



^ permalink raw reply

* [PATCH net v2 1/2] net: airoha: Move ndesc initialization at end of airoha_qdma_init_rx_queue()
From: Lorenzo Bianconi @ 2026-04-20  8:07 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lorenzo Bianconi
  Cc: Simon Horman, linux-arm-kernel, linux-mediatek, netdev
In-Reply-To: <20260420-airoha_qdma_init_rx_queue-fix-v2-0-d99347e5c18d@kernel.org>

If queue entry or DMA descriptor list allocation fails in
airoha_qdma_init_rx_queue routine, airoha_qdma_cleanup() will trigger a
NULL pointer dereference running netif_napi_del() for RX queue NAPIs
since netif_napi_add() has never been executed to this particular RX NAPI.
The issue is due to the early ndesc initialization in
airoha_qdma_init_rx_queue() since airoha_qdma_cleanup() relies on ndesc
value to check if the queue is properly initialized. Fix the issue moving
ndesc initialization at end of airoha_qdma_init_tx routine.
Move page_pool allocation after descriptor list allocation in order to
avoid memory leaks if desc allocation fails.

Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index e1ab15f1ee7d..fc79c456743c 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -745,14 +745,18 @@ static int airoha_qdma_init_rx_queue(struct airoha_queue *q,
 	dma_addr_t dma_addr;
 
 	q->buf_size = PAGE_SIZE / 2;
-	q->ndesc = ndesc;
 	q->qdma = qdma;
 
-	q->entry = devm_kzalloc(eth->dev, q->ndesc * sizeof(*q->entry),
+	q->entry = devm_kzalloc(eth->dev, ndesc * sizeof(*q->entry),
 				GFP_KERNEL);
 	if (!q->entry)
 		return -ENOMEM;
 
+	q->desc = dmam_alloc_coherent(eth->dev, ndesc * sizeof(*q->desc),
+				      &dma_addr, GFP_KERNEL);
+	if (!q->desc)
+		return -ENOMEM;
+
 	q->page_pool = page_pool_create(&pp_params);
 	if (IS_ERR(q->page_pool)) {
 		int err = PTR_ERR(q->page_pool);
@@ -761,11 +765,7 @@ static int airoha_qdma_init_rx_queue(struct airoha_queue *q,
 		return err;
 	}
 
-	q->desc = dmam_alloc_coherent(eth->dev, q->ndesc * sizeof(*q->desc),
-				      &dma_addr, GFP_KERNEL);
-	if (!q->desc)
-		return -ENOMEM;
-
+	q->ndesc = ndesc;
 	netif_napi_add(eth->napi_dev, &q->napi, airoha_qdma_rx_napi_poll);
 
 	airoha_qdma_wr(qdma, REG_RX_RING_BASE(qid), dma_addr);

-- 
2.53.0



^ permalink raw reply related

* [PATCH net v2 2/2] net: airoha: Add size check for TX NAPIs in airoha_qdma_cleanup()
From: Lorenzo Bianconi @ 2026-04-20  8:07 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Lorenzo Bianconi
  Cc: Simon Horman, linux-arm-kernel, linux-mediatek, netdev
In-Reply-To: <20260420-airoha_qdma_init_rx_queue-fix-v2-0-d99347e5c18d@kernel.org>

If airoha_qdma_init routine fails before airoha_qdma_tx_irq_init() runs
successfully for all TX NAPIs, airoha_qdma_cleanup() will
unconditionally runs netif_napi_del() on TX NAPIs, triggering a NULL
pointer dereference. Fix the issue relying on q_tx_irq size value to
check if the TX NAPIs is properly initialized in airoha_qdma_cleanup().
Moreover, run netif_napi_add_tx() just if irq_q queue is properly
allocated.

Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index fc79c456743c..fd8c4f817d85 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -996,8 +996,6 @@ static int airoha_qdma_tx_irq_init(struct airoha_tx_irq_queue *irq_q,
 	struct airoha_eth *eth = qdma->eth;
 	dma_addr_t dma_addr;
 
-	netif_napi_add_tx(eth->napi_dev, &irq_q->napi,
-			  airoha_qdma_tx_napi_poll);
 	irq_q->q = dmam_alloc_coherent(eth->dev, size * sizeof(u32),
 				       &dma_addr, GFP_KERNEL);
 	if (!irq_q->q)
@@ -1007,6 +1005,9 @@ static int airoha_qdma_tx_irq_init(struct airoha_tx_irq_queue *irq_q,
 	irq_q->size = size;
 	irq_q->qdma = qdma;
 
+	netif_napi_add_tx(eth->napi_dev, &irq_q->napi,
+			  airoha_qdma_tx_napi_poll);
+
 	airoha_qdma_wr(qdma, REG_TX_IRQ_BASE(id), dma_addr);
 	airoha_qdma_rmw(qdma, REG_TX_IRQ_CFG(id), TX_IRQ_DEPTH_MASK,
 			FIELD_PREP(TX_IRQ_DEPTH_MASK, size));
@@ -1398,8 +1399,12 @@ static void airoha_qdma_cleanup(struct airoha_qdma *qdma)
 		}
 	}
 
-	for (i = 0; i < ARRAY_SIZE(qdma->q_tx_irq); i++)
+	for (i = 0; i < ARRAY_SIZE(qdma->q_tx_irq); i++) {
+		if (!qdma->q_tx_irq[i].size)
+			continue;
+
 		netif_napi_del(&qdma->q_tx_irq[i].napi);
+	}
 
 	for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++) {
 		if (!qdma->q_tx[i].ndesc)

-- 
2.53.0



^ permalink raw reply related

* RE: [PATCH V13 02/12] PCI: host-generic: Add common helpers for parsing Root Port properties
From: Sherry Sun @ 2026-04-20  8:24 UTC (permalink / raw)
  To: mani@kernel.org, Bjorn Helgaas
  Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	Frank Li, s.hauer@pengutronix.de, kernel@pengutronix.de,
	festevam@gmail.com, lpieralisi@kernel.org, kwilczynski@kernel.org,
	bhelgaas@google.com, Hongxing Zhu, l.stach@pengutronix.de,
	imx@lists.linux.dev, linux-pci@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <viggqsxczf5d5hok4qpqhknalwb46xapsgdxbbgbqhruhyn2hn@wtck4yajmuw7>

> On Fri, Apr 17, 2026 at 02:55:33PM -0500, Bjorn Helgaas wrote:
> > On Fri, Apr 17, 2026 at 03:17:16AM +0000, Sherry Sun wrote:
> > > > On Thu, Apr 16, 2026 at 07:14:12PM +0800, Sherry Sun wrote:
> > > > > Introduce generic helper functions to parse Root Port device
> > > > > tree nodes and extract common properties like reset GPIOs. This
> > > > > allows multiple PCI host controller drivers to share the same
> > > > > parsing logic.
> > > > >
> > > > > Define struct pci_host_port to hold common Root Port properties
> > > > > (currently only reset GPIO descriptor) and add
> > > > > pci_host_common_parse_ports() to parse Root Port nodes from
> > > > > device tree.
> > > >
> > > > Are the Root Port and the RC the only possible places for 'reset'
> > > > GPIO descriptions in DT?  I think PERST# routing is outside the
> > > > PCIe spec, so it seems like a system could provide a PERST# GPIO
> > > > routed to any Switch Upstream Port or Endpoint (I assume a PERST#
> > > > connected to a switch would apply to both the upstream port and
> > > > the downstream ports).
> > >
> > > Thanks for the feedback. You're right that PERST# routing could
> > > theoretically be connected to any device in the hierarchy. However,
> > > for this patch series, I've focused on the most common use case in
> > > practice: use Root Port level PERST# instead of the legacy Root
> > > Complex level PERST#.
> > >
> > > Root Port level PERST# - This is the primary target, where each Root
> > > Port has individual control over devices connected to it.  RC level
> > > PERST# - Legacy binding support, where a single GPIO controls all
> > > ports.
> > >
> > > We can extend this framework later if real hardware emerges that
> > > needs Switch or EP-level PERST# control. I can add a comment
> > > documenting this limitation if needed.
> > >
> > > BTW, Mani and Rob had some great discussions in dt-schema about
> > > PERST# and WAKE# sideband signals settings.
> >
> > > You can check here:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > thub.com%2Fdevicetree-org%2Fdt-
> schema%2Fissues%2F168&data=05%7C02%7C
> > >
> sherry.sun%40nxp.com%7C232644f8bbe64279f77908de9ea20b09%7C686ea1
> d3bc
> > >
> 2b4c6fa92cd99c5c301635%7C0%7C0%7C639122615977862858%7CUnknown
> %7CTWFp
> > >
> bGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4z
> MiIs
> > >
> IkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=r7szCLCsGFN2
> 1ULZ
> > > ibH7Ga%2FH0e6VyIdqznKCJ6yIGM4%3D&reserved=0
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > thub.com%2Fdevicetree-org%2Fdt-
> schema%2Fpull%2F126&data=05%7C02%7Csh
> > >
> erry.sun%40nxp.com%7C232644f8bbe64279f77908de9ea20b09%7C686ea1d
> 3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C639122615977892044%7CUnknown%7
> CTWFpbG
> > >
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiI
> sIk
> > >
> FOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=o3RIy1SfvTGfkX
> 9rm8
> > > dNH2or5SZ7v5bYF%2Fl1XGaf8aA%3D&reserved=0
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > thub.com%2Fdevicetree-org%2Fdt-
> schema%2Fpull%2F170&data=05%7C02%7Csh
> > >
> erry.sun%40nxp.com%7C232644f8bbe64279f77908de9ea20b09%7C686ea1d
> 3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C639122615977910169%7CUnknown%7
> CTWFpbG
> > >
> Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiI
> sIk
> > >
> FOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=d8SBGcqKcjYe1i
> iqs9
> > > %2F%2Bg1o%2FbECHYtnEULg7hTXyKmY%3D&reserved=0
> >
> > The upshot of all those conversations is that WAKE# and PERST# can be
> > routed to arbitrary devices independent of the PCI topology.
> >
> > I think extending host-generic to look for 'reset' in Root Port nodes
> > is the right thing.  My concern is more about where we store it.  This
> > patch saves it in a new "pci_host_port" struct, but someday we'll want
> > a place to save the PERST# GPIOs for several slots behind a switch.
> > Then we'll have two different ways to save the same information.
> >
> 
> Even if there are PERST# GPIOs from the host, connected to downstream
> ports of a PCIe switch, they could be stored in the Root Port's (pci_host_port)
> struct as a list of PERST#. This is what pcie-qcom driver does.
> 
> It is too clumsy to handle PERST# individually for each device. We tried it
> before with pwrctrl, but it always ended up biting us on who gets to control
> the PERST#. We can't let pwrctrl handle PERST# for a switch port and host
> controller driver handle it for RP. And we cannot let pwrctrl handle PERST# for
> all ports, because, host controller drivers also need to control them for RC
> initialization.
> 
> That's why it was decided to handle PERST# for all ports in the host controller
> drivers. So following that pattern, this helper could also be extended to parse
> the PERST# from all ports defined in DT and store them in the same Root Port
> struct.
> 
> It should be trivial to implement this logic in the current helper. @Sherry:
> Could you please implement this logic?

Hi Mani, do you mean the similar logic in this patch?
https://lore.kernel.org/all/20251216-pci-pwrctrl-rework-v2-1-745a563b9be6@oss.qualcomm.com/
If yes, of cause I can do this for current helper functions in pci-host-common.c.

Best Regards
Sherry

^ permalink raw reply

* Re: [PATCH v7 3/4] KVM: arm64: PMU: Introduce FIXED_COUNTERS_ONLY
From: Akihiko Odaki @ 2026-04-20  8:36 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, Kees Cook, Gustavo A. R. Silva,
	Paolo Bonzini, Jonathan Corbet, Shuah Khan, linux-arm-kernel,
	kvmarm, linux-kernel, linux-hardening, devel, kvm, linux-doc,
	linux-kselftest
In-Reply-To: <87ldeic1gk.wl-maz@kernel.org>

On 2026/04/20 2:19, Marc Zyngier wrote:
> On Sat, 18 Apr 2026 09:14:25 +0100,
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> On a heterogeneous arm64 system, KVM's PMU emulation is based on the
>> features of a single host PMU instance. When a vCPU is migrated to a
>> pCPU with an incompatible PMU, counters such as PMCCNTR_EL0 stop
>> incrementing.
>>
>> Although this behavior is permitted by the architecture, Windows does
>> not handle it gracefully and may crash with a division-by-zero error.
>>
>> The current workaround requires VMMs to pin vCPUs to a set of pCPUs
>> that share a compatible PMU. This is difficult to implement correctly in
>> QEMU/libvirt, where pinning occurs after vCPU initialization, and it
>> also restricts the guest to a subset of available pCPUs.
>>
>> Introduce the KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY attribute to
>> create a "fixed-counters-only" PMU. When set, KVM exposes a PMU that is
>> compatible with all pCPUs but that does not support programmable
>> event counters which may have different feature sets on different PMUs.
>>
>> This allows Windows guests to run reliably on heterogeneous systems
>> without crashing, even without vCPU pinning, and enables VMMs to
>> schedule vCPUs across all available pCPUs, making full use of the host
>> hardware.
>>
>> Much like KVM_ARM_VCPU_PMU_V3_IRQ and other read-write attributes, this
>> attribute provides a getter that facilitates kernel and userspace
>> debugging/testing.
> 
> OK, so that's the sales pitch. But how is it implemented? I would like
> to be able to read a high-level description of the implementation
> trade-offs.

Implementation-wise it is very trivial. Essentially the following 
addition in kvm_arm_pmu_v3_get_attr() is the entire implementation:
+	case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
+		if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, 
&vcpu->kvm->arch.flags))
+			return 0;

Both its functionality and code complexity is trivial. So we can argue that:
- the functionality is too trivial to be useful or
- the interface/implementation complexity is so trivial that it does not
   incur maintenance burden

In this case the selftest uses the getter so I was more inclined to have 
it, but adding one just for the selftest sounds too ad-hoc, so here I 
looked into other attributes to ensure that it was not introducing 
inconsistency with existing interfaces.

As the result, I found there are other read-write attributes; in fact 
there are more read-write attributes than write-only ones.

> 
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>>   Documentation/virt/kvm/devices/vcpu.rst |  29 ++++++
>>   arch/arm64/include/asm/kvm_host.h       |   2 +
>>   arch/arm64/include/uapi/asm/kvm.h       |   1 +
>>   arch/arm64/kvm/arm.c                    |   1 +
>>   arch/arm64/kvm/pmu-emul.c               | 155 +++++++++++++++++++++++---------
>>   include/kvm/arm_pmu.h                   |   2 +
>>   6 files changed, 147 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/virt/kvm/devices/vcpu.rst b/Documentation/virt/kvm/devices/vcpu.rst
>> index 60bf205cb373..e0aeb1897d77 100644
>> --- a/Documentation/virt/kvm/devices/vcpu.rst
>> +++ b/Documentation/virt/kvm/devices/vcpu.rst
>> @@ -161,6 +161,35 @@ explicitly selected, or the number of counters is out of range for the
>>   selected PMU. Selecting a new PMU cancels the effect of setting this
>>   attribute.
>>   
>> +1.6 ATTRIBUTE: KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY
>> +------------------------------------------------------
>> +
>> +:Parameters: no additional parameter in kvm_device_attr.addr
>> +
>> +:Returns:
>> +
>> +	 =======  =====================================================
>> +	 -EBUSY   Attempted to set after initializing PMUv3 or running
>> +		  VCPU, or attempted to set for the first time after
>> +		  setting an event filter
>> +	 -ENXIO   Attempted to get before setting
>> +	 -ENODEV  Attempted to set while PMUv3 not supported
>> +	 =======  =====================================================
>> +
>> +If set, PMUv3 will be emulated without programmable event counters. The VCPU
>> +will use any compatible hardware PMU. This attribute is particularly useful on
> 
> Not quite "any PMU". It will use *the* PMU of the physical CPU,
> irrespective of the implementation.

I think:

- this comment
- one on the KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED note
- one on kvm_pmu_create_perf_event()
- and one on kvm_arm_pmu_v3_set_pmu_fixed_counters_only()

All boil down into one question: will it support all possible CPUs, or 
will it support a subset? Let me answer here:

This patch is written to support a subset instead of all possible CPUs. 
If a pCPU does not have a compatible PMU, the pCPU will not be supported 
and cause KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED.

This patch does not enforce all possible CPUs are covered by the 
compatible PMUs. Theoretically speaking, kvm_arm_pmu_get_pmuver_limit() 
enables the PMU emulation when real PMUv3 hardware covers all possible 
CPUs *or* the relevant registers can be trapped with IMPDEF, so some 
pCPU may not have a compatible PMU and only provide the IMPDEF trapping.

Practically, I don't think any sane configuration will ever have such a 
subset support, so we can explicitly enforce all possible CPUs are 
covered by the compatible PMUs if desired.

> 
>> +heterogeneous systems where different hardware PMUs cover different physical
>> +CPUs. The compatibility of hardware PMUs can be checked with
>> +KVM_ARM_VCPU_PMU_V3_SET_PMU. All VCPUs in a VM share this attribute. It isn't
>> +possible to set it for the first time if a PMU event filter is already present.
> 
> "for the first time" gives the impression that it will work if you try
> again. I'd rather we say that "This feature is incompatible with the
> existence of a PMU event filter".

The following sequence will work:
1. Set KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY
2. Set KVM_ARM_VCPU_PMU_V3_FILTER
3. Set KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY

This is to make the behavior conistent with KVM_ARM_VCPU_PMU_V3_SET_PMU.

> 
> Furthermore, the architecture currently describes *two* fixed-function
> counters (cycles and instructions), while KVM only expose the cycle
> counter. I'm all for the extra abstraction, but what does it mean for
> migration if we enable FEAT_PMUv3_ICNTR?

I'll answe this at the end of this email.

> 
>> +
>> +Note that KVM will not make any attempts to run the VCPU on the physical CPUs
>> +with compatible hardware PMUs. This is entirely left to userspace. However,
>> +attempting to run the VCPU on an unsupported CPU will fail and KVM_RUN will
>> +return with exit_reason = KVM_EXIT_FAIL_ENTRY and populate the fail_entry struct
>> +by setting hardware_entry_failure_reason field to
>> +KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED and the cpu field to the processor id.
>> +
> 
> This is mostly a copy-paste of the previous section. How relevant is
> this to the fixed-counters-only feature? If the whole point of this
> stuff is to ensure compatibility across CPUs with different PMU
> implementations, surely what you describe here is the opposite of what
> you want.

Please see the earlier discussion of supported pCPUs.

> 
> My preference would be to move this to a separate patch in any case,
> more on that below.

I will do so with the next version.

> 
>>   2. GROUP: KVM_ARM_VCPU_TIMER_CTRL
>>   =================================
>>   
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 59f25b85be2b..b59e0182472c 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -353,6 +353,8 @@ struct kvm_arch {
>>   #define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS		10
>>   	/* Unhandled SEAs are taken to userspace */
>>   #define KVM_ARCH_FLAG_EXIT_SEA				11
>> +	/* PMUv3 is emulated without progammable event counters */
>> +#define KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY	12
>>   	unsigned long flags;
>>   
>>   	/* VM-wide vCPU feature set */
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index a792a599b9d6..474c84fa757f 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -436,6 +436,7 @@ enum {
>>   #define   KVM_ARM_VCPU_PMU_V3_FILTER		2
>>   #define   KVM_ARM_VCPU_PMU_V3_SET_PMU		3
>>   #define   KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS	4
>> +#define   KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY	5
>>   #define KVM_ARM_VCPU_TIMER_CTRL		1
>>   #define   KVM_ARM_VCPU_TIMER_IRQ_VTIMER		0
>>   #define   KVM_ARM_VCPU_TIMER_IRQ_PTIMER		1
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index 620a465248d1..dca16ca26d32 100644
>> --- a/arch/arm64/kvm/arm.c
>> +++ b/arch/arm64/kvm/arm.c
>> @@ -634,6 +634,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>   	if (has_vhe())
>>   		kvm_vcpu_load_vhe(vcpu);
>>   	kvm_arch_vcpu_load_fp(vcpu);
>> +	kvm_vcpu_load_pmu(vcpu);
>>   	kvm_vcpu_pmu_restore_guest(vcpu);
>>   	if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
>>   		kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
>> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
>> index ef5140bbfe28..d1009c144581 100644
>> --- a/arch/arm64/kvm/pmu-emul.c
>> +++ b/arch/arm64/kvm/pmu-emul.c
>> @@ -326,7 +326,10 @@ u64 kvm_pmu_implemented_counter_mask(struct kvm_vcpu *vcpu)
>>   
>>   static void kvm_pmc_enable_perf_event(struct kvm_pmc *pmc)
>>   {
>> -	if (!pmc->perf_event) {
>> +	struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
>> +
>> +	if (!pmc->perf_event ||
>> +	    !cpumask_test_cpu(vcpu->cpu, &to_arm_pmu(pmc->perf_event->pmu)->supported_cpus)) {
>>   		kvm_pmu_create_perf_event(pmc);
>>   		return;
>>   	}
>> @@ -667,10 +670,8 @@ static bool kvm_pmc_counts_at_el2(struct kvm_pmc *pmc)
>>   	return kvm_pmc_read_evtreg(pmc) & ARMV8_PMU_INCLUDE_EL2;
>>   }
>>   
>> -static int kvm_map_pmu_event(struct kvm *kvm, unsigned int eventsel)
>> +static int kvm_map_pmu_event(struct arm_pmu *pmu, unsigned int eventsel)
>>   {
>> -	struct arm_pmu *pmu = kvm->arch.arm_pmu;
>> -
>>   	/*
>>   	 * The CPU PMU likely isn't PMUv3; let the driver provide a mapping
>>   	 * for the guest's PMUv3 event ID.
> 
> This refactor should be in its own patch. This sort of minor change is
> adding noise to the mean of the patch, for no good reason.

I'll make that change with the next version too.

> 
>> @@ -681,6 +682,23 @@ static int kvm_map_pmu_event(struct kvm *kvm, unsigned int eventsel)
>>   	return eventsel;
>>   }
>>   
>> +static struct arm_pmu *kvm_pmu_probe_armpmu(int cpu)
>> +{
>> +	struct arm_pmu_entry *entry;
>> +	struct arm_pmu *pmu;
>> +
>> +	guard(rcu)();
>> +
>> +	list_for_each_entry_rcu(entry, &arm_pmus, entry) {
>> +		pmu = entry->arm_pmu;
>> +
>> +		if (cpumask_test_cpu(cpu, &pmu->supported_cpus))
>> +			return pmu;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>>   /**
>>    * kvm_pmu_create_perf_event - create a perf event for a counter
>>    * @pmc: Counter context
>> @@ -694,6 +712,12 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
>>   	int eventsel;
>>   	u64 evtreg;
>>   
>> +	if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &vcpu->kvm->arch.flags)) {
>> +		arm_pmu = kvm_pmu_probe_armpmu(vcpu->cpu);
>> +		if (!arm_pmu)
>> +			return;
> 
> How is it possible to not get a PMU here? We don't expose the PMU to a
> guest at all if there are CPUs without PMUs, see the comment in
> kvm_host_pmu_init(). So I'd expect this to never fail.

Please see the earlier comment.

> 
>> +	}
>> +
>>   	evtreg = kvm_pmc_read_evtreg(pmc);
>>   
>>   	kvm_pmu_stop_counter(pmc);
>> @@ -722,7 +746,7 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
>>   	 * Don't create an event if we're running on hardware that requires
>>   	 * PMUv3 event translation and we couldn't find a valid mapping.
>>   	 */
>> -	eventsel = kvm_map_pmu_event(vcpu->kvm, eventsel);
>> +	eventsel = kvm_map_pmu_event(arm_pmu, eventsel);
>>   	if (eventsel < 0)
>>   		return;
>>   
>> @@ -810,42 +834,6 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
>>   	list_add_tail_rcu(&entry->entry, &arm_pmus);
>>   }
>>   
>> -static struct arm_pmu *kvm_pmu_probe_armpmu(void)
>> -{
>> -	struct arm_pmu_entry *entry;
>> -	struct arm_pmu *pmu;
>> -	int cpu;
>> -
>> -	guard(rcu)();
>> -
>> -	/*
>> -	 * It is safe to use a stale cpu to iterate the list of PMUs so long as
>> -	 * the same value is used for the entirety of the loop. Given this, and
>> -	 * the fact that no percpu data is used for the lookup there is no need
>> -	 * to disable preemption.
>> -	 *
>> -	 * It is still necessary to get a valid cpu, though, to probe for the
>> -	 * default PMU instance as userspace is not required to specify a PMU
>> -	 * type. In order to uphold the preexisting behavior KVM selects the
>> -	 * PMU instance for the core during vcpu init. A dependent use
>> -	 * case would be a user with disdain of all things big.LITTLE that
>> -	 * affines the VMM to a particular cluster of cores.
>> -	 *
>> -	 * In any case, userspace should just do the sane thing and use the UAPI
>> -	 * to select a PMU type directly. But, be wary of the baggage being
>> -	 * carried here.
>> -	 */
>> -	cpu = raw_smp_processor_id();
>> -	list_for_each_entry_rcu(entry, &arm_pmus, entry) {
>> -		pmu = entry->arm_pmu;
>> -
>> -		if (cpumask_test_cpu(cpu, &pmu->supported_cpus))
>> -			return pmu;
>> -	}
>> -
>> -	return NULL;
>> -}
>> -
> 
> Same thing for the refactoring of this function. Moving it, changing
> the signature and moving the comment somewhere else would be better
> placed on its own.

This will be in a separate patch with the next version.

> 
>>   static u64 __compute_pmceid(struct arm_pmu *pmu, bool pmceid1)
>>   {
>>   	u32 hi[2], lo[2];
>> @@ -888,6 +876,9 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>>   	u64 val, mask = 0;
>>   	int base, i, nr_events;
>>   
>> +	if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &vcpu->kvm->arch.flags))
>> +		return 0;
>> +
>>   	if (!pmceid1) {
>>   		val = compute_pmceid0(cpu_pmu);
>>   		base = 0;
>> @@ -915,6 +906,26 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>>   	return val & mask;
>>   }
>>   
>> +void kvm_vcpu_load_pmu(struct kvm_vcpu *vcpu)
>> +{
>> +	unsigned long mask = kvm_pmu_enabled_counter_mask(vcpu);
>> +	struct kvm_pmc *pmc;
>> +	struct arm_pmu *cpu_pmu;
> 
> Move these to be inside the loop.

I followed the pattern of other functions, but I agree this new code can 
follow a more modern style. It will be done with the next version.

> 
>> +	int i;
>> +
>> +	for_each_set_bit(i, &mask, 32) {
>> +		pmc = kvm_vcpu_idx_to_pmc(vcpu, i);
>> +		if (!pmc->perf_event)
>> +			continue;
>> +
>> +		cpu_pmu = to_arm_pmu(pmc->perf_event->pmu);
>> +		if (!cpumask_test_cpu(vcpu->cpu, &cpu_pmu->supported_cpus)) {
>> +			kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);
>> +			break;
>> +		}
>> +	}
>> +}
>> +
> 
> Why do we need to inflict this on VMs that do not have the fixed
> counter restriction?

This function is to re-create the perf_event in case the current 
perf_event does not support the pCPU because e.g., the pCPU is a E-core 
while the perf_event only covers the P-cores.

> 
> And even then, all you have to reconfigure is the cycle counter. So
> why the loop? All we want to find out is whether the cycle counter is
> instantiated on the PMU that matches the current CPU.

I just wanted to avoid hardcoding assumptions on the fixed counter(s). 
FEAT_PMUv3_ICNTR will be naturaly handled with a loop, for example.

> 
>>   void kvm_vcpu_reload_pmu(struct kvm_vcpu *vcpu)
>>   {
>>   	u64 mask = kvm_pmu_implemented_counter_mask(vcpu);
>> @@ -1016,6 +1027,9 @@ u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm)
>>   {
>>   	struct arm_pmu *arm_pmu = kvm->arch.arm_pmu;
>>   
>> +	if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags))
>> +		return 0;
>> +
>>   	/*
>>   	 * PMUv3 requires that all event counters are capable of counting any
>>   	 * event, though the same may not be true of non-PMUv3 hardware.
>> @@ -1070,7 +1084,24 @@ static void kvm_arm_set_pmu(struct kvm *kvm, struct arm_pmu *arm_pmu)
>>    */
>>   int kvm_arm_set_default_pmu(struct kvm *kvm)
>>   {
>> -	struct arm_pmu *arm_pmu = kvm_pmu_probe_armpmu();
>> +	/*
>> +	 * It is safe to use a stale cpu to iterate the list of PMUs so long as
>> +	 * the same value is used for the entirety of the loop. Given this, and
>> +	 * the fact that no percpu data is used for the lookup there is no need
>> +	 * to disable preemption.
>> +	 *
>> +	 * It is still necessary to get a valid cpu, though, to probe for the
>> +	 * default PMU instance as userspace is not required to specify a PMU
>> +	 * type. In order to uphold the preexisting behavior KVM selects the
>> +	 * PMU instance for the core during vcpu init. A dependent use
>> +	 * case would be a user with disdain of all things big.LITTLE that
>> +	 * affines the VMM to a particular cluster of cores.
>> +	 *
>> +	 * In any case, userspace should just do the sane thing and use the UAPI
>> +	 * to select a PMU type directly. But, be wary of the baggage being
>> +	 * carried here.
>> +	 */
>> +	struct arm_pmu *arm_pmu = kvm_pmu_probe_armpmu(raw_smp_processor_id());
>>   
>>   	if (!arm_pmu)
>>   		return -ENODEV;
>> @@ -1098,6 +1129,7 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu *vcpu, int pmu_id)
>>   				break;
>>   			}
>>   
>> +			clear_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags);
> 
> Why does this need to be cleared? I'd rather we make sure it is never
> set the first place.

KVM_ARM_VCPU_PMU_V3_SET_PMU and KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY 
can be set on the same VCPU. The last KVM_ARM_VCPU_PMU_V3_SET_PMU or 
KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY setting will be effective.

A VMM may try set these attributes to check if the setting is supported. 
For example, the RFC QEMU patch first uses KVM_ARM_VCPU_PMU_V3_SET_PMU 
to find a compatible PMU that covers all pCPUs, and then falls back to 
KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY. The order of such probing is up 
to the VMM.

This rationale applies also to the next comment.

> 
>>   			kvm_arm_set_pmu(kvm, arm_pmu);
>>   			cpumask_copy(kvm->arch.supported_cpus, &arm_pmu->supported_cpus);
>>   			ret = 0;
>> @@ -1108,11 +1140,42 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu *vcpu, int pmu_id)
>>   	return ret;
>>   }
>>   
>> +static int kvm_arm_pmu_v3_set_pmu_fixed_counters_only(struct kvm_vcpu *vcpu)
>> +{
>> +	struct kvm *kvm = vcpu->kvm;
>> +	struct arm_pmu_entry *entry;
>> +	struct arm_pmu *arm_pmu;
>> +	struct cpumask *supported_cpus = kvm->arch.supported_cpus;
>> +
>> +	lockdep_assert_held(&kvm->arch.config_lock);
>> +
>> +	if (kvm_vm_has_ran_once(kvm) ||
>> +	    (kvm->arch.pmu_filter &&
>> +	     !test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags)))
>> +		return -EBUSY;
>> +
>> +	set_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags);
>> +	kvm_arm_set_nr_counters(kvm, 0);
>> +	cpumask_clear(supported_cpus);
> 
> What is the purpose of this cpumask_clear()? Under what conditions can
> you have something else?
> 
>> +
>> +	guard(rcu)();
>> +
>> +	list_for_each_entry_rcu(entry, &arm_pmus, entry) {
>> +		arm_pmu = entry->arm_pmu;
>> +		cpumask_or(supported_cpus, supported_cpus, &arm_pmu->supported_cpus);
> 
> Why isn't supported_cpus directly set to possible_cpus? Isn't that the
> base requirement that you can run on any CPU at all?

Please see the earlier discussion of supported pCPUs.

> 
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int kvm_arm_pmu_v3_set_nr_counters(struct kvm_vcpu *vcpu, unsigned int n)
>>   {
>>   	struct kvm *kvm = vcpu->kvm;
>>   
>> -	if (!kvm->arch.arm_pmu)
>> +	lockdep_assert_held(&kvm->arch.config_lock);
>> +
>> +	if (!kvm->arch.arm_pmu &&
>> +	    !test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &kvm->arch.flags))
>>   		return -EINVAL;
>>   
>>   	if (n > kvm_arm_pmu_get_max_counters(kvm))
>> @@ -1227,6 +1290,8 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>>   
>>   		return kvm_arm_pmu_v3_set_nr_counters(vcpu, n);
>>   	}
>> +	case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
>> +		return kvm_arm_pmu_v3_set_pmu_fixed_counters_only(vcpu);
>>   	case KVM_ARM_VCPU_PMU_V3_INIT:
>>   		return kvm_arm_pmu_v3_init(vcpu);
>>   	}
>> @@ -1253,6 +1318,9 @@ int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>>   		irq = vcpu->arch.pmu.irq_num;
>>   		return put_user(irq, uaddr);
>>   	}
>> +	case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
>> +		if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY, &vcpu->kvm->arch.flags))
> 
> With 6 occurrences of this test_bit(), it feels like it'd be valuable
> to have a dedicate predicate to help with readability.

I'll add one with the next version.

> 
>> +			return 0;
>>   	}
>>   
>>   	return -ENXIO;
>> @@ -1266,6 +1334,7 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>>   	case KVM_ARM_VCPU_PMU_V3_FILTER:
>>   	case KVM_ARM_VCPU_PMU_V3_SET_PMU:
>>   	case KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS:
>> +	case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
>>   		if (kvm_vcpu_has_pmu(vcpu))
>>   			return 0;
>>   	}
>> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
>> index 96754b51b411..1375cbaf97b2 100644
>> --- a/include/kvm/arm_pmu.h
>> +++ b/include/kvm/arm_pmu.h
>> @@ -56,6 +56,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
>>   void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
>>   void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>>   				    u64 select_idx);
>> +void kvm_vcpu_load_pmu(struct kvm_vcpu *vcpu);
>>   void kvm_vcpu_reload_pmu(struct kvm_vcpu *vcpu);
>>   int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu,
>>   			    struct kvm_device_attr *attr);
>> @@ -161,6 +162,7 @@ static inline u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>>   static inline void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu) {}
>>   static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}
>>   static inline void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) {}
>> +static inline void kvm_vcpu_load_pmu(struct kvm_vcpu *vcpu) {}
>>   static inline void kvm_vcpu_reload_pmu(struct kvm_vcpu *vcpu) {}
>>   static inline u8 kvm_arm_pmu_get_pmuver_limit(void)
>>   {
>>
> 
> In conclusion, I find this patch to be rather messy. For a start, it
> needs to be split in at least 5 patches:
> 
> - at least two for the refactoring
> - one for the PMU core changes
> - one for the UAPI
> - one for documentation

That clarifies the expected granurarity of patches. The next version 
will be in that layout, perhaps with more patches if an additional 
change. Thanks for the guidance.

> 
> I'd also like some clarification on how this is intended to work if we
> enable FEAT_PMUv3_ICNTR, because the definition seems to be designed
> to encompass all fixed-function counters, and I expect this to grow
> over time.

Indeed the UAPI was designed to encompass all fixed-function counters as 
suggested by Oliver.

To support the UAPI, the implementation avoids hardcoding the assumption 
on the fixed counter(s). FEAT_PMUv3_INCTR will be naturaly supported 
once the common code is properly updated (i.e., the size of the event 
counter bitmask is grown the corresponding registers are wired up with a 
proper check of the feature.)

I expect migration will be handled with the conventional register 
getters and setters, but please share if you have a concern.

> 
> I'm also not planning to look at the selftest at this stage.

That is completely understandable; I'll focus on refining the design and 
implementation for the next version first.

Regards,
Akihiko Odaki


^ permalink raw reply

* Re: [PATCH] iommu/arm-smmu-qcom: Fix fastrpc compatible string in ACTLR client match table
From: Bibek Kumar Patro @ 2026-04-20  8:38 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel,
	Dmitry Baryshkov, iommu, linux-arm-msm, linux-arm-kernel,
	linux-kernel, srinivas.kandagatla
In-Reply-To: <aeSly0N7IkXHYExB@QCOM-aGQu4IUr3Y>



On 4/19/2026 3:22 PM, Shawn Guo wrote:
> On Wed, Apr 08, 2026 at 06:38:25PM +0530, bibek.patro@oss.qualcomm.com wrote:
>> From: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> ...
>> Assisted-by: Anthropic:claude-4-6-sonnet
> 
> Nit - coding-assistants.rst suggests format:
> 
>    Assisted-by: AGENT_NAME:MODEL_VERSION
> 
> So I guess this might be better?
> 
>    Assisted-by: Claude:claude-4-6-sonnet
> 

Agreed. Thanks for pointing this out, Shawn.
I will update the Assisted-by tag to follow the recommendation in 
coding-assistants.rst.

Thanks & regards,
Bibek

> Shawn
> 
>> Fixes: 3e35c3e725de ("iommu/arm-smmu: Add ACTLR data and support for qcom_smmu_500")
>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>



^ permalink raw reply

* Re: [PATCH v3 8/8] unwind: arm64: Use sframe to unwind interrupt frames.
From: Jens Remus @ 2026-04-20  8:42 UTC (permalink / raw)
  To: Dylan Hatch
  Cc: Roman Gushchin, Weinan Liu, Will Deacon, Josh Poimboeuf,
	Indu Bhagat, Peter Zijlstra, Steven Rostedt, Catalin Marinas,
	Jiri Kosina, Mark Rutland, Prasanna Kumar T S M, Puranjay Mohan,
	Song Liu, joe.lawrence, linux-toolchains, linux-kernel,
	live-patching, linux-arm-kernel, Heiko Carstens
In-Reply-To: <CADBMgpwjDf44p0ApR1=XVStCyN-0Q6tuywJ4ixLcbaLZOSjjBg@mail.gmail.com>

On 4/20/2026 7:56 AM, Dylan Hatch wrote:
> On Fri, Apr 17, 2026 at 8:45 AM Jens Remus <jremus@linux.ibm.com> wrote:

>>> +     case UNWIND_CFA_RULE_REG_OFFSET:
>>> +     case UNWIND_CFA_RULE_REG_OFFSET_DEREF:
>>> +             if (!regs)
>>
>>                 if (!regs || frame.cfa.regnum > 30)
>>
>>> +                     return -EINVAL;
>>> +             cfa = regs->regs[frame.cfa.regnum];
>>
>> In unwind user this is guarded by a topmost frame check, as arbitrary
>> registers are otherwise not available.  Isn't this necessary in the
>> kernel case?
> 
> It is necessary, though as you point out the way I wrote the check is
> not as obvious as it probably should be.
> 
> The saved state->regs is set when the current frame is recovered from
> the saved PC of a struct pt_regs, and then immediately set back to
> NULL after the next frame has been recovered. In other words, the
> state->regs is only ever set when it is relevant to the current frame,
> which occurs when state->source == KUNWIND_SOURCE_REGS_PC. This only
> happens when the topmost frame is recovered from a pt_regs, or when a
> pt_regs is recovered from the stack due to an interrupt.
> 
> I can make this more readable by adding an explicit check for
> KUNWIND_SOURCE_REGS_PC in addition to state->regs != NULL.

Thanks for the explanation!  Maybe just add an explanation to the commit
message and a short comment above the (!regs) test?

/* regs only available in topmost frame */

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/



^ permalink raw reply

* Re: [PATCH] crypto: tstmgr - guard xxhash tests
From: Herbert Xu @ 2026-04-20  8:45 UTC (permalink / raw)
  To: Hamza Mahfooz
  Cc: linux-crypto, David S. Miller, Maxime Coquelin, Alexandre Torgue,
	linux-stm32, linux-arm-kernel, linux-kernel, Jeff Barnes,
	Paul Monson
In-Reply-To: <aeJw9I38heQRbbe6@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>

On Fri, Apr 17, 2026 at 10:42:12AM -0700, Hamza Mahfooz wrote:
> 
> It appears that commit 6318fbe26e67 ("crypto: testmgr - Hide ENOENT
> errors better"), already does exactly that and it appears to resolve the
> issue that I'm seeing. So, is there any reason it can't be backported to
> stable?

Sure I don't see anything wrong with that.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


^ permalink raw reply

* Re: [PATCH] gpio: rockchip: Fix GPIO after convert to dynamic base allocation
From: Bartosz Golaszewski @ 2026-04-20  8:46 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Heiko Stuebner, Shawn Lin,
	Jonas Karlman
  Cc: Bartosz Golaszewski, linux-gpio, linux-arm-kernel, linux-rockchip,
	linux-kernel
In-Reply-To: <20260416154928.2103388-1-jonas@kwiboo.se>


On Thu, 16 Apr 2026 15:49:28 +0000, Jonas Karlman wrote:
> The commit c8079f83e0bf ("gpio: rockchip: convert to dynamic GPIO base
> allocation") broke GPIO on devices using device trees which don't set
> the gpio-ranges property, something only Rockchip RK35xx SoC DTs do.
> 
> On a Rockchip RK3399 device something like following is now observed:
> 
> [    0.082771] rockchip-gpio ff720000.gpio: probed /pinctrl/gpio@ff720000
> [    0.083531] rockchip-gpio ff730000.gpio: probed /pinctrl/gpio@ff730000
> [    0.084110] rockchip-gpio ff780000.gpio: probed /pinctrl/gpio@ff780000
> [    0.084746] rockchip-gpio ff788000.gpio: probed /pinctrl/gpio@ff788000
> [    0.085389] rockchip-gpio ff790000.gpio: probed /pinctrl/gpio@ff790000
> --
> [    0.212208] rockchip-pinctrl pinctrl: pin 637 is not registered so it cannot be requested
> [    0.212271] rockchip-pinctrl pinctrl: error -EINVAL: pin-637 (gpio3:637)
> [    0.212344] leds-gpio leds: error -EINVAL: Failed to get GPIO '/leds/led-0'
> [    0.212389] leds-gpio leds: probe with driver leds-gpio failed with error -22
> --
> [    0.607545] rockchip-pinctrl pinctrl: pin 519 is not registered so it cannot be requested
> [    0.608775] rockchip-pinctrl pinctrl: error -EINVAL: pin-519 (gpio0:519)
> [    0.610003] dwmmc_rockchip fe320000.mmc: probe with driver dwmmc_rockchip failed with error -22
> --
> [    0.805882] rockchip-pinctrl pinctrl: pin 547 is not registered so it cannot be requested
> [    0.806672] rockchip-pinctrl pinctrl: error -EINVAL: pin-547 (gpio1:547)
> [    0.807301] reg-fixed-voltage regulator-vbus-typec: error -EINVAL: can't get GPIO
> [    0.807307] rockchip-pinctrl pinctrl: pin 602 is not registered so it cannot be requested
> [    0.807970] reg-fixed-voltage regulator-vbus-typec: probe with driver reg-fixed-voltage failed with error -22
> [    0.808692] rockchip-pinctrl pinctrl: error -EINVAL: pin-602 (gpio2:602)
> [    0.810279] reg-fixed-voltage regulator-vcc3v3-pcie: error -EINVAL: can't get GPIO
> [    0.810284] rockchip-pinctrl pinctrl: pin 665 is not registered so it cannot be requested
> [    0.810299] rockchip-pinctrl pinctrl: error -EINVAL: pin-665 (gpio4:665)
> [    0.810960] reg-fixed-voltage regulator-vcc3v3-pcie: probe with driver reg-fixed-voltage failed with error -22
> [    0.811679] reg-fixed-voltage regulator-vcc5v0-host: error -EINVAL: can't get GPIO
> [    0.813943] reg-fixed-voltage regulator-vcc5v0-host: probe with driver reg-fixed-voltage failed with error -22
> --
> [    0.867788] rockchip-pinctrl pinctrl: pin 522 is not registered so it cannot be requested
> [    0.868537] rockchip-pinctrl pinctrl: error -EINVAL: pin-522 (gpio0:522)
> [    0.869166] pwrseq_simple sdio-pwrseq: error -EINVAL: reset GPIOs not ready
> [    0.869798] pwrseq_simple sdio-pwrseq: probe with driver pwrseq_simple failed with error -22
> --
> [    0.940365] rockchip-pinctrl pinctrl: pin 623 is not registered so it cannot be requested
> [    0.941084] rockchip-pinctrl pinctrl: error -EINVAL: pin-623 (gpio3:623)
> [    0.941823] rk_gmac-dwmac fe300000.ethernet: error -EINVAL: Cannot register the MDIO bus
> [    0.942542] rk_gmac-dwmac fe300000.ethernet: error -EINVAL: MDIO bus (id: 0) registration failed
> [    0.943772] rk_gmac-dwmac fe300000.ethernet: probe with driver rk_gmac-dwmac failed with error -22
> 
> [...]

Applied, thanks!

[1/1] gpio: rockchip: Fix GPIO after convert to dynamic base allocation
      https://git.kernel.org/brgl/c/5cd9c6d332f46d1de8b68117fe2a3f1b08ee80ff

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>


^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: amlogic: t7: khadas-vim4: Enable Bluetooth
From: Neil Armstrong @ 2026-04-20  8:47 UTC (permalink / raw)
  To: Ronald Claveau, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260416-add-bluetooth-t7-vim4-v2-3-9a57098fd055@aliel.fr>

On 4/16/26 10:54, Ronald Claveau wrote:
> Enable UART C on the Khadas VIM4 board and attach the BCM43438
>   compatible Bluetooth controller to it. The node configures the RTS/CTS
> hardware flow control, the associated pinmux, the power supplies (vddao_3v3
> and vddao_1v8), the 32 kHz LPO clock shared with the wifi32k fixed
> clock, and the GPIO lines used for host wakeup, device wakeup and
> shutdown.
> 
> Remove clocks and clock-names for UART A, as they are defined in DTSI.

This should be a separate patch.

Neil

> 
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
>   .../dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts   | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> index 69d6118ba57e7..8ea7ae609fbd5 100644
> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> @@ -250,6 +250,23 @@ &sd_emmc_c {
>   
>   &uart_a {
>   	status = "okay";
> -	clocks = <&xtal>, <&xtal>, <&xtal>;
> -	clock-names = "xtal", "pclk", "baud";
> +};
> +
> +&uart_c {
> +	status = "okay";
> +	pinctrl-0 = <&uart_c_pins>;
> +	pinctrl-names = "default";
> +	uart-has-rtscts;
> +
> +	bluetooth {
> +		compatible = "brcm,bcm43438-bt";
> +		shutdown-gpios = <&gpio GPIOX_17 GPIO_ACTIVE_HIGH>;
> +		host-wakeup-gpios = <&gpio GPIOX_18 GPIO_ACTIVE_HIGH>;
> +		device-wakeup-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
> +		max-speed = <3000000>;
> +		clocks = <&wifi32k>;
> +		clock-names = "lpo";
> +		vbat-supply = <&vddao_3v3>;
> +		vddio-supply = <&vddao_1v8>;
> +	};
>   };
> 



^ permalink raw reply

* Re: [PATCH v2 2/3] arm64: dts: amlogic: t7: Add UART controllers nodes
From: Neil Armstrong @ 2026-04-20  8:47 UTC (permalink / raw)
  To: Ronald Claveau, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260416-add-bluetooth-t7-vim4-v2-2-9a57098fd055@aliel.fr>

On 4/16/26 10:54, Ronald Claveau wrote:
> Add device tree nodes for UART B through F (serial@7a000 to
> serial@82000), completing the UART controller description for the T7
> SoC. Each node includes the peripheral clock.
> 
> While at it, move the uart_a node to its correct position in the
> bus address order (0x78000) to comply with the DT requirement that
> nodes be sorted by their reg address. Complete the
> uart_a node with its peripheral clock (CLKID_SYS_UART_A) and the
> associated clock-names, matching the vendor default clock assignment,
> consistent with the other UART nodes.
> 
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
>   arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 61 +++++++++++++++++++++++++----
>   1 file changed, 54 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
> index 4a55d9641bc9b..81c26b1e3e7a4 100644
> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
> @@ -577,13 +577,6 @@ gpio_intc: interrupt-controller@4080 {
>   					<10 11 12 13 14 15 16 17 18 19 20 21>;
>   			};
>   
> -			uart_a: serial@78000 {
> -				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> -				reg = <0x0 0x78000 0x0 0x18>;
> -				interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> -				status = "disabled";
> -			};
> -
>   			gp0: clock-controller@8080 {
>   				compatible = "amlogic,t7-gp0-pll";
>   				reg = <0x0 0x8080 0x0 0x20>;
> @@ -713,6 +706,60 @@ pwm_ao_cd: pwm@60000 {
>   				status = "disabled";
>   			};
>   
> +			uart_a: serial@78000 {
> +				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> +				reg = <0x0 0x78000 0x0 0x18>;
> +				interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
> +				clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_A>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +				status = "disabled";
> +			};
> +
> +			uart_b: serial@7a000 {
> +				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> +				reg = <0x0 0x7a000 0x0 0x18>;
> +				interrupts = <GIC_SPI 169 IRQ_TYPE_EDGE_RISING>;
> +				clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_B>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +				status = "disabled";
> +			};
> +
> +			uart_c: serial@7c000 {
> +				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> +				reg = <0x0 0x7c000 0x0 0x18>;
> +				interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>;
> +				clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_C>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +				status = "disabled";
> +			};
> +
> +			uart_d: serial@7e000 {
> +				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> +				reg = <0x0 0x7e000 0x0 0x18>;
> +				interrupts = <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>;
> +				clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_D>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +				status = "disabled";
> +			};
> +
> +			uart_e: serial@80000 {
> +				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> +				reg = <0x0 0x80000 0x0 0x18>;
> +				interrupts = <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>;
> +				clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_E>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +				status = "disabled";
> +			};
> +
> +			uart_f: serial@82000 {
> +				compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
> +				reg = <0x0 0x82000 0x0 0x18>;
> +				interrupts = <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>;
> +				clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_F>, <&xtal>;
> +				clock-names = "xtal", "pclk", "baud";
> +				status = "disabled";
> +			};
> +
>   			sd_emmc_a: mmc@88000 {
>   				compatible = "amlogic,t7-mmc", "amlogic,meson-axg-mmc";
>   				reg = <0x0 0x88000 0x0 0x800>;
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil


^ permalink raw reply

* Re: [PATCH v2 1/3] arm64: dts: amlogic: t7: Add uart_c pinctrl pins group
From: Neil Armstrong @ 2026-04-20  8:47 UTC (permalink / raw)
  To: Ronald Claveau, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260416-add-bluetooth-t7-vim4-v2-1-9a57098fd055@aliel.fr>

On 4/16/26 10:54, Ronald Claveau wrote:
> Add the pin multiplexing configuration for UART C (TX, RX, CTS, RTS)
> in the T7 SoC pinctrl node, required to route the UART C signals
> through the correct pads before enabling the controller.
> 
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
>   arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
> index 7fe72c94ed623..4a55d9641bc9b 100644
> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
> @@ -553,6 +553,18 @@ mux {
>   						bias-pull-up;
>   					};
>   				};
> +
> +				uart_c_pins: uart-c {
> +					mux {
> +						groups = "uart_c_tx",
> +							 "uart_c_rx",
> +							 "uart_c_cts",
> +							 "uart_c_rts";
> +						bias-pull-up;
> +						output-high;
> +						function = "uart_c";
> +					};
> +				};
>   			};
>   
>   			gpio_intc: interrupt-controller@4080 {
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil


^ permalink raw reply

* Re: [PATCH RFC v3 2/4] mm/pgtable: Make pfn_pte() filter out huge page attributes
From: Will Deacon @ 2026-04-20  8:48 UTC (permalink / raw)
  To: Yin Tirui
  Cc: linux-kernel, linux-mm, x86, linux-arm-kernel, willy, david,
	catalin.marinas, tglx, mingo, bp, dave.hansen, hpa, luto, peterz,
	akpm, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache,
	ryan.roberts, dev.jain, baohua, lance.yang, vbabka, rppt, surenb,
	mhocko, anshuman.khandual, rmclure, kevin.brodsky, apopple, ajd,
	pasha.tatashin, bhe, thuth, coxu, dan.j.williams, yu-cheng.yu,
	yangyicong, baolu.lu, jgross, conor.dooley, Jonathan.Cameron,
	riel, wangkefeng.wang, chenjun102
In-Reply-To: <20260228070906.1418911-3-yintirui@huawei.com>

On Sat, Feb 28, 2026 at 03:09:04PM +0800, Yin Tirui wrote:
> A fundamental principle of page table type safety is that `pte_t` represents
> the lowest level page table entry and should never carry huge page attributes.
> 
> Currently, passing a pgprot with huge page bits (e.g., extracted via
> pmd_pgprot()) into pfn_pte() creates a malformed PTE that retains the huge
> attribute, leading to the necessity of the ugly `pte_clrhuge()` anti-pattern.
> 
> Enforce type safety by making `pfn_pte()` inherently filter out huge page
> attributes:
> - On x86: Strip the `_PAGE_PSE` bit.
> - On ARM64: Mask out the block descriptor bits in `PTE_TYPE_MASK` and
>   enforce the `PTE_TYPE_PAGE` format.
> - On RISC-V: No changes required, as RISC-V leaf PMDs and PTEs share the
>   exact same hardware format and do not use a distinct huge bit.
> 
> Signed-off-by: Yin Tirui <yintirui@huawei.com>
> ---
>  arch/arm64/include/asm/pgtable.h | 4 +++-
>  arch/x86/include/asm/pgtable.h   | 4 ++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index b3e58735c49b..f2a7a40106d2 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -141,7 +141,9 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
>  
>  #define pte_pfn(pte)		(__pte_to_phys(pte) >> PAGE_SHIFT)
>  #define pfn_pte(pfn,prot)	\
> -	__pte(__phys_to_pte_val((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot))
> +	__pte(__phys_to_pte_val((phys_addr_t)(pfn) << PAGE_SHIFT) | \
> +		((pgprot_val(prot) & ~(PTE_TYPE_MASK & ~PTE_VALID)) | \
> +		(PTE_TYPE_PAGE & ~PTE_VALID)))

Why are you touching arch/arm64? We don't implement pte_clrhuge() afaict.
What does this actually fix?

Will


^ permalink raw reply

* Re: [PATCH v2 2/4] soc: amlogic: clk-measure: Add A1 and T7 support
From: Neil Armstrong @ 2026-04-20  8:49 UTC (permalink / raw)
  To: jian.hu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl
  Cc: devicetree, linux-arm-kernel, linux-amlogic, linux-kernel
In-Reply-To: <20260415-clkmsr_a1_t7-v2-2-02b6314427e6@amlogic.com>

On 4/15/26 10:33, Jian Hu via B4 Relay wrote:
> From: Jian Hu <jian.hu@amlogic.com>
> 
> Add support for the A1 and T7 SoC family in amlogic clk measure.
> 
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>   drivers/soc/amlogic/meson-clk-measure.c | 272 ++++++++++++++++++++++++++++++++
>   1 file changed, 272 insertions(+)
> 
> diff --git a/drivers/soc/amlogic/meson-clk-measure.c b/drivers/soc/amlogic/meson-clk-measure.c
> index d862e30a244e..8c4f3cc8c8ab 100644
> --- a/drivers/soc/amlogic/meson-clk-measure.c
> +++ b/drivers/soc/amlogic/meson-clk-measure.c
> @@ -787,6 +787,258 @@ static const struct meson_msr_id clk_msr_s4[] = {
>   
>   };
>   
> +static const struct meson_msr_id clk_msr_a1[] = {
> +	CLK_MSR_ID(0, "tdmout_b_sclk"),
> +	CLK_MSR_ID(1, "tdmout_a_sclk"),
> +	CLK_MSR_ID(2, "tdmin_lb_sclk"),
> +	CLK_MSR_ID(3, "tdmin_b_sclk"),
> +	CLK_MSR_ID(4, "tdmin_a_sclk"),
> +	CLK_MSR_ID(5, "vad"),
> +	CLK_MSR_ID(6, "resamplea"),
> +	CLK_MSR_ID(7, "pdm_sysclk"),
> +	CLK_MSR_ID(8, "pdm_dclk"),
> +	CLK_MSR_ID(9, "locker_out"),
> +	CLK_MSR_ID(10, "locker_in"),
> +	CLK_MSR_ID(11, "spdifin"),
> +	CLK_MSR_ID(12, "tdmin_vad"),
> +	CLK_MSR_ID(13, "au_adc"),
> +	CLK_MSR_ID(14, "au_dac"),
> +	CLK_MSR_ID(16, "spicc_a"),
> +	CLK_MSR_ID(17, "spifc"),
> +	CLK_MSR_ID(18, "sd_emmc_a"),
> +	CLK_MSR_ID(19, "dmcx4"),
> +	CLK_MSR_ID(20, "dmc"),
> +	CLK_MSR_ID(21, "psram"),
> +	CLK_MSR_ID(22, "cecb"),
> +	CLK_MSR_ID(23, "ceca"),
> +	CLK_MSR_ID(24, "ts"),
> +	CLK_MSR_ID(25, "pwm_f"),
> +	CLK_MSR_ID(26, "pwm_e"),
> +	CLK_MSR_ID(27, "pwm_d"),
> +	CLK_MSR_ID(28, "pwm_c"),
> +	CLK_MSR_ID(29, "pwm_b"),
> +	CLK_MSR_ID(30, "pwm_a"),
> +	CLK_MSR_ID(31, "saradc"),
> +	CLK_MSR_ID(32, "usb_bus"),
> +	CLK_MSR_ID(33, "dsp_b"),
> +	CLK_MSR_ID(34, "dsp_a"),
> +	CLK_MSR_ID(35, "axi"),
> +	CLK_MSR_ID(36, "sys"),
> +	CLK_MSR_ID(40, "rng_ring_osc0"),
> +	CLK_MSR_ID(41, "rng_ring_osc1"),
> +	CLK_MSR_ID(42, "rng_ring_osc2"),
> +	CLK_MSR_ID(43, "rng_ring_osc3"),
> +	CLK_MSR_ID(44, "dds_out"),
> +	CLK_MSR_ID(45, "cpu_clk_div16"),
> +	CLK_MSR_ID(46, "gpio_msr"),
> +	CLK_MSR_ID(50, "osc_ring_cpu0"),
> +	CLK_MSR_ID(51, "osc_ring_cpu1"),
> +	CLK_MSR_ID(54, "osc_ring_top0"),
> +	CLK_MSR_ID(55, "osc_ring_top1"),
> +	CLK_MSR_ID(56, "osc_ring_ddr"),
> +	CLK_MSR_ID(57, "osc_ring_dmc"),
> +	CLK_MSR_ID(58, "osc_ring_dspa"),
> +	CLK_MSR_ID(59, "osc_ring_dspb"),
> +	CLK_MSR_ID(60, "osc_ring_rama"),
> +	CLK_MSR_ID(61, "osc_ring_ramb"),
> +};
> +
> +static const struct meson_msr_id clk_msr_t7[] = {
> +	CLK_MSR_ID(0, "sys"),
> +	CLK_MSR_ID(1, "axi"),
> +	CLK_MSR_ID(2, "rtc"),
> +	CLK_MSR_ID(3, "dspa"),
> +	CLK_MSR_ID(4, "dspb"),
> +	CLK_MSR_ID(5, "mali"),
> +	CLK_MSR_ID(6, "sys_cpu_clk_div16"),
> +	CLK_MSR_ID(7, "ceca"),
> +	CLK_MSR_ID(8, "cecb"),
> +	CLK_MSR_ID(10, "fclk_div5"),
> +	CLK_MSR_ID(11, "mpll0"),
> +	CLK_MSR_ID(12, "mpll1"),
> +	CLK_MSR_ID(13, "mpll2"),
> +	CLK_MSR_ID(14, "mpll3"),
> +	CLK_MSR_ID(15, "mpll_50m"),
> +	CLK_MSR_ID(16, "pcie_inp"),
> +	CLK_MSR_ID(17, "pcie_inn"),
> +	CLK_MSR_ID(18, "mpll_test_out"),
> +	CLK_MSR_ID(19, "hifi_pll"),
> +	CLK_MSR_ID(20, "gp0_pll"),
> +	CLK_MSR_ID(21, "gp1_pll"),
> +	CLK_MSR_ID(22, "eth_mppll_50m"),
> +	CLK_MSR_ID(23, "sys_pll_div16"),
> +	CLK_MSR_ID(24, "ddr_dpll_pt"),
> +	CLK_MSR_ID(25, "earcrx_pll"),
> +	CLK_MSR_ID(26, "paie1_clk_inp"),
> +	CLK_MSR_ID(27, "paie1_clk_inn"),
> +	CLK_MSR_ID(28, "amlgdc"),
> +	CLK_MSR_ID(29, "gdc"),
> +	CLK_MSR_ID(30, "mod_eth_phy_ref"),
> +	CLK_MSR_ID(31, "mod_eth_tx"),
> +	CLK_MSR_ID(32, "eth_clk125Mhz"),
> +	CLK_MSR_ID(33, "eth_clk_rmii"),
> +	CLK_MSR_ID(34, "co_clkin_to_mac"),
> +	CLK_MSR_ID(35, "mod_eth_rx_clk_rmii"),
> +	CLK_MSR_ID(36, "co_rx"),
> +	CLK_MSR_ID(37, "co_tx"),
> +	CLK_MSR_ID(38, "eth_phy_rxclk"),
> +	CLK_MSR_ID(39, "eth_phy_plltxclk"),
> +	CLK_MSR_ID(40, "ephy_test"),
> +	CLK_MSR_ID(41, "dsi_b_meas"),
> +	CLK_MSR_ID(42, "hdmirx_apl"),
> +	CLK_MSR_ID(43, "hdmirx_tmds"),
> +	CLK_MSR_ID(44, "hdmirx_cable"),
> +	CLK_MSR_ID(45, "hdmirx_apll_clk_audio"),
> +	CLK_MSR_ID(46, "hdmirx_5m"),
> +	CLK_MSR_ID(47, "hdmirx_2m"),
> +	CLK_MSR_ID(48, "hdmirx_cfg"),
> +	CLK_MSR_ID(49, "hdmirx_hdcp2x_eclk"),
> +	CLK_MSR_ID(50, "vid_pll0_div"),
> +	CLK_MSR_ID(51, "hdmi_vid_pll"),
> +	CLK_MSR_ID(54, "vdac_clk"),
> +	CLK_MSR_ID(55, "vpu_clk_buf"),
> +	CLK_MSR_ID(56, "mod_tcon_clko"),
> +	CLK_MSR_ID(57, "lcd_an_clk_ph2"),
> +	CLK_MSR_ID(58, "lcd_an_clk_ph3"),
> +	CLK_MSR_ID(59, "hdmi_tx_pixel"),
> +	CLK_MSR_ID(60, "vdin_meas"),
> +	CLK_MSR_ID(61, "vpu_clk"),
> +	CLK_MSR_ID(62, "vpu_clkb"),
> +	CLK_MSR_ID(63, "vpu_clkb_tmp"),
> +	CLK_MSR_ID(64, "vpu_clkc"),
> +	CLK_MSR_ID(65, "vid_lock"),
> +	CLK_MSR_ID(66, "vapbclk"),
> +	CLK_MSR_ID(67, "ge2d"),
> +	CLK_MSR_ID(68, "aud_pll"),
> +	CLK_MSR_ID(69, "aud_sck"),
> +	CLK_MSR_ID(70, "dsi_a_meas"),
> +	CLK_MSR_ID(72, "mipi_csi_phy"),
> +	CLK_MSR_ID(73, "mipi_isp"),
> +	CLK_MSR_ID(76, "hdmitx_tmds"),
> +	CLK_MSR_ID(77, "hdmitx_sys"),
> +	CLK_MSR_ID(78, "hdmitx_fe"),
> +	CLK_MSR_ID(80, "hdmitx_prif"),
> +	CLK_MSR_ID(81, "hdmitx_200m"),
> +	CLK_MSR_ID(82, "hdmitx_aud"),
> +	CLK_MSR_ID(83, "hdmitx_pnx"),
> +	CLK_MSR_ID(84, "spicc5"),
> +	CLK_MSR_ID(85, "spicc4"),
> +	CLK_MSR_ID(86, "spicc3"),
> +	CLK_MSR_ID(87, "spicc2"),
> +	CLK_MSR_ID(93, "vdec"),
> +	CLK_MSR_ID(94, "wave521_aclk"),
> +	CLK_MSR_ID(95, "wave521_cclk"),
> +	CLK_MSR_ID(96, "wave521_bclk"),
> +	CLK_MSR_ID(97, "hcodec"),
> +	CLK_MSR_ID(98, "hevcb"),
> +	CLK_MSR_ID(99, "hevcf"),
> +	CLK_MSR_ID(100, "hdmi_aud_pll"),
> +	CLK_MSR_ID(101, "hdmi_acr_ref"),
> +	CLK_MSR_ID(102, "hdmi_meter"),
> +	CLK_MSR_ID(103, "hdmi_vid"),
> +	CLK_MSR_ID(104, "hdmi_aud"),
> +	CLK_MSR_ID(105, "hdmi_dsd"),
> +	CLK_MSR_ID(108, "dsi1_phy"),
> +	CLK_MSR_ID(109, "dsi0_phy"),
> +	CLK_MSR_ID(110, "smartcard"),
> +	CLK_MSR_ID(111, "sar_adc"),
> +	CLK_MSR_ID(113, "sd_emmc_c"),
> +	CLK_MSR_ID(114, "sd_emmc_b"),
> +	CLK_MSR_ID(115, "sd_emmc_a"),
> +	CLK_MSR_ID(116, "gpio_msr"),
> +	CLK_MSR_ID(117, "spicc1"),
> +	CLK_MSR_ID(118, "spicc0"),
> +	CLK_MSR_ID(119, "anakin"),
> +	CLK_MSR_ID(121, "ts_clk(temp sensor)"),
> +	CLK_MSR_ID(122, "ts_a73"),
> +	CLK_MSR_ID(123, "ts_a53"),
> +	CLK_MSR_ID(124, "ts_nna"),
> +	CLK_MSR_ID(130, "audio_vad"),
> +	CLK_MSR_ID(131, "acodec_dac_clk_x128"),
> +	CLK_MSR_ID(132, "audio_locker_in"),
> +	CLK_MSR_ID(133, "audio_locker_out"),
> +	CLK_MSR_ID(134, "audio_tdmout_c_sclk"),
> +	CLK_MSR_ID(135, "audio_tdmout_b_sclk"),
> +	CLK_MSR_ID(136, "audio_tdmout_a_sclk"),
> +	CLK_MSR_ID(137, "audio_tdmin_lb_sclk"),
> +	CLK_MSR_ID(138, "audio_tdmin_c_sclk"),
> +	CLK_MSR_ID(139, "audio_tdmin_b_sclk"),
> +	CLK_MSR_ID(140, "audio_tdmin_a_sclk"),
> +	CLK_MSR_ID(141, "audio_resamplea"),
> +	CLK_MSR_ID(142, "audio_pdm_sysclk"),
> +	CLK_MSR_ID(143, "audio_spdifoutb_mst"),
> +	CLK_MSR_ID(144, "audio_spdifout_mst"),
> +	CLK_MSR_ID(145, "audio_spdifin_mst"),
> +	CLK_MSR_ID(146, "audio_pdm_dclk"),
> +	CLK_MSR_ID(147, "audio_resampleb"),
> +	CLK_MSR_ID(148, "earcrx_pll_dmac"),
> +	CLK_MSR_ID(156, "pwm_ao_h"),
> +	CLK_MSR_ID(157, "pwm_ao_g"),
> +	CLK_MSR_ID(158, "pwm_ao_f"),
> +	CLK_MSR_ID(159, "pwm_ao_e"),
> +	CLK_MSR_ID(160, "pwm_ao_d"),
> +	CLK_MSR_ID(161, "pwm_ao_c"),
> +	CLK_MSR_ID(162, "pwm_ao_b"),
> +	CLK_MSR_ID(163, "pwm_ao_a"),
> +	CLK_MSR_ID(164, "pwm_f"),
> +	CLK_MSR_ID(165, "pwm_e"),
> +	CLK_MSR_ID(166, "pwm_d"),
> +	CLK_MSR_ID(167, "pwm_c"),
> +	CLK_MSR_ID(168, "pwm_b"),
> +	CLK_MSR_ID(169, "pwm_a"),
> +	CLK_MSR_ID(170, "aclkm"),
> +	CLK_MSR_ID(171, "mclk_pll"),
> +	CLK_MSR_ID(172, "a73_sys_pll_div16"),
> +	CLK_MSR_ID(173, "a73_cpu_clk_div16"),
> +	CLK_MSR_ID(176, "rng_ring_0"),
> +	CLK_MSR_ID(177, "rng_ring_1"),
> +	CLK_MSR_ID(178, "rng_ring_2"),
> +	CLK_MSR_ID(179, "rng_ring_3"),
> +	CLK_MSR_ID(180, "am_ring_out0"),
> +	CLK_MSR_ID(181, "am_ring_out1"),
> +	CLK_MSR_ID(182, "am_ring_out2"),
> +	CLK_MSR_ID(183, "am_ring_out3"),
> +	CLK_MSR_ID(184, "am_ring_out4"),
> +	CLK_MSR_ID(185, "am_ring_out5"),
> +	CLK_MSR_ID(186, "am_ring_out6"),
> +	CLK_MSR_ID(187, "am_ring_out7"),
> +	CLK_MSR_ID(188, "am_ring_out8"),
> +	CLK_MSR_ID(189, "am_ring_out9"),
> +	CLK_MSR_ID(190, "am_ring_out10"),
> +	CLK_MSR_ID(191, "am_ring_out11"),
> +	CLK_MSR_ID(192, "am_ring_out12"),
> +	CLK_MSR_ID(193, "am_ring_out13"),
> +	CLK_MSR_ID(194, "am_ring_out14"),
> +	CLK_MSR_ID(195, "am_ring_out15"),
> +	CLK_MSR_ID(196, "am_ring_out16"),
> +	CLK_MSR_ID(197, "am_ring_out17"),
> +	CLK_MSR_ID(198, "am_ring_out18"),
> +	CLK_MSR_ID(199, "am_ring_out19"),
> +	CLK_MSR_ID(200, "mipi_csi_phy0"),
> +	CLK_MSR_ID(201, "mipi_csi_phy1"),
> +	CLK_MSR_ID(202, "mipi_csi_phy2"),
> +	CLK_MSR_ID(203, "mipi_csi_phy3"),
> +	CLK_MSR_ID(204, "vid_pll1_div"),
> +	CLK_MSR_ID(205, "vid_pll2_div"),
> +	CLK_MSR_ID(206, "am_ring_out20"),
> +	CLK_MSR_ID(207, "am_ring_out21"),
> +	CLK_MSR_ID(208, "am_ring_out22"),
> +	CLK_MSR_ID(209, "am_ring_out23"),
> +	CLK_MSR_ID(210, "am_ring_out24"),
> +	CLK_MSR_ID(211, "am_ring_out25"),
> +	CLK_MSR_ID(212, "am_ring_out26"),
> +	CLK_MSR_ID(213, "am_ring_out27"),
> +	CLK_MSR_ID(214, "am_ring_out28"),
> +	CLK_MSR_ID(215, "am_ring_out29"),
> +	CLK_MSR_ID(216, "am_ring_out30"),
> +	CLK_MSR_ID(217, "am_ring_out31"),
> +	CLK_MSR_ID(218, "am_ring_out32"),
> +	CLK_MSR_ID(219, "enc0_if"),
> +	CLK_MSR_ID(220, "enc2"),
> +	CLK_MSR_ID(221, "enc1"),
> +	CLK_MSR_ID(222, "enc0")
> +};
> +
>   static int meson_measure_id(struct meson_msr_id *clk_msr_id,
>   			    unsigned int duration)
>   {
> @@ -1026,6 +1278,18 @@ static const struct meson_msr_data clk_msr_s4_data = {
>   	.reg = &msr_reg_offset_v2,
>   };
>   
> +static const struct meson_msr_data clk_msr_a1_data = {
> +	.msr_table = (void *)clk_msr_a1,
> +	.msr_count = ARRAY_SIZE(clk_msr_a1),
> +	.reg = &msr_reg_offset_v2,
> +};
> +
> +static const struct meson_msr_data clk_msr_t7_data = {
> +	.msr_table = (void *)clk_msr_t7,
> +	.msr_count = ARRAY_SIZE(clk_msr_t7),
> +	.reg = &msr_reg_offset_v2,
> +};
> +
>   static const struct of_device_id meson_msr_match_table[] = {
>   	{
>   		.compatible = "amlogic,meson-gx-clk-measure",
> @@ -1059,6 +1323,14 @@ static const struct of_device_id meson_msr_match_table[] = {
>   		.compatible = "amlogic,s4-clk-measure",
>   		.data = &clk_msr_s4_data,
>   	},
> +	{
> +		.compatible = "amlogic,a1-clk-measure",
> +		.data = &clk_msr_a1_data,
> +	},
> +	{
> +		.compatible = "amlogic,t7-clk-measure",
> +		.data = &clk_msr_t7_data,
> +	},
>   	{ /* sentinel */ }
>   };
>   MODULE_DEVICE_TABLE(of, meson_msr_match_table);
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil


^ permalink raw reply

* Re: [PATCH v2 4/9] pmdomain: core: Add initial fine grained sync_state support
From: Geert Uytterhoeven @ 2026-04-20  8:49 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm,
	Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
	Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
	Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
	Jonathan Hunter, Dmitry Baryshkov, linux-arm-kernel, linux-kernel,
	Geert Uytterhoeven
In-Reply-To: <20260410104058.83748-5-ulf.hansson@linaro.org>

Hi Ulf,

On Fri, 10 Apr 2026 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> A onecell (#power-domain-cells = <1 or 2>; in DT) power domain provider
> typically provides multiple independent power domains, each with their own
> corresponding consumers. In these cases we have to wait for all consumers
> for all the provided power domains before the ->sync_state() callback gets
> called for the supplier.
>
> In a first step to improve this, let's implement support for fine grained
> sync_state support a per genpd basis by using the ->queue_sync_state()
> callback. To take step by step, let's initially limit the improvement to
> the internal genpd provider driver and to its corresponding genpd devices
> for onecell providers.
>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks for your patch!

> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c

> +static void genpd_parse_for_consumer(struct device_node *sup,
> +                                    struct device_node *con)
> +{
> +       struct generic_pm_domain *genpd;
> +       int i;

unsigned int?

> +
> +       for (i = 0; ; i++) {

... and you could move it inside the for-statement.

> +               struct of_phandle_args pd_args;
> +
> +               if (of_parse_phandle_with_args(con, "power-domains",
> +                                              "#power-domain-cells",
> +                                               i, &pd_args))

Checkpatch reports a superfluous space before the "i".

> +                       break;

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


^ permalink raw reply

* Re: [PATCH v2 4/4] arm64: dts: amlogic: t7: Add clk measure support
From: Neil Armstrong @ 2026-04-20  8:52 UTC (permalink / raw)
  To: Jian Hu, Ronald Claveau
  Cc: devicetree, linux-arm-kernel, linux-amlogic, linux-kernel,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
	Jerome Brunet, Martin Blumenstingl
In-Reply-To: <64cde9f6-4f28-4ba7-8362-aac28887ff22@amlogic.com>

On 4/20/26 05:25, Jian Hu wrote:
> Hi Ronald,
> 
> 
> Thanks for your review.
> 
> On 4/17/2026 5:48 PM, Ronald Claveau wrote:
>> [ EXTERNAL EMAIL ]
>>
>> Hello Jian,
>>
>> On 4/15/26 10:33 AM, Jian Hu via B4 Relay wrote:
>>> From: Jian Hu <jian.hu@amlogic.com>
>>>
>>> Add the clock measure device to the T7 SoC family.
>>>
>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>> ---
>>>   arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>> index 7fe72c94ed62..cec2ea74850d 100644
>>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>> @@ -701,6 +701,11 @@ pwm_ao_cd: pwm@60000 {
>>>                                status = "disabled";
>>>                        };
>>>
>>> +                     clock-measurer@48000 {
>>> +                             compatible = "amlogic,t7-clk-measure";
>>> +                             reg = <0x0 0x48000 0x0 0x1c>;
>>> +                     };
>>> +
>> Can you please order by reg, it should be between pwm_ao_gh and pwm_ab.
>> Thank you.
> 
> 
> According to the "Order of Nodes" chapter in Documentation/devicetree/bindings/dts-coding-style.rst,
> 
> nodes of the same type should be grouped together, and this takes higher priority.
> 
> So I have placed the clock-measure node after all PWM nodes to avoid splitting the PWM group.

This is not something we ever followed in the past, and I don't think it makes sens here.


"""
Alternatively for some subarchitectures, nodes of the same type can be
grouped together, e.g. all I2C controllers one after another even if this
breaks unit address ordering.
"""

This doesn't apply here, so order strictly by address.

Neil

> 
> 
> Best regards,
> 
> Jian
> 



^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Will Deacon @ 2026-04-20  8:55 UTC (permalink / raw)
  To: Yeoreum Yun
  Cc: Marc Zyngier, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
	zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, peterhuewe,
	jarkko, jgg, sudeep.holla, oupton, joey.gouly, suzuki.poulose,
	yuzenghui, catalin.marinas
In-Reply-To: <aeS4rAeVQ0yJIPYw@e129823.arm.com>

On Sun, Apr 19, 2026 at 12:12:44PM +0100, Yeoreum Yun wrote:
> Hi Marc,
> 
> > On Sat, 18 Apr 2026 11:34:30 +0100,
> > Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > >
> > > > > @@ -2035,6 +2037,16 @@ static int __init ffa_init(void)
> > > > >  	u32 buf_sz;
> > > > >  	size_t rxtx_bufsz = SZ_4K;
> > > > >
> > > > > +	/*
> > > > > +	 * When pKVM is enabled, the FF-A driver must be initialized
> > > > > +	 * after pKVM initialization. Otherwise, pKVM cannot negotiate
> > > > > +	 * the FF-A version or obtain RX/TX buffer information,
> > > > > +	 * which leads to failures in FF-A calls.
> > > > > +	 */
> > > > > +	if (IS_ENABLED(CONFIG_KVM) && is_protected_kvm_enabled() &&
> > > > > +	    !is_kvm_arm_initialised())
> > > > > +		return -EPROBE_DEFER;
> > > > > +
> > > >
> > > > That's still fundamentally wrong: pkvm is not ready until
> > > > finalize_pkvm() has finished, and that's not indicated by
> > > > is_kvm_arm_initialised().
> > >
> > > Thanks. I miss the TSC bit set in here.
> >
> > That's the least of the problems. None of the infrastructure is in
> > place at this stage...
> >
> > > IMHO, I'd like to make an new state check function --
> > > is_pkvm_arm_initialised() so that ff-a driver to know whether
> > > pkvm is initialised.
> >
> > Doesn't sound great, TBH.
> >
> > > or any other suggestion?
> >
> > Instead of adding more esoteric predicates, I'd rather you build on an
> > existing infrastructure. You have a dependency on KVM, use something
> > that is designed to enforce dependencies. Device links spring to mind
> > as something designed for that.
> >
> > Can you look into enabling this for KVM? If that's possible, then it
> > should be easy enough to delay the actual KVM registration after pKVM
> > is finalised.
> 
> or what about some event notifier? Just like:

This seems a bit over-engineered to me. Why don't you just split the
FF-A initialisation into two steps: an early part which does the version
negotiation and then a later part which can fit in with whatever
dependencies you have on the TPM?

Will


^ permalink raw reply

* Re: [PATCH] gpio: aspeed: fix AST2700 debounce selector bit definitions
From: Bartosz Golaszewski @ 2026-04-20  9:15 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Joel Stanley, Andrew Jeffery,
	Billy Tsai
  Cc: Bartosz Golaszewski, linux-gpio, linux-arm-kernel, linux-aspeed,
	linux-kernel
In-Reply-To: <20260415-gpio-fix-v1-1-b08a89b31e6f@aspeedtech.com>


On Wed, 15 Apr 2026 18:24:42 +0800, Billy Tsai wrote:
> The AST2700 datasheet defines reg_debounce_sel1 as the low bit and
> reg_debounce_sel2 as the high bit. The current driver uses the AST2600
> mapping instead, where sel1 is the high bit and sel2 is the low bit.
> 
> As a result, the debounce selector bits are programmed in reverse on
> AST2700. Swap the G7 sel1/sel2 bit definitions so the driver matches the
> hardware definition.
> 
> [...]

Applied, thanks!

[1/1] gpio: aspeed: fix AST2700 debounce selector bit definitions
      https://git.kernel.org/brgl/c/e31eee4a961077d60ef2362507240c6743c1c2ae

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>


^ permalink raw reply

* Re: [PATCH v2 4/4] arm64: dts: amlogic: t7: Add clk measure support
From: Ronald Claveau @ 2026-04-20  9:16 UTC (permalink / raw)
  To: Jian Hu
  Cc: devicetree, linux-arm-kernel, linux-amlogic, linux-kernel,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl
In-Reply-To: <64cde9f6-4f28-4ba7-8362-aac28887ff22@amlogic.com>

Hi Jian,

On 4/20/26 5:25 AM, Jian Hu wrote:
> Hi Ronald,
> 
> 
> Thanks for your review.
> 
> On 4/17/2026 5:48 PM, Ronald Claveau wrote:
>> [ EXTERNAL EMAIL ]
>>
>> Hello Jian,
>>
>> On 4/15/26 10:33 AM, Jian Hu via B4 Relay wrote:
>>> From: Jian Hu <jian.hu@amlogic.com>
>>>
>>> Add the clock measure device to the T7 SoC family.
>>>
>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>> ---
>>>   arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/
>>> arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>> index 7fe72c94ed62..cec2ea74850d 100644
>>> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
>>> @@ -701,6 +701,11 @@ pwm_ao_cd: pwm@60000 {
>>>                                status = "disabled";
>>>                        };
>>>
>>> +                     clock-measurer@48000 {
>>> +                             compatible = "amlogic,t7-clk-measure";
>>> +                             reg = <0x0 0x48000 0x0 0x1c>;
>>> +                     };
>>> +
>> Can you please order by reg, it should be between pwm_ao_gh and pwm_ab.
>> Thank you.
> 
> 
> According to the "Order of Nodes" chapter in Documentation/devicetree/
> bindings/dts-coding-style.rst,
> 
> nodes of the same type should be grouped together, and this takes higher
> priority.
> 
> So I have placed the clock-measure node after all PWM nodes to avoid
> splitting the PWM group.
> 

Thanks for your answer.

The documentation says nodes "shall be ordered by unit address" as the
primary rule.
The grouping by type is described as an alternative ("Alternatively"),
applicable to some subarchitectures, not as a rule that takes higher
priority.

So to me, I understand it as, unless your subarchitecture has an
established convention of grouping PWM nodes together, ordering by reg
remains the correct default here. And, in my opinion, sticking to a
single sorting method improves readability.


-- 
Best regards,
Ronald


^ permalink raw reply

* Re: [PATCH 2/3] gpio: axiado: add SGPIO controller support
From: Bartosz Golaszewski @ 2026-04-20  9:25 UTC (permalink / raw)
  To: Petar Stepanovic
  Cc: linux-gpio, devicetree, linux-arm-kernel, linux-kernel,
	Tzu-Hao Wei, Swark Yang, Prasad Bolisetty, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Harshit Shah, SriNavmani A
In-Reply-To: <20260414-axiado-ax3000-sgpio-controller-v1-2-b5c7e4c2e69b@axiado.com>

On Tue, 14 Apr 2026 15:48:33 +0200, Petar Stepanovic
<pstepanovic@axiado.com> said:
> Add support for the Axiado SGPIO controller.
>
> The controller provides a serialized interface for GPIOs with
> configurable direction and interrupt support.
>
> The driver registers the controller as a gpio_chip and uses
> regmap for register access.
>
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
> ---
>  drivers/gpio/Kconfig             |  18 +
>  drivers/gpio/Makefile            |   1 +
>  drivers/gpio/gpio-axiado-sgpio.c | 780 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 799 insertions(+)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index bd185482a7fd..42c56d157092 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -198,6 +198,24 @@ config GPIO_ATH79
>  	  Select this option to enable GPIO driver for
>  	  Atheros AR71XX/AR724X/AR913X SoC devices.
>
> +config GPIO_AXIADO_SGPIO
> +	bool "Axiado SGPIO support"
> +	depends on OF_GPIO

You don't need this.

> +	depends on ARCH_AXIADO || COMPILE_TEST
> +	select GPIO_GENERIC

You don't seem to be using this.

> +	select GPIOLIB_IRQCHIP
> +	select REGMAP
> +	help
> +	  Enable support for the Axiado Serial GPIO (SGPIO) controller.
> +
> +	  The SGPIO controller provides a serialized interface for
> +	  controlling multiple GPIO signals over a limited number of
> +	  physical lines. It supports configurable data direction and
> +	  interrupt handling.
> +
> +	  This driver integrates with the Linux GPIO subsystem and
> +	  exposes the controller as a standard GPIO provider.
> +
>  config GPIO_RASPBERRYPI_EXP
>  	tristate "Raspberry Pi 3 GPIO Expander"
>  	default RASPBERRYPI_FIRMWARE
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 2421a8fd3733..909a97551807 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -42,6 +42,7 @@ obj-$(CONFIG_GPIO_ARIZONA)		+= gpio-arizona.o
>  obj-$(CONFIG_GPIO_ASPEED)		+= gpio-aspeed.o
>  obj-$(CONFIG_GPIO_ASPEED_SGPIO)		+= gpio-aspeed-sgpio.o
>  obj-$(CONFIG_GPIO_ATH79)		+= gpio-ath79.o
> +obj-$(CONFIG_GPIO_AXIADO_SGPIO)		+= gpio-axiado-sgpio.o
>  obj-$(CONFIG_GPIO_BCM_KONA)		+= gpio-bcm-kona.o
>  obj-$(CONFIG_GPIO_BCM_XGS_IPROC)	+= gpio-xgs-iproc.o
>  obj-$(CONFIG_GPIO_BD71815)		+= gpio-bd71815.o
> diff --git a/drivers/gpio/gpio-axiado-sgpio.c b/drivers/gpio/gpio-axiado-sgpio.c
> new file mode 100644
> index 000000000000..8cd349ec6f53
> --- /dev/null
> +++ b/drivers/gpio/gpio-axiado-sgpio.c
> @@ -0,0 +1,780 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022-2026 Axiado Corporation
> + */

Please add a blank line here...

> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/spinlock.h>
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +#include <linux/gpio/driver.h>
> +
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +
> +#include <linux/regmap.h>
> +

... and keep the includes together as well as ordered alphabetically.

> +struct sgpio_reg_offsets {
> +	u32 mux_0;
> +	u32 preset_0;
> +	u32 count_0;
> +	u32 pos_0;
> +
> +	u32 mux_1;
> +	u32 ld;
> +	u32 ld_ss;
> +
> +	u32 preset_1;
> +	u32 count_1;
> +	u32 pos_1;
> +
> +	u32 mux_2;
> +	u32 dout;
> +	u32 dout_ss;
> +
> +	u32 preset_2;
> +	u32 count_2;
> +	u32 pos_2;
> +
> +	u32 mux_3;
> +	u32 preset_3;
> +	u32 count_3;
> +	u32 pos_3;
> +
> +	u32 mux_4;
> +	u32 oe;
> +	u32 oe_ss;
> +
> +	u32 preset_4;
> +	u32 count_4;
> +	u32 pos_4;
> +
> +	u32 mask;
> +	u32 ctrl_en;
> +	u32 ctrl_en_pos;
> +
> +	u32 din_ss;
> +	u32 status;
> +};
> +
> +static const struct sgpio_reg_offsets sgpio_offsets_512 = {
> +	.mux_0 = 0x000,
> +	.preset_0 = 0x1dc,
> +	.count_0 = 0x1f0,
> +	.pos_0 = 0x204,
> +
> +	.mux_1 = 0x004,
> +	.ld = 0x014,
> +	.ld_ss = 0x0d8,
> +
> +	.preset_1 = 0x1e0,
> +	.count_1 = 0x1f4,
> +	.pos_1 = 0x208,
> +
> +	.mux_2 = 0x008,
> +	.dout = 0x054,
> +	.dout_ss = 0x158,
> +
> +	.preset_2 = 0x1e4,
> +	.count_2 = 0x1f8,
> +	.pos_2 = 0x20c,
> +
> +	.mux_3 = 0x00c,
> +	.preset_3 = 0x1e8,
> +	.count_3 = 0x1fc,
> +	.pos_3 = 0x210,
> +
> +	.mux_4 = 0x010,
> +	.oe = 0x0d4,
> +	.oe_ss = 0x1d8,
> +
> +	.preset_4 = 0x1ec,
> +	.count_4 = 0x200,
> +	.pos_4 = 0x214,
> +
> +	.mask = 0x224,
> +	.ctrl_en = 0x218,
> +	.ctrl_en_pos = 0x21c,
> +
> +	.din_ss = 0x198,
> +	.status = 0x228,
> +};
> +
> +static const struct sgpio_reg_offsets sgpio_offsets_128 = {
> +	.mux_0 = 0x000,
> +	.preset_0 = 0x08c,
> +	.count_0 = 0x0a0,
> +	.pos_0 = 0x0b4,
> +
> +	.mux_1 = 0x004,
> +	.ld = 0x014,
> +	.ld_ss = 0x048,
> +
> +	.preset_1 = 0x090,
> +	.count_1 = 0x0a4,
> +	.pos_1 = 0x0b8,
> +
> +	.mux_2 = 0x008,
> +	.dout = 0x024,
> +	.dout_ss = 0x068,
> +
> +	.preset_2 = 0x094,
> +	.count_2 = 0x0a8,
> +	.pos_2 = 0x0bc,
> +
> +	.mux_3 = 0x00c,
> +	.preset_3 = 0x098,
> +	.count_3 = 0x0ac,
> +	.pos_3 = 0x0c0,
> +
> +	.mux_4 = 0x010,
> +	.oe = 0x044,
> +	.oe_ss = 0x088,
> +
> +	.preset_4 = 0x09c,
> +	.count_4 = 0x0b0,
> +	.pos_4 = 0x0c4,
> +
> +	.mask = 0x0d4,
> +	.ctrl_en = 0x0c8,
> +	.ctrl_en_pos = 0x0cc,
> +
> +	.din_ss = 0x078,
> +	.status = 0x0d8,
> +};
> +
> +#define MAX_SGPIO_PINS 512
> +#define MAX_OFFSET_REG 16
> +#define MAX_SLICE_COUNT 5
> +
> +struct ax3000_slice_info {
> +	u32 out_mux;
> +	u32 sgpio_mux;
> +	u32 slice_mux;
> +	u32 reg[MAX_OFFSET_REG];
> +	u32 reg_ss[MAX_OFFSET_REG];
> +	u32 preset;
> +	u32 count;
> +	u32 pos;
> +};
> +
> +struct ax3000_sgpio {
> +	u32 preset_value;
> +	u32 count_value;
> +	u32 pos_reg;
> +	struct ax3000_slice_info
> +		slices[MAX_SLICE_COUNT]; /* 0=clk,1=load,2=out,3=in,4=oe */
> +	spinlock_t lock;
> +	int ngpios;
> +	int max_sgpio_pins;
> +	int max_offset_regs;
> +	struct gpio_chip chip;
> +	u32 irq_unmasked[MAX_SGPIO_PINS];
> +	int parent_irq;
> +	struct regmap *regmap;
> +	u32 regmap_base_offset;
> +	struct sgpio_reg_offsets *regs;
> +};
> +
> +static int sgpio_set_irq_type(struct irq_data *d, unsigned int type);
> +static void sgpio_mask_irq(struct irq_data *d);
> +static void sgpio_unmask_irq(struct irq_data *d);
> +static void sgpio_irq_shutdown(struct irq_data *d);
> +
> +static const struct irq_chip axiado_sgpio_irqchip = {
> +	.name = "axiado-sgpio",
> +	.irq_mask = sgpio_mask_irq,
> +	.irq_unmask = sgpio_unmask_irq,
> +	.irq_set_type = sgpio_set_irq_type,
> +	.irq_shutdown = sgpio_irq_shutdown,
> +	.flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND,
> +};
> +
> +static void ax3000_sgpio_set(struct gpio_chip *chip, unsigned int offset,
> +			     int value)
> +{
> +	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
> +	unsigned long flags;
> +	u32 bank = (offset / 2) / 32;
> +	u32 position = (offset / 2) % 32;
> +
> +	spin_lock_irqsave(&sgpio->lock, flags);

Please use guards for locks.

> +	if (value)
> +		sgpio->slices[2].reg_ss[bank] |= BIT(position);
> +	else
> +		sgpio->slices[2].reg_ss[bank] &= ~BIT(position);
> +
> +	spin_unlock_irqrestore(&sgpio->lock, flags);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->dout_ss +
> +			     (bank * 4),
> +		     sgpio->slices[2].reg_ss[bank]);
> +}
> +
> +static int ax3000_sgpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
> +	u32 bank = (offset / 2) / 32;
> +	u32 position = (offset / 2) % 32;
> +
> +	if (offset % 2 == 0)
> +		return !!(sgpio->slices[3].reg_ss[bank] & BIT(position));
> +	else
> +		return !!(sgpio->slices[2].reg_ss[bank] & BIT(position));
> +}
> +
> +static int ax3000_sgpio_dir_in(struct gpio_chip *chip, unsigned int offset)
> +{
> +	if (!(offset % 2))
> +		return 0;
> +	else
> +		return -EINVAL;
> +}
> +
> +static int ax3000_sgpio_dir_out(struct gpio_chip *chip, unsigned int offset,
> +				int value)
> +{
> +	if (offset % 2) {
> +		if (chip->set)
> +			chip->set(chip, offset, value);
> +		return 0;
> +	} else {
> +		return -EINVAL;
> +	}
> +}
> +
> +static irqreturn_t sgpio_irq_handler(int irq, void *arg)
> +{
> +	struct ax3000_sgpio *sgpio = (struct ax3000_sgpio *)arg;
> +	unsigned long flags;
> +	u32 status, new_value;
> +	u32 changed_value;
> +	int i, bit, reg_ptr;
> +
> +	/* Read-on-clear (ACK) parent cause */
> +	regmap_read(sgpio->regmap,
> +		    sgpio->regmap_base_offset + sgpio->regs->status, &status);
> +	status >>= 16;
> +
> +	bool has_shifted_layout = (sgpio->max_offset_regs == MAX_OFFSET_REG);
> +
> +	reg_ptr = has_shifted_layout ? 16 - DIV_ROUND_UP(sgpio->ngpios, 32) : 0;
> +
> +	for (i = 0; i < DIV_ROUND_UP(sgpio->ngpios, 32); i++, reg_ptr++) {
> +		if (status & BIT(reg_ptr)) {
> +			regmap_read(sgpio->regmap,
> +				    sgpio->regmap_base_offset +
> +					    sgpio->regs->din_ss + (reg_ptr * 4),
> +				    &new_value);
> +			spin_lock_irqsave(&sgpio->lock, flags);
> +			changed_value = sgpio->slices[3].reg_ss[i] ^ new_value;
> +			sgpio->slices[3].reg_ss[i] = new_value;
> +			spin_unlock_irqrestore(&sgpio->lock, flags);
> +
> +			while (changed_value) {
> +				bit = __ffs(changed_value);
> +				changed_value &= ~BIT(bit);
> +
> +				irq_hw_number_t hwirq = i * 32 + bit;
> +
> +				if (sgpio->irq_unmasked[hwirq]) {
> +					unsigned int child_irq;
> +
> +					child_irq = irq_find_mapping(sgpio->chip.irq.domain,
> +								     hwirq);
> +
> +					if (child_irq)
> +						handle_nested_irq(child_irq);
> +				}
> +			}
> +		}
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void sgpio_hw_init(struct ax3000_sgpio *sgpio)
> +{
> +	u32 bank;
> +	u32 position;
> +	int i = 0;
> +	bool has_shifted_layout = (sgpio->max_offset_regs == MAX_OFFSET_REG);
> +
> +	/* slice A0, Clock Pin - 0 */
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_0, 0x306);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_0,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_0,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_0, 0x1f001f);
> +
> +	/* Slice B1, Data Load Pin - 1 */
> +	bank = (sgpio->ngpios - 1) / 32;
> +	position = (sgpio->ngpios - 1) % 32;
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_1,
> +		     has_shifted_layout ? 0x30c : 0x304);
> +
> +	for (i = 0; i < bank; i++) {
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->ld +
> +				     (i * 4),
> +			     0xffffffff);
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->ld_ss +
> +				     (i * 4),
> +			     0xffffffff);
> +	}
> +
> +	if (position) {
> +		u32 val;
> +
> +		val = sgpio->slices[1].reg_ss[i];
> +		val |= GENMASK(position - 1, 0);
> +
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->ld +
> +				     (i * 4),
> +			     val);
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->ld_ss +
> +				     (i * 4),
> +			     val);
> +	}
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_1,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_1,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_1,
> +		     sgpio->pos_reg);
> +
> +	/* Slice C2, Data Out Pin - 2 */
> +	bank = sgpio->ngpios / 32;
> +	position = sgpio->ngpios % 32;
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_2,
> +		     has_shifted_layout ? 0x30c : 0x304);
> +
> +	for (i = 0; i < bank; i++) {
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->dout +
> +				     (i * 4),
> +			     sgpio->slices[2].reg_ss[i]);
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->dout_ss +
> +				     (i * 4),
> +			     sgpio->slices[2].reg_ss[i]);
> +	}
> +
> +	if (position) {
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->dout +
> +				     (i * 4),
> +			     sgpio->slices[2].reg_ss[i]);
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->dout_ss +
> +				     (i * 4),
> +			     sgpio->slices[2].reg_ss[i]);
> +	}
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_2,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_2,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_2,
> +		     sgpio->pos_reg);
> +
> +	/* Slice D3, Data In Pin - 3 */
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_3, 0x14C);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_3,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_3,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_3,
> +		     sgpio->pos_reg);
> +
> +	/* Slice E4, Output Enable for respective pins */
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_4,
> +		     has_shifted_layout ? 0x10c : 0x104);
> +	regmap_write(sgpio->regmap, sgpio->regmap_base_offset + sgpio->regs->oe,
> +		     0xffffffff);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->oe_ss,
> +		     0xffffffff);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_4,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_4,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_4, 0x1f001f);
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mask, 0xdfff);
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->ctrl_en, 0xffff);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->ctrl_en_pos,
> +		     0xffff);
> +}
> +
> +static int sgpio_set_irq_type(struct irq_data *d, unsigned int type)
> +{
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_BOTH:
> +	case IRQ_TYPE_EDGE_RISING:
> +	case IRQ_TYPE_EDGE_FALLING:
> +		irq_set_handler_locked(d, handle_edge_irq);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void sgpio_mask_irq(struct irq_data *d)
> +{
> +	struct gpio_chip *chip;
> +	struct ax3000_sgpio *sgpio;
> +	u32 irq_num;
> +
> +	chip = irq_data_get_irq_chip_data(d);
> +	if (!chip) {
> +		pr_err("Unable to get gpio_chip for IRQ\n");
> +		return;
> +	}
> +
> +	sgpio = gpiochip_get_data(chip);
> +	if (!sgpio) {
> +		pr_err("Unable to get chip data\n");
> +		return;
> +	}
> +
> +	irq_num = irqd_to_hwirq(d);
> +	sgpio->irq_unmasked[irq_num / 2] = 0;
> +}
> +
> +static void sgpio_unmask_irq(struct irq_data *d)
> +{
> +	struct gpio_chip *chip;
> +	struct ax3000_sgpio *sgpio;
> +	u32 irq_num;
> +
> +	chip = irq_data_get_irq_chip_data(d);
> +	if (!chip) {
> +		pr_err("Unable to get gpio_chip for IRQ\n");
> +		return;
> +	}
> +
> +	sgpio = gpiochip_get_data(chip);
> +	if (!sgpio) {
> +		pr_err("Unable to get chip data\n");
> +		return;
> +	}
> +
> +	irq_num = irqd_to_hwirq(d);
> +	sgpio->irq_unmasked[irq_num / 2] = 1;
> +}
> +
> +static void sgpio_irq_shutdown(struct irq_data *d)
> +{
> +	sgpio_mask_irq(d);
> +}
> +
> +static int sgpio_probe(struct platform_device *pdev)
> +{
> +	int rc;
> +	int irq;
> +	int i;
> +	const __be32 *prop;
> +	struct gpio_irq_chip *girq;
> +	struct ax3000_sgpio *sgpio;
> +	u32 variant;
> +	u32 dout_value;
> +	u32 bus_frequency;
> +	u32 apb_frequency;
> +	int dout_reverse;
> +
> +	void __iomem *base;
> +
> +	const struct regmap_config regmap_config = {
> +		.reg_bits = 32,
> +		.val_bits = 32,
> +		.reg_stride = 4,
> +	};
> +
> +	sgpio = devm_kzalloc(&pdev->dev, sizeof(*sgpio), GFP_KERNEL);
> +	if (!sgpio)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&sgpio->lock);
> +
> +	sgpio->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +
> +	if (sgpio->regmap) {
> +		rc = of_property_read_u32(pdev->dev.of_node, "reg",
> +					  &sgpio->regmap_base_offset);

Why are you mixing of_property_*() with device_property_*()?

> +		if (rc) {
> +			dev_err(&pdev->dev, "Failed to read reg property: %d\n",
> +				rc);
> +			return rc;
> +		}
> +		dev_info(&pdev->dev, "Using regmap with base offset: 0x%x\n",
> +			 sgpio->regmap_base_offset);
> +	} else {
> +		base = devm_platform_ioremap_resource(pdev, 0);
> +		if (IS_ERR(base))
> +			return PTR_ERR(base);
> +
> +		sgpio->regmap =
> +			devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);
> +
> +		if (IS_ERR(sgpio->regmap))
> +			return PTR_ERR(sgpio->regmap);
> +
> +		sgpio->regmap_base_offset = 0;
> +
> +		dev_info(&pdev->dev, "Using MMIO regmap\n");
> +	}
> +
> +	rc = device_property_read_u32(&pdev->dev, "ngpios", &sgpio->ngpios);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Could not read ngpios property\n");
> +		return -EINVAL;
> +	}
> +
> +	if (device_property_read_u32(&pdev->dev, "design-variant", &variant)) {
> +		dev_err(&pdev->dev, "design-variant not specified in DT\n");
> +		return -EINVAL;
> +	}
> +
> +	if (variant == 128) {
> +		sgpio->regs = &sgpio_offsets_128;
> +		sgpio->max_sgpio_pins = 128;
> +		sgpio->max_offset_regs = 4;
> +	} else if (variant == 512) {
> +		sgpio->regs = &sgpio_offsets_512;
> +		sgpio->max_sgpio_pins = 512;
> +		sgpio->max_offset_regs = 16;
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	if (sgpio->ngpios > sgpio->max_sgpio_pins) {
> +		dev_err(&pdev->dev, "ngpio is greater than 512 pins\n");
> +		return -EINVAL;
> +	}
> +
> +	rc = device_property_read_u32(&pdev->dev, "bus-frequency",
> +				      &bus_frequency);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Could not read bus-frequency property\n");
> +		return -EINVAL;
> +	}
> +
> +	rc = device_property_read_u32(&pdev->dev, "apb-frequency",
> +				      &apb_frequency);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Could not read apb-frequency property\n");
> +		return -EINVAL;
> +	}
> +
> +	sgpio->preset_value = (apb_frequency / bus_frequency) - 1;
> +	sgpio->count_value = sgpio->preset_value;
> +
> +	u32 pos;
> +
> +	pos = sgpio->ngpios - 1;
> +	sgpio->pos_reg = (pos << 16) | pos;
> +
> +	prop = of_get_property(pdev->dev.of_node, "dout-init", NULL);
> +	if (!prop) {
> +		dev_err(&pdev->dev, "Failed to get dout-init\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < sgpio->max_offset_regs; i++) {
> +		sgpio->slices[2].reg_ss[i] = 0;
> +		dout_value = be32_to_cpu(prop[i]);
> +
> +		for (dout_reverse = 0; dout_reverse < 32; ++dout_reverse) {
> +			sgpio->slices[2].reg_ss[i] <<= 1;
> +			sgpio->slices[2].reg_ss[i] |= (dout_value & 1);
> +			dout_value >>= 1;
> +		}
> +	}
> +
> +	sgpio_hw_init(sgpio);
> +
> +	irq = platform_get_irq(pdev, 0);
> +

Unnecessary newline.

> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get parent IRQ: %d\n", irq);
> +		return irq;
> +	}
> +	/* Store parent IRQ for cleanup */
> +	sgpio->parent_irq = irq;
> +
> +	rc = devm_request_threaded_irq(&pdev->dev, irq, NULL, sgpio_irq_handler,
> +				       IRQF_ONESHOT, "axiado-sgpio", sgpio);
> +
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Failed to request threaded IRQ %d: %d\n",
> +			irq, rc);
> +		return rc;
> +	}
> +
> +	sgpio->chip.parent = &pdev->dev;
> +	sgpio->chip.ngpio = sgpio->ngpios * 2;
> +	sgpio->chip.owner = THIS_MODULE;
> +	sgpio->chip.direction_input = ax3000_sgpio_dir_in;
> +	sgpio->chip.direction_output = ax3000_sgpio_dir_out;
> +	sgpio->chip.get = ax3000_sgpio_get;
> +	sgpio->chip.set = ax3000_sgpio_set;
> +	sgpio->chip.label = dev_name(&pdev->dev);
> +	sgpio->chip.base = -1;
> +
> +	girq = &sgpio->chip.irq;
> +
> +	girq->chip = &axiado_sgpio_irqchip;
> +	girq->handler = handle_edge_irq;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->num_parents = 1;
> +	girq->parents =
> +		devm_kcalloc(&pdev->dev, 1, sizeof(*girq->parents), GFP_KERNEL);
> +	if (!girq->parents) {
> +		dev_err(&pdev->dev, "Failed to allocate parents array\n");

Drop this message, returning -ENOMEM is enough.

> +		return -ENOMEM;
> +	}
> +	girq->parents[0] = irq;
> +
> +	rc = devm_gpiochip_add_data(&pdev->dev, &sgpio->chip, sgpio);
> +	if (rc < 0) {
> +		dev_err(&pdev->dev, "Could not register gpiochip, %d\n", rc);
> +		return rc;

Use return dev_err_probe() here and elsewhere.

> +	}
> +
> +	/* Store driver data for remove() */
> +	platform_set_drvdata(pdev, sgpio);
> +	dev_info(&pdev->dev, "SGPIO registered with %d GPIOs\n",
> +		 sgpio->chip.ngpio);

No need for this info message, please drop it.

> +
> +	return 0;
> +}
> +
> +static int sgpio_remove(struct platform_device *pdev)
> +{
> +	struct ax3000_sgpio *sgpio = platform_get_drvdata(pdev);
> +	int i;
> +
> +	if (!sgpio)
> +		return 0;
> +
> +	/* Disable interrupts in hardware */
> +	if (sgpio->regs) {
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->mask,
> +			     0x0);
> +		regmap_write(sgpio->regmap,
> +			     sgpio->regmap_base_offset + sgpio->regs->ctrl_en,
> +			     0x0);
> +	}
> +
> +	/* Disable and synchronize parent IRQ to avoid races with handlers */
> +	if (sgpio->parent_irq >= 0) {
> +		disable_irq(sgpio->parent_irq);
> +		synchronize_irq(sgpio->parent_irq);
> +	}
> +
> +	/* Ensure all GPIO IRQ handlers complete before removal */
> +	if (sgpio->chip.irq.domain) {
> +		struct irq_domain *domain = sgpio->chip.irq.domain;
> +		unsigned int irq;
> +		int hwirq;
> +
> +		for (hwirq = 0; hwirq < sgpio->chip.ngpio; hwirq++) {
> +			irq = irq_find_mapping(domain, hwirq);
> +			if (irq) {
> +				disable_irq(irq);
> +				synchronize_irq(irq);
> +			}
> +		}
> +	}
> +
> +	/* Clear internal IRQ state */
> +	for (i = 0; i < sgpio->max_sgpio_pins; i++)
> +		sgpio->irq_unmasked[i] = 0;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ax_sgpio_match[] = {
> +	{ .compatible = "axiado,sgpio" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ax_sgpio_match);
> +
> +static struct platform_driver sgpio_driver = {
> +	.driver = {
> +		.name = "sgpio",
> +		.owner = THIS_MODULE,
> +		.of_match_table = ax_sgpio_match,
> +	},
> +	.probe = sgpio_probe,
> +	.remove = sgpio_remove,
> +};
> +
> +static int __init ax_sgpio_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&sgpio_driver);
> +	if (ret < 0) {
> +		pr_err("Failed to register SGPIO driver\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit ax_sgpio_exit(void)
> +{
> +	platform_driver_unregister(&sgpio_driver);
> +}
> +
> +module_init(ax_sgpio_init);
> +module_exit(ax_sgpio_exit);

Just use module_platform_driver().

> +
> +MODULE_DESCRIPTION("Axiado Serial GPIO Driver");
> +MODULE_AUTHOR("Axiado Corporation");
> +MODULE_LICENSE("GPL");
>
> --
> 2.34.1
>
>

Bart


^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Yeoreum Yun @ 2026-04-20  9:25 UTC (permalink / raw)
  To: Will Deacon
  Cc: Marc Zyngier, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
	zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, peterhuewe,
	jarkko, jgg, sudeep.holla, oupton, joey.gouly, suzuki.poulose,
	yuzenghui, catalin.marinas
In-Reply-To: <aeXp7WSqpXNytNPG@willie-the-truck>

Hi Will,

> On Sun, Apr 19, 2026 at 12:12:44PM +0100, Yeoreum Yun wrote:
> > Hi Marc,
> >
> > > On Sat, 18 Apr 2026 11:34:30 +0100,
> > > Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > >
> > > > > > @@ -2035,6 +2037,16 @@ static int __init ffa_init(void)
> > > > > >  	u32 buf_sz;
> > > > > >  	size_t rxtx_bufsz = SZ_4K;
> > > > > >
> > > > > > +	/*
> > > > > > +	 * When pKVM is enabled, the FF-A driver must be initialized
> > > > > > +	 * after pKVM initialization. Otherwise, pKVM cannot negotiate
> > > > > > +	 * the FF-A version or obtain RX/TX buffer information,
> > > > > > +	 * which leads to failures in FF-A calls.
> > > > > > +	 */
> > > > > > +	if (IS_ENABLED(CONFIG_KVM) && is_protected_kvm_enabled() &&
> > > > > > +	    !is_kvm_arm_initialised())
> > > > > > +		return -EPROBE_DEFER;
> > > > > > +
> > > > >
> > > > > That's still fundamentally wrong: pkvm is not ready until
> > > > > finalize_pkvm() has finished, and that's not indicated by
> > > > > is_kvm_arm_initialised().
> > > >
> > > > Thanks. I miss the TSC bit set in here.
> > >
> > > That's the least of the problems. None of the infrastructure is in
> > > place at this stage...
> > >
> > > > IMHO, I'd like to make an new state check function --
> > > > is_pkvm_arm_initialised() so that ff-a driver to know whether
> > > > pkvm is initialised.
> > >
> > > Doesn't sound great, TBH.
> > >
> > > > or any other suggestion?
> > >
> > > Instead of adding more esoteric predicates, I'd rather you build on an
> > > existing infrastructure. You have a dependency on KVM, use something
> > > that is designed to enforce dependencies. Device links spring to mind
> > > as something designed for that.
> > >
> > > Can you look into enabling this for KVM? If that's possible, then it
> > > should be easy enough to delay the actual KVM registration after pKVM
> > > is finalised.
> >
> > or what about some event notifier? Just like:
>
> This seems a bit over-engineered to me. Why don't you just split the
> FF-A initialisation into two steps: an early part which does the version
> negotiation and then a later part which can fit in with whatever
> dependencies you have on the TPM?

Sorry, I may have misunderstood your suggestion and
I might be in missing your point.

But, The issue here is that FFA_VERSION, FFA_RXTX_MAP, and
FFA_PARTITION_INFO_GET, which are invoked from ffa_init()
as part of early initialisation, must be trapped by pKVM.

In other words, even the early part of the initialization,
including version negotiation, needs to happen after pKVM
is initialized.

Because of this dependency, simply splitting the FF-A
initialization into two phases within the driver does not
seem sufficient, as it still requires knowing when pKVM
has been initialized.

Am I missing something?

--
Sincerely,
Yeoreum Yun


^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: amlogic: t7: khadas-vim4: Enable Bluetooth
From: Ronald Claveau @ 2026-04-20  9:26 UTC (permalink / raw)
  To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <c9c3227f-2a46-47b5-963f-e784184f7f31@linaro.org>

On 4/20/26 10:47 AM, Neil Armstrong wrote:
>> Enable UART C on the Khadas VIM4 board and attach the BCM43438
>>   compatible Bluetooth controller to it. The node configures the RTS/CTS
>> hardware flow control, the associated pinmux, the power supplies
>> (vddao_3v3
>> and vddao_1v8), the 32 kHz LPO clock shared with the wifi32k fixed
>> clock, and the GPIO lines used for host wakeup, device wakeup and
>> shutdown.
>>
>> Remove clocks and clock-names for UART A, as they are defined in DTSI.
> 
> This should be a separate patch.

Thanks for your feedback.
I will then add the remove redundant clocks before that one.

-- 
Best regards,
Ronald


^ permalink raw reply

* [PATCH] ACPI: arm64: cpuidle: Tolerate platforms with no deep PSCI idle states
From: Breno Leitao @ 2026-04-20  9:27 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Catalin Marinas,
	Will Deacon, Rafael J. Wysocki, Len Brown, Huisong Li
  Cc: Rafael J. Wysocki, linux-acpi, linux-arm-kernel, linux-kernel,
	pjaroszynski, rmikey, kernel-team, stable, Breno Leitao

Commit cac173bea57d ("ACPI: processor: idle: Rework the handling of
acpi_processor_ffh_lpi_probe()") moved the acpi_processor_ffh_lpi_probe()
call from acpi_processor_setup_cpuidle_dev(), where its return value was
ignored, to acpi_processor_get_power_info(), where it is now treated as
a hard failure. As a result, platforms where psci_acpi_cpu_init_idle()
returned -ENODEV stopped registering any cpuidle states, forcing CPUs to
busy-poll when idle.

On NVIDIA Grace (aarch64) systems with PSCIv1.1, pr->power.count is 1
(only WFI, no deep PSCI states beyond it), so the previous
"count = pr->power.count - 1; if (count <= 0) return -ENODEV;" check
returned -ENODEV for all 72 CPUs and disabled cpuidle entirely.

The lpi_states count is already validated in acpi_processor_get_lpi_info(),
so the check here is redundant. Simplify the loop to iterate over
lpi_states[1..power.count). When only WFI is present, the loop body
simply does not execute and the function returns 0, which is the correct
outcome: there is nothing to validate for FFH and no error to report.

Suggested-by: Huisong Li <lihuisong@huawei.com>
Cc: stable@vger.kernel.org
Fixes: cac173bea57d ("ACPI: processor: idle: Rework the handling of acpi_processor_ffh_lpi_probe()")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/acpi/arm64/cpuidle.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/cpuidle.c b/drivers/acpi/arm64/cpuidle.c
index 801f9c4501425..c68a5db8ebba8 100644
--- a/drivers/acpi/arm64/cpuidle.c
+++ b/drivers/acpi/arm64/cpuidle.c
@@ -16,7 +16,7 @@
 
 static int psci_acpi_cpu_init_idle(unsigned int cpu)
 {
-	int i, count;
+	int i;
 	struct acpi_lpi_state *lpi;
 	struct acpi_processor *pr = per_cpu(processors, cpu);
 
@@ -30,14 +30,10 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
 	if (!psci_ops.cpu_suspend)
 		return -EOPNOTSUPP;
 
-	count = pr->power.count - 1;
-	if (count <= 0)
-		return -ENODEV;
-
-	for (i = 0; i < count; i++) {
+	for (i = 1; i < pr->power.count; i++) {
 		u32 state;
 
-		lpi = &pr->power.lpi_states[i + 1];
+		lpi = &pr->power.lpi_states[i];
 		/*
 		 * Only bits[31:0] represent a PSCI power_state while
 		 * bits[63:32] must be 0x0 as per ARM ACPI FFH Specification

---
base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
change-id: 20260413-ffh-93f68b2f46a3

Best regards,
--  
Breno Leitao <leitao@debian.org>



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox