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

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

On 05/18 13:33, Catalin Marinas wrote:
> On Mon, May 18, 2026 at 12:31:31PM +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.
> > 
> > [0]
> >  BUG: using smp_processor_id() in preemptible [00000000] code: cca-workload-at/134
> >  caller is debug_smp_processor_id+0x20/0x2c
> >  CPU: 0 UID: 0 PID: 134 Comm: cca-workload-at Not tainted 7.0.0-rc1-gc74a64d12073 #1 PREEMPT
> >  Hardware name: linux,dummy-virt (DT)
> >  Call trace:
> >   [...]
> >   check_preemption_disabled+0xf8/0x100
> >   debug_smp_processor_id+0x20/0x2c
> >   arm_cca_report_new+0x54/0x230
> >   tsm_report_read+0x184/0x260
> >   tsm_report_outblob_read+0x18/0x38
> >   configfs_bin_read_iter+0xf4/0x1dc
> >   vfs_read+0x230/0x31c
> >   [...]
> > 
> > Fixes: 7999edc484ca ("virt: arm-cca-guest: TSM_REPORT support for realms")
> > Signed-off-by: Kohei Enju <enju.kohei@fujitsu.com>
> > ---
> >  drivers/virt/coco/arm-cca-guest/arm-cca-guest.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> > index 0c9ea24a200c..2d450caee3e4 100644
> > --- a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> > +++ b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> > @@ -108,7 +108,7 @@ static int arm_cca_report_new(struct tsm_report *report, void *data)
> >  	 * allocate outblob based on the returned value from the 'init'
> >  	 * call and that cannot be done in an atomic context.
> >  	 */
> > -	cpu = smp_processor_id();
> > +	cpu = raw_smp_processor_id();
> 
> That's just hiding the warning which might be genuine, irrespective of
> what the comment says. Sashiko has some good points:
> 
> https://sashiko.dev/#/patchset/20260518033157.1865498-1-enju.kohei@fujitsu.com
> 
> Basically what guarantees that the cpu won't go offline? Can we use
> migrate_disable() and ignore the smp_call_function_single() altogether?
> It looks like a hack anyway.

Hi Catalin,
Thank you for reviewing.

You've raised a very valid point about raw_smp_processor_id()
potentially hiding a genuine issue. I agree this would be a concern in
most contexts.

However, this implementation was intentionally designed not to block CPU
hotplug:
https://lore.kernel.org/linux-arm-kernel/7a83461d-40fd-4e61-8833-5dae2abaf82b@arm.com/

As mentioned in the thread above, the potential failure from the target
CPU going offline (resulting in -ENXIO) is an expected and tolerated
condition in this path.
Using migrate_disable() would go against the non-blocking design goal.

Given the context, the debug warning looks false positive for our
specific use case to me, and I believe raw_smp_processor_id() correctly
reflects the design intent by simply acquiring a CPU number without
debug checks.

> 
> We should also look at the other unrelated findings in this function

Regarding the other unrelated findings by Sashiko, I'll take a look at
them. Thanks for the heads-up.

Thanks,
Kohei

> 
> -- 
> Catalin
>