Re: [PATCH v3 0/4] XOR Math Fixups: translation & position

[Dan Williams <dan.j.williams@intel.com>]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> Changes in v3:
> - Patch 2: Perform the 'chunk' check on Modulo decodes only
> - Patch 1: Fold cxl_translate() into cxl_dpa_to_hpa()	(Jonathan)
>   Jonathan asked for a rename of cxl_translate to cxl_dpa_to_hpa()
>   but the latter already existed and the work of cxl_translate() was
>   minimal. They are now one.
> - Remove the mention of XOR's purpose in Patch 2 commit log (Dan)
> - Reword hamming weight wrt XORALLBITS code comment (Jonathan)
> - Post a unit test upstream[1]  (Dan, Jonathan)
> - Remove Reviewed-by Tags on Patch 1 & 2 due to rework
> - Add Diego's Tested-by tag to Patch 2,3
> 
> Link to v2:
> https://lore.kernel.org/cover.1714159486.git.alison.schofield@intel.com/
> 
> [1] https://lore.kernel.org/20240624210644.495563-1-alison.schofield@intel.com/
> 
> 
> Begin cover letter:
> 
> Rather than repeat the individual patch commit message content,
> let me describe the flow of this set:

For a future cover letter, a lead in like this does not capture
potential new reviewers compared to a boilerplate flow of:

"Kernel fails at X... this is bad because... here is the flow of the
patches to get the kernel back on track..."

> 
> Patch 1: Rename an existing fn - cxl_trace_hpa()-> cxl_dpa_to_hpa()
> A tiny, yet essential cleanup to take first.
> 
> Patch 2: cxl: Restore XOR'd position bits during address translation
> The problem fixed in this patch, bad HPA translations with XOR math,
> came to my attention recently. 
> 
> Patch 3 & Patch 4 are paired. Patch 3 presents the new method for
> verifying a target position in the list and Patch 4 removes the
> old method. These could be squashed.
> 
> FYI - the reason I don't present the code removal first is because
> I think it is easier to read the diff if I leave in the old root
> decoder call back setup for calc_hb, insert the new call back along
> the same path, and then rip out the defunct calc_hb. That's the
> way I created the patchset and it may be an easier way for reviewers
> to follow along with the root decoder callback setup.

If I did not say it before, I appreciate notes like this, and agree this
makes the patches easier to read.