[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] cdrom disc type - is this patch correct? (unbreaks rec
|
From: |
Juergen Lock |
|
Subject: |
Re: [Qemu-devel] cdrom disc type - is this patch correct? (unbreaks recent FreeBSD guest's -cdrom access) |
|
Date: |
Mon, 19 Nov 2007 00:37:05 +0100 (CET) |
Oops, I seem to have missed this post, sorry for not answering earlier...
In article <address@hidden> you write:
>On Tue, Nov 13 2007, Juergen Lock wrote:
>> Hi!
>>
>> Yesterday I learned that FreeBSD 7.0-BETA2 guests will no longer
>> read from the emulated cd drive, apparently because of this commit:
>>
>http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/ata/atapi-cd.c.diff?r1=1.193;r2=1.193.2.1
>> The following patch file added to the qemu-devel port fixes the issue
>> for me, is it also correct? (making the guest see a dvd in the drive
>> when it is inserted, previously it saw the drive as empty.)
>>
>> The second hunk is already in qemu cvs so remove it if you want to
>> test on that. ISO used for testing:
>>
>ftp://ftp.freebsd.org:/pub/FreeBSD/ISO-IMAGES-i386/7.0/7.0-BETA2-i386-disc1.iso
>> (test by either selecting fixit->cdrom or by trying to install, just
>> booting it will always work because that goes thru the bios.)
>>
>> Index: qemu/hw/ide.c
>> @@ -1339,6 +1341,8 @@
>> case 0x2a:
>> cpu_to_ube16(&buf[0], 28 + 6);
>> buf[2] = 0x70;
>> + if (bdrv_is_inserted(s->bs))
>> + buf[2] = 0x40;
>
>medium type code has been obsoleted since at least 1999. Looking back at
>even older docs, 0x70 is 'door closed, no disc present'. 0x40 is a
>reserved value though, I would not suggest using that. Given that
>freebsd breaks, my suggest change would be the below - keep the 0x70 for
>when no disc is really inserted, but don't set anything if there is.
>
>diff --git a/hw/ide.c b/hw/ide.c
>index 5f76c27..52d4c78 100644
>--- a/hw/ide.c
>+++ b/hw/ide.c
>@@ -1344,7 +1344,10 @@ static void ide_atapi_cmd(IDEState *s)
> break;
> case 0x2a:
> cpu_to_ube16(&buf[0], 28 + 6);
>- buf[2] = 0x70;
>+ if (!bdrv_is_inserted(s->bs))
>+ buf[2] = 0x70;
>+ else
>+ buf[2] = 0;
> buf[3] = 0;
> buf[4] = 0;
> buf[5] = 0;
>
Well that (0) doesn't work. The relevant FreeBSD header,
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/ata/atapi-cd.h?rev=1.46.2.1;content-type=text%2Fx-cvsweb-markup
, defines the following:
[...]
/* CDROM Capabilities and Mechanical Status Page */
struct cappage {
/* mode page data header */
u_int16_t data_length;
u_int8_t medium_type;
#define MST_TYPE_MASK_LOW 0x0f
#define MST_FMT_NONE 0x00
#define MST_DATA_120 0x01
#define MST_AUDIO_120 0x02
#define MST_COMB_120 0x03
#define MST_PHOTO_120 0x04
#define MST_DATA_80 0x05
#define MST_AUDIO_80 0x06
#define MST_COMB_80 0x07
#define MST_PHOTO_80 0x08
#define MST_TYPE_MASK_HIGH 0x70
#define MST_CDROM 0x00
#define MST_CDR 0x10
#define MST_CDRW 0x20
#define MST_DVD 0x40
#define MST_NO_DISC 0x70
#define MST_DOOR_OPEN 0x71
#define MST_FMT_ERROR 0x72
u_int8_t dev_spec;
u_int16_t unused;
u_int16_t blk_desc_len;
/* capabilities page */
u_int8_t page_code;
#define ATAPI_CDROM_CAP_PAGE 0x2a
u_int8_t param_len;
u_int16_t media;
#define MST_READ_CDR 0x0001
#define MST_READ_CDRW 0x0002
#define MST_READ_PACKET 0x0004
#define MST_READ_DVDROM 0x0008
#define MST_READ_DVDR 0x0010
#define MST_READ_DVDRAM 0x0020
#define MST_WRITE_CDR 0x0100
#define MST_WRITE_CDRW 0x0200
#define MST_WRITE_TEST 0x0400
#define MST_WRITE_DVDR 0x1000
#define MST_WRITE_DVDRAM 0x2000
u_int16_t capabilities;
#define MST_AUDIO_PLAY 0x0001
#define MST_COMPOSITE 0x0002
#define MST_AUDIO_P1 0x0004
#define MST_AUDIO_P2 0x0008
#define MST_MODE2_f1 0x0010
#define MST_MODE2_f2 0x0020
#define MST_MULTISESSION 0x0040
#define MST_BURNPROOF 0x0080
#define MST_READ_CDDA 0x0100
#define MST_CDDA_STREAM 0x0200
#define MST_COMBINED_RW 0x0400
#define MST_CORRECTED_RW 0x0800
#define MST_SUPPORT_C2 0x1000
#define MST_ISRC 0x2000
#define MST_UPC 0x4000
u_int8_t mechanism;
#define MST_LOCKABLE 0x01
#define MST_LOCKED 0x02
#define MST_PREVENT 0x04
#define MST_EJECT 0x08
#define MST_MECH_MASK 0xe0
#define MST_MECH_CADDY 0x00
#define MST_MECH_TRAY 0x20
#define MST_MECH_POPUP 0x40
#define MST_MECH_CHANGER 0x80
#define MST_MECH_CARTRIDGE 0xa0
uint8_t audio;
#define MST_SEP_VOL 0x01
#define MST_SEP_MUTE 0x02
u_int16_t max_read_speed; /* max raw data rate in bytes/1000 */
u_int16_t max_vol_levels; /* number of discrete volume levels */
u_int16_t buf_size; /* internal buffer size in bytes/1024 */
u_int16_t cur_read_speed; /* current data rate in bytes/1000 */
u_int8_t reserved3;
u_int8_t misc;
u_int16_t max_write_speed; /* max raw data rate in bytes/1000 */
u_int16_t cur_write_speed; /* current data rate in bytes/1000 */
u_int16_t copy_protect_rev;
u_int16_t reserved4;
};
[...]
and in
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/ata/atapi-cd.c?rev=1.193.2.1;content-type=text%2Fx-cvsweb-markup
a check is done like this:
[...]
static int
acd_geom_access(struct g_provider *pp, int dr, int dw, int de)
{
device_t dev = pp->geom->softc;
struct acd_softc *cdp = device_get_ivars(dev);
int timeout = 60, track;
/* check for media present, waiting for loading medium just in case */
while (timeout--) {
if (!acd_mode_sense(dev, ATAPI_CDROM_CAP_PAGE,
(caddr_t)&cdp->cap, sizeof(cdp->cap)) &&
cdp->cap.page_code == ATAPI_CDROM_CAP_PAGE) {
if ((cdp->cap.medium_type == MST_FMT_NONE) ||
(cdp->cap.medium_type == MST_NO_DISC) ||
(cdp->cap.medium_type == MST_DOOR_OPEN) ||
(cdp->cap.medium_type == MST_FMT_ERROR))
return EIO;
else
break;
}
pause("acdld", hz / 2);
}
[...]
There have been reports of this also being broken on real hw tho, like,
http://lists.freebsd.org/pipermail/freebsd-current/2007-November/079760.html
so I'm not sure what to make of this...
Thanx,
Juergen