netdev - Re: [PATCH v2 04/15] cxl: add capabilities field to cxl_dev

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: <20240809132514.00003229.zhiw@nvidia.com>
Date: Fri, 9 Aug 2024 13:25:14 +0300
From: Zhi Wang <zhiw@...dia.com>
To: Alejandro Lucero Palau <alucerop@....com>
CC: Dave Jiang <dave.jiang@...el.com>, <alejandro.lucero-palau@....com>,
	<linux-cxl@...r.kernel.org>, <netdev@...r.kernel.org>,
	<dan.j.williams@...el.com>, <martin.habets@...inx.com>,
	<edward.cree@....com>, <davem@...emloft.net>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <edumazet@...gle.com>, <richard.hughes@....com>,
	<targupta@...dia.com>
Subject: Re: [PATCH v2 04/15] cxl: add capabilities field to cxl_dev_state

On Tue, 23 Jul 2024 14:43:24 +0100
Alejandro Lucero Palau <alucerop@....com> wrote:

> 
> On 7/19/24 20:01, Dave Jiang wrote:
> >
> >>   
> >> -static int cxl_probe_regs(struct cxl_register_map *map)
> >> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t
> >> caps) {
> >>   	struct cxl_component_reg_map *comp_map;
> >>   	struct cxl_device_reg_map *dev_map;
> >> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct
> >> cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV:
> >>   		dev_map = &map->device_map;
> >>   		cxl_probe_device_regs(host, base, dev_map);
> >> -		if (!dev_map->status.valid ||
> >> !dev_map->mbox.valid ||
> >> +		if (!dev_map->status.valid ||
> >> +		    ((caps & CXL_DRIVER_CAP_MBOX) &&
> >> !dev_map->mbox.valid) || !dev_map->memdev.valid) {
> >>   			dev_err(host, "registers not found:
> >> %s%s%s\n", !dev_map->status.valid ? "status " : "",
> >> -				!dev_map->mbox.valid ? "mbox " :
> >> "",
> >> +				((caps & CXL_DRIVER_CAP_MBOX) &&
> >> !dev_map->mbox.valid) ? "mbox " : "",
> > According to the r3.1 8.2.8.2.1, the device status registers and
> > the primary mailbox registers are both mandatory if regloc id=3
> > block is found. So if the type2 device does not implement a mailbox
> > then it shouldn't be calling cxl_pci_setup_regs(pdev,
> > CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init
> > right? If the type2 device defines a regblock with id=3 but without
> > a mailbox, then isn't that a spec violation?
> >
> > DJ
> 
> 
> Right. The code needs to support the possibility of a Type2 having a 
> mailbox, and if it is not supported, the rest of the dvsec regs 
> initialization needs to be performed. This is not what the code does 
> now, so I'll fix this.
> 
> 
> A wider explanation is, for the RFC I used a test driver based on
> QEMU emulating a Type2 which had a CXL Device Register Interface
> defined (03h) but not a CXL Device Capability with id 2 for the
> primary mailbox register, breaking the spec as you spotted.
> 
> 

Because SFC driver uses (the 8.2.8.5.1.1 Memory Device Status
Register) to determine if the memory media is ready or not (in PATCH 6).
That register should be in a regloc id=3 block.

According to the spec paste above, the device that has regloc block
id=3 needs to have device status and mailbox.

Curious, does the SFC device have to implement the mailbox in this case
for spec compliance?

Previously, I always think that "CXL Memory Device" == "CXL Type-3
device" in the CXL spec.

Now I am little bit confused if a type-2 device that supports cxl.mem
== "CXL Memory Device" mentioned in the spec.

If the answer == Y, then having regloc id ==3 and mailbox turn
mandatory for a type-2 device that support cxl.mem for the spec
compliance.

If the answer == N, then a type-2 device can use approaches other than
Memory Device Status Register to determine the readiness of the memory?

ZW

> Thanks.
> 
>