| Message ID | 1613669204-6464-6-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CSS Mesurement Block | expand |
On Thu, 18 Feb 2021 18:26:44 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > Measurement block format 1 is made available by the extended > measurement block facility and is indicated in the SCHIB by > the bit in the PMCW. > > The MBO is specified in the SCHIB of each channel and the MBO > defined by the SCHM instruction is ignored. > > The test of the MB format 1 is just skipped if the feature is > not available. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > lib/s390x/css.h | 16 ++++++++++++++ > lib/s390x/css_lib.c | 25 ++++++++++++++++++++- > s390x/css.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 93 insertions(+), 1 deletion(-) > (...) > diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c > index 4c8a6ae..1f09f93 100644 > --- a/lib/s390x/css_lib.c > +++ b/lib/s390x/css_lib.c > @@ -298,7 +298,7 @@ static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi, > pmcw->flags2 &= ~PMCW_MBF1; > > pmcw->mbi = mbi; > - schib.mbo = mb; > + schib.mbo = mb & ~0x3f; Merge this into the patch introducing the function? > } else { > pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME); > } > @@ -527,3 +527,26 @@ void enable_io_isc(uint8_t isc) > value = (uint64_t)isc << 24; > lctlg(6, value); > } > + > +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb) > +{ > + struct pmcw *pmcw = &schib.pmcw; > + int cc; > + > + /* Read the SCHIB for this subchannel */ > + cc = stsch(schid, &schib); > + if (cc) { > + report(0, "stsch: sch %08x failed with cc=%d", schid, cc); > + return; > + } > + > + /* Update the SCHIB to enable the measurement block */ > + pmcw->flags |= PMCW_MBUE; > + pmcw->flags2 |= PMCW_MBF1; > + schib.mbo = mb; > + > + /* Tell the CSS we want to modify the subchannel */ > + expect_pgm_int(); > + cc = msch(schid, &schib); > + check_pgm_int_code(PGM_INT_CODE_OPERAND); > +} > diff --git a/s390x/css.c b/s390x/css.c > index b65aa89..576df48 100644 > --- a/s390x/css.c > +++ b/s390x/css.c > @@ -257,6 +257,58 @@ end: > report_prefix_pop(); > } > > +/* > + * test_schm_fmt1: > + * With measurement block format 1 the mesurement block is > + * dedicated to a subchannel. > + */ > +static void test_schm_fmt1(void) > +{ > + struct measurement_block_format1 *mb1; > + > + report_prefix_push("Format 1"); > + > + if (!test_device_sid) { > + report_skip("No device"); > + goto end; > + } > + > + if (!css_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK)) { > + report_skip("Extended measurement block not available"); > + goto end; > + } > + > + /* Allocate zeroed Measurement block */ > + mb1 = alloc_io_mem(sizeof(struct measurement_block_format1), 0); > + if (!mb1) { > + report_abort("measurement_block_format1 allocation failed"); > + goto end; > + } > + > + schm(NULL, 0); /* Stop any previous measurement */ > + schm(0, SCHM_MBU); > + > + /* Expect error for non aligned MB */ > + report_prefix_push("Unaligned MB origin"); > + msch_with_wrong_fmt1_mbo(test_device_sid, (uint64_t)mb1 + 1); > + report_prefix_pop(); > + > + /* Clear the measurement block for the next test */ > + memset(mb1, 0, sizeof(*mb1)); > + > + /* Expect success */ > + report_prefix_push("Valid MB address and index"); > + report(start_measure((u64)mb1, 0, true) && > + mb1->ssch_rsch_count == SCHM_UPDATE_CNT, > + "SSCH measured %d", mb1->ssch_rsch_count); > + report_prefix_pop(); > + > + schm(NULL, 0); /* Stop the measurement */ Same here, I think you should call css_disable_mb(). > + free_io_mem(mb1, sizeof(struct measurement_block_format1)); > +end: > + report_prefix_pop(); > +} > + > static struct { > const char *name; > void (*func)(void); > @@ -268,6 +320,7 @@ static struct { > { "sense (ssch/tsch)", test_sense }, > { "measurement block (schm)", test_schm }, > { "measurement block format0", test_schm_fmt0 }, > + { "measurement block format1", test_schm_fmt1 }, > { NULL, NULL } > }; >
On 2/23/21 2:29 PM, Cornelia Huck wrote: > On Thu, 18 Feb 2021 18:26:44 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> Measurement block format 1 is made available by the extended >> measurement block facility and is indicated in the SCHIB by >> the bit in the PMCW. >> >> The MBO is specified in the SCHIB of each channel and the MBO >> defined by the SCHM instruction is ignored. >> >> The test of the MB format 1 is just skipped if the feature is >> not available. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> lib/s390x/css.h | 16 ++++++++++++++ >> lib/s390x/css_lib.c | 25 ++++++++++++++++++++- >> s390x/css.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 93 insertions(+), 1 deletion(-) >> > > (...) > >> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c >> index 4c8a6ae..1f09f93 100644 >> --- a/lib/s390x/css_lib.c >> +++ b/lib/s390x/css_lib.c >> @@ -298,7 +298,7 @@ static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi, >> pmcw->flags2 &= ~PMCW_MBF1; >> >> pmcw->mbi = mbi; >> - schib.mbo = mb; >> + schib.mbo = mb & ~0x3f; indeed, looks like a later change. ...snip... >> + */ >> +static void test_schm_fmt1(void) >> +{ >> + ...snip... >> + /* Expect success */ >> + report_prefix_push("Valid MB address and index"); >> + report(start_measure((u64)mb1, 0, true) && >> + mb1->ssch_rsch_count == SCHM_UPDATE_CNT, >> + "SSCH measured %d", mb1->ssch_rsch_count); >> + report_prefix_pop(); >> + >> + schm(NULL, 0); /* Stop the measurement */ > > Same here, I think you should call css_disable_mb(). I agree. Thanks, Pierre
On 2/18/21 6:26 PM, Pierre Morel wrote: > Measurement block format 1 is made available by the extended > measurement block facility and is indicated in the SCHIB by > the bit in the PMCW. > > The MBO is specified in the SCHIB of each channel and the MBO > defined by the SCHM instruction is ignored. > > The test of the MB format 1 is just skipped if the feature is > not available. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > lib/s390x/css.h | 16 ++++++++++++++ > lib/s390x/css_lib.c | 25 ++++++++++++++++++++- > s390x/css.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 93 insertions(+), 1 deletion(-) > > diff --git a/lib/s390x/css.h b/lib/s390x/css.h > index dabe54a..1e5e4b5 100644 > --- a/lib/s390x/css.h > +++ b/lib/s390x/css.h > @@ -387,4 +387,20 @@ struct measurement_block_format0 { > uint32_t initial_cmd_resp_time; > }; > > +struct measurement_block_format1 { > + uint32_t ssch_rsch_count; > + uint32_t sample_count; > + uint32_t device_connect_time; > + uint32_t function_pending_time; > + uint32_t device_disconnect_time; > + uint32_t cu_queuing_time; > + uint32_t device_active_only_time; > + uint32_t device_busy_time; > + uint32_t initial_cmd_resp_time; > + uint32_t irq_delay_time; > + uint32_t irq_prio_delay_time; > +}; > + > +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb); > + > #endif > diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c > index 4c8a6ae..1f09f93 100644 > --- a/lib/s390x/css_lib.c > +++ b/lib/s390x/css_lib.c > @@ -298,7 +298,7 @@ static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi, > pmcw->flags2 &= ~PMCW_MBF1; > > pmcw->mbi = mbi; > - schib.mbo = mb; > + schib.mbo = mb & ~0x3f; > } else { > pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME); > } > @@ -527,3 +527,26 @@ void enable_io_isc(uint8_t isc) > value = (uint64_t)isc << 24; > lctlg(6, value); > } > + > +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb) > +{ > + struct pmcw *pmcw = &schib.pmcw; > + int cc; > + > + /* Read the SCHIB for this subchannel */ > + cc = stsch(schid, &schib); > + if (cc) {> + report(0, "stsch: sch %08x failed with cc=%d", schid, cc); > + return; > + } > + > + /* Update the SCHIB to enable the measurement block */ > + pmcw->flags |= PMCW_MBUE; > + pmcw->flags2 |= PMCW_MBF1; > + schib.mbo = mb; > + > + /* Tell the CSS we want to modify the subchannel */ > + expect_pgm_int(); > + cc = msch(schid, &schib); > + check_pgm_int_code(PGM_INT_CODE_OPERAND); Why would you expect a PGM in a library function are PGMs normal for IO instructions? oO Is this a test function which should be part of your test file in s390x/*.c or is it part of the IO library which should: - Abort if an initialization failed and we can assume that future tests are now useless - Return an error so the test can report an error - Return success > +} > diff --git a/s390x/css.c b/s390x/css.c > index b65aa89..576df48 100644 > --- a/s390x/css.c > +++ b/s390x/css.c > @@ -257,6 +257,58 @@ end: > report_prefix_pop(); > } > > +/* > + * test_schm_fmt1: > + * With measurement block format 1 the mesurement block is > + * dedicated to a subchannel. > + */ > +static void test_schm_fmt1(void) > +{ > + struct measurement_block_format1 *mb1; > + > + report_prefix_push("Format 1"); > + > + if (!test_device_sid) { > + report_skip("No device"); > + goto end; > + } > + > + if (!css_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK)) { > + report_skip("Extended measurement block not available"); > + goto end; > + } > + > + /* Allocate zeroed Measurement block */ > + mb1 = alloc_io_mem(sizeof(struct measurement_block_format1), 0); > + if (!mb1) { > + report_abort("measurement_block_format1 allocation failed"); > + goto end; > + } > + > + schm(NULL, 0); /* Stop any previous measurement */ > + schm(0, SCHM_MBU); > + > + /* Expect error for non aligned MB */ > + report_prefix_push("Unaligned MB origin"); > + msch_with_wrong_fmt1_mbo(test_device_sid, (uint64_t)mb1 + 1); > + report_prefix_pop(); > + > + /* Clear the measurement block for the next test */ > + memset(mb1, 0, sizeof(*mb1)); > + > + /* Expect success */ > + report_prefix_push("Valid MB address and index"); > + report(start_measure((u64)mb1, 0, true) && > + mb1->ssch_rsch_count == SCHM_UPDATE_CNT, > + "SSCH measured %d", mb1->ssch_rsch_count); > + report_prefix_pop(); > + > + schm(NULL, 0); /* Stop the measurement */ > + free_io_mem(mb1, sizeof(struct measurement_block_format1)); > +end: > + report_prefix_pop(); > +} > + > static struct { > const char *name; > void (*func)(void); > @@ -268,6 +320,7 @@ static struct { > { "sense (ssch/tsch)", test_sense }, > { "measurement block (schm)", test_schm }, > { "measurement block format0", test_schm_fmt0 }, > + { "measurement block format1", test_schm_fmt1 }, Output will then be: "measurement block format1: Format 1: Report message" Wouldn't it make more sense to put the format 0 and 1 tests into test_schm() so we'd have: "measurement block (schm): Format 0: Report message" ? > { NULL, NULL } > }; > >
On 2/26/21 11:02 AM, Janosch Frank wrote: > On 2/18/21 6:26 PM, Pierre Morel wrote: >> Measurement block format 1 is made available by the extended >> measurement block facility and is indicated in the SCHIB by >> the bit in the PMCW. ... >> +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb) >> +{ >> + struct pmcw *pmcw = &schib.pmcw; >> + int cc; >> + >> + /* Read the SCHIB for this subchannel */ >> + cc = stsch(schid, &schib); >> + if (cc) {> + report(0, "stsch: sch %08x failed with cc=%d", schid, cc); >> + return; >> + } >> + >> + /* Update the SCHIB to enable the measurement block */ >> + pmcw->flags |= PMCW_MBUE; >> + pmcw->flags2 |= PMCW_MBF1; >> + schib.mbo = mb; >> + >> + /* Tell the CSS we want to modify the subchannel */ >> + expect_pgm_int(); >> + cc = msch(schid, &schib); >> + check_pgm_int_code(PGM_INT_CODE_OPERAND); > > Why would you expect a PGM in a library function are PGMs normal for IO > instructions? oO > > Is this a test function which should be part of your test file in > s390x/*.c or is it part of the IO library which should: > > - Abort if an initialization failed and we can assume that future tests > are now useless > - Return an error so the test can report an error > - Return success Now it looks clear to me that this test belongs to the tests and not the lib. I put it there to avoid exporting the SCHIB, but after all, why not exporting the SCHIB, we may need access to it from other tests again in the future. > >> +} >> diff --git a/s390x/css.c b/s390x/css.c >> index b65aa89..576df48 100644 >> --- a/s390x/css.c >> +++ b/s390x/css.c >> @@ -257,6 +257,58 @@ end: >> report_prefix_pop(); >> } >> >> +/* >> + * test_schm_fmt1: >> + * With measurement block format 1 the mesurement block is >> + * dedicated to a subchannel. >> + */ >> +static void test_schm_fmt1(void) >> +{ >> + struct measurement_block_format1 *mb1; >> + >> + report_prefix_push("Format 1"); >> + >> + if (!test_device_sid) { >> + report_skip("No device"); >> + goto end; >> + } ... >> + report(start_measure((u64)mb1, 0, true) && >> + mb1->ssch_rsch_count == SCHM_UPDATE_CNT, >> + "SSCH measured %d", mb1->ssch_rsch_count); >> + report_prefix_pop(); >> + >> + schm(NULL, 0); /* Stop the measurement */ >> + free_io_mem(mb1, sizeof(struct measurement_block_format1)); >> +end: >> + report_prefix_pop(); >> +} >> + >> static struct { >> const char *name; >> void (*func)(void); >> @@ -268,6 +320,7 @@ static struct { >> { "sense (ssch/tsch)", test_sense }, >> { "measurement block (schm)", test_schm }, >> { "measurement block format0", test_schm_fmt0 }, >> + { "measurement block format1", test_schm_fmt1 }, > > Output will then be: > "measurement block format1: Format 1: Report message" > > Wouldn't it make more sense to put the format 0 and 1 tests into > test_schm() so we'd have: > "measurement block (schm): Format 0: Report message" ? too much prefix push... rationalizing will make the goto disapear... Thanks for review Regards, Pierre
diff --git a/lib/s390x/css.h b/lib/s390x/css.h index dabe54a..1e5e4b5 100644 --- a/lib/s390x/css.h +++ b/lib/s390x/css.h @@ -387,4 +387,20 @@ struct measurement_block_format0 { uint32_t initial_cmd_resp_time; }; +struct measurement_block_format1 { + uint32_t ssch_rsch_count; + uint32_t sample_count; + uint32_t device_connect_time; + uint32_t function_pending_time; + uint32_t device_disconnect_time; + uint32_t cu_queuing_time; + uint32_t device_active_only_time; + uint32_t device_busy_time; + uint32_t initial_cmd_resp_time; + uint32_t irq_delay_time; + uint32_t irq_prio_delay_time; +}; + +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb); + #endif diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c index 4c8a6ae..1f09f93 100644 --- a/lib/s390x/css_lib.c +++ b/lib/s390x/css_lib.c @@ -298,7 +298,7 @@ static bool schib_update_mb(int schid, uint64_t mb, uint16_t mbi, pmcw->flags2 &= ~PMCW_MBF1; pmcw->mbi = mbi; - schib.mbo = mb; + schib.mbo = mb & ~0x3f; } else { pmcw->flags &= ~(PMCW_MBUE | PMCW_DCTME); } @@ -527,3 +527,26 @@ void enable_io_isc(uint8_t isc) value = (uint64_t)isc << 24; lctlg(6, value); } + +void msch_with_wrong_fmt1_mbo(unsigned int schid, uint64_t mb) +{ + struct pmcw *pmcw = &schib.pmcw; + int cc; + + /* Read the SCHIB for this subchannel */ + cc = stsch(schid, &schib); + if (cc) { + report(0, "stsch: sch %08x failed with cc=%d", schid, cc); + return; + } + + /* Update the SCHIB to enable the measurement block */ + pmcw->flags |= PMCW_MBUE; + pmcw->flags2 |= PMCW_MBF1; + schib.mbo = mb; + + /* Tell the CSS we want to modify the subchannel */ + expect_pgm_int(); + cc = msch(schid, &schib); + check_pgm_int_code(PGM_INT_CODE_OPERAND); +} diff --git a/s390x/css.c b/s390x/css.c index b65aa89..576df48 100644 --- a/s390x/css.c +++ b/s390x/css.c @@ -257,6 +257,58 @@ end: report_prefix_pop(); } +/* + * test_schm_fmt1: + * With measurement block format 1 the mesurement block is + * dedicated to a subchannel. + */ +static void test_schm_fmt1(void) +{ + struct measurement_block_format1 *mb1; + + report_prefix_push("Format 1"); + + if (!test_device_sid) { + report_skip("No device"); + goto end; + } + + if (!css_general_feature(CSSC_EXTENDED_MEASUREMENT_BLOCK)) { + report_skip("Extended measurement block not available"); + goto end; + } + + /* Allocate zeroed Measurement block */ + mb1 = alloc_io_mem(sizeof(struct measurement_block_format1), 0); + if (!mb1) { + report_abort("measurement_block_format1 allocation failed"); + goto end; + } + + schm(NULL, 0); /* Stop any previous measurement */ + schm(0, SCHM_MBU); + + /* Expect error for non aligned MB */ + report_prefix_push("Unaligned MB origin"); + msch_with_wrong_fmt1_mbo(test_device_sid, (uint64_t)mb1 + 1); + report_prefix_pop(); + + /* Clear the measurement block for the next test */ + memset(mb1, 0, sizeof(*mb1)); + + /* Expect success */ + report_prefix_push("Valid MB address and index"); + report(start_measure((u64)mb1, 0, true) && + mb1->ssch_rsch_count == SCHM_UPDATE_CNT, + "SSCH measured %d", mb1->ssch_rsch_count); + report_prefix_pop(); + + schm(NULL, 0); /* Stop the measurement */ + free_io_mem(mb1, sizeof(struct measurement_block_format1)); +end: + report_prefix_pop(); +} + static struct { const char *name; void (*func)(void); @@ -268,6 +320,7 @@ static struct { { "sense (ssch/tsch)", test_sense }, { "measurement block (schm)", test_schm }, { "measurement block format0", test_schm_fmt0 }, + { "measurement block format1", test_schm_fmt1 }, { NULL, NULL } };
Measurement block format 1 is made available by the extended measurement block facility and is indicated in the SCHIB by the bit in the PMCW. The MBO is specified in the SCHIB of each channel and the MBO defined by the SCHM instruction is ignored. The test of the MB format 1 is just skipped if the feature is not available. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- lib/s390x/css.h | 16 ++++++++++++++ lib/s390x/css_lib.c | 25 ++++++++++++++++++++- s390x/css.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-)