linux-kernel - Re: [PATCH RFC 2/2] cxl/memdev: Remove temporary variables from cxl_memdev

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: <679a9abe292a4_210e3e294d@iweiny-mobl.notmuch>
Date: Wed, 29 Jan 2025 15:16:46 -0600
From: Ira Weiny <ira.weiny@...el.com>
To: Alejandro Lucero Palau <alucerop@....com>, Ira Weiny
	<ira.weiny@...el.com>, Dave Jiang <dave.jiang@...el.com>, Dan Williams
	<dan.j.williams@...el.com>, Davidlohr Bueso <dave@...olabs.net>, "Jonathan
 Cameron" <jonathan.cameron@...wei.com>, Alison Schofield
	<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC 2/2] cxl/memdev: Remove temporary variables from
 cxl_memdev_state

Alejandro Lucero Palau wrote:
> 
> <snip>
> 
> >
> >> FWIW, I had (as part of current in-progress v10) similar struct than
> >> used here for Type2 initialization when there is no mailbox.
> >>
> >>
> >> I had added a specific function for initialising that struct, but my
> >> idea now with this change is to have cxl_mem_dev_info initialized by the
> >> driver before calling cxl_dev_state_identify,
> > Why is the accelerator calling cxl_dev_state_identify?  I did not see that
> > in v9.  My idea was that was a mailbox only call which is only needed for
> > memdevs.  And cxl_add_partition() can be called by accelerators as a
> > convenience function to aid in creating cxl_dpa_info.  (This and cxl_test
> > needed that function shared so it just got left in mbox.c)
> 
> 
> No in v9 ... that predates Dan's DPA patches.
> 
> 
> Type2 without an mbox needs to give those values obtained from 
> CXL_MBOX_OP_IDENTIFY. I'm using an struct for passing those values to 
> cxl_dev_state_identify for having same function serving Type2 with and 
> without mbox.

But in the case of no mbox where does the type2 get those values from
within cxl_dev_state_identify()?

The idea with this patch change is to only call cxl_dev_state_identify IFF
the device has a mailbox.  Are you saying the same thing?

> I could have another function instead and calling that one 
> for Type2 with mbox. My idea is to keep similar initialization than 
> Type3 pci driver.

Agreed.  But without an mbox I'm hoping you don't need to call
cxl_dev_state_identify at all if you don't need to query the device...  ie
don't have a mailbox.

> 
> 
> >> and inside that function
> > I'm not clear which 'that function' you are referring to here.
> 
> 
> cxl_dev_state_identify
> 
> 
> >> checking if total_bytes already != 0 for avoiding call the mbox command
> >> for getting the info. This will support both cases for Type2, with and
> >> without mailbox.
> > I think I agree with you except the != 0 to avoid mailbox commands.
> >
> > Unless I am miss-understanding Dan we need to get to a place where mailbox
> > commands stop filling in structures unless those work for both type 2 and
> > 3 __and__ are optional.  Because putting in special checks for the type
> > within a cxl/core/mailbox call is wrong IMO.
> 
> 
> As I said, I can avoid that check with a wrapper for Type2, then only 
> Type2 with mbox and supporting that command (is it mandatory if an 
> mbox?) will end up calling cxl_dev_state_identify.

Even if type 2 has a mailbox the partition information may or may not need
to be in the identify command.

Why force any type 2 to call that mailbox command?

Ira