On 2020-07-09 16:22, Cornelia Huck wrote:
On Thu, 9 Jul 2020 15:58:07 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 2020-07-09 15:52, Cornelia Huck wrote:
On Thu, 9 Jul 2020 15:41:56 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 2020-07-09 15:30, Cornelia Huck wrote:
On Thu, 9 Jul 2020 15:12:05 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
On 2020-07-09 13:40, Cornelia Huck wrote:
On Thu, 9 Jul 2020 10:07:47 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
(...)
+/*
+ * css_msch: enable subchannel and set with specified ISC
"css_enable: enable the subchannel with the specified ISC"
?
+ * @schid: Subchannel Identifier
+ * @isc : number of the interruption subclass to use
+ * Return value:
+ * On success: 0
+ * On error the CC of the faulty instruction
+ * or -1 if the retry count is exceeded.
+ */
+int css_enable(int schid, int isc)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int retry_count = 0;
+ uint16_t flags;
+ int cc;
+
+ /* Read the SCHIB for this subchannel */
+ cc = stsch(schid, &schib);
+ if (cc) {
+ report_info("stsch: sch %08x failed with cc=%d", schid, cc);
+ return cc;
+ }
+
+ flags = PMCW_ENABLE | (isc << PMCW_ISC_SHIFT);
+ if ((pmcw->flags & flags) == flags) {
I think you want (pmcw->flags & PMCW_ENABLE) == PMCW_ENABLE -- this
catches the case of "subchannel has been enabled before, but with a
different isc".
If with a different ISC, we need to modify the ISC.
Don't we ?
I think that's a policy decision (I would probably fail and require a
disable before setting another isc, but that's a matter of taste).
Regardless, I think the current check doesn't even catch the 'different
isc' case?
hum, right.
If it is OK I remove this one.
And I must rework the same test I do later
in this patch.
So, you mean checking for PMCW_ENABLE? Or not at all?
(I'd check for PMCW_ENABLE.)
- if ((pmcw->flags & flags) == flags) {
+ if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
report_info("stsch: sch %08x already enabled", schid);
return 0;
}
I keep both, otherwise I return 0 without setting the ISC.
Ah, I missed the 'return 0'.
then I have another error:
retry:
/* Update the SCHIB to enable the channel and set the ISC */
+ pmcw->flags &= ~(PMCW_ISC_MASK | PMCW_ENABLE);
Maybe ~PMCW_ISC_MASK is enough?
yes
pmcw->flags |= flags;
and finaly the same as the first later...
- if ((pmcw->flags & flags) == flags) {
+ if ((pmcw->flags & (PMCW_ISC_MASK | PMCW_ENABLE)) == flags) {
I think you can keep that as-is.
I don't thing so, I just stored the pmcw.
if ISC is stored as 3 and I want 1 it is a false positive.
Same error as you showed me before.
?
report_info("stsch: sch %08x successfully modified
after %d retries",
schid, retry_count);
is better I think.
What do you think?
It's probably the right direction.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
