Re: [PATCH rdma-next 02/23] net/mlx5: Derive adversarial mode from CC guest memory encryption

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] mlx5_is_adversarial() in
  drivers/net/ethernet/mellanox/mlx5/core/main.c is exported…
- [Low] mlx5_is_adversarial(void) takes no device argument and returns
  cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT), so the decision applies to…

--

net/mlx5: Derive adversarial mode from CC guest memory encryption

This patch adds mlx5_is_adversarial(), exported from mlx5_core. It
reports whether the driver should treat the NIC as untrusted. The result
comes from cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT), so it is true in
any confidential computing guest.

> Add mlx5_is_adversarial() to report whether the driver treats the NIC
> as untrusted. In a confidential computing guest, the hypervisor and any
> passthrough device are outside the guest's trust boundary, so the mode
> is derived from cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT).
>
> This turns adversarial mode on automatically in such guests, without
> any administrator configuration.

[Severity: Low]
This isn't a bug, but is the last sentence accurate for this patch on its
own? Nothing calls mlx5_is_adversarial() at this commit, so this patch by
itself does not change behaviour in a CC guest.

The callers arrive later in the series:

  "net/mlx5: Add capability filtering in adversarial mode" (caps_filter.c)

  "RDMA/mlx5: Derive mlx5_ib_adversarial_mode global from mlx5_core
  adversarial API" (drivers/infiniband/hw/mlx5/main.c)

Could the changelog say that the users come in follow-up patches?

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 1a45474ce03db..f9e10464a71b5 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c

[ ... ]

> @@ -484,6 +485,12 @@ bool mlx5_is_roce_on(struct mlx5_core_dev *dev)
>  }
>  EXPORT_SYMBOL(mlx5_is_roce_on);
>  
> +bool mlx5_is_adversarial(void)
> +{
> +	return cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT);

[Severity: Low]
This isn't a bug, but this check treats every device in a CC guest as
adversarial, and the helper takes no device argument. That matches the
current tree, which has no guest-side way to detect a trusted (T=1)
device.

ib_register_device() in drivers/infiniband/core/device.c uses the same
check and documents the assumption:

	/*
	 * In a CoCo guest every device is currently assumed to be untrusted
	 * (T=0) and therefore subject to DMA bouncing. Once trusted (T=1)
	 * device detection is wired up, narrow this check to exclude such
	 * devices.
	 */
	if (dma_device && cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
		device->cc_dma_bounce = 1;

Would a similar comment here, or a note in the changelog, be worth
adding? Once T=1 detection exists, narrowing this check would also mean
giving mlx5_is_adversarial() a device argument.

> +}
> +EXPORT_SYMBOL_GPL(mlx5_is_adversarial);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103163949.1554-1-yishaih%40nvidia.com