Re: [PATCH v2] virt: arm-cca-guest: use raw variant of smp_processor_id() in arm_cca_report_new()

[Kohei Enju <enju.kohei@fujitsu.com>]

On 06/09 14:44, Suzuki K Poulose wrote:
> On 09/06/2026 14:16, Kohei Enju wrote:
> > On 06/03 12:48, Will Deacon wrote:
> > > On Tue, Jun 02, 2026 at 04:48:43PM +0100, Suzuki K Poulose wrote:
> > > > On 02/06/2026 12:01, Will Deacon wrote:
> > > > > On Tue, May 19, 2026 at 07:12:08PM +0900, Kohei Enju wrote:
> > > > > > With CONFIG_DEBUG_PREEMPT=y, smp_processor_id() becomes an alias of
> > > > > > debug_smp_processor_id(). This debug function complains when certain
> > > > > > conditions that ensure CPU ID stability are not met, specifically when
> > > > > > it's called from a preemptible context.
> > > > > > 
> > > > > > In arm_cca_report_new(), which runs in a preemptible context,
> > > > > > smp_processor_id() triggers a splat [0] due to this.
> > > > > > 
> > > > > > However, the CPU ID obtained here is used as the target CPU for
> > > > > > smp_call_function_single() to designate a specific CPU for subsequent
> > > > > > operations, not to assert that the current thread will continue to
> > > > > > execute on the same CPU. Therefore, snapshotting the CPU ID itself is
> > > > > > correct, and thus there's no actual harm except for the splat.
> > > > > > 
> > > > > > Use raw_smp_processor_id() instead, to directly retrieve the current CPU
> > > > > > ID without the debug checks, avoiding the unnecessary warning message
> > > > > > while preserving the correct functional behavior.
> > > > > 
> > > > > That's pretty disgusting imo so I'd like to see some more justification
> > > > > for this approach.
> > > > > 
> > > > > > Note that while migrate_disable() would pin the task to the current CPU,
> > > > > > this path should not block CPU hotplug events. Therefore, we snapshot
> > > > > > the current CPU ID and accept that smp_call_function_single() may fail
> > > > > > if the CPU goes offline.
> > > > > 
> > > > > Why shouldn't it block CPU hotplug events? What happens if the CPU goes
> > > > > offline and comes back online again during the loop of continue calls?
> > > > 
> > > > It need not. It can continue the calls. The RMM keeps track of the internal
> > > > progress in the "REC" object for this "VCPU". Hotplug ON/OFF
> > > > doesn't change the REC object in CCA Guest. So, a REC can come back and
> > > > execute it. But the Linux could fail the operation if the CPU isn't
> > > > available for fetching the report, after we do a RSI_ATTEST_TOKEN_INIT.
> > > 
> > > I couldn't really shake that out of the RMM spec tbh:
> > > RSI_ATTESTATION_TOKEN_CONTINUE is allowed to return RSI_ERROR_UNKNOWN
> > > and I couldn't find anything about hotplug.
> 
> Like I said, we are dealing with Virtual CPUs (RECs here) and the only
> condition that the RMM enforces is the _CONTINUE calls are issued on the
> same VCPU (REC). Hotplug doesn't change the REC (as a REC cannot be
> modified or added once the Realm is ACTIVE).
> 
> > > 
> > > But my main point, really, is why are we not using migrate_disable()
> > > here? I can't see the justification.
> 
> There is no reason, why we couldn't use this. TBH, I wasn't aware of the
> helper ;-) at the time this was initially designed and we didn't want to
> block CPU hotplug. We could always request a new report, it is not like
> aborting a report generation has any side effects.
> 
> > 
> > Hi Will,
> > Sorry for the late reply.
> > 
> > I agree that using migrate_disable() makes this path simpler and
> > clearer.
> > 
> > I've reviewed the discussion where the original commit was introduced:
> >      https://lore.kernel.org/linux-arm-kernel/7a83461d-40fd-4e61-8833-5dae2abaf82b@arm.com/
> > but I couldn't find a strong reason why we shouldn't block CPU hotplug
> > or use migrate_disable(), even though I can see the current design was
> > intentional.
> > 
> > So I'm happy to rework the patch to use migrate_disable() and remove
> > the smp_call_function_single() calls if there are no objections.
> 
> I am fine with removing with migrate_disable() approach.

Hi Suzuki,

Thank you for your clarification, and explanation in this discussion.
I'll work on v3 soon.

> 
> Cheers
> Suzuki
> 
> > 
> > > 
> > > Will
> > > 
> 
>