On 08/04/17 12:49, Paolo Bonzini wrote:
> On 04/08/2017 10:36, Hannes Reinecke wrote:
>> The LUN0 emulation is just that, an emulation for a non-existing
>> LUN0. So we should be returning LUN_NOT_SUPPORTED for any request
>> coming from any other LUN.
>> And we should be aborting unhandled commands with INVALID OPCODE,
>> not LUN NOT SUPPORTED.
>>
>> Signed-off-by: Hannes Reinecke <h...@suse.com>
>> ---
>> hw/scsi/scsi-bus.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index 8419c75..79a222f 100644
>> --- a/hw/scsi/scsi-bus.c
>> +++ b/hw/scsi/scsi-bus.c
>> @@ -583,6 +583,11 @@ static int32_t scsi_target_send_command(SCSIRequest
>> *req, uint8_t *buf)
>> {
>> SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
>>
>> + if (req->lun != 0) {
>> + scsi_req_build_sense(req, SENSE_CODE(LUN_NOT_SUPPORTED));
>> + scsi_req_complete(req, CHECK_CONDITION);
>> + return 0;
>> + }
>> switch (buf[0]) {
>> case REPORT_LUNS:
>> if (!scsi_target_emulate_report_luns(r)) {
>> @@ -613,7 +618,7 @@ static int32_t scsi_target_send_command(SCSIRequest
>> *req, uint8_t *buf)
>> case TEST_UNIT_READY:
>> break;
>> default:
>> - scsi_req_build_sense(req, SENSE_CODE(LUN_NOT_SUPPORTED));
>> + scsi_req_build_sense(req, SENSE_CODE(INVALID_OPCODE));
>> scsi_req_complete(req, CHECK_CONDITION);
>> return 0;
>> illegal_request:
>>
>
> I am queuing this one since it's an independent bugfix.
This patch (ded6ddc5a7b9, "scsi: clarify sense codes for LUN0
emulation", 2017-08-04) seems to confuse the media detection in edk2's
"MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c".
Namely, when it enumerates the {targets}x{LUNs} matrix on the
virtio-scsi HBA, it now reports the following message, for each
(target,LUN) pair to which no actual SCSI device (like disk or CD-ROM)
is assigned on the command line:
ScsiDisk: Sense Key = 0x5 ASC = 0x25!
Unfortunately, this is not all that happens -- the ScsiDiskDxe driver
even installs a BlockIo protocol instance on the handle (again, there is
no media, and no actual SCSI device), on which further protocols are
stacked, such as BlockIo2:
ScsiDisk: Sense Key = 0x5 ASC = 0x25!
InstallProtocolInterface: [EfiBlockIoProtocol] 13A59A3A8
InstallProtocolInterface: [EfiBlockIo2Protocol] 13A59A3D8
InstallProtocolInterface: [EfiDiskInfoProtocol] 13A59A4D0
In turn, in BDS, UEFI boot options are auto-generated for these devices,
which is not nice, given that this procedure in BDS is very
pflash-intensive, and pflash access is remarkably slow on aarch64 KVM.
For example, if I use one virtio-scsi HBA, and put a CD-ROM on target 0,
LUN 0, and a disk on target 1, LUN 0, then edk2 will create protocol
interfaces, and matching boot options, for
2 targets * 7 LUNs/target = 14 LUNs
of which only 2 make sense.
If I revert the patch (on top of v2.10.0-rc3), then everything works as
before -- BlockIo protocol instances are produced only for actual
devices (with media).
I guess the path forward is to fix the ScsiDiskDxe driver in edk2; the
new ASC should be recognized.
My question is, how *exactly* did this patch change the reported sense
key and ASC? That is, what did they use to be *before*? INVALID_OPCODE?
Thanks!
Laszlo
