Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

[Michael Schmitz]

Hi Finn,

>>> The SEL-with-ATN process is discussed in the "Configuration Register 
>>> 3" section, Bit 3, in 
>>> http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR53C9X.txt
>>
>> What chip revision is this doc for?
> 
> It can only be FAS100A. That's the document which David cited when he 
> posted his ESP_CONFIG2_SCSI2ENAB patch in 2014, which is why I was using 
> it. I think the documents Kars cited may be more relevant in this case:
> http://www.spinics.net/lists/linux-scsi/msg73856.html
> (those documents are all available from bitsavers).

Thanks, I'll look at those again.

>> As far as I can make out, it should be fas100a? The esp_scsi.h comments 
>> suggest the tagged queuing bit is for revisions fas236. The chips used 
>> on Macs were probably esp236 or fas236? Depending on the
> 
> The Quadras I've used have ESP236 chips, according to esp_scsi detection. 

Right - we've already established that bit 6 (ESP_CONFIG3_TBMS) is what
we need (plus possibly bit 5).

>> The ESP driver explicitly clears this bit and only sets it for the 
>> FAS236 variant (using it as fast clock bit there). The logic for setting 
>> a target's config3 value refer to the stored config for target 0, and 
>> the only place I can see that get initialized is again in esp_get_rev(). 
>> First to 5, then to 0 once it's clear the register even exists. We could 
>> set bit 3 here, but it may not matter for the chip revisions that we are 
>> interested in... Better set bit 6 (ESP_CONFIG3_TBMS) in our 
>> esp_slave_configure() hook?
> 
> That hook lets particular board-specific drivers disable particular 
> features for particular targets. It would be better, I think, to fix this 
> in the core driver, so that any target is free to use three byte messages 
> regardless of the board-specific driver (except ESP100 I guess).

That would be the best solution, but from what I understand, no one has
reported this bug for revisions > FAS236. I'd rather not touch these
cases - can't test them.

> 
> Besides, ESP_CONFIG3_EWIDE == ESP_CONFIG3_TBMS, so this bit will get wiped 
> in esp_reset_cleanup() anyway. Probably this should be conditional on 
> FASHME. I think there's a similar bug in esp_data_bytes_sent().

Oops - these bits should be cleared only for FAS100A and HME. I don't
think we redo domain validation after a bus reset so clearing the bits
here means they are lost for good.
Can't spot a bug in esp_data_bytes_sent() though.

>>> Anyway, is safe to do this?
>>>
>>>         esp->ops->send_dma_cmd(esp, esp->command_block_dma,
>>>                                2, 2, 1, ESP_CMD_DMA | ESP_CMD_TI);
>>>
>>>         /* ACK the message.  */
>>>         scsi_esp_cmd(esp, ESP_CMD_MOK);
>>>
>>> For PIO and PDMA, the transfer will be finished before the MOK 
>>> ("Message Accepted") command, but for DMA this seems to be racey...
>>
>> It sure is racey. Is there a DMA complete interrupt that you can wait 
>> for before proceeding with the ACK?
> 
> That code comes from esp_reconnect_with_tag(). The interrupt you asked 
> about never shows up, hence, "Reconnect IRQ2 timeout".

OK, I see the later loop to check for the FDONE status bit. If the ESP
chip never reads in the missing tag bytes, we ACK a message that we
haven't really fully received. But we're treating this as an error
anyway so the cleanup will take care of that. In the normal case, I
suppose the ESP chip has read all message bytes into the FIFO so the ACK
there is probably OK.

Could be moved until after the IRQ2 checking loop to make sure (not
certain what this'll do to the target - is there a max. time to ACK the
message before the target stuffs up?).

> Anyway, there must about 3 potential patches from this discussion. I say 
> we do as Geert suggests and move the discussion to the kernel.org lists 
> (with code, of course).

Agreed - the three patches I see so far would be

- set config2 SCSI2 enable bit as originally proposed by Dave,

- correct the esp_reset_cleanup() bug so a reset won't wipe out our
config3 ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM bits

- set ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM for all targets in either
esp_slave_configure() or esp_reset_esp()

Is that what you had in mind?

Cheers,

	Michael

Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

[Finn Thain]

On Tue, 1 Nov 2016, Michael Schmitz wrote:

> > It would be better, I think, to fix this in the core driver, so that 
> > any target is free to use three byte messages regardless of the 
> > board-specific driver (except ESP100 I guess).
> 
> That would be the best solution, but from what I understand, no one has 
> reported this bug for revisions > FAS236. I'd rather not touch these 
> cases - can't test them.

Even untested patches can carry low risk, given enough review, in which 
case we should avoid hacks.

Anyway, based on activity on the list, it seems likely that there are 
testers for the affected silicon. Besides ESP236 and FAS236, it is 
probably only FASHME and PCSCSI.

> 
> > 
> > Besides, ESP_CONFIG3_EWIDE == ESP_CONFIG3_TBMS, so this bit will get 
> > wiped in esp_reset_cleanup() anyway. Probably this should be 
> > conditional on FASHME. I think there's a similar bug in 
> > esp_data_bytes_sent().
> 
> Oops - these bits should be cleared only for FAS100A and HME. I don't 
> think we redo domain validation after a bus reset so clearing the bits 
> here means they are lost for good. Can't spot a bug in 
> esp_data_bytes_sent() though.

It's not a bug, it's just that ESP_CONFIG3_EWIDE == ESP_CONFIG3_TBMS:

	if (esp->prev_cfg3 & ESP_CONFIG3_EWIDE)
		fifo_cnt <<= 1;

> 
> >>> Anyway, is safe to do this?
> >>>
> >>>         esp->ops->send_dma_cmd(esp, esp->command_block_dma,
> >>>                                2, 2, 1, ESP_CMD_DMA | ESP_CMD_TI);
> >>>
> >>>         /* ACK the message.  */
> >>>         scsi_esp_cmd(esp, ESP_CMD_MOK);
> >>>
> >>> For PIO and PDMA, the transfer will be finished before the MOK 
> >>> ("Message Accepted") command, but for DMA this seems to be racey...
> >>
> >> It sure is racey. Is there a DMA complete interrupt that you can wait 
> >> for before proceeding with the ACK?
> > 
> > That code comes from esp_reconnect_with_tag(). The interrupt you asked 
> > about never shows up, hence, "Reconnect IRQ2 timeout".
> 
> OK, I see the later loop to check for the FDONE status bit. If the ESP 
> chip never reads in the missing tag bytes, we ACK a message that we 
> haven't really fully received. But we're treating this as an error 
> anyway so the cleanup will take care of that.
> 

The question is, did we cause the error by releasing ACK during DMA.

But perhaps the chip is smart enough to handle the two commands 
back-to-back: Transfer Information followed by Accept Message.

> In the normal case, I suppose the ESP chip has read all message bytes 
> into the FIFO so the ACK there is probably OK.
> 
> Could be moved until after the IRQ2 checking loop to make sure (not 
> certain what this'll do to the target - is there a max. time to ACK the 
> message before the target stuffs up?).

The complication is that the spec demands that certain messages be 
acknowleged with /ATN negated before the final negation of /ACK. I think 
that negating /ACK with /ATN asserted signals message rejection.

Perhaps that's the problem here: scsi_esp_cmd(esp, ESP_CMD_SATN) occurs 
after the transfer, but instead probably should be scsi_esp_cmd(esp, 
ESP_CMD_RATN) before scsi_esp_cmd(esp, ESP_CMD_MOK). It would be easy to 
believe that some targets don't tolerate this.

Anyway, I suspect our problem lies in this reselection message transfer, 
not in the configuration registers.

> 
> > Anyway, there must about 3 potential patches from this discussion. I say 
> > we do as Geert suggests and move the discussion to the kernel.org lists 
> > (with code, of course).
> 
> Agreed - the three patches I see so far would be
> 
> - set config2 SCSI2 enable bit as originally proposed by Dave,
> 
> - correct the esp_reset_cleanup() bug so a reset won't wipe out our 
>   config3 ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM bits
> 
> - set ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM for all targets in either 
>   esp_slave_configure() or esp_reset_esp()
> 
> Is that what you had in mind?

Something like that. All of which is based on the notion that the config2 
SCSI2 Enable bit could be useful for an initiator. But that notion looks 
more and more dubious to me --

What use could an initiator have for the Group Two Command Block bit 
(ESP_CONFIG3_GTM)?

What use could an initiator have for the QTAG Control bit 
(ESP_CONFIG3_TBMS) when the target never controls /ATN? (I only remembered 
this yesterday, so please ignore my earlier comments about reselection and 
SEL-with-ATN.)

-- 

> 
> Cheers,
> 
> 	Michael

Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

[Michael Schmitz]

Hi Finn,


>>> Besides, ESP_CONFIG3_EWIDE == ESP_CONFIG3_TBMS, so this bit will get 
>>> wiped in esp_reset_cleanup() anyway. Probably this should be 
>>> conditional on FASHME. I think there's a similar bug in 
>>> esp_data_bytes_sent().
>>
>> Oops - these bits should be cleared only for FAS100A and HME. I don't 
>> think we redo domain validation after a bus reset so clearing the bits 
>> here means they are lost for good. Can't spot a bug in 
>> esp_data_bytes_sent() though.
> 
> It's not a bug, it's just that ESP_CONFIG3_EWIDE == ESP_CONFIG3_TBMS:
> 
> 	if (esp->prev_cfg3 & ESP_CONFIG3_EWIDE)
> 		fifo_cnt <<= 1;

I see - having this bit set would have side effects here.


>>> That code comes from esp_reconnect_with_tag(). The interrupt you asked 
>>> about never shows up, hence, "Reconnect IRQ2 timeout".
>>
>> OK, I see the later loop to check for the FDONE status bit. If the ESP 
>> chip never reads in the missing tag bytes, we ACK a message that we 
>> haven't really fully received. But we're treating this as an error 
>> anyway so the cleanup will take care of that.
>>
> 
> The question is, did we cause the error by releasing ACK during DMA.
> 
> But perhaps the chip is smart enough to handle the two commands 
> back-to-back: Transfer Information followed by Accept Message.

You can queue two commands in the command fifo - the Accept Message
would only be run once the Transfer Information is complete.

We can read back the command register to see which command was last
excecuting (or still is). I suspect it's the Transfer Information here.

> The complication is that the spec demands that certain messages be 
> acknowleged with /ATN negated before the final negation of /ACK. I think 
> that negating /ACK with /ATN asserted signals message rejection.

Yep, that's my reading as well.

> 
> Perhaps that's the problem here: scsi_esp_cmd(esp, ESP_CMD_SATN) occurs 
> after the transfer, but instead probably should be scsi_esp_cmd(esp, 

Where? In esp_reconnect, we don't get to that point when returning from
esp_reconnect_with_tag() after timeout. And without timeout, there's
still the check for ESP_CMD_FLAG_ABORT which I don't see set anywhere.

> ESP_CMD_RATN) before scsi_esp_cmd(esp, ESP_CMD_MOK). It would be easy to 
> believe that some targets don't tolerate this.
> 
> Anyway, I suspect our problem lies in this reselection message transfer, 
> not in the configuration registers.

How does the target signal end of message in phase? Stop handshaking, or
change phase? ATN going false?

With the DMA programmed for two tag bytes, and the reselection
originating from a tagged command, I would expect the target to send the
two tag bytes. Would it help to do the message transfer without DMA?

> 
>>
>>> Anyway, there must about 3 potential patches from this discussion. I say 
>>> we do as Geert suggests and move the discussion to the kernel.org lists 
>>> (with code, of course).
>>
>> Agreed - the three patches I see so far would be
>>
>> - set config2 SCSI2 enable bit as originally proposed by Dave,
>>
>> - correct the esp_reset_cleanup() bug so a reset won't wipe out our 
>>   config3 ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM bits
>>
>> - set ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM for all targets in either 
>>   esp_slave_configure() or esp_reset_esp()
>>
>> Is that what you had in mind?
> 
> Something like that. All of which is based on the notion that the config2 
> SCSI2 Enable bit could be useful for an initiator. But that notion looks 
> more and more dubious to me --
> 
> What use could an initiator have for the Group Two Command Block bit 
> (ESP_CONFIG3_GTM)?

Upon closer reading of the docs, none.

> What use could an initiator have for the QTAG Control bit 
> (ESP_CONFIG3_TBMS) when the target never controls /ATN? (I only remembered 
> this yesterday, so please ignore my earlier comments about reselection and 
> SEL-with-ATN.)

I had thought the reselection process similar to selection. But if the
target does not signal further message bytes by keepin ATN asserted,
that bit might not matter here.

Are we dealing with SCSI disks that fool the domain validation into
assuming tagged queuing capability, and then ignore the tag message
bytes, sending only the identify message byte on reselection?

Trying to make reconnect_with_tag work for these disks would be foolish
indeed ...

Cheers,

	Michael

Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

[Finn Thain]

Wed, 2 Nov 2016 20:53:50 +1100 (AEDT)
On Wed, 2 Nov 2016, Michael Schmitz wrote:

> 
> > The complication is that the spec demands that certain messages be 
> > acknowleged with /ATN negated before the final negation of /ACK. I 
> > think that negating /ACK with /ATN asserted signals message rejection.
> 
> Yep, that's my reading as well.
> 
> > 
> > Perhaps that's the problem here: scsi_esp_cmd(esp, ESP_CMD_SATN) 
> > occurs after the transfer,
> 
> Where? In esp_reconnect, we don't get to that point when returning from 
> esp_reconnect_with_tag() after timeout.

Well, the point is, we potentially release ACK (either after the 
Information Transfer command or after the Accept Message command) with ATN 
still asserted.

> And without timeout, there's still the check for ESP_CMD_FLAG_ABORT 
> which I don't see set anywhere.

I don't know what that's about. The git history may explain where it comes 
from.

> 
> > but instead probably should be scsi_esp_cmd(esp, ESP_CMD_RATN) before 
> > scsi_esp_cmd(esp, ESP_CMD_MOK). It would be easy to believe that some 
> > targets don't tolerate this.
> > 
> > Anyway, I suspect our problem lies in this reselection message 
> > transfer, not in the configuration registers.
> 
> How does the target signal end of message in phase? Stop handshaking, or 
> change phase?

Either would do. To stop handshaking would imply entering bus free phase.

> ATN going false?

AIUI, only the initiator can drive ATN.

> 
> With the DMA programmed for two tag bytes, and the reselection 
> originating from a tagged command, I would expect the target to send the 
> two tag bytes.

Sure, but I think the initator is messing it up along the way.

> Would it help to do the message transfer without DMA?

I would, just to see more clearly what was going on (command queue, 
control lines, bus phase).

Recall that Tuomas said he tried PIO for this with zorro_esp without any 
improvement. And I first saw this bug on a Quadra 660av, on which mac_esp 
uses PIO for all transfers.

> 
> > 
> >>
> >>> Anyway, there must about 3 potential patches from this discussion. I 
> >>> say we do as Geert suggests and move the discussion to the 
> >>> kernel.org lists (with code, of course).
> >>
> >> Agreed - the three patches I see so far would be
> >>
> >> - set config2 SCSI2 enable bit as originally proposed by Dave,
> >>
> >> - correct the esp_reset_cleanup() bug so a reset won't wipe out our 
> >>   config3 ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM bits
> >>
> >> - set ESP_CONFIG3_TBMS and ESP_CONFIG3_GTM for all targets in either 
> >>   esp_slave_configure() or esp_reset_esp()
> >>
> >> Is that what you had in mind?
> > 
> > Something like that. All of which is based on the notion that the 
> > config2 SCSI2 Enable bit could be useful for an initiator. But that 
> > notion looks more and more dubious to me --
> > 
> > What use could an initiator have for the Group Two Command Block bit 
> > (ESP_CONFIG3_GTM)?
> 
> Upon closer reading of the docs, none.
> 
> > What use could an initiator have for the QTAG Control bit 
> > (ESP_CONFIG3_TBMS) when the target never controls /ATN? (I only 
> > remembered this yesterday, so please ignore my earlier comments about 
> > reselection and SEL-with-ATN.)
> 
> I had thought the reselection process similar to selection. But if the 
> target does not signal further message bytes by keepin ATN asserted, 
> that bit might not matter here.

If only the initator drives ATN, the documentation can only be referring 
to an ESP device in target mode.

> 
> Are we dealing with SCSI disks that fool the domain validation into 
> assuming tagged queuing capability, and then ignore the tag message 
> bytes, sending only the identify message byte on reselection?

I don't think so.

> 
> Trying to make reconnect_with_tag work for these disks would be foolish
> indeed ...

My recollection from 2009 is that this particular disk did work with 
mac_esp on one of my Quadras, but not on my 660av. And three people have 
reported the problem now (to my knowledge) so I don't blame the disks.

-- 

> 
> Cheers,
> 
> 	Michael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

[Michael Schmitz]

Hi Finn,

got sidetracked once again ...

>> Where? In esp_reconnect, we don't get to that point when returning from 
>> esp_reconnect_with_tag() after timeout.
> 
> Well, the point is, we potentially release ACK (either after the 
> Information Transfer command or after the Accept Message command) with ATN 
> still asserted.

Who asserts ATN in case of reselection? I always thought that the
target, by initiating reselection, assumes the role of initiator and is
in charge of driving bus phase changes (including control of ATN).

>> How does the target signal end of message in phase? Stop handshaking, or 
>> change phase?
> 
> Either would do. To stop handshaking would imply entering bus free phase.
> 
>> ATN going false?
> 
> AIUI, only the initiator can drive ATN.

So the initiator would have to read two tag message bytes if a target
reselects that had a tagged command disconnected? From the initiator
side, that should work (if an untagged commmand has been issued, no
tagged commands are sent so we expect either tagged or untagged
reselections).

>> With the DMA programmed for two tag bytes, and the reselection 
>> originating from a tagged command, I would expect the target to send the 
>> two tag bytes.
> 
> Sure, but I think the initator is messing it up along the way.

Can't see how - reconnect_with_tag() is only used if no untagged command
is on record for the target.

> 
>> Would it help to do the message transfer without DMA?
> 
> I would, just to see more clearly what was going on (command queue, 
> control lines, bus phase).

Not sure we can get control lines read - bus phases are available IIRC.

> Recall that Tuomas said he tried PIO for this with zorro_esp without any 
> improvement. And I first saw this bug on a Quadra 660av, on which mac_esp 
> uses PIO for all transfers.

PIO does get set up for the expected number of message bytes though. Is
there  a check for phase mismatch during PIO?

>>> What use could an initiator have for the QTAG Control bit 
>>> (ESP_CONFIG3_TBMS) when the target never controls /ATN? (I only 
>>> remembered this yesterday, so please ignore my earlier comments about 
>>> reselection and SEL-with-ATN.)
>>
>> I had thought the reselection process similar to selection. But if the 
>> target does not signal further message bytes by keepin ATN asserted, 
>> that bit might not matter here.
> 
> If only the initator drives ATN, the documentation can only be referring 
> to an ESP device in target mode.

The documentation mentions a command descriptor block to be transferred
- would only apply to ESP in target mode again.

> My recollection from 2009 is that this particular disk did work with 
> mac_esp on one of my Quadras, but not on my 660av. And three people have 
> reported the problem now (to my knowledge) so I don't blame the disks.

What ESP versions are used in the Quadras vs 660av?

Cheers,

	Michael

Re: esp_scsi, was Re: Modified Linux 4.1.20 (mac+scsi) on Quadra 660av

[Finn Thain]

On Wed, 9 Nov 2016, Michael Schmitz wrote:
> 
> > Well, the point is, we potentially release ACK (either after the 
> > Information Transfer command or after the Accept Message command) with 
> > ATN still asserted.
> 
> Who asserts ATN in case of reselection? I always thought that the 
> target, by initiating reselection, assumes the role of initiator and is 
> in charge of driving bus phase changes (including control of ATN).
> 

The difference between selection and reselection is the SEL signal. For 
selection, the host controls SEL. For reselection, the device controls it.

If the device disconnects and reconnects a command, it doesn't alter the 
initiator/target roles for that command. The host always controls ATN.

> >> How does the target signal end of message in phase? Stop handshaking, 
> >> or change phase?
> > 
> > Either would do. To stop handshaking would imply entering bus free 
> > phase.
> > 
> >> ATN going false?
> > 
> > AIUI, only the initiator can drive ATN.
> 
> So the initiator would have to read two tag message bytes if a target 
> reselects that had a tagged command disconnected?

Right, the target contols the bus phase, so it will send as many bytes as 
it wants. The initiator gets to alter that behaviour by signalling with 
ATN. Hence esp_scsi needs to be more careful with ATN during reselection.

> From the initiator side, that should work (if an untagged commmand has 
> been issued, no tagged commands are sent so we expect either tagged or 
> untagged reselections).
> 
> >> With the DMA programmed for two tag bytes, and the reselection 
> >> originating from a tagged command, I would expect the target to send 
> >> the two tag bytes.
> > 
> > Sure, but I think the initator is messing it up along the way.
> 
> Can't see how - reconnect_with_tag() is only used if no untagged command 
> is on record for the target.

Please see prior discussion of "Queue Tag Enable" bit, which relates to 
target response to ACK with and without ATN. Please also refer to the 
first para quoted at the top of this message.

> 
> > 
> >> Would it help to do the message transfer without DMA?
> > 
> > I would, just to see more clearly what was going on (command queue, 
> > control lines, bus phase).
> 
> Not sure we can get control lines read - bus phases are available IIRC.
> 
> > Recall that Tuomas said he tried PIO for this with zorro_esp without 
> > any improvement. And I first saw this bug on a Quadra 660av, on which 
> > mac_esp uses PIO for all transfers.
> 
> PIO does get set up for the expected number of message bytes though. Is 
> there a check for phase mismatch during PIO?
> 

There is in mac_esp. But you really need to try it.

> > 
> > If only the initator drives ATN, the documentation can only be 
> > referring to an ESP device in target mode.
> 
> The documentation mentions a command descriptor block to be transferred
> - would only apply to ESP in target mode again.
> 
> > My recollection from 2009 is that this particular disk did work with 
> > mac_esp on one of my Quadras, but not on my 660av. And three people have 
> > reported the problem now (to my knowledge) so I don't blame the disks.
> 
> What ESP versions are used in the Quadras vs 660av?

All are ESP236 according to esp_scsi detection.

-- 

> 
> Cheers,
> 
> 	Michael
> 
> 
>