Messages in this thread | | | Date | Thu, 17 Nov 2022 15:55:15 -0800 | From | Alison Schofield <> | Subject | Re: [PATCH v3 2/6] cxl/mbox: Add GET_POISON_LIST mailbox command |
| |
On Wed, Nov 16, 2022 at 12:41:18PM +0000, Jonathan Cameron wrote: > On Thu, 10 Nov 2022 19:12:40 -0800 > alison.schofield@intel.com wrote: > > > From: Alison Schofield <alison.schofield@intel.com> > > > > CXL devices maintain a list of locations that are poisoned or result > > in poison if the addresses are accessed by the host. > > > > Per the spec (CXL 3.0 8.2.9.8.4.1), the device returns this Poison > > list as a set of Media Error Records that include the source of the > > error, the starting device physical address and length. The length is > > the number of adjacent DPAs in the record and is in units of 64 bytes. > > > > Retrieve the list and log each Media Error Record as a trace event of > > type 'cxl_poison'. > > > > When the poison list is requested by region, include the region name > > and uuid in the trace event. > > > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > Hi Alison, > > I've forgotten most of previous discussions around versions of this series > so I may well repeat things that were covered earlier! > > A few things inline. > > Thanks, > > Jonathan Thanks Jonathan. Got 'em all. > > > > --- snip > > > > > > + memcpy(&val, id.poison_list_max_mer, 3); > > This is ugly. I've lost track of last discussion about get_unaligned_le24() > and using it on elements of a packed structure. At very least can we > do a memcpy to a u8[3] array and then use get_unaligned_le24() on that if > we can't use it directly on the structure element? >
Done, like this - val = get_unaligned_le24(id.poison_list_max_mer);
> snip > > + > > +int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len, > > + struct cxl_region *cxlr) > > +{ > > + struct cxl_dev_state *cxlds = cxlmd->cxlds; > > + const char *memdev_name = dev_name(&cxlmd->dev); > Could just do this where it's used rather than here. > > > + const char *pcidev_name = dev_name(cxlds->dev); > Same with this. >
Done.
> snip > ... > >
| |