Re: [PATCH V2 3/7] iommu/vt-d: Support Enhanced Command Interface
From: Liang, Kan
Date: Thu Jan 19 2023 - 08:25:03 EST
On 2023-01-19 3:55 a.m., Baolu Lu wrote:
> On 2023/1/19 4:50, kan.liang@xxxxxxxxxxxxxxx wrote:
>> +#ifdef CONFIG_INTEL_IOMMU
>> +#define ecmd_get_status_code(res) ((res & 0xff) >> 1)
>> +
>> +/*
>> + * Function to submit a command to the enhanced command interface. The
>> + * valid enhanced command descriptions are defined in Table 47 of the
>> + * VT-d spec. The VT-d hardware implementation may support some but not
>> + * all commands, which can be determined by checking the Enhanced
>> + * Command Capability Register.
>> + *
>> + * Return values:
>> + * - 0: Command successful without any error;
>> + * - Negative: software error value;
>> + * - Nonzero positive: failure status code defined in Table 48.
>> + */
>> +int ecmd_submit_sync(struct intel_iommu *iommu, u8 ecmd, u64 oa, u64 ob)
>> +{
>> + unsigned long flags;
>> + u64 res;
>> + int ret;
>> +
>> + if (!cap_ecmds(iommu->cap))
>> + return -ENODEV;
>> +
>> + raw_spin_lock_irqsave(&iommu->register_lock, flags);
>> +
>> + res = dmar_readq(iommu->reg + DMAR_ECRSP_REG);
>> + if (res & DMA_ECMD_ECRSP_IP) {
>> + ret = -EBUSY;
>> + goto err;
>> + }
>> +
>> + /*
>> + * Unconditionally write the operand B, because
>> + * - There is no side effect if an ecmd doesn't require an
>> + * operand B, but we set the register to some value.
>> + * - It's not invoked in any critical path. The extra MMIO
>> + * write doesn't bring any performance concerns.
>> + */
>> + dmar_writeq(iommu->reg + DMAR_ECEO_REG, ob);
>> + dmar_writeq(iommu->reg + DMAR_ECMD_REG, ecmd | (oa <<
>> DMA_ECMD_OA_SHIFT));
>> +
>> + IOMMU_WAIT_OP(iommu, DMAR_ECRSP_REG, dmar_readq,
>> + !(res & DMA_ECMD_ECRSP_IP), res);
>> +
>> + if (res & DMA_ECMD_ECRSP_IP) {
>> + ret = -ETIMEDOUT;
>> + goto err;
>> + }
>> +
>> + ret = ecmd_get_status_code(res);
>> +err:
>> + raw_spin_unlock_irqrestore(&iommu->register_lock, flags);
>> +
>> + return ret;
>> +}
>> +#endif /* CONFIG_INTEL_IOMMU */
>
> Can we remove the "#ifdef CONFIG_INTEL_IOMMU"?
In dmar.c, no, there will be a compiler warning when the
CONFIG_INTEL_IOMMU is not set.
> Or if this is currently
> only intel-iommu specific, how about moving it to drivers/iommu/intel
> /iommu.c?
>
Yes, it should OK to move it to iommu.c to avoid the "#ifdef
CONFIG_INTEL_IOMMU". Now, it's intel-iommu specific.
Thanks,
Kan