Re: [PATCH v2 20/28] KVM: arm64: Reject non-coherent DMA when using forced writeback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: arm64: kvm_arch_register_noncoherent_dma() bugs VM for coherent VFIO devs
--

commit e65fa223b5ab689dd29e0940ed61947cf8f5ac8a
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: Reject non-coherent DMA when using forced writeback

This commit implements kvm_arch_register_noncoherent_dma() on arm64 to mark a
VM as bugged when non-coherent DMA is registered on a system using stage-2
forced writeback (ARM64_HAS_STAGE2_FWB).

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index d593ba9280933..f08a450184d96 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -133,6 +133,20 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
>  }
>  
> +void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
> +{
> +	if (!cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
> +		return;
> +
> +	pr_err_once("kvm: noncoherent DMA unsupported with FWB, see kvm_arm.nofwb\n");
> +	kvm_vm_bugged(kvm);
> +}

[Severity: High]
Will calling kvm_vm_bugged() here break VFIO passthrough for cache-coherent
devices on systems with ARM64_HAS_STAGE2_FWB that do not advertise
IOMMU_CAP_ENFORCE_CACHE_COHERENCY?

When userspace attaches a VFIO group or device file to a VM via
KVM_DEV_VFIO_FILE_ADD, virt/kvm/vfio.c:kvm_vfio_update_coherency() checks
kvm_vfio_file_enforced_coherent():

	list_for_each_entry(kvf, &kv->file_list, node) {
		if (!kvm_vfio_file_enforced_coherent(kvf->file)) {
			noncoherent = true;
			break;
		}
	}

	if (noncoherent != kv->noncoherent) {
		kv->noncoherent = noncoherent;

		if (kv->noncoherent)
			kvm_arch_register_noncoherent_dma(dev->kvm);

That helper calls drivers/vfio/vfio_main.c:vfio_file_enforced_coherent(),
which queries IOMMU_CAP_ENFORCE_CACHE_COHERENCY:

	device = vfio_device_from_file(file);
	if (device)
		return device_iommu_capable(device->dev,
					    IOMMU_CAP_ENFORCE_CACHE_COHERENCY);

While VFIO and iommufd already require IOMMU_CAP_CACHE_COHERENCY at device
registration and bind time (__vfio_register_dev() and iommufd_bind_iommu()),
IOMMU_CAP_ENFORCE_CACHE_COHERENCY is much narrower on arm64. In
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:arm_smmu_capable(), it is gated
on arm_smmu_master_canwbs():

	case IOMMU_CAP_CACHE_COHERENCY:
		/* Assume that a coherent TCU implies coherent TBUs */
		return master->smmu->features & ARM_SMMU_FEAT_COHERENCY;
	case IOMMU_CAP_ENFORCE_CACHE_COHERENCY:
		return arm_smmu_master_canwbs(master);

And IOMMU_FWSPEC_PCI_RC_CANWBS is only set in
drivers/acpi/arm64/iort.c:iort_iommu_configure_id() for ACPI PCI root
complexes whose IORT node sets ACPI_IORT_MF_CANWBS:

		if (fwspec && iort_pci_rc_supports_canwbs(node))
			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_CANWBS;

As a result, device_iommu_capable(dev, IOMMU_CAP_ENFORCE_CACHE_COHERENCY)
returns false on Device Tree systems, ACPI systems with IORT tables that only
set CCA=1 (ACPI_IORT_MF_COHERENCY) without ACPI_IORT_MF_CANWBS, non-PCI
platform devices, and systems using other arm64 IOMMU drivers (such as
arm-smmu, virtio-iommu, apple-dart, or qcom_iommu).

On any of those systems with ARM64_HAS_STAGE2_FWB, won't attaching a normal
cache-coherent VFIO device invoke kvm_arch_register_noncoherent_dma() and
immediately terminate the VM via kvm_vm_bugged()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023205610.1044532-1-oupton@kernel.org?part=20