linux-kernel - Re: [PATCH v2 2/6] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <3644ad6e-d800-c84b-9d62-6dda8462450f@hisilicon.com>
Date:   Tue, 16 Nov 2021 19:37:20 +0800
From:   Yicong Yang <yangyicong@...ilicon.com>
To:     Robin Murphy <robin.murphy@....com>, <gregkh@...uxfoundation.org>,
        <helgaas@...nel.org>, <alexander.shishkin@...ux.intel.com>,
        <lorenzo.pieralisi@....com>, <will@...nel.org>,
        <mark.rutland@....com>, <mathieu.poirier@...aro.org>,
        <suzuki.poulose@....com>, <mike.leach@...aro.org>,
        <leo.yan@...aro.org>, <jonathan.cameron@...wei.com>,
        <daniel.thompson@...aro.org>, <joro@...tes.org>,
        <john.garry@...wei.com>, <shameerali.kolothum.thodi@...wei.com>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <coresight@...ts.linaro.org>, <linux-pci@...r.kernel.org>,
        <linux-perf-users@...r.kernel.org>,
        <iommu@...ts.linux-foundation.org>
CC:     <yangyicong@...ilicon.com>, <prime.zeng@...wei.com>,
        <linuxarm@...wei.com>, <zhangshaokun@...ilicon.com>,
        <liuqi115@...wei.com>
Subject: Re: [PATCH v2 2/6] hwtracing: Add trace function support for
 HiSilicon PCIe Tune and Trace device

On 2021/11/16 18:56, Robin Murphy wrote:
> On 2021-11-16 09:06, Yicong Yang via iommu wrote:
> [...]
>> +/*
>> + * Get RMR address if provided by the firmware.
>> + * Return 0 if the IOMMU doesn't present or the policy of the
>> + * IOMMU domain is passthrough or we get a usable RMR region.
>> + * Otherwise a negative value is returned.
>> + */
>> +static int hisi_ptt_get_rmr(struct hisi_ptt *hisi_ptt)
>> +{
>> +    struct pci_dev *pdev = hisi_ptt->pdev;
>> +    struct iommu_domain *iommu_domain;
>> +    struct iommu_resv_region *region;
>> +    LIST_HEAD(list);
>> +
>> +    /*
>> +     * Use direct DMA if IOMMU does not present or the policy of the
>> +     * IOMMU domain is passthrough.
>> +     */
>> +    iommu_domain = iommu_get_domain_for_dev(&pdev->dev);
>> +    if (!iommu_domain || iommu_domain->type == IOMMU_DOMAIN_IDENTITY)
>> +        return 0;
>> +
>> +    iommu_get_resv_regions(&pdev->dev, &list);
>> +    list_for_each_entry(region, &list, list)
>> +        if (region->type == IOMMU_RESV_DIRECT &&
>> +            region->length >= HISI_PTT_TRACE_BUFFER_SIZE) {
>> +            hisi_ptt->trace_ctrl.has_rmr = true;
>> +            hisi_ptt->trace_ctrl.rmr_addr = region->start;
>> +            hisi_ptt->trace_ctrl.rmr_length = region->length;
>> +            break;
>> +        }
>> +
>> +    iommu_put_resv_regions(&pdev->dev, &list);
>> +    return hisi_ptt->trace_ctrl.has_rmr ? 0 : -ENOMEM;
>> +}
> 
> No.
> 
> The whole point of RMRs is for devices that are already configured to access the given address range in a manner beyond the kernel's control. If you can do this, it proves that you should not have an RMR in the first place.
> 
> The notion of a kernel driver explicitly configuring its device to DMA into any random RMR that looks big enough is so egregiously wrong that I'm almost lost for words...
> 

our bios will reserve such a region and reported it through iort. the device will write to the region and in the driver we need to access the region
to get the traced data. the region is reserved exclusively and will not be accessed by kernel or other devices.

is it ok to let bios configure the address to the device and from CPU side we just read it?

Thanks,
Yicong