Re: [edk2] [PATCH 0/2] MdeModulePkg/.../IdeMode: restore handling of "short reads"

2016-01-19 Thread Laszlo Ersek 
On 01/19/16 01:34, Laszlo Ersek wrote:
> The series
> 
>   [edk2] [patch 0/2] Fix a DVD device compatbility issue
>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/3681
> 
> fixed an IDE timeout masking (misreporting) bug in the AtaAtapiPassThru
> driver. (And, separately, it increased SCSI disk timeouts to 30s.)
> However, the IDE change that improved the reporting of timeouts
> regressed the handling of short reads.
> 
> Users reported this in ,
> running the code in question as part of OVMF, on QEMU's i440fx IDE
> emulation. These patches aim to fix the short reads.
> 
> Cc: Feng Tian 
> Cc: Star Zeng 
> Cc: John Snow 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   MdeModulePkg/.../IdeMode: actualize DRQReady*() comment blocks
>   MdeModulePkg/.../IdeMode: report early finish of packet read as
> success
> 
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 37 
>  1 file changed, 31 insertions(+), 6 deletions(-)
> 

Committed to SVN as r19684 and r19685.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/2] MdeModulePkg/.../IdeMode: restore handling of "short reads"

2016-01-19 Thread Laszlo Ersek 
On 01/19/16 06:40, Tian, Feng wrote:
> Laszlo & John
> 
> Thanks for your detail analysis. It's very helpful for me.
> 
> Reviewed-by: Feng Tian 

Thank you! I'll soon commit the patches with your R-b.

> I am thinking shall we update
> EFI_SCSI_INQUIRY_DATA/ATAPI_INQUIRY_DATA data structure as well to
> only contain the first 36 bytes?  According to ATAPI SFF 8020i & SPC4
> spec, they all state the inquiry data includes at least 36 bytes. But
> we defines it as 96 bytes or more.

I checked the SPC4 ("6.4.2 Standard INQUIRY data"). Indeed it says "at
least 36 bytes".

However, the diagram ("Table 137 — Standard INQUIRY data format") does
list a bunch of optional data in the [36, 96) left-inclusive,
right-exclusive offset range: "CLOCKING", "QAS", "IUS", up to eight
vendor descriptors, and other vendor-specific data.

Although all these seem to be lumped together in the
EFI_SCSI_INQUIRY_DATA.Reserved_5_95 field, I think it's useful to carry
them there. That field can simplify debugging, debug dumping inquiry
data, and maybe even allow some drivers to check for vendor-specific stuff.

Also, there could be edk2 forks (open source or proprietary) that rely
on the current *sizes* of EFI_SCSI_INQUIRY_DATA and ATAPI_INQUIRY_DATA.

After all, these structures are defined in:
- MdePkg/Include/IndustryStandard/Scsi.h
- MdePkg/Include/IndustryStandard/Atapi.h

which seems to imply client platforms are allowed to include them, and
more-or-less rely on them being "stable".

Just my opinion of course.

Thanks!
Laszlo



> 
> Thanks
> Feng
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Tuesday, January 19, 2016 08:35
> To: edk2-de...@ml01.01.org
> Cc: Tian, Feng; Zeng, Star; John Snow
> Subject: [PATCH 0/2] MdeModulePkg/.../IdeMode: restore handling of "short 
> reads"
> 
> The series
> 
>   [edk2] [patch 0/2] Fix a DVD device compatbility issue
>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/3681
> 
> fixed an IDE timeout masking (misreporting) bug in the AtaAtapiPassThru 
> driver. (And, separately, it increased SCSI disk timeouts to 30s.) However, 
> the IDE change that improved the reporting of timeouts regressed the handling 
> of short reads.
> 
> Users reported this in ,
> running the code in question as part of OVMF, on QEMU's i440fx IDE emulation. 
> These patches aim to fix the short reads.
> 
> Cc: Feng Tian 
> Cc: Star Zeng 
> Cc: John Snow 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   MdeModulePkg/.../IdeMode: actualize DRQReady*() comment blocks
>   MdeModulePkg/.../IdeMode: report early finish of packet read as
> success
> 
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 37 
>  1 file changed, 31 insertions(+), 6 deletions(-)
> 
> --
> 1.8.3.1
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 0/2] MdeModulePkg/.../IdeMode: restore handling of "short reads"

2016-01-18 Thread Laszlo Ersek 
The series

  [edk2] [patch 0/2] Fix a DVD device compatbility issue
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/3681

fixed an IDE timeout masking (misreporting) bug in the AtaAtapiPassThru
driver. (And, separately, it increased SCSI disk timeouts to 30s.)
However, the IDE change that improved the reporting of timeouts
regressed the handling of short reads.

Users reported this in ,
running the code in question as part of OVMF, on QEMU's i440fx IDE
emulation. These patches aim to fix the short reads.

Cc: Feng Tian 
Cc: Star Zeng 
Cc: John Snow 

Thanks
Laszlo

Laszlo Ersek (2):
  MdeModulePkg/.../IdeMode: actualize DRQReady*() comment blocks
  MdeModulePkg/.../IdeMode: report early finish of packet read as
success

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 37 
 1 file changed, 31 insertions(+), 6 deletions(-)

-- 
1.8.3.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/2] MdeModulePkg/.../IdeMode: restore handling of "short reads"

2016-01-18 Thread Tian, Feng 
Laszlo & John

Thanks for your detail analysis. It's very helpful for me.

Reviewed-by: Feng Tian 

I am thinking shall we update EFI_SCSI_INQUIRY_DATA/ATAPI_INQUIRY_DATA data 
structure as well to only contain the first 36 bytes?  According to ATAPI SFF 
8020i & SPC4 spec, they all state the inquiry data includes at least 36 bytes. 
But we defines it as 96 bytes or more.

Thanks
Feng

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, January 19, 2016 08:35
To: edk2-de...@ml01.01.org
Cc: Tian, Feng; Zeng, Star; John Snow
Subject: [PATCH 0/2] MdeModulePkg/.../IdeMode: restore handling of "short reads"

The series

  [edk2] [patch 0/2] Fix a DVD device compatbility issue
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/3681

fixed an IDE timeout masking (misreporting) bug in the AtaAtapiPassThru driver. 
(And, separately, it increased SCSI disk timeouts to 30s.) However, the IDE 
change that improved the reporting of timeouts regressed the handling of short 
reads.

Users reported this in ,
running the code in question as part of OVMF, on QEMU's i440fx IDE emulation. 
These patches aim to fix the short reads.

Cc: Feng Tian 
Cc: Star Zeng 
Cc: John Snow 

Thanks
Laszlo

Laszlo Ersek (2):
  MdeModulePkg/.../IdeMode: actualize DRQReady*() comment blocks
  MdeModulePkg/.../IdeMode: report early finish of packet read as
success

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 37 
 1 file changed, 31 insertions(+), 6 deletions(-)

--
1.8.3.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel