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

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

Posted by Kohei Enju 1 week ago

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();
 
 	info.challenge = desc->inblob;
 	info.challenge_size = desc->inblob_len;
-- 
2.47.3

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

Posted by Catalin Marinas 6 days, 16 hours ago

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.

We should also look at the other unrelated findings in this function
from Sashiko.

-- 
Catalin

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

Posted by Kohei Enju 6 days, 15 hours ago

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
>

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

Posted by Catalin Marinas 6 days, 11 hours ago

Hi Kohei,

On Mon, May 18, 2026 at 10:38:53PM +0900, Kohei Enju wrote:
> On 05/18 13:33, Catalin Marinas wrote:
> > On Mon, May 18, 2026 at 12:31:31PM +0900, Kohei Enju wrote:
> > > 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.
[...]
> 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.

Thanks, I wasn't aware of the old discussion. If user-space can
tolerate, than it's fine.

Would you mind updating the comment above the changed line? It talks
about not allocating memory in atomic context, so migrate_disable()
would solve this. Just mention that it can't block CPU hotplug events
either and user-space can handle spurious errors.

With that:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

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

Posted by Kohei Enju 6 days, 2 hours ago

On 05/18 18:21, Catalin Marinas wrote:
> Hi Kohei,
> 
> On Mon, May 18, 2026 at 10:38:53PM +0900, Kohei Enju wrote:
> > On 05/18 13:33, Catalin Marinas wrote:
> > > On Mon, May 18, 2026 at 12:31:31PM +0900, Kohei Enju wrote:
> > > > 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.
> [...]
> > 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.
> 
> Thanks, I wasn't aware of the old discussion. If user-space can
> tolerate, than it's fine.
> 
> Would you mind updating the comment above the changed line? It talks
> about not allocating memory in atomic context, so migrate_disable()
> would solve this. Just mention that it can't block CPU hotplug events
> either and user-space can handle spurious errors.

Sure, I'm happy to do. Thank you for the suggestion.
I'll work on v2.

Thanks,
Kohei

> 
> With that:
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>

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

Posted by Suzuki K Poulose 6 days, 20 hours ago

On 18/05/2026 04:31, 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>

Thank for the fix,

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.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();
>   
>   	info.challenge = desc->inblob;
>   	info.challenge_size = desc->inblob_len;

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

Posted by Gavin Shan 1 week ago

On 5/18/26 1:31 PM, 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(-)
> 
Reviewed-by: Gavin Shan <gshan@redhat.com>