linux-kernel - Re: [PATCH v4 3/4] fpga: dfl: add basic support DFHv1

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]

Date:   Tue, 1 Nov 2022 15:37:19 -0700 (PDT)
From:   matthew.gerlach@...ux.intel.com
To:     Xu Yilun <yilun.xu@...el.com>
cc:     hao.wu@...el.com, russell.h.weight@...el.com,
        basheer.ahmed.muddebihal@...el.com, trix@...hat.com,
        mdf@...nel.org, linux-fpga@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        tianfei.zhang@...el.com, corbet@....net, jirislaby@...nel.org,
        geert+renesas@...der.be, andriy.shevchenko@...ux.intel.com,
        niklas.soderlund+renesas@...natech.se, macro@...am.me.uk,
        johan@...nel.org, lukas@...ner.de, ilpo.jarvinen@...ux.intel.com,
        marpagan@...hat.com
Subject: Re: [PATCH v4 3/4] fpga: dfl: add basic support DFHv1



On Sat, 29 Oct 2022, matthew.gerlach@...ux.intel.com wrote:

>
>>
>>>  	if (!finfo)
>>>  		return -ENOMEM;
>>> 
>>> +	if (dfh_psize > 0) {
>>> +		memcpy_fromio(finfo->params,
>>> +			      binfo->ioaddr + ofst + DFHv1_PARAM_HDR, 
>>> dfh_psize);
>>> +		finfo->param_size = dfh_psize;
>>> +	}
>>> +
>>>  	finfo->fid = fid;
>>>  	finfo->revision = revision;
>>> -	finfo->mmio_res.start = binfo->start + ofst;
>>> -	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>>> +	finfo->dfh_version = dfh_ver;
>>>  	finfo->mmio_res.flags = IORESOURCE_MEM;
>>> -	finfo->irq_base = irq_base;
>>> -	finfo->nr_irqs = nr_irqs;
>>> +	if (dfh_ver == 1) {
>>> +		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_ADDR);
>>> +		if (v & DFHv1_CSR_ADDR_REL)
>>> +			finfo->mmio_res.start = v & ~DFHv1_CSR_ADDR_REL;
>>> +		else
>>> +			finfo->mmio_res.start = binfo->start + ofst +
>>> +					       FIELD_GET(DFHv1_CSR_ADDR_MASK, 
>>> v);
>>> +
>>> +		v = readq(binfo->ioaddr + ofst + DFHv1_CSR_SIZE_GRP);
>>> +		finfo->mmio_res.end = finfo->mmio_res.start +
>>> +				      FIELD_GET(DFHv1_CSR_SIZE_GRP_SIZE, v) - 
>>> 1;
>> 
>> So for dflv1, no feature header resource for dfl_device, is it a problem
>> for dfl_uio? Does userspace driver need the raw feature header?
> These are two very good questions.  The dfl_uio driver question is 
> particularly relevent because user space is looking at the GUIDs.
>

In the case of dfl_uio driver, user space will definitely want to look at 
the feature header for the GUID and the parameters.  Since DFHv1 can have 
the DFH header and the feature registers in non-contiguous memory 
locations, a resource for the dfl_device will be required.  In earlier
revisions of this patch set, a second resource was added called csr_res 
pointing to the feature's register while mmio_res pointed at the header.
Do we just need better names or do we need an array of named resources?

>> 
>>> +	} else {
>>> +		finfo->mmio_res.start = binfo->start + ofst;
>>> +		finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>>> +	}
>>> +
>>> +	ret = parse_feature_irqs(binfo, ofst, finfo);
>>> +	if (ret) {
>>> +		kfree(finfo);
>>> +		return ret;
>>> +	}
>>>