| 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
| ||
|
Message-ID: <dd035701-62f0-0a68-ed4d-5347d26753f8@linux.intel.com>
Date: Fri, 13 Jan 2023 13:19:59 -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 3/7] iommu/vt-d: Support Enhanced Command Interface
On 2023-01-13 8:55 a.m., Baolu Lu wrote:
> On 2023/1/12 4:25, kan.liang@...ux.intel.com wrote:
>> From: Kan Liang <kan.liang@...ux.intel.com>
>>
>> The Enhanced Command Register is to submit command and operand of
>> enhanced commands to DMA Remapping hardware. It can supports upto 256
>> enhanced commands.
>>
>> There is a HW register to indicate the availability of all 256 enhanced
>> commands. Each bit stands for each command. But there isn't an existing
>> interface to read/write all 256 bits. Introduce the u64 ecmdcap[4] to
>> store the existence of each enhanced command. Read 4 times to get
>> all of them in map_iommu().
>>
>> Add a helper to facilitate an enhanced command launch. Make sure hardware
>> complete the command.
>>
>> Add a helper to facilitate the check of PMU essentials.
>>
>> The helpers will be used later.
>>
>> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
>> ---
>> drivers/iommu/intel/dmar.c | 63 +++++++++++++++++++++++++++++++++++++
>> drivers/iommu/intel/iommu.h | 46 +++++++++++++++++++++++++++
>> 2 files changed, 109 insertions(+)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>> index 91bb48267df2..dcafa32875c1 100644
>> --- a/drivers/iommu/intel/dmar.c
>> +++ b/drivers/iommu/intel/dmar.c
>> @@ -1025,6 +1025,16 @@ static int map_iommu(struct intel_iommu *iommu,
>> struct dmar_drhd_unit *drhd)
>> goto release;
>> }
>> }
>> +
>> + if (cap_ecmds(iommu->cap)) {
>> + int i;
>> +
>> + for (i = 0; i < DMA_MAX_NUM_ECMDCAP; i++) {
>> + iommu->ecmdcap[i] = dmar_readq(iommu->reg + DMAR_ECCAP_REG +
>> + i * DMA_ECMD_REG_STEP);
>> + }
>> + }
>> +
>> err = 0;
>> goto out;
>> @@ -2434,3 +2444,56 @@ bool dmar_platform_optin(void)
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(dmar_platform_optin);
>> +
>> +#ifdef CONFIG_INTEL_IOMMU
>
> Why do you need above #ifdef?
>
>> +#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, bool has_ob, 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;
>> + }
>> +
>> + if (has_ob)
>> + dmar_writeq(iommu->reg + DMAR_ECEO_REG, ob);
>
> The ecmds that require a Operand B are statically defined in the spec,
> right? What will it look like if we define a static ignore_ob(ecmd)?
>
>> + 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 */
>> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
>> index 5bcefbea55c9..f227107434ac 100644
>> --- a/drivers/iommu/intel/iommu.h
>> +++ b/drivers/iommu/intel/iommu.h
>> @@ -130,6 +130,10 @@
>> #define DMAR_PERFOVFOFF_REG 0x318
>> #define DMAR_PERFCNTROFF_REG 0x31c
>> #define DMAR_PERFEVNTCAP_REG 0x380
>> +#define DMAR_ECMD_REG 0x400
>> +#define DMAR_ECEO_REG 0x408
>> +#define DMAR_ECRSP_REG 0x410
>> +#define DMAR_ECCAP_REG 0x430
>> #define DMAR_VCCAP_REG 0xe30 /* Virtual command capability
>> register */
>> #define DMAR_VCMD_REG 0xe00 /* Virtual command register */
>> #define DMAR_VCRSP_REG 0xe10 /* Virtual command response
>> register */
>> @@ -304,6 +308,26 @@
>> #define DMA_CCMD_SID(s) (((u64)((s) & 0xffff)) << 16)
>> #define DMA_CCMD_DID(d) ((u64)((d) & 0xffff))
>> +/* ECMD_REG */
>> +#define DMA_MAX_NUM_ECMD 256
>> +#define DMA_MAX_NUM_ECMDCAP (DMA_MAX_NUM_ECMD / 64)
>> +#define DMA_ECMD_REG_STEP 8
>> +#define DMA_ECMD_ENABLE 0xf0
>> +#define DMA_ECMD_DISABLE 0xf1
>> +#define DMA_ECMD_FREEZE 0xf4
>> +#define DMA_ECMD_UNFREEZE 0xf5
>> +#define DMA_ECMD_OA_SHIFT 16
>> +#define DMA_ECMD_ECRSP_IP 0x1
>> +#define DMA_ECMD_ECCAP3 3
>> +#define DMA_ECMD_ECCAP3_ECNTS (1ULL << 48)
>> +#define DMA_ECMD_ECCAP3_DCNTS (1ULL << 49)
>> +#define DMA_ECMD_ECCAP3_FCNTS (1ULL << 52)
>> +#define DMA_ECMD_ECCAP3_UFCNTS (1ULL << 53)
>> +#define DMA_ECMD_ECCAP3_ESSENTIAL (DMA_ECMD_ECCAP3_ECNTS | \
>> + DMA_ECMD_ECCAP3_DCNTS | \
>> + DMA_ECMD_ECCAP3_FCNTS | \
>> + DMA_ECMD_ECCAP3_UFCNTS)
>> +
>> /* FECTL_REG */
>> #define DMA_FECTL_IM (((u32)1) << 31)
>> @@ -600,6 +624,7 @@ struct intel_iommu {
>> u64 cap;
>> u64 ecap;
>> u64 vccap;
>> + u64 ecmdcap[DMA_MAX_NUM_ECMDCAP];
>> u32 gcmd; /* Holds TE, EAFL. Don't need SRTP, SFL, WBF */
>> raw_spinlock_t register_lock; /* protect register handling */
>> int seq_id; /* sequence id of the iommu */
>> @@ -841,6 +866,15 @@ extern const struct iommu_ops intel_iommu_ops;
>> extern int intel_iommu_sm;
>> extern int iommu_calculate_agaw(struct intel_iommu *iommu);
>> extern int iommu_calculate_max_sagaw(struct intel_iommu *iommu);
>> +extern int ecmd_submit_sync(struct intel_iommu *iommu, u8 ecmd,
>> + u64 oa, bool has_ob, u64 ob);
>> +
>> +static inline bool ecmd_has_pmu_essential(struct intel_iommu *iommu)
>> +{
>> + return (iommu->ecmdcap[DMA_ECMD_ECCAP3] &
>> DMA_ECMD_ECCAP3_ESSENTIAL) ==
>> + DMA_ECMD_ECCAP3_ESSENTIAL;
>> +}
>> +
>> extern int dmar_disabled;
>> extern int intel_iommu_enabled;
>> #else
>> @@ -852,6 +886,18 @@ static inline int
>> iommu_calculate_max_sagaw(struct intel_iommu *iommu)
>> {
>> return 0;
>> }
>> +
>> +static inline int ecmd_submit_sync(struct intel_iommu *iommu, u8 ecmd,
>> + u64 oa, bool has_ob, u64 ob)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static inline bool ecmd_has_pmu_essential(struct intel_iommu *iommu)
>> +{
>> + return false;
>> +}
>
> Is there any need to call these helpers when INTEL_IOMMU is not
> configured?
No, I will remove them from non-INTEL_IOMMU.
Thanks,
Kan
>
>> +
>> #define dmar_disabled (1)
>> #define intel_iommu_enabled (0)
>> #define intel_iommu_sm (0)
>
> --
> Best regards,
> baolu
Powered by blists - more mailing lists