linux-kernel - Re: [PATCH V2 3/7] iommu/vt-d: Support Enhanced Command Interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b9619bee-7f5c-81db-710d-20550ea01517@linux.intel.com>
Date:   Thu, 19 Jan 2023 08:24:25 -0500
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>, joro@...tes.org,
        will@...nel.org, dwmw2@...radead.org, robin.murphy@....com,
        robert.moore@...el.com, rafael.j.wysocki@...el.com,
        lenb@...nel.org, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 3/7] iommu/vt-d: Support Enhanced Command Interface



On 2023-01-19 3:55 a.m., Baolu Lu wrote:
> On 2023/1/19 4:50, kan.liang@...ux.intel.com 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