linux-kernel - Re: [PATCH 1/1] iommu/vt-d: Tweak the description of a DMA fault

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]

Date:   Thu, 13 May 2021 11:13:21 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     "Raj, Ashok" <ashok.raj@...el.com>
Cc:     baolu.lu@...ux.intel.com, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>, kevin.tian@...el.com,
        jacob.jun.pan@...el.com, yi.l.liu@...el.com,
        sanjay.k.kumar@...el.com, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] iommu/vt-d: Tweak the description of a DMA fault

On 5/13/21 12:56 AM, Raj, Ashok wrote:
> On Wed, May 12, 2021 at 02:50:12PM +0800, Lu Baolu wrote:
>> The Intel IOMMU driver reports the DMA fault reason in a decimal number
>> while the VT-d specification uses a hexadecimal one. It's inconvenient
>> that users need to covert them everytime before consulting the spec.
>> Let's use hexadecimal number for a DMA fault reason.
>>
>> The fault message uses 0xffffffff as PASID for DMA requests w/o PASID.
>> This is confusing. Tweak this by adding "w/o PASID" explicitly.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
> 
> Maybe simpler to call it NO_PASID, and just PASID 0xxxxx instead?

Yeah, it's okay for me.

> 
> with the minor suggestions below
> 
> Reviewed-by: Ashok Raj <ashok.raj@...el.com>

Thanks!

> 
>> ---
>>   drivers/iommu/intel/dmar.c | 22 ++++++++++++++--------
>>   1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>> index 1757ac1e1623..11e37d2c2af2 100644
>> --- a/drivers/iommu/intel/dmar.c
>> +++ b/drivers/iommu/intel/dmar.c
>> @@ -1911,15 +1911,21 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
>>   	reason = dmar_get_fault_reason(fault_reason, &fault_type);
>>   
>>   	if (fault_type == INTR_REMAP)
>> -		pr_err("[INTR-REMAP] Request device [%02x:%02x.%d] fault index %llx [fault reason %02d] %s\n",
>> -			source_id >> 8, PCI_SLOT(source_id & 0xFF),
>> -			PCI_FUNC(source_id & 0xFF), addr >> 48,
>> -			fault_reason, reason);
>> -	else
>> -		pr_err("[%s] Request device [%02x:%02x.%d] PASID %x fault addr %llx [fault reason %02d] %s\n",
>> +		pr_err("[INTR-REMAP] Request device [%02x:%02x.%d] fault index %llx [fault reason %02xh] %s\n",
>> +		       source_id >> 8, PCI_SLOT(source_id & 0xFF),
>> +		       PCI_FUNC(source_id & 0xFF), addr >> 48,
>> +		       fault_reason, reason);
>> +	else if (pasid == INVALID_IOASID)
>> +		pr_err("[%s w/o PASID] Request device [%02x:%02x.%d] fault addr %llx [fault reason %02xh] %s\n",
>>   		       type ? "DMA Read" : "DMA Write",
>>   		       source_id >> 8, PCI_SLOT(source_id & 0xFF),
>> -		       PCI_FUNC(source_id & 0xFF), pasid, addr,
>> +		       PCI_FUNC(source_id & 0xFF), addr,
>> +		       fault_reason, reason);
>> +	else
>> +		pr_err("[%s w/ PASID %x] Request device [%02x:%02x.%d] fault addr %llx [fault reason %02xh] %s\n",
> 
> Can you always lead hex values with 0x?

Yes.

> 
>> +		       type ? "DMA Read" : "DMA Write", pasid,
>> +		       source_id >> 8, PCI_SLOT(source_id & 0xFF),
>> +		       PCI_FUNC(source_id & 0xFF), addr,
>>   		       fault_reason, reason);
>>   	return 0;
>>   }
>> @@ -1987,7 +1993,7 @@ irqreturn_t dmar_fault(int irq, void *dev_id)
>>   		if (!ratelimited)
>>   			/* Using pasid -1 if pasid is not present */
>>   			dmar_fault_do_one(iommu, type, fault_reason,
>> -					  pasid_present ? pasid : -1,
>> +					  pasid_present ? pasid : INVALID_IOASID,
>>   					  source_id, guest_addr);
>>   
>>   		fault_index++;
>> -- 
>> 2.25.1
>>
> 

Best regards,
baolu