Quoting Claudio Imbrenda (2022-11-25 15:03:48)
> On Thu, 24 Nov 2022 14:44:28 +0100
> Nico Boehr <nrb@xxxxxxxxxxxxx> wrote:
[...]
> > diff --git a/lib/s390x/cmm.c b/lib/s390x/cmm.c
> > new file mode 100644
> > index 000000000000..5da02fe628f9
> > --- /dev/null
> > +++ b/lib/s390x/cmm.c
[...]
> > +/*
> > + * Set CMM page states on pagebuf.
> > + * pagebuf must point to page_count consecutive pages.
> > + * page_count must be a multiple of 4.
> > + */
> > +void cmm_set_page_states(uint8_t *pagebuf, int page_count)
>
> this could be an unsigned int (but maybe unsigned long would be better)
Yes, changed to unsigned long.
[...]
> > +/*
> > + * Verify CMM page states on pagebuf.
> > + * Page states must have been set by cmm_set_page_states on pagebuf before.
> > + * page_count must be a multiple of 4.
> > + *
> > + * If page states match the expected result, will return a cmm_verify_result
> > + * with verify_failed false. All other fields are then invalid.
> > + * If there is a mismatch, the returned struct will have verify_failed true
> > + * and will be filled with details on the first mismatch encountered.
> > + */
> > +struct cmm_verify_result cmm_verify_page_states(uint8_t *pagebuf, int page_count)
>
> same here
Yes, done.
>
> > +{
> > + struct cmm_verify_result result = {
> > + .verify_failed = true
> > + };
> > + int i, expected_mask, actual_mask;
> > + unsigned long addr;
> > +
> > + assert(page_count % 4 == 0);
> > +
> > + for (i = 0; i < page_count; i++) {
> > + addr = (unsigned long)(pagebuf + i * PAGE_SIZE);
> > + actual_mask = essa(ESSA_GET_STATE, addr);
Oh, essa() returns unsigned long, so let's make actual_mask, expected_mask etc. unsigned long, too.
> > + /* usage state in bits 60 and 61 */
> > + actual_mask = BIT((actual_mask >> 2) & 0x3);
> > + expected_mask = allowed_essa_state_masks[i % ARRAY_SIZE(allowed_essa_state_masks)];
> > + if (!(actual_mask & expected_mask)) {
> > + result.page_mismatch_idx = i;
> > + result.page_mismatch_addr = addr;
> > + result.expected_mask = expected_mask;
> > + result.actual_mask = actual_mask;
> > + result.verify_failed = true;
>
> it's already true, you don't need to set it again
OK, removed.
[...]
> > +void cmm_report_verify(struct cmm_verify_result const *result)
> > +{
> > + if (result->verify_failed)
> > + report_fail("page state mismatch: first page idx = %d, addr = %lx, expected_mask = 0x%x, actual_mask = 0x%x", result->page_mismatch_idx, result->page_mismatch_addr, result->expected_mask, result->actual_mask);
>
> this line looks longer than 120 columns
Yes, wrapped it.
> > diff --git a/lib/s390x/cmm.h b/lib/s390x/cmm.h
> > new file mode 100644
> > index 000000000000..41dcc2f953fd
> > --- /dev/null
> > +++ b/lib/s390x/cmm.h
[...]
> > +struct cmm_verify_result {
> > + bool verify_failed;
> > + char expected_mask;
> > + char actual_mask;
> > + int page_mismatch_idx;
>
> maybe also unsigned long to be consistent with
> cmm_set_page_states
Yes, done.
