Date:   Fri, 27 Jan 2023 14:04:06 -0800
From:   Dan Williams <dan.j.williams@intel.com>
To:     Alison Schofield <alison.schofield@intel.com>,
        Dan Williams <dan.j.williams@intel.com>
CC:     Ira Weiny <ira.weiny@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        "Steven Rostedt" <rostedt@goodmis.org>,
        <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re:
Message-ID: <63d44a562a629_3a36e5294a4@dwillia2-xfh.jf.intel.com.notmuch>
References: <cover.1674070170.git.alison.schofield@intel.com>
 <63d32fe783ee2_ea2222949f@dwillia2-xfh.jf.intel.com.notmuch>
 <Y9P3ipL28W7N1bLm@aschofie-mobl2>
 <63d423218e854_3a36e5294fd@dwillia2-xfh.jf.intel.com.notmuch>
 <Y9RDxWwKX+OPl7V1@aschofie-mobl2>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y9RDxWwKX+OPl7V1@aschofie-mobl2>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?XcZPfJtJtiJtiH3LJUSQ26kvgR/LIyUR/ULRgFtLlS7x9x88iAZUj3od/WWv?=
 =?us-ascii?Q?ZXSk/UuiVWk7GINe+MsuUa+1ssZgsn9/e+sAz0Ne+bCxvzUOxzNZZiDEkDcY?=
 =?us-ascii?Q?F6MGqAw69uB7nagQvU+pi6sksYVm0LQJ//wv/Y8yHxm47FU3/DLT48s8DrQW?=
 =?us-ascii?Q?pU/plP460VfmUig74JvOKjY5VIFmGZD+5J36Qd8iZWxgHon2jcg0NH0IgLRJ?=
 =?us-ascii?Q?S0923HOBvvDZQgcLYBfVH/zknouKrjSx/kUcD48WvSmRrBVY8lt85vvouPRp?=
 =?us-ascii?Q?lrSmgR5Hfs5MX+wllWKCmsxvfzFmSFS2ivSYBGSvbwgPLo5kW1N6lfS2ANBQ?=
 =?us-ascii?Q?SAQ+ip+oJG1sZ/sJaXaXPxdv/zk6Er6dEVGKpVW+7YO1wkkvTJFXAsjDkY89?=
 =?us-ascii?Q?O4A0fTObD7bfn+ZyBg/3p6CHs+TrXG61XmuGPAw5Y0yL2qE/FBFDo1PkroD0?=
 =?us-ascii?Q?e8m4bLV6X6AtZUmzwkzCgUdfM7xFByOqsAsPlJ0Vp7yNgvS+di7ftXdeYosA?=
 =?us-ascii?Q?/jmKVlRivsQO2sDRhV1SPCoE4evvTRodCQR7WKVrLP9QmcQGfRPqU+DdeBZD?=
 =?us-ascii?Q?t51vadAVf+4bF7ilRy/jADfmP9CuJdCDk64JbbZHrCVZ6jumZeMnKhi+acse?=
 =?us-ascii?Q?6XitQq5pAksCppxgTgelXqohXrDKxn5oJ4McMtsCXbj6U5f7AMkTboyvO9Ah?=
 =?us-ascii?Q?BvKB0k9aKZtBGS5kIm8fJDMicJ5kuTR4UyE53uvMIil3GY0YT12UEGkfdpEj?=
 =?us-ascii?Q?loYq26Zt1pny3FlsrBzVpmGRp7wHvNe2tWTyC8j+ey1FeCAXGtRWs7SmOQP3?=
 =?us-ascii?Q?HYJkOjeaXMGTpJlEGWpDSleWn7k7pJzppiyvbvecHYfV+EUDujRM2bqbTgij?=
 =?us-ascii?Q?4Ian2w6WM2CZdFZD4nLZRSbZ4EtmMfaeeZjooQi5x8kddWVU4syYfTaTuNU4?=
 =?us-ascii?Q?cMUS58Wk6Atz8fzh3z30p7GP9f5yuwHrbeXcJXed01sUKAhC85yADX3TBGEY?=
 =?us-ascii?Q?cLXhgLVFjkoi2XErcPi190MdnbpYDTyT6Pw7rwXnP21+syx1tXbeyqxzI7Sg?=
 =?us-ascii?Q?1eM9UjOx8eRi2hi+Op7+/j9r8Q+xqwifb/DpVVZ8tYx/lw4clnW/vU03yKWF?=
 =?us-ascii?Q?p4tjjzmeFdTmLMVY/xT6f96VqqM/Ua7i6HM14jHrxB25c1t+eKkMA68SHObR?=
 =?us-ascii?Q?XvtP4npBJJcTpbT+/K+Rye/auTenv8gzOSnVzqDWlcE6iLZQGVB4sid/wcqi?=
 =?us-ascii?Q?Nj+M/DT019+xqwwiLj3n+NVanT8UgEV+SYs95Capnm5acpAyG0QzzxobWkJB?=
 =?us-ascii?Q?b0bhSRkSqwwiEhndNr7mCZ8yWxg0LEfs0+R2BDKPGPXBWsPYmo2TLBq+4cIg?=
 =?us-ascii?Q?WpL8+epmpH0wQTMqufAG5j5jyqOYObi7nACuDi1gW35w0AJRx7X5HrSxkmc6?=
 =?us-ascii?Q?R7EpmnC8fXeKr9x/92Chs5LW/tQ4DImWKrY2nR0u/vEyeLtHFCoEy+dh20cT?=
 =?us-ascii?Q?Q31lTIY13ZYQTVeADqcgTpTnh4UQtIWZJlS1VasAuYGNX7bd21FESKb4NCWR?=
 =?us-ascii?Q?+OlE9DAgj3Gkh6rabbY1FgaA6WHztUe2+2o9MX6y4MErlOxfGWK+ZXvSwHBl?=
 =?us-ascii?Q?Gw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3377c3c4-8e54-4ca1-8059-08db00b269d0
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2023 22:04:08.4226
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3RSidCcnG15o0drWIOM7VoSwbYs6Pvsk7Fo+Z2TsUDo0ayIJv1+3+QWmK5uwlxfGp9LMsE6B1Tqx5r4H7zjBDQxb9e/U6/J2OykaBRrfDLk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8134
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Alison Schofield wrote:
> On Fri, Jan 27, 2023 at 11:16:49AM -0800, Dan Williams wrote:
> > Alison Schofield wrote:
> > > On Thu, Jan 26, 2023 at 05:59:03PM -0800, Dan Williams wrote:
> > > > alison.schofield@ wrote:
> > > > > From: Alison Schofield <alison.schofield@intel.com>
> > > > > 
> > > > > Subject: [PATCH v5 0/5] CXL Poison List Retrieval & Tracing
> > > > > 
> > > > > Changes in v5:
> > > > > - Rebase on cxl/next 
> > > > > - Use struct_size() to calc mbox cmd payload .min_out
> > > > > - s/INTERNAL/INJECTED mocked poison record source
> > > > > - Added Jonathan Reviewed-by tag on Patch 3
> > > > > 
> > > > > Link to v4:
> > > > > https://lore.kernel.org/linux-cxl/cover.1671135967.git.alison.schofield@intel.com/
> > > > > 
> > > > > Add support for retrieving device poison lists and store the returned
> > > > > error records as kernel trace events.
> > > > > 
> > > > > The handling of the poison list is guided by the CXL 3.0 Specification
> > > > > Section 8.2.9.8.4.1. [1] 
> > > > > 
> > > > > Example, triggered by memdev:
> > > > > $ echo 1 > /sys/bus/cxl/devices/mem3/trigger_poison_list
> > > > > cxl_poison: memdev=mem3 pcidev=cxl_mem.3 region= region_uuid=00000000-0000-0000-0000-000000000000 dpa=0x0 length=0x40 source=Internal flags= overflow_time=0
> > > > 
> > > > I think the pcidev= field wants to be called something like "host" or
> > > > "parent", because there is no strict requirement that a 'struct
> > > > cxl_memdev' is related to a 'struct pci_dev'. In fact in that example
> > > > "cxl_mem.3" is a 'struct platform_device'. Now that I think about it, I
> > > > think all CXL device events should be emitting the PCIe serial number
> > > > for the memdev.
> > > ]
> > > 
> > > Will do, 'host' and add PCIe serial no.
> > > 
> > > > 
> > > > I will look in the implementation, but do region= and region_uuid= get
> > > > populated when mem3 is a member of the region?
> > > 
> > > Not always.
> > > In the case above, where the trigger was by memdev, no.
> > > Region= and region_uuid= (and in the follow-on patch, hpa=) only get
> > > populated if the poison was triggered by region, like the case below.
> > > 
> > > It could be looked up for the by memdev cases. Is that wanted?
> > 
> > Just trying to understand the semantics. However, I do think it makes sense
> > for a memdev trigger to lookup information on all impacted regions
> > across all of the device's DPA and the region trigger makes sense to
> > lookup all memdevs, but bounded by the DPA that contributes to that
> > region. I just want to avoid someone having to trigger the region to get
> > extra information that was readily available from a memdev listing.
> > 
> 
> Dan - 
> 
> Confirming my take-away from this email, and our chat:
> 
> Remove the by-region trigger_poison_list option entirely. User space
> needs to trigger by-memdev the memdevs participating in the region and
> filter those events by region.
> 
> Add the region info (region name, uuid) to the TRACE_EVENTs when the
> poisoned DPA is part of any region.

That's what I was thinking, yes. So the internals of
cxl_mem_get_poison() will take the cxl_region_rwsem for read and compare
the device's endpoint decoder settings against the media error records
to do the region (and later HPA) lookup.