| Message ID | 20231117-fix-cdat-cs-v1-2-ffc2b116ca6c@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/cdat: Fixes for CXL CDAT processing | expand |
On 11/17/23 18:14, Ira Weiny wrote: > The addition of the DCD support for CXL type-3 devices extended the CDAT > table large enough that the checksum being returned was incorrect.[1] > > This was because the checksum value was using the header length field > rather than each of the 4 bytes of the length field. This was > previously not seen because the length of the CDAT data was less than > 256 thus resulting in an equivalent checksum value. > > Properly calculate the checksum for the CDAT header. > > [1] https://lore.kernel.org/all/20231116-fix-cdat-devm-free-v1-1-b148b40707d7@intel.com/ > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > hw/cxl/cxl-cdat.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c > index 24829cf2428d..d93e2e4e64f2 100644 > --- a/hw/cxl/cxl-cdat.c > +++ b/hw/cxl/cxl-cdat.c > @@ -95,8 +95,15 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) > /* For now, no runtime updates */ > cdat_header->sequence = 0; > cdat_header->length += sizeof(CDATTableHeader); > - sum += cdat_header->revision + cdat_header->sequence + > - cdat_header->length; > + > + do { > + uint8_t *buf = (uint8_t *)cdat_header; > + > + for (i = 0; i < sizeof(*cdat_header); i++) { > + sum += buf[i]; > + } > + } while (0); Why the empty do/while loop? > + > /* Sum of all bytes including checksum must be 0 */ > cdat_header->checksum = ~sum + 1; > >
Dave Jiang wrote: > [snip] > > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c > > index 24829cf2428d..d93e2e4e64f2 100644 > > --- a/hw/cxl/cxl-cdat.c > > +++ b/hw/cxl/cxl-cdat.c > > @@ -95,8 +95,15 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) > > /* For now, no runtime updates */ > > cdat_header->sequence = 0; > > cdat_header->length += sizeof(CDATTableHeader); > > - sum += cdat_header->revision + cdat_header->sequence + > > - cdat_header->length; > > + > > + do { > > + uint8_t *buf = (uint8_t *)cdat_header; > > + > > + for (i = 0; i < sizeof(*cdat_header); i++) { > > + sum += buf[i]; > > + } > > + } while (0); > > Why the empty do/while loop? Because I used the loop for debugging and forgot to clean up after it was tested. I'll send a v2, Ira
diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c index 24829cf2428d..d93e2e4e64f2 100644 --- a/hw/cxl/cxl-cdat.c +++ b/hw/cxl/cxl-cdat.c @@ -95,8 +95,15 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) /* For now, no runtime updates */ cdat_header->sequence = 0; cdat_header->length += sizeof(CDATTableHeader); - sum += cdat_header->revision + cdat_header->sequence + - cdat_header->length; + + do { + uint8_t *buf = (uint8_t *)cdat_header; + + for (i = 0; i < sizeof(*cdat_header); i++) { + sum += buf[i]; + } + } while (0); + /* Sum of all bytes including checksum must be 0 */ cdat_header->checksum = ~sum + 1;
The addition of the DCD support for CXL type-3 devices extended the CDAT table large enough that the checksum being returned was incorrect.[1] This was because the checksum value was using the header length field rather than each of the 4 bytes of the length field. This was previously not seen because the length of the CDAT data was less than 256 thus resulting in an equivalent checksum value. Properly calculate the checksum for the CDAT header. [1] https://lore.kernel.org/all/20231116-fix-cdat-devm-free-v1-1-b148b40707d7@intel.com/ Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- hw/cxl/cxl-cdat.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)