linux-kernel - Re: [PATCH 0/3] cxl: Preparation of type2 accelerators support

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: <8734nrtmjy.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Tue, 30 Jul 2024 14:34:57 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Alejandro Lucero Palau <alucerop@....com>
Cc: Dan Williams <dan.j.williams@...el.com>,  Dave Jiang
 <dave.jiang@...el.com>,  <linux-cxl@...r.kernel.org>,
  <linux-kernel@...r.kernel.org>,  "Davidlohr Bueso" <dave@...olabs.net>,
  Jonathan Cameron <jonathan.cameron@...wei.com>,  Alison Schofield
 <alison.schofield@...el.com>,  Vishal Verma <vishal.l.verma@...el.com>,
  Ira Weiny <ira.weiny@...el.com>
Subject: Re: [PATCH 0/3] cxl: Preparation of type2 accelerators support

Hi, Alejandro,

Alejandro Lucero Palau <alucerop@....com> writes:

> Hi,
>
>
> I'm a bit "surprised" by this patchset. As you rightly say in this
> cover letter, there is a patchset under review, and I have not seen
> you commenting there on the concerns stated in this cover letter.
>
>
> I would say that is the first thing you should do, at least, to
> comment there, suggest to do other way, pointing to other needed
> changes, and if things do not go well for reaching an agreement, then
> a patchset like this could make sense exposing another way.

Sorry for confusing.  The patchset is just some preparation work for
type2 accelerator support.  Your patchset is more complete.  I just want
to relief your overhead a bit.

> Anyway, I think a CXL maintainer should say something about it, but
> after 24hours, I had to say something.
>
>
> About testing these changes, I wonder how did you proceed. If you have
> used an emulated Type2 device, as I did with the first patchset
> version, you should trigger some of the problems I found, what makes
> any Type2 initialization for getting a memdev and finally a cxl region
> to fail. In other words, testing these changes can not be a partly
> initialised Type2 but a complete one. This does not mean a patch
> fixing a known and obvious issue for supporting Type2 should not be
> approved if not tested, since the Type2 support is not yet there, but
> mentioning testing makes things confusing, at least for me.

Yes.  To test this patchset, I need more complete type2 support.  I used
some patches from your and Dan's patchset on top of this patchset to do
the test.

>
> On 7/29/24 09:46, Huang Ying wrote:
>> There have been 2 series to add type 2 accelerators support in [1] and [2].
>>
>> [1] https://lore.kernel.org/linux-cxl/168592149709.1948938.8663425987110396027.stgit@dwillia2-xfh.jf.intel.com/
>> [2] https://lore.kernel.org/linux-cxl/20240516081202.27023-1-alucerop@amd.com/
>>
>> Both provide relative complete support, but both are long too.  To
>> make it easier to review, some preparation of type2 accelerators
>> support is implemented in this series.  More complete support can be
>> implemented based on it.
>>
>> This series has been tested with cxl_test via mocking a type2
>> accelerator as in [1] above.  Because more type2 accelerators support
>> than that provided by this series is needed to simulate the device,
>> the cxl_test patch isn't included in the series.
>>
>> Huang Ying (3):
>>    cxl: Set target type of root decoder based on CFMWS restrictions
>>    cxl: Set target type of region with that of root decoder
>>    cxl: Avoid to create dax regions for type2 accelerators
>>
>>   drivers/cxl/acpi.c        |  5 ++++-
>>   drivers/cxl/core/region.c | 11 ++++++++++-
>>   2 files changed, 14 insertions(+), 2 deletions(-)

--
Best Regards,
Huang, Ying