Re: [edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

2019-02-19 Thread Laszlo Ersek
On 02/19/19 09:49, Gerd Hoffmann wrote:
>   Hi,
> 
>> (2) If I change the cmdline to "lun=5", then the exchange is:
> 
> Not supported (by the usb protocol).
> 
> The protocol has a control message to query the number of devices (grep
> for GetMaxLun in qemu).  LUNs are not allowed to be sparse.  So, with a
> single storage device the LUN must be zero.  With two devices the LUNs
> must be 0,1, with three 0,1,2 etc., you get the idea :)

Yes, Phil explained the same -- I originally missed this part of the
documentation (because the requirement is documented, so the fault is
clearly mine).

However, once I fixed the configuration (so that I'd have LUNs 0 and 1),
edk2 still tripped an assert. Please see my message elsewhere in this
thread; the fixed config case is marked as item (4).

Thanks!
Laszlo

> Maybe usb-bot should check for that and throw an error, I think right
> now we only have the generic scsi code check which will verify the lun
> isn't too big (<= 15) but will not check the non-sparse requirement.
> 
> cheers,
>   Gerd
> 

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


Re: [edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

2019-02-19 Thread Gerd Hoffmann
  Hi,

> (2) If I change the cmdline to "lun=5", then the exchange is:

Not supported (by the usb protocol).

The protocol has a control message to query the number of devices (grep
for GetMaxLun in qemu).  LUNs are not allowed to be sparse.  So, with a
single storage device the LUN must be zero.  With two devices the LUNs
must be 0,1, with three 0,1,2 etc., you get the idea :)

Maybe usb-bot should check for that and throw an error, I think right
now we only have the generic scsi code check which will verify the lun
isn't too big (<= 15) but will not check the non-sparse requirement.

cheers,
  Gerd

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


Re: [edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

2019-02-13 Thread Laszlo Ersek
On 02/13/19 18:00, Philippe Mathieu-Daudé wrote:
> On 2/13/19 9:37 AM, Laszlo Ersek wrote:
>>
>> using QEMU, when I specify a nonzero LUN for the hard disk that sits on
>> the "SCSI bus" that embodies the USB Bulk Only Transfer device, then
>> UsbMassStorageDxe fails to recognize the hard disk.
>>
>> (1) Consider the following QEMU command line snippet:
>>
>>   -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
>>   -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
>>   -device usb-bot,bus=xhci1.0,port=3,id=bot1 \
> 
> Do you have a specific need to use the 'usb-bot' device?

Nothing beyond .

>>   -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \

[...]

>> In this case, edk2 recognizes the disk and things work fine.
>>
>> (In fact, for lun=0, the QemuBootOrderLib pattern matching / translation
>> works fine as well -- verifying which was my original goal, before I ran
>> into the issues below, for nonzero LUNs. But, I digress.)
>>
>>
>> (2) If I change the cmdline to "lun=5", then the exchange is:
> 
> From qemu/docs/usb-storage.txt:
> 
>   The LUN numbers must be continuous, i.e. for three devices you must
>   use 0+1+2. The 0+1+5 numbering from the "usb-uas" example isn't going
>   to work with "usb-bot".
> 
> A failure is expected :/

OK, that explains the issue in (2). Wrong config. Thanks!

[...]

>> (3) Starting again from the original command line, if I change "lun=0"
>> to "lun=1" (rather than to "lun=5"), then OVMF even hangs, with the
>> following log:

[...]

>>> ASSERT MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c(1915): TrsRing != ((void *) 
>>> 0)
>>
>> In this case, edk2 seems to recognize that a nonzero LUN is available on
>> the target, but the initialization never completes, and then an
>> assertion fails, apparently in the lower-level XHCI transport code.
> 
> Can you try using the 'usb-uas' device instead of the 'usb-bot'?

Thanks, but no, thanks. :)

For USB storage options, I prefer the absolute minimum. I thought that
usb-storage was the end of the story -- it works perfectly fine; please
see the scope in:
- https://bugzilla.redhat.com/show_bug.cgi?id=1458192
- https://github.com/tianocore/edk2/commit/f9c59fa44ae2

Due to , usb-bot now
looks relevant as well. I'm trying to see how that maps to the existent
usb-storage support code, and what extensions if any are needed.

"usb-uas" remains totally out of scope though.

--*--

Anyway, now I realize that my test (3) was invalid too, because, by
*changing* lun0 to lun1 (rather than adding lun1 after lun0), I again
created a discontiguous LUN space.

(4) Unfortunately, the same assertion failure hits in edk2, even if I
add *both* lun0 and lun1:

  -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
  -drive id=disk2,if=none,format=raw,readonly,file=$APPDISK \
  -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
  -device usb-bot,bus=xhci1.0,port=4,id=bot1 \
  -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \
  -device scsi-hd,drive=disk2,bus=bot1.0,lun=1,bootindex=2 \

Based on the last paragraphs in "docs/usb-storage.txt" (specifically
step (2b)), I'd expect this to work -- do you agree?

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


Re: [edk2] nonzero LUN on USB Bulk Only Transfer fails with QEMU+edk2

2019-02-13 Thread Philippe Mathieu-Daudé
Hi Laszlo,

On 2/13/19 9:37 AM, Laszlo Ersek wrote:
> Hi,
> 
> using QEMU, when I specify a nonzero LUN for the hard disk that sits on
> the "SCSI bus" that embodies the USB Bulk Only Transfer device, then
> UsbMassStorageDxe fails to recognize the hard disk.
> 
> (1) Consider the following QEMU command line snippet:
> 
>   -drive id=disk1,if=none,format=raw,readonly,file=$APPDISK \
>   -device qemu-xhci,id=xhci1,p3=15,addr=02.0 \
>   -device usb-bot,bus=xhci1.0,port=3,id=bot1 \

Do you have a specific need to use the 'usb-bot' device?

>   -device scsi-hd,drive=disk1,bus=bot1.0,lun=0,bootindex=1 \
> 
> For this (i.e., lun=0), I see the following INQUIRY exchange between
> UsbMassStorageDxe and QEMU:
> 
>> Lun=0 InquiryCmd {
>> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
>> Lun=0 InquiryCmd }
>> Lun=0 InquiryData {
>> Lun=0 InquiryData 00 00 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
>> Lun=0 InquiryData 10 51 45 4D 55 20 48 41 52 44 44 49 53 4B 20 20 20
>> Lun=0 InquiryData 20 32 2E 35 2B
>> Lun=0 InquiryData }
> 
> The vendor and product ID fields translate to "QEMU" and
> "QEMU_HARDDISK___", respectively. (For easier reading, I replaced the
> space characters with underscores, for the sake of better
> readability/wrapping.)
> 
> In this case, edk2 recognizes the disk and things work fine.
> 
> (In fact, for lun=0, the QemuBootOrderLib pattern matching / translation
> works fine as well -- verifying which was my original goal, before I ran
> into the issues below, for nonzero LUNs. But, I digress.)
> 
> 
> (2) If I change the cmdline to "lun=5", then the exchange is:

>From qemu/docs/usb-storage.txt:

  The LUN numbers must be continuous, i.e. for three devices you must
  use 0+1+2. The 0+1+5 numbering from the "usb-uas" example isn't going
  to work with "usb-bot".

A failure is expected :/

> 
>> Lun=0 InquiryCmd {
>> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
>> Lun=0 InquiryCmd }
>> Lun=0 InquiryData {
>> Lun=0 InquiryData 00 3F 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
>> Lun=0 InquiryData 10 51 45 4D 55 20 54 41 52 47 45 54 20 20 20 20 20
>> Lun=0 InquiryData 20 32 2E 35 00
>> Lun=0 InquiryData }
> 
> Here the product ID is unchanged, but the vendor ID becomes
> "QEMU_TARGET_".
> 
> And, edk2 fails to recognize the device:
> 
>> UsbBootGetParams: Found an unsupported peripheral type[31]
>> UsbMassInitMedia: UsbBootGetParams (Unsupported)
>> UsbMassInitNonLun: UsbMassInitMedia (Unsupported)
>> USBMassDriverBindingStart: UsbMassInitNonLun (Unsupported)
> 
> This happens because the first byte in the response is 0x3F. QEMU sets
> this byte in
> 
>> r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;
> 
> in function scsi_target_emulate_inquiry(), file "hw/scsi/scsi-bus.c".
> 
> And edk2 parses the byte in UsbBootInquiry() and UsbBootGetParams()
> (file "MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c"):
> 
>> typedef struct {
>>   UINT8 Pdt;///< Peripheral Device Type (low 5 bits)
>>   UINT8 Removable;  ///< Removable Media (highest bit)
>>   UINT8 Reserved0[2];
>>   UINT8 AddLen; ///< Additional length
>>   UINT8 Reserved1[3];
>>   UINT8 VendorID[8];
>>   UINT8 ProductID[16];
>>   UINT8 ProductRevision[4];
>> } USB_BOOT_INQUIRY_DATA;
> 
>> #define USB_BOOT_PDT(Pdt)   ((Pdt) & 0x1f)
> 
>>   UsbMass->Pdt  = (UINT8) (USB_BOOT_PDT (UsbMass->InquiryData.Pdt));
> 
>>   //
>>   // According to USB Mass Storage Specification for Bootability, only 
>> following
>>   // 4 Peripheral Device Types are in spec.
>>   //
>>   if ((UsbMass->Pdt != USB_PDT_DIRECT_ACCESS) &&
>>(UsbMass->Pdt != USB_PDT_CDROM) &&
>>(UsbMass->Pdt != USB_PDT_OPTICAL) &&
>>(UsbMass->Pdt != USB_PDT_SIMPLE_DIRECT)) {
>> DEBUG ((EFI_D_ERROR, "UsbBootGetParams: Found an unsupported peripheral 
>> type[%d]\n", UsbMass->Pdt));
>> return EFI_UNSUPPORTED;
>>   }
> 
> It looks likely that at least one of edk2 and QEMU has a bug. Which one
> is it?

Both implementation looks correct at first glance, QEMU replies a
standard SCSI INQUIRY format, while EDK2 expects a "USB Mass Storage
Specification
 for Bootability, Revision 1.0" INQUIRY format.

> Or is the command line incorrect perhaps? (I.e., is it invalid to
> specify *only* lun=5? Does the LUN space have to be contiguous?)
> 
> 
> (3) Starting again from the original command line, if I change "lun=0"
> to "lun=1" (rather than to "lun=5"), then OVMF even hangs, with the
> following log:
> 
>> UsbMassInitMultiLun: Start to initialize No.0 logic unit
>> Lun=0 InquiryCmd {
>> Lun=0 InquiryCmd 00 12 00 00 00 24 00 00 00 00 00 00 00
>> Lun=0 InquiryCmd }
>> Lun=0 InquiryData {
>> Lun=0 InquiryData 00 3F 00 05 12 1F 00 00 10 51 45 4D 55 20 20 20 20
>> Lun=0 InquiryData 10 51 45 4D 55 20 54 41 52 47 45 54 20 20 20 20 20
>> Lun=0 InquiryData 20 32 2