[Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
The following patch implements fixes to the CD-ROM IDE/ATAPI emulation
since ide.c revision 1.66 and that prevents installation of OpenSolaris
guests because of timeouts like :
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1);
timeout: abort request, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: abort device, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: reset target, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: reset bus, target=0 lun=0
It has been tested in Gentoo Linux 2007.0 amd64 (64bit) and x86 (32bit) hosts
using several different versions of Linux, FreeBSD, OpenBSD, NetBSD,
DragonFlyBSD, OpenSolaris and Windows 2000 guests
The patch implements changes to the following IDE commands as described by :
* change the model name (used by "INQUIRY" and "IDENTIFY DEVICE") to DVD
* recognize and honor "Allocation Length" parameter in "GET CONFIGURATION"
* only set "current profile" for the "GET CONFIGURATION" response if a profile
is current (CD or DVD loaded)
* calculate "data length" including all headers
* refactor code and add comments to help document references to all
implemented standards (ATAPI-4, SPC-3 and MMC-6)
Carlo
---
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.79
diff -u -p -r1.79 ide.c
--- hw/ide.c24 Dec 2007 14:33:24 - 1.79
+++ hw/ide.c26 Dec 2007 07:11:01 -
@@ -1,5 +1,5 @@
/*
- * QEMU IDE disk and CD-ROM Emulator
+ * QEMU IDE disk and CD/DVD-ROM Emulator
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
@@ -540,7 +540,7 @@ static void ide_atapi_identify(IDEState
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */
+padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
#ifdef USE_DMA_CDROM
put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -1634,12 +1634,13 @@ static void ide_atapi_cmd(IDEState *s)
buf[6] = 0; /* reserved */
buf[7] = 0; /* reserved */
padstr8(buf + 8, 8, "QEMU");
-padstr8(buf + 16, 16, "QEMU CD-ROM");
+padstr8(buf + 16, 16, "QEMU DVD-ROM");
padstr8(buf + 32, 4, QEMU_VERSION);
ide_atapi_cmd_reply(s, 36, max_len);
break;
case GPCMD_GET_CONFIGURATION:
{
+uint32_t len;
uint64_t total_sectors;
/* only feature 0 is supported */
@@ -1648,17 +1649,27 @@ static void ide_atapi_cmd(IDEState *s)
ASC_INV_FIELD_IN_CMD_PACKET);
break;
}
-memset(buf, 0, 32);
+max_len = ube16_to_cpu(packet + 7);
bdrv_get_geometry(s->bs, &total_sectors);
-buf[3] = 16;
-buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current
profile */
-buf[10] = 0x10 | 0x1;
-buf[11] = 0x08; /* size of profile list */
+memset(buf, 0, 32);
+if (total_sectors) {
+if (total_sectors > 1433600) {
+buf[7] = 0x10; /* DVD-ROM */
+} else {
+buf[7] = 0x08; /* CD-ROM */
+}
+} else {
+buf[7] = 0x00; /* no current profile */
+}
+buf[10] = 0x02 | 0x01; /* persistent and current */
+buf[11] = 0x08; /* size of profile list = 4 bytes per profile */
buf[13] = 0x10; /* DVD-ROM profile */
-buf[14] = buf[7] == 0x10; /* (in)active */
+buf[14] = buf[13] == buf[7]; /* (in)active */
buf[17] = 0x08; /* CD-ROM profile */
-buf[18] = buf[7] == 0x08; /* (in)active */
-ide_atapi_cmd_reply(s, 32, 32);
+buf[18] = buf[17] == buf[7]; /* (in)active */
+len = 8 + 4 + buf[11]; /* headers + size of profile list */
+cpu_to_ube32(buf, len - 4); /* data length */
+ide_atapi_cmd_reply(s, len, max_len);
break;
}
default:
[Qemu-devel] [PATCH] ensure all invocations to bdrv_{read, write} use (uint8_t *) for its third parameter
Trivial fix that ensures that all buffers used for bdrv_read or bdrv_write
are from an array of the uint8_t type
Carlo
---
Index: block-vvfat.c
===
RCS file: /sources/qemu/qemu/block-vvfat.c,v
retrieving revision 1.16
diff -u -p -r1.16 block-vvfat.c
--- block-vvfat.c 24 Dec 2007 13:26:04 - 1.16
+++ block-vvfat.c 4 Jan 2008 07:57:20 -
@@ -340,7 +340,7 @@ typedef struct BDRVVVFATState {
int current_fd;
mapping_t* current_mapping;
unsigned char* cluster; /* points to current cluster */
-unsigned char* cluster_buffer; /* points to a buffer to hold temp data */
+uint8_t* cluster_buffer; /* points to a buffer to hold temp data */
unsigned int current_cluster;
/* write support */
Index: block.c
===
RCS file: /sources/qemu/qemu/block.c,v
retrieving revision 1.53
diff -u -p -r1.53 block.c
--- block.c 24 Dec 2007 16:10:43 - 1.53
+++ block.c 4 Jan 2008 07:57:21 -
@@ -459,7 +459,7 @@ int bdrv_commit(BlockDriverState *bs)
BlockDriver *drv = bs->drv;
int64_t i, total_sectors;
int n, j;
-unsigned char sector[512];
+uint8_t sector[512];
if (!drv)
return -ENOMEDIUM;
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
Can someone please comment on the mergability of this patch? or in what needs
to be done to it so that it can be committed?
The patch is still current and the bug it fixes would otherwise prevent
OpenSolaris guests to be installed in qemu. the MMC-6 command it fixes (GET
CONFIGURATION) has been verified to behave as per the SPECs and compared to
behave just like real hardware does.
Carlo
On Wed, Dec 26, 2007 at 01:36:15AM -0600, Carlo Marcelo Arenas Belon wrote:
> The following patch implements fixes to the CD-ROM IDE/ATAPI emulation
> since ide.c revision 1.66 and that prevents installation of OpenSolaris
> guests because of timeouts like :
>
> WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1);
> timeout: abort request, target=0 lun=0
> WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
> timeout: abort device, target=0 lun=0
> WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
> timeout: reset target, target=0 lun=0
> WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
> timeout: reset bus, target=0 lun=0
>
> It has been tested in Gentoo Linux 2007.0 amd64 (64bit) and x86 (32bit) hosts
> using several different versions of Linux, FreeBSD, OpenBSD, NetBSD,
> DragonFlyBSD, OpenSolaris and Windows 2000 guests
>
> The patch implements changes to the following IDE commands as described by :
>
> * change the model name (used by "INQUIRY" and "IDENTIFY DEVICE") to DVD
> * recognize and honor "Allocation Length" parameter in "GET CONFIGURATION"
> * only set "current profile" for the "GET CONFIGURATION" response if a profile
> is current (CD or DVD loaded)
> * calculate "data length" including all headers
> * refactor code and add comments to help document references to all
> implemented standards (ATAPI-4, SPC-3 and MMC-6)
>
> Carlo
> ---
> Index: hw/ide.c
> ===
> RCS file: /sources/qemu/qemu/hw/ide.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 ide.c
> --- hw/ide.c 24 Dec 2007 14:33:24 - 1.79
> +++ hw/ide.c 26 Dec 2007 07:11:01 -
> @@ -1,5 +1,5 @@
> /*
> - * QEMU IDE disk and CD-ROM Emulator
> + * QEMU IDE disk and CD/DVD-ROM Emulator
> *
> * Copyright (c) 2003 Fabrice Bellard
> * Copyright (c) 2006 Openedhand Ltd.
> @@ -540,7 +540,7 @@ static void ide_atapi_identify(IDEState
> put_le16(p + 21, 512); /* cache size in sectors */
> put_le16(p + 22, 4); /* ecc bytes */
> padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
> -padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */
> +padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
> put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
> #ifdef USE_DMA_CDROM
> put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
> @@ -1634,12 +1634,13 @@ static void ide_atapi_cmd(IDEState *s)
> buf[6] = 0; /* reserved */
> buf[7] = 0; /* reserved */
> padstr8(buf + 8, 8, "QEMU");
> -padstr8(buf + 16, 16, "QEMU CD-ROM");
> +padstr8(buf + 16, 16, "QEMU DVD-ROM");
> padstr8(buf + 32, 4, QEMU_VERSION);
> ide_atapi_cmd_reply(s, 36, max_len);
> break;
> case GPCMD_GET_CONFIGURATION:
> {
> +uint32_t len;
> uint64_t total_sectors;
>
> /* only feature 0 is supported */
> @@ -1648,17 +1649,27 @@ static void ide_atapi_cmd(IDEState *s)
> ASC_INV_FIELD_IN_CMD_PACKET);
> break;
> }
> -memset(buf, 0, 32);
> +max_len = ube16_to_cpu(packet + 7);
> bdrv_get_geometry(s->bs, &total_sectors);
> -buf[3] = 16;
> -buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current
> profile */
> -buf[10] = 0x10 | 0x1;
> -buf[11] = 0x08; /* size of profile list */
> +memset(buf, 0, 32);
> +if (total_sectors) {
> +if (total_sectors > 1433600) {
> +buf[7] = 0x10; /* DVD-ROM */
> +} else {
> +buf[7] = 0x08; /* CD-ROM */
> +}
> +} else {
> +buf[7] = 0x00; /* no current profile */
> +}
> +buf[10] = 0x02 | 0x01; /* persistent and current */
> +buf[11] = 0x08; /* size of profile list = 4 bytes per profile */
> buf[13] = 0x
Re: [Qemu-devel] [PATCH] ensure all invocations to bdrv_{read, write} use (uint8_t *) for its third parameter
On Fri, Jan 04, 2008 at 01:20:39PM +, Thiemo Seufer wrote:
> Carlo Marcelo Arenas Belon wrote:
> > Trivial fix that ensures that all buffers used for bdrv_read or bdrv_write
> > are from an array of the uint8_t type
>
> Do we have a host where this actually makes a difference?
No that I know of (even if I'd heard horror stories about some bizarre old
architecure where sizeof(char) was 3 which I hope I never ever find),
and sorry for stirring this long thread by not being clear about my objective,
and not coming to clarify it before as I had no access until now to my email.
as you said this patch makes no difference in the logic at all in any
architecture that qemu uses (which is why I said it was a trivial fix)
but is IMHO a more correct version of the code because it is consistently
using the same type everywhere instead of different types with different names
in different places, even if they have the same storage requirements; after
all there is a reason why bdrv_read and bdrv_write where defined as using
(uint8_t *) for their buffer parameter since version 1.1 of the vl.h file.
I came to look for it when a merge conflict in kvm flipped the second
invocation from block.c into an "unsigned char *sector[512]" instead as you
can see by the proposed fix here :
http://article.gmane.org/gmane.comp.emulators.kvm.devel/11510
Carlo
> > ---
> > Index: block-vvfat.c
> > ===
> > RCS file: /sources/qemu/qemu/block-vvfat.c,v
> > retrieving revision 1.16
> > diff -u -p -r1.16 block-vvfat.c
> > --- block-vvfat.c 24 Dec 2007 13:26:04 - 1.16
> > +++ block-vvfat.c 4 Jan 2008 07:57:20 -
> > @@ -340,7 +340,7 @@ typedef struct BDRVVVFATState {
> > int current_fd;
> > mapping_t* current_mapping;
> > unsigned char* cluster; /* points to current cluster */
> > -unsigned char* cluster_buffer; /* points to a buffer to hold temp data
> > */
> > +uint8_t* cluster_buffer; /* points to a buffer to hold temp data */
> > unsigned int current_cluster;
> >
> > /* write support */
> > Index: block.c
> > ===
> > RCS file: /sources/qemu/qemu/block.c,v
> > retrieving revision 1.53
> > diff -u -p -r1.53 block.c
> > --- block.c 24 Dec 2007 16:10:43 - 1.53
> > +++ block.c 4 Jan 2008 07:57:21 -
> > @@ -459,7 +459,7 @@ int bdrv_commit(BlockDriverState *bs)
> > BlockDriver *drv = bs->drv;
> > int64_t i, total_sectors;
> > int n, j;
> > -unsigned char sector[512];
> > +uint8_t sector[512];
> >
> > if (!drv)
> > return -ENOMEDIUM;
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
On Sat, Jan 05, 2008 at 01:02:30AM +, Stuart Brady wrote: > On Fri, Jan 04, 2008 at 06:25:25PM -0600, Rob Landley wrote: > > On Friday 04 January 2008 04:02:07 Carlo Marcelo Arenas Belon wrote: > > > > > > -buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current > > > > profile */ > > > > Where does the constant come from, anyway? > > 1433600? Seems it's the number of 512 KiB blocks in a 700 MiB CD image > (700 * 1024 * 2). yes, that magic constant corresponds to the maximum expected size for a CD-ROM and is used to detect if the profile used should be for DVD-ROM or not. It came from the original implementation patch for "GET CONFIGURATION" committed in revision 1.66 of ide.c by Filip Navarra as "Partial IDE DVD emulation" http://cvs.savannah.nongnu.org/viewvc/qemu/hw/ide.c?root=qemu&r1=1.65&r2=1.66 Carlo
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
On Fri, Jan 04, 2008 at 06:25:25PM -0600, Rob Landley wrote: > On Friday 04 January 2008 04:02:07 Carlo Marcelo Arenas Belon wrote: > > Can someone please comment on the mergability of this patch? or in what > > needs to be done to it so that it can be committed? > > > > The patch is still current and the bug it fixes would otherwise prevent > > OpenSolaris guests to be installed in qemu. the MMC-6 command it fixes > > (GET CONFIGURATION) has been verified to behave as per the SPECs and > > compared to behave just like real hardware does. > > > > Carlo > > Well, I'm no expert but the patch is small enough that I can read it. thanks > > > padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */ > > > -padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */ > > > +padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */ > > > put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) > > > */ #ifdef USE_DMA_CDROM > > > put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */ > > > @@ -1634,12 +1634,13 @@ static void ide_atapi_cmd(IDEState *s) > > > buf[6] = 0; /* reserved */ > > > buf[7] = 0; /* reserved */ > > > padstr8(buf + 8, 8, "QEMU"); > > > -padstr8(buf + 16, 16, "QEMU CD-ROM"); > > > +padstr8(buf + 16, 16, "QEMU DVD-ROM"); > > > padstr8(buf + 32, 4, QEMU_VERSION); > > I take it Solaris is actually checking these strings? Or is this a cosmetic > change? (Not a _bad_ change, I'm just curious.) this is just cosmetic. in 2 of the RESENDs I did it was actually a second optional patch on a series of 2. this RESEND had it all together just because I though I had probably a better chance of having it reviewed if it was 1 mail instead of 3 (including the patch series description) and since I got mostly no feedback until now. the real fix is on the "GET CONFIGURATION" implementation which is done below. > > > @@ -1648,17 +1649,27 @@ static void ide_atapi_cmd(IDEState *s) > > > ASC_INV_FIELD_IN_CMD_PACKET); > > > break; > > > } > > > -memset(buf, 0, 32); > > This moved a few lines down, but that just seems to be churn. it is structured in a way that could be later structured away into a function when/if more profiles are added. this way the first part of the code reads the input parameters and initialized the buffer it will use later to generate a response in the logical order. > > > +max_len = ube16_to_cpu(packet + 7); > > Why are you doing math on a value _before_ you adjust its endianness from a > potentially foreign format into the CPU native one? I take it that gives you > a pointer from which a 16 byte value is fetched? yes, this adds not to the value but the pointer where the packet is stored and that contains on byte 7 (MSB) and 8 (LSB) the value for the Allocation Length parameter as described in Table 275 of T10/1836-D Revision 1 (*) > > > bdrv_get_geometry(s->bs, &total_sectors); > > Calculating stuff to use later rather than modifying buf[]. Ok. I just kept this from the original patch, but it could be extracted instead from s->nb_sectors as well, but though it would be probably easier if I just kept the unrelated changes to a minimum. > > > -buf[3] = 16; > > The new code doesn't directly set buf[3] anymore, leaving it 0 from the > memset. I take it this is now handled by the cpu_to_ube32() targeting 4 > bytes of buf[] much later? yes, the response as described in Table 277 of T10/1836-D Revision 1 contains a 4 byte "Data Length" field (LSB is byte 3) and I am calculating it at the end instead of hardcoding it at the beginning, so that this code could adapt if later extended to add more profiles (CD-RW, HD-DVD, ..). > > > -buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current > > > profile */ > > This becomes 3-state now. You added a state 0 when there's no cdrom in the > drive. The less intrusive change would be keeping the above line and adding > a second line: "if (!total_sectors) buf[7] = 0;" Not actually any larger, > codewise, and a less intrusive change. I refactored this code so that it is hopefully more clear in the intended effect, which is to set the default profile to either of the available profiles depending on the kind of media that was available, and as it is done by real hardware. Again, even if the refactoring was more of an intrusive change, it also allows fo
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
On Sat, Jan 05, 2008 at 10:28:34AM +, Stuart Brady wrote:
> On Fri, Jan 04, 2008 at 09:53:09PM -0600, Rob Landley wrote:
> > Except that according to http://en.wikipedia.org/wiki/CD-ROM it's actually
> > 703
> > and 1/8 binary megabytes (360,000 sectors *2048 bytes), which would be
> > 144.
>
> Apparently that value comes from 75 sectors per second * 80 minutes...
> 75*80*60 = 36, and of course, 36*2048/512 = 144, although
> it actually seems that it should be one sector less than 80 minutes,
> which is 35 2048-byte sectors or 1439996 512-byte chunks.
>
> BTW, there are/were also 90 and 99 minute 'CD-Rs' -- Wikipedia's page on
> CD-Rs describes them, but they were never very popular, and a lot of
> drives can't read the discs.
the exact number of sectors is really not that relevant, as the whole point
here is to try to detect if it is a CD (700MB) or a DVD (4.7GB) and the logic
is just assuming that if it has more sectors than you should normally expect
in a CD, then it is a DVD.
attached the program I used in the guests (only works on Linux) to poke the
emulated drive (or a physical drive if you feel like) and compare the responses
(you will need to take a look at the SPEC tables to interpret the data though)
for my own tests (using a linux guest with -cdrom /dev/cdrom in my linux
host that has a DVD-+RW drive) :
700MB CD-R = 1374880 (with FreeSBIE 2.0.1)
4.7GB DVD-R = 6939520 (with SXDE 9/07)
feel free to report back with the value to use then if you happen to have a CD
that is completely full but I had already enough problems trying to get this
merged without trying to change the code that much to try to guess a better
magic number than the one was originally used (I like 1440000 though)
Carlo
/*
ide-atapi
Copyright (c) 2007 Carlo Marcelo Arenas Belon
ide-atapi is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License version 2
as published by the Free Software Foundation.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include
#include
#include
#include
#include
#include
#include
#include
int main (int argc, char *argv[])
{
struct cdrom_generic_command cgc;
struct request_sense sense;
unsigned char buf[250];
int i;
if (argc < 2) {
printf("Usage: %s \n", argv[0]);
printf("\n");
printf(" device: where the commands are send\n");
printf("\n");
return 1;
}
memset (&cgc, 0, sizeof(struct cdrom_generic_command));
memset (&sense, 0, sizeof(struct request_sense));
memset (&buf, 0, sizeof(buf));
int fd = open (argv[1], O_RDONLY | O_NONBLOCK);
if (fd < 0) {
printf("couldn't open device %s\n", argv[1]);
return 1;
}
cgc.cmd[0] = GPCMD_GET_CONFIGURATION;
cgc.cmd[1] = 0x00;
cgc.cmd[8] = sizeof(buf);
cgc.timeout = 100;
cgc.buffer = buf;
cgc.buflen = sizeof(buf);
cgc.data_direction = CGC_DATA_READ;
cgc.sense = &sense;
cgc.quiet = 0;
i = ioctl (fd, CDROM_SEND_PACKET, &cgc);
if (i < 0) {
printf("command failed\n");
close (fd);
return 1;
}
printf("Response raw dump:\n");
for (i = 0; i
Re: [Qemu-devel] qemu-cvs FreeBSD guests, cirrus, vmwarevga emulation - experimental qemu-devel FreeBSD port update available for testing
On Sun, Jan 06, 2008 at 11:44:50PM +0100, Juergen Lock wrote: > Also, still slirp causes qemu to crash on amd64 hosts when just > trying to access a webpage from inside a guest. I never though slirp will ever work in an amd64 (judging by all the pointer <-> integer size mismatches) or any other LP64 architecture, regardless of the guest OS being used, which is why I never even bother to test. > So, can anyone reproduce any of these problems on e.g. a Linux host? does anyone have a woking setup for slirp for linux 64bit hosts? Carlo
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
On Sun, Jan 06, 2008 at 01:57:38PM +, Stuart Brady wrote: > On Sat, Jan 05, 2008 at 08:22:33PM -0600, Carlo Marcelo Arenas Belon wrote: > > > the exact number of sectors is really not that relevant, as the whole point > > here is to try to detect if it is a CD (700MB) or a DVD (4.7GB) and the > > logic > > is just assuming that if it has more sectors than you should normally expect > > in a CD, then it is a DVD. > > My answer was quite relevant to Rob's question, which was "Where does > the constant come from, anyway?" Yes, and my comment didn't meant it wasn't relevant, but that the exact value isn't as important as finding the upper possible limit that can be used to assume that anything bigger than that has to be a DVD instead. when I looked at the patch originally, tried to find something in the specifications that could be used to define a "standard" CD size but couldn't find anything, because as other people pointed out, there isn't a "standard" CD size even if most of the CDs out there are 80min large. > As for the code, there's a choice between using an incorrect value, and > correctly detecting for the vast majority of cases, and using the > correct value and correctly detecting for 100% of cases. Perhaps "only > marginally broken" is "good enough", seeing as nobody's complained. Agree, and that is why I said using 144 will be probably better and provided a tool that can be used to generate this call in the guests (only for Linux though), so that the maximum value could be found empirically. Since this is meant to work for ISO images in a file as well as devices with physical CDs on them, I suspect (and remember the original code which included this magic value wasn't mine) that the number of sectors might be the only reliable indication of media size, but will look at it again and see if there is any other metadata available in all cases which could be used instead. > > but I had already enough problems trying to get this > > merged without trying to change the code that much to try to guess a better > > magic number than the one was originally used (I like 144 though) > > Sorry, but did anyone complain? > > No. Not sure what you mean by this, but having a patch resent several times with no comments is IMHO more problematic that complains. Carlo
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
On Sun, Jan 06, 2008 at 03:32:27PM +0100, Andreas Färber wrote: > Either way, shouldn't it be a preprocessor define rather than a magic > number, maybe something like MAX_SECTORS_CD? Then it can more easily > be found and changed, where necessary. Point taken, will fix that if we still have to rely on the number of sectors to detect the media type. Carlo
Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support
On Sun, Jan 06, 2008 at 03:22:15AM -0600, Rob Landley wrote: > > > > > @@ -1648,17 +1649,27 @@ static void ide_atapi_cmd(IDEState *s) > > > > > ASC_INV_FIELD_IN_CMD_PACKET); > > > > > break; > > > > > } > > > > > -memset(buf, 0, 32); > > > > > > This moved a few lines down, but that just seems to be churn. > > > > it is structured in a way that could be later structured away into a > > function when/if more profiles are added. > > Makes sense. I should also said that unless done after as shown by the patch, reading the parameter will be imposible, because the buffer used for the IDE commands is reused for input/output as shown by lines 1295 to 1300 at the beginning of ide_atapi_cmd : const uint8_t *packet; uint8_t *buf; int max_len; packet = s->io_buffer; buf = s->io_buffer; > > > > > +max_len = ube16_to_cpu(packet + 7); > > > > > > Why are you doing math on a value _before_ you adjust its endianness from > > > a potentially foreign format into the CPU native one? I take it that > > > gives you a pointer from which a 16 byte value is fetched? > > > > yes, this adds not to the value but the pointer where the packet is stored > > and that contains on byte 7 (MSB) and 8 (LSB) the value for the Allocation > > Length parameter as described in Table 275 of T10/1836-D Revision 1 (*) > > Is dereferencing a word value at an odd address alignment safe on all the > potential host platforms? (I dunno if qemu runs on an arm host, nor if the > ube16_to_cpu value is already dealing with this. Just curious.) good point, and I am affraid that it might be unsafe in those architectures with strict alignment requirements, but sadly there is no mechanism available (at least for the ide emulation) to emulate the hardware buffers in a safe way. could someone with most experience on this hint into the right direction to approach this problem and that affects several other emulated commandas as well? > > > > > -buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* > > > > > current profile */ > > > > > > This becomes 3-state now. You added a state 0 when there's no cdrom in > > > the drive. The less intrusive change would be keeping the above line and > > > adding a second line: "if (!total_sectors) buf[7] = 0;" Not actually any > > > larger, codewise, and a less intrusive change. > > > > I refactored this code so that it is hopefully more clear in the intended > > effect, which is to set the default profile to either of the available > > profiles depending on the kind of media that was available, and as it is > > done by real hardware. > > > > Again, even if the refactoring was more of an intrusive change, it also > > allows for more flexibility to expand this code to support more profiles in > > a cleaner way. > > I take it buf[7]=0 is what real hardware returns when there's no disk in the > drive? yes > > It is clearer on why the size of the response includes the profile > > definitions plus the 2 headers, remember that buf[11] is now a constant > > because we are defining only 2 profiles by hand, but the aim was to clean > > the code and make it extensible (and I hoped self explanatory) even if it > > makes the computer do some math or is not as compact as the original one, > > after all this code doesn't need to be optimized for speed and, well I > > trust compilers ;) > > I.E. if the value gets set in a more complicated way, it propagates naturally > to all the places it needs to go. > > *shrug* I suppose I can see trying to have fewer instances of each magic > constant... will try to make a simpler version then that could be used at least as an intermediate step and that is less intrusive than this one, while avoiding so may magic values. Carlo
Re: [Qemu-devel] QEMU version 0.9.1
On Sun, Jan 06, 2008 at 11:03:45PM +0100, Fabrice Bellard wrote: > > QEMU version 0.9.1 is out ! and if you want to install an OpenSolaris guest on it, apply the attached patch over it. the patch prevents OpenSolaris from overflowing a small buffer when querying the emulated CDROM for its capabilities and getting more data than requested at install time. beware that there are still other problems with the implementation of that command that are being addressed in a bigger patch that is still under revision. Carlo Index: hw/ide.c === RCS file: /sources/qemu/qemu/hw/ide.c,v retrieving revision 1.79 diff -u -p -r1.79 ide.c --- hw/ide.c24 Dec 2007 14:33:24 - 1.79 +++ hw/ide.c7 Jan 2008 05:24:16 - @@ -1648,6 +1648,7 @@ static void ide_atapi_cmd(IDEState *s) ASC_INV_FIELD_IN_CMD_PACKET); break; } +max_len = ube16_to_cpu(packet + 7); memset(buf, 0, 32); bdrv_get_geometry(s->bs, &total_sectors); buf[3] = 16; @@ -1658,7 +1659,7 @@ static void ide_atapi_cmd(IDEState *s) buf[14] = buf[7] == 0x10; /* (in)active */ buf[17] = 0x08; /* CD-ROM profile */ buf[18] = buf[7] == 0x08; /* (in)active */ -ide_atapi_cmd_reply(s, 32, 32); +ide_atapi_cmd_reply(s, 32, max_len); break; } default:
[Qemu-devel] [RFC] ide: multi-profile DVD-ROM support v2
The following patch re-implements the "GET CONFIGURATION" MMC-6 command to
match the published SPEC.
This is a re-write of the original patch series published but including the
feedback received :
http://lists.gnu.org/archive/html/qemu-devel/2007-11/msg00849.html
Important changes from the previous patch :
* Use a 99min CD size as the bigger possible sector count for CD profile
* Don't recalculate the number of sectors
* Use an inline helper function to set the profiles in a cleaner way
* Avoid extra computations from constants while reducing the use of magic
numbers and using defines when possible.
Carlo
---
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.79
diff -u -p -r1.79 ide.c
--- hw/ide.c24 Dec 2007 14:33:24 - 1.79
+++ hw/ide.c7 Jan 2008 11:57:43 -
@@ -1,5 +1,5 @@
/*
- * QEMU IDE disk and CD-ROM Emulator
+ * QEMU IDE disk and CD/DVD-ROM Emulator
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
@@ -284,6 +284,44 @@
* of MODE_SENSE_POWER_PAGE */
#define GPMODE_CDROM_PAGE 0x0d
+/* Some generally useful CD-ROM information */
+#define CD_MINS 99 /* max. minutes per CD */
+#define CD_SECS 60 /* seconds per minute */
+#define CD_FRAMES 75 /* frames per second */
+#define CD_FRAMESIZE2048 /* bytes per frame, "cooked" mode */
+#define CD_MAX_BYTES 912384000 /* multiply all of the above */
+#define CD_MAX_SECTORS 1782000 /* divide by 512 */
+
+/* Profile list from MMC-6 revision 1 table 91 */
+#define MMC_PROFILE_NONE0x
+#define MMC_PROFILE_CD_ROM 0x0008
+#define MMC_PROFILE_CD_R0x0009
+#define MMC_PROFILE_CD_RW 0x000A
+#define MMC_PROFILE_DVD_ROM 0x0010
+#define MMC_PROFILE_DVD_R_SR0x0011
+#define MMC_PROFILE_DVD_RAM 0x0012
+#define MMC_PROFILE_DVD_RW_RO 0x0013
+#define MMC_PROFILE_DVD_RW_SR 0x0014
+#define MMC_PROFILE_DVD_R_DL_SR 0x0015
+#define MMC_PROFILE_DVD_R_DL_JR 0x0016
+#define MMC_PROFILE_DVD_RW_DL 0x0017
+#define MMC_PROFILE_DVD_DDR 0x0018
+#define MMC_PROFILE_DVD_PLUS_RW 0x001A
+#define MMC_PROFILE_DVD_PLUS_R 0x001B
+#define MMC_PROFILE_DVD_PLUS_RW_DL 0x002A
+#define MMC_PROFILE_DVD_PLUS_R_DL 0x002B
+#define MMC_PROFILE_BD_ROM 0x0040
+#define MMC_PROFILE_BD_R_SRM0x0041
+#define MMC_PROFILE_BD_R_RRM0x0042
+#define MMC_PROFILE_BD_RE 0x0043
+#define MMC_PROFILE_HDDVD_ROM 0x0050
+#define MMC_PROFILE_HDDVD_R 0x0051
+#define MMC_PROFILE_HDDVD_RAM 0x0052
+#define MMC_PROFILE_HDDVD_RW0x0053
+#define MMC_PROFILE_HDDVD_R_DL 0x0058
+#define MMC_PROFILE_HDDVD_RW_DL 0x005A
+#define MMC_PROFILE_INVALID 0x
+
#define ATAPI_INT_REASON_CD 0x01 /* 0 = data transfer */
#define ATAPI_INT_REASON_IO 0x02 /* 1 = transfer to the host */
#define ATAPI_INT_REASON_REL0x04
@@ -540,7 +578,7 @@ static void ide_atapi_identify(IDEState
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */
+padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
#ifdef USE_DMA_CDROM
put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -1290,6 +1328,22 @@ static void ide_atapi_cmd_read(IDEState
}
}
+static inline uint8_t ide_atapi_set_profile(uint8_t *buf, uint8_t *index,
+uint16_t profile)
+{
+uint8_t *buf_profile = buf + 12; /* start of profiles */
+
+buf_profile += ((*index) * 4); /* start of indexed profile */
+cpu_to_ube16 (buf_profile, profile);
+buf_profile[2] = ((buf_profile[0] == buf[6]) && (buf_profile[1] ==
buf[7]));
+
+/* each profile adds 4 bytes to the response */
+(*index)++;
+buf[11] += 4; /* Additional Length */
+
+return 4;
+}
+
static void ide_atapi_cmd(IDEState *s)
{
const uint8_t *packet;
@@ -1634,13 +1688,13 @@ static void ide_atapi_cmd(IDEState *s)
buf[6] = 0; /* reserved */
buf[7] = 0; /* reserved */
padstr8(buf + 8, 8, "QEMU");
-padstr8(buf + 16, 16, "QEMU CD-ROM");
+padstr8(buf + 16, 16, "QEMU DVD-ROM");
padstr8(buf + 32, 4, QEMU_VERSION);
ide_atapi_cmd_reply(s, 36, max_len);
break;
case GPCMD_GET_CONFIGURATION:
{
-uint64_t total_sectors;
+uint32_t len;
/* only feature 0 is supported */
if (packet[2] !=
[Qemu-devel] [RFC] ide: multi-profile DVD-ROM support v2.1
This is version 2.1 of the patch to re-implement the "GET CONFIGURATION"
MMC-6 command as used by the IDE emulation to match the published SPEC
and that was originally published in :
http://lists.gnu.org/archive/html/qemu-devel/2007-11/msg00849.html
Important changes from the previous patches :
* Use a 99min CD size as the bigger possible sector count for CD profile
* Don't recalculate the number of sectors
* Use an inline helper function to set the profiles in a cleaner way
* Avoid extra computations from constants except for #define values
* Reduce the use of magic numbers and use defines when possible
Remaining issues that will need to be addressed in future versions :
* MMC-6 also applies to SCSI devices and so the definitions might need
to be moved to a common header when that code is developed.
* The use of the buffer might not be safe for unaligned access in some
architectures, but the same applies to all other commands that are
currently using the io_buffer directly as the hardware does.
* The heuristic used tries to guess the kind of media from the size of
it and is not that reliable for really small DVDs that could fit in a
CD.
* The response uses the io_buffer and is therefore limited to the size
of it (not really a problem now when the maximum response size will
be 20 bytes) but could be a problem when more features/profiles are
implemented.
* When using the host_device driver media changes could go unnoticed
and result in the wrong profile being selected due to limitations
in the current implementation of the ide emulation.
Testing is appreciated
Carlo
---
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.79
diff -u -p -r1.79 ide.c
--- hw/ide.c24 Dec 2007 14:33:24 - 1.79
+++ hw/ide.c8 Jan 2008 12:02:40 -
@@ -1,5 +1,5 @@
/*
- * QEMU IDE disk and CD-ROM Emulator
+ * QEMU IDE disk and CD/DVD-ROM Emulator
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
@@ -284,6 +284,44 @@
* of MODE_SENSE_POWER_PAGE */
#define GPMODE_CDROM_PAGE 0x0d
+/* Some generally useful CD-ROM information */
+#define CD_MINS 99 /* max. minutes per CD */
+#define CD_SECS 60 /* seconds per minute */
+#define CD_FRAMES 75 /* frames per second */
+#define CD_FRAMESIZE2048 /* bytes per frame, "cooked" mode */
+#define CD_MAX_BYTES (CD_MINS * CD_SECS * CD_FRAMES * CD_FRAMESIZE)
+#define CD_MAX_SECTORS (CD_MAX_BYTES / 512)
+
+/* Profile list from MMC-6 revision 1 table 91 */
+#define MMC_PROFILE_NONE0x
+#define MMC_PROFILE_CD_ROM 0x0008
+#define MMC_PROFILE_CD_R0x0009
+#define MMC_PROFILE_CD_RW 0x000A
+#define MMC_PROFILE_DVD_ROM 0x0010
+#define MMC_PROFILE_DVD_R_SR0x0011
+#define MMC_PROFILE_DVD_RAM 0x0012
+#define MMC_PROFILE_DVD_RW_RO 0x0013
+#define MMC_PROFILE_DVD_RW_SR 0x0014
+#define MMC_PROFILE_DVD_R_DL_SR 0x0015
+#define MMC_PROFILE_DVD_R_DL_JR 0x0016
+#define MMC_PROFILE_DVD_RW_DL 0x0017
+#define MMC_PROFILE_DVD_DDR 0x0018
+#define MMC_PROFILE_DVD_PLUS_RW 0x001A
+#define MMC_PROFILE_DVD_PLUS_R 0x001B
+#define MMC_PROFILE_DVD_PLUS_RW_DL 0x002A
+#define MMC_PROFILE_DVD_PLUS_R_DL 0x002B
+#define MMC_PROFILE_BD_ROM 0x0040
+#define MMC_PROFILE_BD_R_SRM0x0041
+#define MMC_PROFILE_BD_R_RRM0x0042
+#define MMC_PROFILE_BD_RE 0x0043
+#define MMC_PROFILE_HDDVD_ROM 0x0050
+#define MMC_PROFILE_HDDVD_R 0x0051
+#define MMC_PROFILE_HDDVD_RAM 0x0052
+#define MMC_PROFILE_HDDVD_RW0x0053
+#define MMC_PROFILE_HDDVD_R_DL 0x0058
+#define MMC_PROFILE_HDDVD_RW_DL 0x005A
+#define MMC_PROFILE_INVALID 0x
+
#define ATAPI_INT_REASON_CD 0x01 /* 0 = data transfer */
#define ATAPI_INT_REASON_IO 0x02 /* 1 = transfer to the host */
#define ATAPI_INT_REASON_REL0x04
@@ -540,7 +578,7 @@ static void ide_atapi_identify(IDEState
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */
+padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
#ifdef USE_DMA_CDROM
put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -1290,6 +1328,22 @@ static void ide_atapi_cmd_read(IDEState
}
}
+static inline uint8_t ide_atapi_set_profile(uint8_t *buf, uint8_t *index,
+uint16_t profile)
+{
+uint8_t *buf_profile = buf + 12; /* s
[Qemu-devel] [PATCH] ide: multi-profile DVD-ROM support v2.2
This is version 2.2 of the patch to re-implement the "GET CONFIGURATION"
MMC-6 command as used by the IDE emulation to match the published SPEC
and that was originally published in :
http://lists.gnu.org/archive/html/qemu-devel/2007-11/msg00849.html
Important changes from the previous patches :
* Use a 80min size as the bigger orange book compatible sector count for CD
profile
* Don't recalculate the number of sectors
* Use an inline helper function to set the profiles
Carlo
---
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.79
diff -u -r1.79 ide.c
--- hw/ide.c24 Dec 2007 14:33:24 - 1.79
+++ hw/ide.c9 Jan 2008 12:33:54 -
@@ -1,5 +1,5 @@
/*
- * QEMU IDE disk and CD-ROM Emulator
+ * QEMU IDE disk and CD/DVD-ROM Emulator
*
* Copyright (c) 2003 Fabrice Bellard
* Copyright (c) 2006 Openedhand Ltd.
@@ -284,6 +284,58 @@
* of MODE_SENSE_POWER_PAGE */
#define GPMODE_CDROM_PAGE 0x0d
+/*
+ * Based on values from but extending CD_MINS
+ * to the maximum common size allowed by the Orange's Book ATIP
+ *
+ * 90 and 99 min CDs are also available but using them as the
+ * upper limit reduces the effectiveness of the heuristic to
+ * detect DVDs burned to less than 25% of their maximum capacity
+ */
+
+/* Some generally useful CD-ROM information */
+#define CD_MINS 80 /* max. minutes per CD */
+#define CD_SECS 60 /* seconds per minute */
+#define CD_FRAMES 75 /* frames per second */
+#define CD_FRAMESIZE2048 /* bytes per frame, "cooked" mode */
+#define CD_MAX_BYTES (CD_MINS * CD_SECS * CD_FRAMES * CD_FRAMESIZE)
+#define CD_MAX_SECTORS (CD_MAX_BYTES / 512)
+
+/*
+ * The MMC values are not IDE specific and might need to be moved
+ * to a common header if they are also needed for the SCSI emulation
+ */
+
+/* Profile list from MMC-6 revision 1 table 91 */
+#define MMC_PROFILE_NONE0x
+#define MMC_PROFILE_CD_ROM 0x0008
+#define MMC_PROFILE_CD_R0x0009
+#define MMC_PROFILE_CD_RW 0x000A
+#define MMC_PROFILE_DVD_ROM 0x0010
+#define MMC_PROFILE_DVD_R_SR0x0011
+#define MMC_PROFILE_DVD_RAM 0x0012
+#define MMC_PROFILE_DVD_RW_RO 0x0013
+#define MMC_PROFILE_DVD_RW_SR 0x0014
+#define MMC_PROFILE_DVD_R_DL_SR 0x0015
+#define MMC_PROFILE_DVD_R_DL_JR 0x0016
+#define MMC_PROFILE_DVD_RW_DL 0x0017
+#define MMC_PROFILE_DVD_DDR 0x0018
+#define MMC_PROFILE_DVD_PLUS_RW 0x001A
+#define MMC_PROFILE_DVD_PLUS_R 0x001B
+#define MMC_PROFILE_DVD_PLUS_RW_DL 0x002A
+#define MMC_PROFILE_DVD_PLUS_R_DL 0x002B
+#define MMC_PROFILE_BD_ROM 0x0040
+#define MMC_PROFILE_BD_R_SRM0x0041
+#define MMC_PROFILE_BD_R_RRM0x0042
+#define MMC_PROFILE_BD_RE 0x0043
+#define MMC_PROFILE_HDDVD_ROM 0x0050
+#define MMC_PROFILE_HDDVD_R 0x0051
+#define MMC_PROFILE_HDDVD_RAM 0x0052
+#define MMC_PROFILE_HDDVD_RW0x0053
+#define MMC_PROFILE_HDDVD_R_DL 0x0058
+#define MMC_PROFILE_HDDVD_RW_DL 0x005A
+#define MMC_PROFILE_INVALID 0x
+
#define ATAPI_INT_REASON_CD 0x01 /* 0 = data transfer */
#define ATAPI_INT_REASON_IO 0x02 /* 1 = transfer to the host */
#define ATAPI_INT_REASON_REL0x04
@@ -540,7 +592,7 @@
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */
+padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
#ifdef USE_DMA_CDROM
put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -1290,6 +1342,22 @@
}
}
+static inline uint8_t ide_atapi_set_profile(uint8_t *buf, uint8_t *index,
+uint16_t profile)
+{
+uint8_t *buf_profile = buf + 12; /* start of profiles */
+
+buf_profile += ((*index) * 4); /* start of indexed profile */
+cpu_to_ube16 (buf_profile, profile);
+buf_profile[2] = ((buf_profile[0] == buf[6]) && (buf_profile[1] ==
buf[7]));
+
+/* each profile adds 4 bytes to the response */
+(*index)++;
+buf[11] += 4; /* Additional Length */
+
+return 4;
+}
+
static void ide_atapi_cmd(IDEState *s)
{
const uint8_t *packet;
@@ -1634,13 +1702,13 @@
buf[6] = 0; /* reserved */
buf[7] = 0; /* reserved */
padstr8(buf + 8, 8, "QEMU");
-padstr8(buf + 16, 16, "QEMU CD-ROM");
+padstr8(buf + 16, 16, "QEMU DVD-ROM");
padstr8(buf + 32, 4, QEMU_VERSION);
ide_atapi_cmd_reply(s, 36, max_len);
Re: [Qemu-devel] qemu-0.9.0 fedora 7, cdrom and ide emulation error
> Can someone tell me the exact changeset/patch to fix this issue? changeset : cvs -q diff -u -r1.63 -r1.64 hw/ide.c patch : http://thread.gmane.org/gmane.comp.emulators.qemu/19160 Carlo
Re: [Qemu-devel] Compilation error on Ubuntu 6.06 and 7.10 with gcc-3.4
On Sun, Jan 27, 2008 at 06:01:22PM +, Stefano Stabellini wrote: > I can confirm this, I have the same problem on Kubuntu 7.10 i386 using > either gcc-3.4 or gcc-3.3. architectural limitation for x86 triggered by cpu-exec.c version 1.131, reverting to 1.130 allows the compilation to proceed Carlo
Re: [Qemu-devel] Under WinXP, Solaris installation does not work in qemu 0.9.1 but does work in qemu 0.9.0
On Wed, Jan 30, 2008 at 05:31:05PM +0300, Dmitry Bolshakov wrote: > > qemu-0.9.1: > -builded by myself too > http://qemu-forum.ipi.fi/viewtopic.php?f=5&t=4269 qemu 0.9.1 was released with a known bug which prevents installing solaris guests with timeouts in the CD device and which was finally fixed with the patch from : http://lists.gnu.org/archive/html/qemu-devel/2008-01/msg00211.html Carlo
[Qemu-devel] [PATCH] restore rw support for vvfat
The attached patch, restores support for writable block devices using
Virtual FAT disk images.
RW support using a generated qcow base image was modified after qemu 0.8.2
was released; while adding AIO support to block-qcow.c in release 1.8; and
resulting in a broken qcow image based in an inexistent "fat:" base
when "fat:rw" was requested.
Carlo
Index: block-qcow.c
===
RCS file: /sources/qemu/qemu/block-qcow.c,v
retrieving revision 1.15
diff -u -r1.15 block-qcow.c
--- block-qcow.c11 Nov 2007 02:51:16 - 1.15
+++ block-qcow.c11 Mar 2008 07:29:00 -
@@ -752,11 +752,15 @@
header_size = sizeof(header);
backing_filename_len = 0;
if (backing_file) {
-header.backing_file_offset = cpu_to_be64(header_size);
-backing_filename_len = strlen(backing_file);
-header.backing_file_size = cpu_to_be32(backing_filename_len);
-header_size += backing_filename_len;
-header.mtime = cpu_to_be32(0);
+if (strcmp(backing_file, "fat:")) {
+header.backing_file_offset = cpu_to_be64(header_size);
+backing_filename_len = strlen(backing_file);
+header.backing_file_size = cpu_to_be32(backing_filename_len);
+header_size += backing_filename_len;
+} else {
+/* special backing file for vvfat */
+backing_file = NULL;
+}
header.cluster_bits = 9; /* 512 byte cluster to avoid copying
unmodifyed sectors */
header.l2_bits = 12; /* 32 KB L2 tables */
Re: [Qemu-devel] --disable-gfx-check still wants SDL.h?
On Wed, Mar 12, 2008 at 07:24:54PM -0700, David Barrett wrote: > Is it possible to compile/run qemu on a system that does not have SDL? ./configure --disable-sdl --disable-gfx-check Carlo
Re: [Qemu-devel] FW: [kvm-devel] CPU consumption for SMP windows guests.
On Sun, Aug 26, 2007 at 02:10:01AM -0700, Igor Lvovsky wrote: >As it was mentioned in forum (by Avi) it looks like the problem in the >windows Idle loop, that spinning instead of executing a 'hlt' instruction. could you provide a reference to this thread? >So, the solution lies in ACPI area. We need to modify a little bit ACPI >tables as following: if windows is using an ACPI enabled HAL, but not it if it is using an MPS enabled HAL like : halmps.dll (Multiprocessor PCs) which you would get is using (-no-acpi) as explained below. >Notes: The UP guest uses the HAL with the halaacpi.dll and windows Idle >loop work fine without ACPI support (I mean bios don't expose the CPU as >ACPI device). if using -no-acpi then Windows will instead select the MPS HAL for UP instead of the ACPI ones. hal.dll (Standard PCs) if using ACPI then Windows will either use (dependent on the hardware support) : halacpi.dll (Advanced Configuration and Power Interface (ACPI) PCs) halapic.dll (Advanced Programmable Interrupt Controller (APIC) PCs) halaacpi.dll (APIC ACPI PCs) in the case if Windows 2000 with your BIOS then it will use halaacpi.dll when running qemu with ACPI enabled (without -no-acpi) and regardless of how many processors are defined by -smp. >I attached the patch for BOCHS bios and compiled bios.bin. this bios.bin is broken as it won't enable SMP even if ACPI is disabled (hence using MPS) as can be seen if you try to start an SMP enabled Linux (like knoppix livecd). I was able to reproduce the broken BIOS by applying the suggested (with some tweaks) patches to the current BOCHS CVS though and so it might be as well a regression added since qemu's branch was created and compiled, but haven't yet been able to dissect that bug as I am not sure either what version was used originally for qemu's bios.bin it does reduce the CPU consumption though, but it might be as well as a side effect of removing all contention between CPUs since there is only 1 anyway. Carlo
Re: [Qemu-devel] [patch] make qemu work with GCC 4
On Wed, Aug 29, 2007 at 03:30:45PM +0200, Michael Matz wrote: > I'm only a compiler developer hitting qemu hard enough until it works with > gcc4 :-) slightly offtopic, but in the last GCC summit 2007 there was a presentation with an interesting propossal which makes GCC specifically generate code chunks that would be reused in a different context like it is done with dyngen for qemu as shown by : http://www.gccsummit.org/2007/view_abstract.php?content_key=30 with a description (but sadly no code) in the proceedings (pages 117 to 130) available from : http://ols2006.108.redhat.com/2007/GCC-Reprints/GCC2007-Proceedings.pdf Carlo
[Qemu-devel] [RFC/PATCH] remove $check_gfx from configure
The following patch removes check_gfx from qemu's configure as the check it was trying to enforce is no longer valid. If neither sdl or cocoa are available, the video output for the console will be still available from the vnc server (which can't be disabled). Carlo --- Index: configure === RCS file: /sources/qemu/qemu/configure,v retrieving revision 1.165 diff -u -r1.165 configure --- configure 18 Oct 2007 20:51:48 - 1.165 +++ configure 23 Oct 2007 01:01:50 - @@ -98,7 +98,6 @@ kqemu="no" profiler="no" cocoa="no" -check_gfx="yes" check_gcc="yes" softmmu="yes" linux_user="no" @@ -276,8 +275,6 @@ ;; --enable-cocoa) cocoa="yes" ; coreaudio="yes" ; sdl="no" ;; - --disable-gfx-check) check_gfx="no" - ;; --disable-gcc-check) check_gcc="no" ;; --disable-system) softmmu="no" @@ -966,12 +963,10 @@ ;; esac -if test "$target_user_only" = "no" -a "$check_gfx" = "yes" \ --a "$sdl" = "no" -a "$cocoa" = "no" ; then -echo "ERROR: QEMU requires SDL or Cocoa for graphical output" -echo "To build QEMU without graphical output configure with --disable-gfx-check" -echo "Note that this will disable all output from the virtual graphics card." -exit 1; +if test "$target_user_only" = "no" -a "$sdl" = "no" -a "$cocoa" = "no" ; then +echo "WARNING: QEMU requires SDL or Cocoa for graphical output" +echo "Note that in order to be able to attach to the console you'll" +echo "need to start qemu's vncserver with --vnc and use a vnc client." fi #echo "Creating $config_mak, $config_h and $target_dir/Makefile"
[Qemu-devel] [PATCH] add --disable-sdl to configure's help
The following patch adds a description for --disable-sdl for configure Carlo --- Index: configure === RCS file: /sources/qemu/qemu/configure,v retrieving revision 1.165 diff -u -r1.165 configure --- configure 18 Oct 2007 20:51:48 - 1.165 +++ configure 23 Oct 2007 01:12:17 - @@ -377,6 +374,7 @@ echo " --make=MAKE use specified make [$make]" echo " --install=INSTALLuse specified install [$install]" echo " --static enable static build [$static]" +echo " --disable-sdldisable SDL" echo " --enable-cocoa enable COCOA (Mac OS X only)" echo " --enable-mingw32 enable Win32 cross compilation with mingw32" echo " --enable-adlib enable Adlib emulation"
[Qemu-devel] [PATCH] show usage and abort if unknown option is passed to configure
The following patch prints an error if an unknown option is passed to configure and directs the script to show usage information. Carlo --- Index: configure === RCS file: /sources/qemu/qemu/configure,v retrieving revision 1.165 diff -u -r1.165 configure --- configure 18 Oct 2007 20:51:48 - 1.165 +++ configure 23 Oct 2007 01:23:58 - @@ -306,6 +303,8 @@ *) echo "undefined SPARC architecture. Exiting";exit 1;; esac ;; + *) echo "ERROR: unknown option $opt"; show_help="yes" + ;; esac done
[Qemu-devel] [PATCH] add gcc 3.4.6 to the list of compilers to look for
The following patch adds gcc-3.4.6 (as used by gentoo's gcc-3.4.6-r2 package) to the list of compilers to test for compatibility. This is also the last version for the gcc-3.4.x and gcc-3.x series which hasn't had an update in 19 months and therefore most likely stable enough to be called by full version number. All other distributions that don't have a binary with this name will just skip this entry and therefore should been unaffected by the change. Carlo --- Index: configure === RCS file: /sources/qemu/qemu/configure,v retrieving revision 1.165 diff -u -r1.165 configure --- configure 18 Oct 2007 20:51:48 - 1.165 +++ configure 23 Oct 2007 01:38:17 - @@ -23,7 +23,7 @@ cross_prefix="" cc="gcc" gcc3_search="yes" -gcc3_list="gcc-3.4 gcc34 gcc-3.3.6 gcc-3.3 gcc33 gcc-3.2 gcc32" +gcc3_list="gcc-3.4.6 gcc-3.4 gcc34 gcc-3.3.6 gcc-3.3 gcc33 gcc-3.2 gcc32" host_cc="gcc" ar="ar" make="make"
[Qemu-devel] [RFC] normalize error messages and use stderr in configure
The following patch normalizes all (most) messages as generated by configure
so they are flagged as ERROR or WARNING (left some of the ones that look more
informative without a flag even if they result in a fatal error).
All those messages are directed to stderr so they can be filtered/identified
correctly if analyzed mechanically. Usage information is left in stdout as it
doesn't denote abnormal behavior.
The patch is based on a modified tree (which includes several of the previous
patches sent) and won't apply cleanly and so it is not ready for merge yet.
Carlo
---
Index: configure
===
RCS file: /sources/qemu/qemu/configure,v
retrieving revision 1.165
diff -u -r1.165 configure
--- configure 18 Oct 2007 20:51:48 - 1.165
+++ configure 23 Oct 2007 02:49:16 -
@@ -169,10 +168,10 @@
if test -f /opt/SUNWspro/prod/lib/libsunmath.so.1; then
needs_libsunmath="yes"
else
-echo "QEMU will not link correctly on Solaris 8/X86 or 9/x86
without"
-echo "libsunmath from the Sun Studio compilers tools, due to a
lack of"
-echo "C99 math features in libm.so in Solaris 8/x86 and
Solaris 9/x86"
-echo "Studio 11 can be downloaded from www.sun.com."
+echo "QEMU will not link correctly on Solaris 8/X86 or 9/x86
without" 1>&2
+echo "libsunmath from the Sun Studio compilers tools, due to a
lack of" 1>&2
+echo "C99 math features in libm.so in Solaris 8/x86 and
Solaris 9/x86" 1>&2
+echo "Studio 11 can be downloaded from www.sun.com." 1>&2
exit 1
fi
fi
@@ -303,9 +300,11 @@
target_cpu="sparc"; cpu="sparc" ;;
v9)SP_CFLAGS="-m64 -mcpu=ultrasparc -D__sparc_${sparc_cpu}__";
SP_LDFLAGS="-m64"
target_cpu="sparc64"; cpu="sparc64" ;;
-*) echo "undefined SPARC architecture. Exiting";exit 1;;
+*) echo "ERROR: undefined SPARC architecture. Exiting" 1>&2; exit
1;;
esac
;;
+ *) echo "ERROR: unknown option $opt" 1>&2; show_help="yes"
+ ;;
esac
done
@@ -412,7 +412,7 @@
if $cc -c -o $TMPO $TMPC 2> /dev/null ; then
: C compiler works ok
else
-echo "ERROR: \"$cc\" either does not exist or does not work"
+echo "ERROR: \"$cc\" either does not exist or does not work" 1>&2
exit 1
fi
@@ -431,26 +431,26 @@
int main(){return 0;}
EOF
if "$cc" -o $TMPE $TMPC 2> /dev/null ; then
- echo "WARNING: \"$cc\" looks like gcc 4.x"
+ echo "WARNING: \"$cc\" looks like gcc 4.x" 1>&2
found_compat_cc="no"
if test "$gcc3_search" = "yes" ; then
- echo "Looking for gcc 3.x"
+ echo "Looking for gcc 3.x" 1>&2
for compat_cc in $gcc3_list ; do
if "$cross_prefix$compat_cc" --version 2> /dev/null | fgrep
'(GCC) 3.' > /dev/null 2>&1 ; then
- echo "Found \"$compat_cc\""
+ echo "Found \"$compat_cc\"" 1>&2
cc="$cross_prefix$compat_cc"
found_compat_cc="yes"
break
fi
done
if test "$found_compat_cc" = "no" ; then
- echo "gcc 3.x not found!"
+ echo "ERROR: gcc 3.x not found!" 1>&2
fi
fi
if test "$found_compat_cc" = "no" ; then
- echo "QEMU is known to have problems when compiled with gcc 4.x"
- echo "It is recommended that you use gcc 3.x to build QEMU"
- echo "To use this compiler anyway, configure with
--disable-gcc-check"
+ echo "QEMU is known to have problems when compiled with gcc 4.x"
1>&2
+ echo "It is recommended that you use gcc 3.x to build QEMU" 1>&2
+ echo "To use this compiler anyway, configure with
--disable-gcc-check" 1>&2
exit 1;
fi
fi
@@ -467,30 +467,30 @@
if test "$solarisrev" -eq 10 -a "$check_gcc" = "yes" ; then
solgcc=`which $cc`
if test "$solgcc" = "/usr/sfw/bin/gcc" ; then
- echo "Solaris 10/FCS gcc in /usr/sfw/bin will not compiled qemu
correctly."
- echo "please get gcc-3.4.3 or later, from www.blastwave.org using
pkg-get -i gcc3"
- echo "or get the latest patch from SunSolve for gcc"
+ echo "Solaris 10/FCS gcc in /usr/sfw/bin will not compiled qemu
correctly." 1>&2
+ echo "please get gcc-3.4.3 or later, from www.blastwave.org using
pkg-get -i gcc3" 1>&2
+ echo "or get the latest patch from SunSolve for gcc" 1>&2
exit 1
fi
fi
solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
if test -z "$solinst" ; then
-echo "Solaris install program not found. Use --install=/usr/ucb/install or"
-echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
-echo "to get ginstall which is used by default (
[Qemu-devel] [RESEND] [PATCH] show usage and abort if unknown option is passed to configure
Any comments on this?, is ignoring unknown parameters to configure a feature as I suspect to prevent build failures on users that assume that it is autoconf based and try to enforce the defaults with an unknown option like --enable-sdl? Carlo - Forwarded message from Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]> - Date: Mon, 22 Oct 2007 20:31:35 -0500 From: Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]> To: qemu-devel@nongnu.org Subject: [PATCH] show usage and abort if unknown option is passed to configure User-Agent: Mutt/1.4.1i The following patch prints an error if an unknown option is passed to configure and directs the script to show usage information. Carlo --- Index: configure === RCS file: /sources/qemu/qemu/configure,v retrieving revision 1.165 diff -u -r1.165 configure --- configure 18 Oct 2007 20:51:48 - 1.165 +++ configure 23 Oct 2007 01:23:58 - @@ -306,6 +303,8 @@ *) echo "undefined SPARC architecture. Exiting";exit 1;; esac ;; + *) echo "ERROR: unknown option $opt"; show_help="yes" + ;; esac done - End forwarded message -
[Qemu-devel] [PATCH] add declaration for kqemu_cpu_interrupt to block-raw.c
The following patch includes "exec-all.h" in block-raw.c when QEMU_IMG is not
defined to avoid the following implicit declaration :
qemu/block-raw.c: In function `aio_signal_handler':
qemu/block-raw.c:253: warning: implicit declaration of function
`kqemu_cpu_interrupt'
Carlo
--
Index: block-raw.c
===
RCS file: /sources/qemu/qemu/block-raw.c,v
retrieving revision 1.26
diff -u -r1.26 block-raw.c
--- block-raw.c 11 Nov 2007 02:51:16 - 1.26
+++ block-raw.c 11 Nov 2007 08:10:05 -
@@ -57,11 +57,13 @@
#include
#endif
-//#define DEBUG_FLOPPY
+#if !defined(QEMU_IMG)
+#include "exec-all.h"
+#endif
+//#define DEBUG_FLOPPY
//#define DEBUG_BLOCK
-#if defined(DEBUG_BLOCK) && !defined(QEMU_IMG)
-#include "exec-all.h"
+#if defined(DEBUG_BLOCK)
#define DEBUG_BLOCK_PRINT(formatCstr, args...) do { if (loglevel != 0) \
{ fprintf(logfile, formatCstr, ##args); fflush(logfile); } } while (0)
#else
[Qemu-devel] [PATCH] 64bit clean cris-softmmu
The following patch makes cris-softmmu compile in a 64bit host without
warnings by defining that the target CPU is 32bit explicitly and casting the
use of pointers into an uint32_t.
Carlo
---
Index: target-cris/cpu.h
===
RCS file: /sources/qemu/qemu/target-cris/cpu.h,v
retrieving revision 1.3
diff -u -r1.3 cpu.h
--- target-cris/cpu.h 10 Nov 2007 15:15:52 - 1.3
+++ target-cris/cpu.h 11 Nov 2007 09:53:49 -
@@ -21,6 +21,7 @@
#ifndef CPU_CRIS_H
#define CPU_CRIS_H
+#define TARGET_PHYS_ADDR_BITS 32
#define TARGET_LONG_BITS 32
#include "cpu-defs.h"
Index: target-cris/op_helper.c
===
RCS file: /sources/qemu/qemu/target-cris/op_helper.c,v
retrieving revision 1.3
diff -u -r1.3 op_helper.c
--- target-cris/op_helper.c 29 Oct 2007 14:39:49 - 1.3
+++ target-cris/op_helper.c 11 Nov 2007 10:05:31 -
@@ -60,7 +60,7 @@
if (__builtin_expect(ret, 0)) {
if (retaddr) {
/* now we have a real cpu fault */
-pc = (target_phys_addr_t)retaddr;
+pc = (target_phys_addr_t)(unsigned long)retaddr;
tb = tb_find_pc(pc);
if (tb) {
/* the PC is inside the translated code. It means that we have
[Qemu-devel] [PATCH/RFC] overflow and register size mismatch in sh4-softmmu
as shown by the following warning when compiling HEAD :
qemu/target-sh4/translate.c: In function `cpu_sh4_reset':
qemu/target-sh4/translate.c:139: warning: overflow in implicit constant
conversion
the problem was introduced in version 1.11 of that file and is being triggered
by the fact that the following assignment :
env->fp_status.float_rounding_mode = float_round_to_zero;
is trying to assign the value of float_round_to_zero which is defined in
softfloat-native.h as :
enum {
float_round_nearest_even = FE_TONEAREST,
float_round_down = FE_DOWNWARD,
float_round_up = FE_UPWARD,
float_round_to_zero = FE_TOWARDZERO
};
where FE_TOWARDZERO = 0xc00 and sizeof(env->fp_status.float_rounding_mode) ==
1 as shown by :
typedef struct float_status {
signed char float_rounding_mode;
signed char floatx80_rounding_precision;
} float_status;
float_status fp_status;
the following patch changes the logic to use a helper function just like other
targets and has been tested in x86 and amd64 to compile correctly, but I have
no way to test it and should be ideally validated by anyone that knows the sh4
emulation better and has a way to confirm that it is functionally equivalent.
Carlo
---
Index: target-sh4/translate.c
===
RCS file: /sources/qemu/qemu/target-sh4/translate.c,v
retrieving revision 1.19
diff -u -r1.19 translate.c
--- target-sh4/translate.c 10 Nov 2007 15:15:54 - 1.19
+++ target-sh4/translate.c 11 Nov 2007 13:01:31 -
@@ -136,7 +136,7 @@
env->fp_status.float_rounding_mode = float_round_nearest_even; /* ?! */
#else
env->fpscr = 0x00040001; /* CPU reset value according to SH4 manual */
-env->fp_status.float_rounding_mode = float_round_to_zero;
+set_float_rounding_mode(float_round_to_zero, &env->fp_status);
#endif
env->mmucr = 0;
}
Re: [Qemu-devel] qemu target-alpha/op_helper.c target-arm/op_hel...
On Sun, Nov 11, 2007 at 12:35:55PM +, Fabrice Bellard wrote: > Modified files: > target-alpha : op_helper.c > target-arm : op_helper.c > target-cris: op_helper.c > target-m68k: op_helper.c > target-ppc : op_helper.c > > Log message: > fixed invalid type why was target_phys_addr_t and invalid type for the program counter? or you mean that it is better to cast it to unsigned long so that you can never overflow the pointer and so using unsigned long was better? Carlo
[Qemu-devel] [PATCH] hw/sh7750.c: use TARGET_FMT_plx to printf target_phys_addr_t
The following patch changes the formatting string from %08x to TARGET_FMT_plx
to accommodate for 64bit hosts.
Carlo
---
Index: hw/sh7750.c
===
RCS file: /sources/qemu/qemu/hw/sh7750.c,v
retrieving revision 1.9
diff -u -r1.9 sh7750.c
--- hw/sh7750.c 4 Oct 2007 21:53:54 - 1.9
+++ hw/sh7750.c 11 Nov 2007 15:27:31 -
@@ -180,13 +180,13 @@
static void error_access(const char *kind, target_phys_addr_t addr)
{
-fprintf(stderr, "%s to %s (0x%08x) not supported\n",
+fprintf(stderr, "%s to %s (0x" TARGET_FMT_plx ") not supported\n",
kind, regname(addr), addr);
}
static void ignore_access(const char *kind, target_phys_addr_t addr)
{
-fprintf(stderr, "%s to %s (0x%08x) ignored\n",
+fprintf(stderr, "%s to %s (0x" TARGET_FMT_plx ") ignored\n",
kind, regname(addr), addr);
}
Re: [Qemu-devel] error compiling hw/sh7750.c
On Sun, Nov 11, 2007 at 09:30:26AM -0500, Ben Taylor wrote: > So the macro turns the last _INTC_ARRAY(NULL) into > > "NULL, sizeof(NULL)/sizeof(*NULL) in my 64bit linux using gcc-4.1.2 it becomes instead : ((void *)0), sizeof(((void *)0))/sizeof(*((void *)0)) what version of gcc (gcc -v) are you using in that Solaris 10 box? (HINT: not all available versions of gcc compile qemu correctly) : http://www.opensolaris.org/os/project/qemu/host/gcc-failures/ Carlo
Re: [Qemu-devel] error compiling hw/sh7750.c
On Sun, Nov 11, 2007 at 10:06:34AM -0600, Carlo Marcelo Arenas Belon wrote:
> On Sun, Nov 11, 2007 at 09:30:26AM -0500, Ben Taylor wrote:
> > So the macro turns the last _INTC_ARRAY(NULL) into
> >
> > "NULL, sizeof(NULL)/sizeof(*NULL)
>
> in my 64bit linux using gcc-4.1.2 it becomes instead :
>
> ((void *)0), sizeof(((void *)0))/sizeof(*((void *)0))
>
> what version of gcc (gcc -v) are you using in that Solaris 10 box? (HINT: not
> all available versions of gcc compile qemu correctly) :
>
> http://www.opensolaris.org/os/project/qemu/host/gcc-failures/
That didn't sound as clear as it should, so I booted an OpenSolaris Nevada 70b
using the sun provided gcc :
$ uname -a
SunOS dell 5.11 snv_70b i86pc i386 i86pc
$ gcc -v
Reading specs from /usr/sfw/lib/gcc/i386-pc-solaris2.11/3.4.3/specs
Configured with: /builds2/sfwnv-gate/usr/src/cmd/gcc/gcc-3.4.3/configure
--prefix=/usr/sfw --with-as=/usr/sfw/bin/gas --with-gnu-as
--with-ld=/usr/ccs/bin/ld --without-gnu-ld --enable-languages=c,c++,f77,objc
--enable-shared
Thread model: posix
gcc version 3.4.3 (csl-sol210-3_4-20050802)
and run a test, the problem is in /usr/include/iso/stdio_iso.h that defined
NULL as an int and not a pointer as shown by :
fndef NULL
#if defined(_LP64)
#define NULL0L
#else
#define NULL0
#endif
#endif
contradicting (at least in spirit) the C99 (ISO/IEC 9899:TCS) standard that
says :
"A pointer to void shall have the same representation and alignment
requirements as a pointer to a character type"
and therefore makes sizeof(*NULL) == 1 solving the problem to compile as it is
done if applying the following patch :
Carlo
---
--- qemu/hw/sh7750.cSun Oct 7 09:31:11 2007
+++ qemu/hw/sh7750.cSun Nov 11 16:13:13 2007
@@ -29,6 +29,9 @@
#include "sh7750_regnames.h"
#include "sh_intc.h"
+#undef NULL
+#define NULL ((void *)0)
+
#define NB_DEVICES 4
typedef struct SH7750State {
Re: [Qemu-devel] error compiling hw/sh7750.c
On Sun, Nov 11, 2007 at 05:24:06PM +, Paul Brook wrote: > > +#undef NULL > > +#define NULL ((void *)0) > > Absolutely not. this was not meant to be a final solution, or even be committed in qemu, just a stop gap solution answering Ben so that he can get their build to complete and have a somehow equivalent definition of what the code is doing in linux (where I presume was tested) until it can be fixed in a portable way. Ben's solution was to replace _INTC_ARRAY(NULL) with "NULL, NULL" but it should had been instead "NULL, 4" or "NULL, 8" depending on what ISA he is compiling it for, doing the "#undef/#define" leads to that and, yes I agree it is awful. Carlo
Re: [Qemu-devel] qemu configure
> Changes by: Fabrice Bellard07/11/11 20:24:30 > > Log message: > better to disable -Werror by default as 64 bit hosts still have warnings amazing work, haven't ever seen a cleaner compilation in 64bit linux before. thank you Carlo
[Qemu-devel] [PATCH] remove duplicated tlb_fill definitions for i386 and cris targets
The following patch removes a duplicated tlb_fill definition from the per
target specific exec.h for i386 and cris in favor of the generic one in
exec-all.h.
It also rearranges cris' op_helper implement it only in softmmu mode like all
other targets.
Carlo
---
Index: target-cris/exec.h
===
RCS file: /sources/qemu/qemu/target-cris/exec.h,v
retrieving revision 1.2
diff -u -r1.2 exec.h
--- target-cris/exec.h 14 Oct 2007 07:07:06 - 1.2
+++ target-cris/exec.h 12 Nov 2007 04:58:48 -
@@ -46,7 +46,6 @@
int cpu_cris_handle_mmu_fault (CPUState *env, target_ulong address, int rw,
int mmu_idx, int is_softmmu);
-void tlb_fill (target_ulong addr, int is_write, int mmu_idx, void *retaddr);
#if !defined(CONFIG_USER_ONLY)
#include "softmmu_exec.h"
Index: target-cris/op_helper.c
===
RCS file: /sources/qemu/qemu/target-cris/op_helper.c,v
retrieving revision 1.4
diff -u -r1.4 op_helper.c
--- target-cris/op_helper.c 11 Nov 2007 12:35:55 - 1.4
+++ target-cris/op_helper.c 12 Nov 2007 04:58:48 -
@@ -22,6 +22,10 @@
#include
#include "exec.h"
+/*/
+/* Softmmu support */
+#if !defined (CONFIG_USER_ONLY)
+
#define MMUSUFFIX _mmu
#ifdef __s390__
# define GETPC() ((void*)((unsigned long)__builtin_return_address(0) &
0x7fffUL))
@@ -73,6 +77,8 @@
env = saved_env;
}
+#endif
+
void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
int is_asi)
{
Index: target-i386/exec.h
===
RCS file: /sources/qemu/qemu/target-i386/exec.h,v
retrieving revision 1.39
diff -u -r1.39 exec.h
--- target-i386/exec.h 11 Nov 2007 19:45:49 - 1.39
+++ target-i386/exec.h 12 Nov 2007 04:58:48 -
@@ -164,8 +164,6 @@
void cpu_x86_flush_tlb(CPUX86State *env, target_ulong addr);
int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr,
int is_write, int mmu_idx, int is_softmmu);
-void tlb_fill(target_ulong addr, int is_write, int mmu_idx,
- void *retaddr);
void __hidden cpu_lock(void);
void __hidden cpu_unlock(void);
void do_interrupt(int intno, int is_int, int error_code,
[Qemu-devel] [PATCH] tests/runcom.c: use sys/vm86.h definition of vm86 system call
The following patch fixes a compilation/type mismatch issue for runcom by removing the _syscall2 macro call (deprecated since kernel 2.6.18) and using the definition from for the vm86 system call instead. Carlo --- Index: tests/runcom.c === RCS file: /sources/qemu/qemu/tests/runcom.c,v retrieving revision 1.5 diff -u -r1.5 runcom.c --- tests/runcom.c 17 Sep 2007 08:09:54 - 1.5 +++ tests/runcom.c 12 Nov 2007 07:30:50 - @@ -12,6 +12,7 @@ #include #include +#include //#define SIGTEST @@ -21,8 +22,6 @@ return (type) (res); \ } while (0) -_syscall2(int, vm86, int, func, struct vm86plus_struct *, v86) - #define COM_BASE_ADDR0x10100 void usage(void)
[Qemu-devel] [PATCH] [RESEND] hw/sh7750.c: use TARGET_FMT_plx to printf target_phys_addr_t
The following patch changes the formatting string from %08x to TARGET_FMT_plx
to accommodate for compilation in 64bit hosts and that manifests with the
following warning :
qemu/hw/sh7750.c: In function `error_access':
qemu/hw/sh7750.c:186: warning: unsigned int format, different type arg (arg 5)
qemu/hw/sh7750.c: In function `ignore_access':
qemu/hw/sh7750.c:192: warning: unsigned int format, different type arg (arg 5)
Carlo
---
Index: sh7750.c
===
RCS file: /sources/qemu/qemu/hw/sh7750.c,v
retrieving revision 1.11
diff -u -r1.11 sh7750.c
--- sh7750.c17 Nov 2007 17:14:48 - 1.11
+++ sh7750.c18 Nov 2007 21:08:37 -
@@ -182,13 +182,13 @@
static void error_access(const char *kind, target_phys_addr_t addr)
{
-fprintf(stderr, "%s to %s (0x%08x) not supported\n",
+fprintf(stderr, "%s to %s (0x" TARGET_FMT_plx ") not supported\n",
kind, regname(addr), addr);
}
static void ignore_access(const char *kind, target_phys_addr_t addr)
{
-fprintf(stderr, "%s to %s (0x%08x) ignored\n",
+fprintf(stderr, "%s to %s (0x" TARGET_FMT_plx ") ignored\n",
kind, regname(addr), addr);
}
[Qemu-devel] [PATCH] ide: fix GPCMD_GET_CONFIGURATION for OpenSolaris guests
The following patch complements "Partial IDE DVD emulation" which was added in
revision 1.66 and that was generating the following timeouts for OpenSolaris
guests when trying to access the ATAPI cdrom (during installation for example):
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1);
timeout: abort request, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: abort device, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: reset target, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: reset bus, target=0 lun=0
It has been tested not to introduce any regressions with Linux, BSD and
Windows 2K guests and to fix the problem by installing a new guest with
Nexenta OpenSolaris alpha7.
The changes required are :
* change the model name (used by "INQUIRY" and "IDENTIFY DEVICE") to DVD
* recognize and honor "Allocation Length" parameter in "GET CONFIGURATION"
* only set "current profile" for the "GET CONFIGURATION" response if a profile
is current (CD or DVD loaded)
* calculate "data length" including all headers
* refactor code and add comments to help document references to all related
standards (ATAPI-4, SPC-3 and MMC-6)
Carlo
PS. some parts of the current implementation are for ATA-2 and not all those
standards are implemented fully AFAIK
---
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.72
diff -u -p -r1.72 ide.c
--- hw/ide.c18 Nov 2007 01:44:37 - 1.72
+++ hw/ide.c26 Nov 2007 07:43:43 -
@@ -541,7 +541,7 @@ static void ide_atapi_identify(IDEState
put_le16(p + 21, 512); /* cache size in sectors */
put_le16(p + 22, 4); /* ecc bytes */
padstr((uint8_t *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-padstr((uint8_t *)(p + 27), "QEMU CD-ROM", 40); /* model */
+padstr((uint8_t *)(p + 27), "QEMU DVD-ROM", 40); /* model */
put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
#ifdef USE_DMA_CDROM
put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -1630,12 +1630,13 @@ static void ide_atapi_cmd(IDEState *s)
buf[6] = 0; /* reserved */
buf[7] = 0; /* reserved */
padstr8(buf + 8, 8, "QEMU");
-padstr8(buf + 16, 16, "QEMU CD-ROM");
+padstr8(buf + 16, 16, "QEMU DVD-ROM");
padstr8(buf + 32, 4, QEMU_VERSION);
ide_atapi_cmd_reply(s, 36, max_len);
break;
case GPCMD_GET_CONFIGURATION:
{
+uint32_t len;
int64_t total_sectors;
/* only feature 0 is supported */
@@ -1644,17 +1645,27 @@ static void ide_atapi_cmd(IDEState *s)
ASC_INV_FIELD_IN_CMD_PACKET);
break;
}
-memset(buf, 0, 32);
+max_len = ube16_to_cpu(packet + 7);
bdrv_get_geometry(s->bs, &total_sectors);
-buf[3] = 16;
-buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current
profile */
-buf[10] = 0x10 | 0x1;
-buf[11] = 0x08; /* size of profile list */
+memset(buf, 0, 32);
+if (total_sectors) {
+if (total_sectors > 1433600) {
+buf[7] = 0x10; /* DVD-ROM */
+} else {
+buf[7] = 0x08; /* CD-ROM */
+}
+} else {
+buf[7] = 0x00; /* no current profile */
+}
+buf[10] = 0x02 | 0x01; /* persistent and current */
+buf[11] = 0x08; /* size of profile list = 4 bytes per profile */
buf[13] = 0x10; /* DVD-ROM profile */
-buf[14] = buf[7] == 0x10; /* (in)active */
+buf[14] = buf[13] == buf[7]; /* (in)active */
buf[17] = 0x08; /* CD-ROM profile */
-buf[18] = buf[7] == 0x08; /* (in)active */
-ide_atapi_cmd_reply(s, 32, 32);
+buf[18] = buf[17] == buf[7]; /* (in)active */
+len = 8 + 4 + buf[11]; /* headers + size of profile list */
+cpu_to_ube32(buf, len - 4); /* data length */
+ide_atapi_cmd_reply(s, len, max_len);
break;
}
default:
[Qemu-devel] [PATCH] ide: remove leftover support for 82371FB (Step A1)
The following patch removes the remaining support for the 82371FB (Step A1)
IDE chip which was added to ide.c in revision 1.53 and then removed (as
it was never used in any profile and was considered dead code) in 1.57.
This code was added to piix_pci.c in revision 1.9 and after the corresponding
code was removed from ide.c was generating the following warning :
qemu/hw/piix_pci.c:318: warning: 'piix_init' defined but not used
Carlo
---
Index: hw/piix_pci.c
===
RCS file: /sources/qemu/qemu/hw/piix_pci.c,v
retrieving revision 1.14
diff -u -p -r1.14 piix_pci.c
--- hw/piix_pci.c 18 Nov 2007 01:44:37 - 1.14
+++ hw/piix_pci.c 26 Nov 2007 09:38:57 -
@@ -314,31 +314,6 @@ static int piix_load(QEMUFile* f, void *
return pci_device_load(d, f);
}
-static int piix_init(PCIBus *bus, int devfn)
-{
-PCIDevice *d;
-uint8_t *pci_conf;
-
-d = pci_register_device(bus, "PIIX", sizeof(PCIDevice),
-devfn, NULL, NULL);
-register_savevm("PIIX", 0, 2, piix_save, piix_load, d);
-
-piix3_dev = d;
-pci_conf = d->config;
-
-pci_conf[0x00] = 0x86; // Intel
-pci_conf[0x01] = 0x80;
-pci_conf[0x02] = 0x2E; // 82371FB PIIX PCI-to-ISA bridge
-pci_conf[0x03] = 0x12;
-pci_conf[0x08] = 0x02; // Step A1
-pci_conf[0x0a] = 0x01; // class_sub = PCI_ISA
-pci_conf[0x0b] = 0x06; // class_base = PCI_bridge
-pci_conf[0x0e] = 0x80; // header_type = PCI_multifunction, generic
-
-piix3_reset(d);
-return d->devfn;
-}
-
int piix3_init(PCIBus *bus, int devfn)
{
PCIDevice *d;
[Qemu-devel] [PATCH 1/5] qemu: GET_CONFIGURATION fixes for MMC-6 DVD-ROM implementation
The following patch complements "Partial IDE DVD emulation", added in ide.c
revision 1.66 and that was generating the following timeouts for OpenSolaris
guests when trying to access the ATAPI cdrom (during installation for example):
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1);
timeout: abort request, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: abort device, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: reset target, target=0 lun=0
WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1):
timeout: reset bus, target=0 lun=0
The changes required to the "GET CONFIGURATION" implementation are :
* recognize and honor "Allocation Length" command parameter
* only set "current profile" for the response if a profile is current (either
CD or DVD loaded)
* calculate "data length" including all headers
* refactor code and add comments to help document references to all implemented
standards (ATAPI-4, SPC-3 and MMC-6)
Signed-off-by: Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]>
---
qemu/hw/ide.c | 27 +++
1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c
index 329d053..29fc7a9 100644
--- a/qemu/hw/ide.c
+++ b/qemu/hw/ide.c
@@ -1648,6 +1648,7 @@ static void ide_atapi_cmd(IDEState *s)
break;
case GPCMD_GET_CONFIGURATION:
{
+uint32_t len;
int64_t total_sectors;
/* only feature 0 is supported */
@@ -1656,17 +1657,27 @@ static void ide_atapi_cmd(IDEState *s)
ASC_INV_FIELD_IN_CMD_PACKET);
break;
}
-memset(buf, 0, 32);
+max_len = ube16_to_cpu(packet + 7);
bdrv_get_geometry(s->bs, &total_sectors);
-buf[3] = 16;
-buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current
profile */
-buf[10] = 0x10 | 0x1;
-buf[11] = 0x08; /* size of profile list */
+memset(buf, 0, 32);
+if (total_sectors) {
+if (total_sectors > 1433600) {
+buf[7] = 0x10; /* DVD-ROM */
+} else {
+buf[7] = 0x08; /* CD-ROM */
+}
+} else {
+buf[7] = 0x00; /* no current profile */
+}
+buf[10] = 0x02 | 0x01; /* persistent and current */
+buf[11] = 0x08; /* size of profile list = 4 bytes per profile */
buf[13] = 0x10; /* DVD-ROM profile */
-buf[14] = buf[7] == 0x10; /* (in)active */
+buf[14] = buf[13] == buf[7]; /* (in)active */
buf[17] = 0x08; /* CD-ROM profile */
-buf[18] = buf[7] == 0x08; /* (in)active */
-ide_atapi_cmd_reply(s, 32, 32);
+buf[18] = buf[17] == buf[7]; /* (in)active */
+len = 8 + 4 + buf[11]; /* headers + size of profile list */
+cpu_to_ube32(buf, len - 4); /* data length */
+ide_atapi_cmd_reply(s, len, max_len);
break;
}
default:
--
1.5.2.5
[Qemu-devel] [PATCH 4/5] qemu: ide INQUIRY and IDENTIFY DEVICE report DVD-ROM model
This patch complements "Partial IDE DVD emulation" which was added in ide.c revision 1.66 so that the CD-ROM identifies itself as a DVD-ROM to "INQUIRY" and "IDENTIFY DEVICE" commands Signed-off-by: Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]> --- qemu/hw/ide.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c index a8d4339..a97d519 100644 --- a/qemu/hw/ide.c +++ b/qemu/hw/ide.c @@ -533,7 +533,7 @@ static void ide_atapi_identify(IDEState *s) put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ padstr((uint8_t *)(p + 23), QEMU_VERSION, 8); /* firmware version */ -padstr((uint8_t *)(p + 27), "QEMU CD-ROM", 40); /* model */ +padstr((uint8_t *)(p + 27), "QEMU DVD-ROM", 40); /* model */ put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */ #ifdef USE_DMA_CDROM put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */ @@ -1622,7 +1622,7 @@ static void ide_atapi_cmd(IDEState *s) buf[6] = 0; /* reserved */ buf[7] = 0; /* reserved */ padstr8(buf + 8, 8, "QEMU"); -padstr8(buf + 16, 16, "QEMU CD-ROM"); +padstr8(buf + 16, 16, "QEMU DVD-ROM"); padstr8(buf + 32, 4, QEMU_VERSION); ide_atapi_cmd_reply(s, 36, max_len); break; -- 1.5.2.5
[Qemu-devel] [PATCH 5/5] qemu: piix_pci: remove 82371FB support
This patch removes support for 82371FB Step A1 (AKA triton) chips as they are
superseded by 82371SB and are currently dead code as all other
references were removed already from ide.c since revision 1.57
Signed-off-by: Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]>
---
qemu/hw/piix_pci.c | 25 -
1 files changed, 0 insertions(+), 25 deletions(-)
diff --git a/qemu/hw/piix_pci.c b/qemu/hw/piix_pci.c
index 3c04e3a..f40dad9 100644
--- a/qemu/hw/piix_pci.c
+++ b/qemu/hw/piix_pci.c
@@ -311,31 +311,6 @@ static int piix_load(QEMUFile* f, void *opaque, int
version_id)
return pci_device_load(d, f);
}
-int piix_init(PCIBus *bus, int devfn)
-{
-PCIDevice *d;
-uint8_t *pci_conf;
-
-d = pci_register_device(bus, "PIIX", sizeof(PCIDevice),
-devfn, NULL, NULL);
-register_savevm("PIIX", 0, 2, piix_save, piix_load, d);
-
-piix3_dev = d;
-pci_conf = d->config;
-
-pci_conf[0x00] = 0x86; // Intel
-pci_conf[0x01] = 0x80;
-pci_conf[0x02] = 0x2E; // 82371FB PIIX PCI-to-ISA bridge
-pci_conf[0x03] = 0x12;
-pci_conf[0x08] = 0x02; // Step A1
-pci_conf[0x0a] = 0x01; // class_sub = PCI_ISA
-pci_conf[0x0b] = 0x06; // class_base = PCI_bridge
-pci_conf[0x0e] = 0x80; // header_type = PCI_multifunction, generic
-
-piix3_reset(d);
-return d->devfn;
-}
-
int piix3_init(PCIBus *bus, int devfn)
{
PCIDevice *d;
--
1.5.2.5
Re: [Qemu-devel] PATCH: ide.c: send irq for WIN_DIAGNOSE
On Thu, Nov 29, 2007 at 02:05:36PM +0100, Tristan Gingold wrote: > according to ATA std: which ATA std? > The pending interrupt condition shall be set by: > ??? the completion of a command; or > > This patch sends an irq for WIN_DIAGNOSE (and WIN_SRST) DEVICE RESET or DEVICE DIAGNOSTIC in T13/1153D revision 18 don't ask for an irq. what is the use case you are trying to solve? which guest OS? Carlo
[Qemu-devel] [PATCH 2/2] ide: report model as DVD-ROM for INQUIRY and IDENTIFY DEVICE commands
This patch complements "Partial IDE DVD emulation" and the previous patch to reflect that the device is now able to support DVDs by changing the reported model to be "DVD-ROM" instead of "CD-ROM". Carlo --- Index: hw/ide.c === RCS file: /sources/qemu/qemu/hw/ide.c,v retrieving revision 1.72 diff -u -p -r1.72 ide.c --- hw/ide.c18 Nov 2007 01:44:37 - 1.72 +++ hw/ide.c30 Nov 2007 16:57:52 - @@ -1,5 +1,5 @@ /* - * QEMU IDE disk and CD-ROM Emulator + * QEMU IDE disk and CD/DVD-ROM Emulator * * Copyright (c) 2003 Fabrice Bellard * Copyright (c) 2006 Openedhand Ltd. @@ -541,7 +541,7 @@ static void ide_atapi_identify(IDEState put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ padstr((uint8_t *)(p + 23), QEMU_VERSION, 8); /* firmware version */ -padstr((uint8_t *)(p + 27), "QEMU CD-ROM", 40); /* model */ +padstr((uint8_t *)(p + 27), "QEMU DVD-ROM", 40); /* model */ put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */ #ifdef USE_DMA_CDROM put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */ @@ -1630,7 +1630,7 @@ static void ide_atapi_cmd(IDEState *s) buf[6] = 0; /* reserved */ buf[7] = 0; /* reserved */ padstr8(buf + 8, 8, "QEMU"); -padstr8(buf + 16, 16, "QEMU CD-ROM"); +padstr8(buf + 16, 16, "QEMU DVD-ROM"); padstr8(buf + 32, 4, QEMU_VERSION); ide_atapi_cmd_reply(s, 36, max_len); break;
[Qemu-devel] [PATCH 0/2] ide: fix GPCMD_GET_CONFIGURATION for correct DVD-ROM emulation
The following patch series complements "Partial IDE DVD emulation" which was added in revision 1.66 of ide.c and that was generating the following timeouts for OpenSolaris guests when trying to access the ATAPI CD-ROM (during installation for example): WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1); timeout: abort request, target=0 lun=0 WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1): timeout: abort device, target=0 lun=0 WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1): timeout: reset target, target=0 lun=0 WARNING: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL PROTECTED] (ata1): timeout: reset bus, target=0 lun=0 It has been tested to fix the problem and not to introduce any regression with several releases of Linux, BSD (FreeBSD, OpenBSD, NetBSD and DragonflyBSD), Windows 2K and OpenSolaris (Nexenta, Indiana, SXDE) and Solaris 10 guests in x86 (32bit) and amd64 (64bit) hosts and to match exact responses when compared to responses received by physical SATA and ATAPI DVD-ROM devices. Patch 1/2: fixes GET_CONFIGURATION to allow OpenSolaris CD-ROM access Patch 2/2: uses DVD-ROM as model name for INQUIRY and IDENTIFY DEVICE Carlo
[Qemu-devel] [PATCH] sh4: define explicitly that the target CPU is 32 bit
The following patch enforces that the sh4 target is 32 bit to prevent qemu to expand incorrectly to a 64 bit wide cpu if compiled in a 64 bit host. Carlo --- Index: target-sh4/cpu.h === RCS file: /sources/qemu/qemu/target-sh4/cpu.h,v retrieving revision 1.12 diff -u -r1.12 cpu.h --- target-sh4/cpu.h10 Nov 2007 15:15:53 - 1.12 +++ target-sh4/cpu.h30 Nov 2007 16:08:38 - @@ -22,6 +22,7 @@ #include "config.h" +#define TARGET_PHYS_ADDR_BITS 32 #define TARGET_LONG_BITS 32 #define TARGET_HAS_ICE 1
Re: [Qemu-devel] [PATCH] [RESEND] hw/sh7750.c: use TARGET_FMT_plx to printf target_phys_addr_t
On Fri, Nov 30, 2007 at 05:37:35PM +0200, Blue Swirl wrote: > On 11/30/07, Carlo Marcelo Arenas Belon <[EMAIL PROTECTED]> wrote: > > which might not be what was intended originally and might be uncovering a > > bug somewhere else and based on the fact that apparently (and this gets > > confusing as it seems to be inconsistently used everywhere in qemu) : > > > > target_phys_addr_t = physical address of the host > > ram_addr_t = physical address of the guest > > No, target_phys_addr_t is the physical address of the emulated target > system. For host addresses ram_addr_t, unsigned long or even int is > used. Host addresses are of course virtual, Qemu is a user space > application until someone makes it run in bare metal without OS. thanks, that makes much more sense that the convoluted idea I got from the code, and now that I recovered my sanity can see finally where the bug is. Carlo
[Qemu-devel] [PATCH 1/2] ide: fix GPCMD_GET_CONFIGURATION for OpenSolaris guests
The following patch implements the following changes to the implementation of
"GET CONFIGURATION" as part of the initial MMC-6 support added to ide.c so it
would emulate a multi profile device with DVD-ROM capabilities :
* recognize and honor "Allocation Length" command parameter
* corrected flags for response to match bits for persistent and current as
requested by the standard
* only set "current profile" for the response if a profile is current (either
CD or DVD loaded)
* calculate "data length" including all headers
* refactor code and add comments to help document references to all partially
implemented standards (ATAPI-4, SPC-3 and MMC-6)
Carlo
---
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.72
diff -u -p -r1.72 ide.c
--- hw/ide.c18 Nov 2007 01:44:37 - 1.72
+++ hw/ide.c30 Nov 2007 16:29:49 -
@@ -1636,6 +1636,7 @@ static void ide_atapi_cmd(IDEState *s)
break;
case GPCMD_GET_CONFIGURATION:
{
+uint32_t len;
int64_t total_sectors;
/* only feature 0 is supported */
@@ -1644,17 +1645,27 @@ static void ide_atapi_cmd(IDEState *s)
ASC_INV_FIELD_IN_CMD_PACKET);
break;
}
-memset(buf, 0, 32);
+max_len = ube16_to_cpu(packet + 7);
bdrv_get_geometry(s->bs, &total_sectors);
-buf[3] = 16;
-buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current
profile */
-buf[10] = 0x10 | 0x1;
-buf[11] = 0x08; /* size of profile list */
+memset(buf, 0, 32);
+if (total_sectors) {
+if (total_sectors > 1433600) {
+buf[7] = 0x10; /* DVD-ROM */
+} else {
+buf[7] = 0x08; /* CD-ROM */
+}
+} else {
+buf[7] = 0x00; /* no current profile */
+}
+buf[10] = 0x02 | 0x01; /* persistent and current */
+buf[11] = 0x08; /* size of profile list = 4 bytes per profile */
buf[13] = 0x10; /* DVD-ROM profile */
-buf[14] = buf[7] == 0x10; /* (in)active */
+buf[14] = buf[13] == buf[7]; /* (in)active */
buf[17] = 0x08; /* CD-ROM profile */
-buf[18] = buf[7] == 0x08; /* (in)active */
-ide_atapi_cmd_reply(s, 32, 32);
+buf[18] = buf[17] == buf[7]; /* (in)active */
+len = 8 + 4 + buf[11]; /* headers + size of profile list */
+cpu_to_ube32(buf, len - 4); /* data length */
+ide_atapi_cmd_reply(s, len, max_len);
break;
}
default:
Re: [Qemu-devel] PATCH: ide.c: send irq for WIN_DIAGNOSE
On Thu, Nov 29, 2007 at 05:46:08PM +0100, Tristan Gingold wrote: > On Nov 29, 2007, at 4:07 PM, Carlo Marcelo Arenas Belon wrote: > >On Thu, Nov 29, 2007 at 02:05:36PM +0100, Tristan Gingold wrote: > > >> The pending interrupt condition shall be set by: > >> ??? the completion of a command; or > >> > >>This patch sends an irq for WIN_DIAGNOSE (and WIN_SRST) > > > >DEVICE RESET or DEVICE DIAGNOSTIC in T13/1153D revision 18 don't > >ask for an > >irq. > > Well, not just after the command is executed. But according to 9.5.1 > of 1153D: > > l) After completing the above steps, Device 0 shall assert INTRQ if > nIEN is cleared to zero. > > So the IRQ is asserted at the end of diagnostic. right my bad, missed that on my copy of ATA-4 while looking for a match to your description of the mis-implementation, but why are you asserting one also for the reset? If I am reading the spec and the code right, the patch should be instead : Index: hw/ide.c === RCS file: /sources/qemu/qemu/hw/ide.c,v retrieving revision 1.72 diff -u -r1.72 ide.c --- hw/ide.c18 Nov 2007 01:44:37 - 1.72 +++ hw/ide.c30 Nov 2007 14:02:33 - @@ -2042,6 +2053,7 @@ ide_set_signature(s); s->status = 0x00; /* NOTE: READY is _not_ set */ s->error = 0x01; +ide_set_irq(s); break; case WIN_SRST: if (!s->is_cdrom) > >what is the use case you are trying to solve? which guest OS? > > The OS timeout during diagnostic. is there a way to reproduce that timeout on the guest OS you are using? if using Linux and smartctl, you will get a timeout but not because of this but because SMART is not supported (which might be also a bug, but at least not this bug) Carlo
Re: [Qemu-devel] [PATCH] [RESEND] hw/sh7750.c: use TARGET_FMT_plx to printf target_phys_addr_t
On Fri, Nov 30, 2007 at 02:36:32PM +0900, Magnus Damm wrote: > On Nov 19, 2007 6:18 AM, Carlo Marcelo Arenas Belon > <[EMAIL PROTECTED]> wrote: > > The following patch changes the formatting string from %08x to > > TARGET_FMT_plx > > to accommodate for compilation in 64bit hosts and that manifests with the > > following warning : > > > > qemu/hw/sh7750.c: In function `error_access': > > qemu/hw/sh7750.c:186: warning: unsigned int format, different type arg > > (arg 5) > > qemu/hw/sh7750.c: In function `ignore_access': > > qemu/hw/sh7750.c:192: warning: unsigned int format, different type arg > > (arg 5) > > This patch works fine on 32 bit x86 hosts. Please apply. Thanks, forgot to mention that I tested it of course as well in 32 bit x86 where the code is equivalent as cpu-defs.h defines for 32 bit targets : #define TARGET_FMT_plx "%08x" For 64 bit targets, it will use a 64 bit type for physical addresses and therefore a 64 bit wide format as defined by : #define TARGET_FMT_plx "%016" PRIx64 which might not be what was intended originally and might be uncovering a bug somewhere else and based on the fact that apparently (and this gets confusing as it seems to be inconsistently used everywhere in qemu) : target_phys_addr_t = physical address of the host ram_addr_t = physical address of the guest and so all this function should had been using ram_addr_t instead, and that would need to be redefined to be 64 bit safe and have as well a new formatting string to match that. > > Index: sh7750.c > > === > > RCS file: /sources/qemu/qemu/hw/sh7750.c,v > > retrieving revision 1.11 > > diff -u -r1.11 sh7750.c > > --- sh7750.c17 Nov 2007 17:14:48 - 1.11 > > +++ sh7750.c18 Nov 2007 21:08:37 - > > Could you please create the diff from the top level directory next > time? That way it can be applied with patch -p0 or -p1 directly in the > top level directory which makes patch handling much easier. Thanks! sure, sorry about that, made the mistake when rebasing the patch for this RESEND after a week has past without any feedback. Carlo
Re: [Qemu-devel] [PATCH] sh4: define explicitly that the target CPU is 32 bit
On Fri, Nov 30, 2007 at 04:28:09PM +, Paul Brook wrote: > On Friday 30 November 2007, Carlo Marcelo Arenas Belon wrote: > > The following patch enforces that the sh4 target is 32 bit to prevent qemu > > to expand incorrectly to a 64 bit wide cpu if compiled in a 64 bit host. > > This is wrong. As the comment in cpu-defs.h says, target_phys_addr_t may need > to be bigger than the actual target address space. > > What exactly are you trying to fix? in a generic way, that the CPU width of the host (as defined by the size of the type that is used to store a target_phys_addr_t) that is used to build the emulator affects in any way the size of the emulated target physical address size or its representation. in the sh4 specific case, it doesn't make sense for sh4 to print an access error to a physical address that is 64 bit long when it is a 32 bit CPU and that is what would happen unless the patch is applied. if anything the following definition from cpu-defs.h is invalid for a representation of a 32 bit physical address : #define TARGET_FMT_plx "%016" PRIx64 Carlo
[Qemu-devel] Re: Outstanding patches - Nov 30
On Fri, Nov 30, 2007 at 03:26:31PM +0900, Magnus Damm wrote: Thanks, from the list there are also 2 trivial patches I sent for cris and tests (build problem for runcom) which I'll RESEND then. > Date: Nov 26, 2007 > From: Carlo Marcelo Arenas Belon > Subject: [Qemu-devel] [PATCH] ide: fix GPCMD_GET_CONFIGURATION for > OpenSolaris guests this includes one mandatory change and one optional one which I will split better in two as explained later. > Date: Nov 28, 2007 > From: Laurent Vivier > Subject: [Qemu-devel] [PATCH 0/2] Open disk images with O_DIRECT > Subject: [Qemu-devel] [PATCH 1/2] Add "directio" parameter to "-drive" > Subject: [Qemu-devel] [PATCH 2/2] Direct IDE I/O > > From: Carlo Marcelo Arenas Belon > Date: Nov 29, 2007 > Subject: [Qemu-devel] [PATCH 1/5] qemu: GET_CONFIGURATION fixes for > MMC-6 DVD-ROM implementation > Subject: [Qemu-devel] [PATCH 4/5] qemu: ide INQUIRY and IDENTIFY > DEVICE report DVD-ROM model > Subject: [Qemu-devel] [PATCH 5/5] qemu: piix_pci: remove 82371FB support > Note: Incomplete patch set? Same fixes as Nov 26, 2007 but renamed? my bad, this was sent as a crosspost when posted to kvm-devel (which I thought might be a good idea as it split the first patch for DVD-ROM support in too more digestible pieces and might help on getting any ACKs, NACKs or comments), but now in retrospective where probably only more confusing than helpful. the missing patches weren't included because they where cherry picked from qemu's cvs from the IDE emulation and therefore are irrelevant for qemu but needed in kvm. please ignore these, and will re-base and re-send the original patch for GPCMD_GET_CONFIGURATION split on two instead to simplify patching. Carlo
Re: [Qemu-devel] gcc
On Mon, Dec 03, 2007 at 08:20:28PM -0600, Rick Vernam wrote: > any comments on the current status of moving beyond dependency on GCC 3.3.6? you meant dependency on GCC < 4 right?, I use gcc 3.4.6 and there shouldn't be any reasons AFAIK that wouldn't work. Carlo
[Qemu-devel] yet another proposed solution for gcc 4.x
Greetings,
attached patch, adds a ./configure option for setting the C compiler that will
be used to build op.c for each of the targets; letting the user compile
everything else with gcc 4.x if configured as the default C compiler while
isolating the opcode generation which currently relies in gcc 3.x for dyngen
to be able to make sense of it.
tested it in a gentoo 2006.0 system (amd64) using gcc-4.1.1 to compile
qemu-0.8.1 and the CVS HEAD, by instructing qemu to use gcc-3.4.5 for op.c,
configuring it with :
./configure --op-cc=gcc-3.4.5 --disable-gcc-check
didn't modify the gcc-check function in ./configure to test $op_cc instead to
minimize the changes proposed, and because I wasn't sure that doing so was
beneficial to the end user without a description of the options that can be
used to select a different compiler for different parts of the code.
FYI gcc-4.1.1 is not able to compile qemu-0.8.1 or the CVS HEAD because it
confuses dyngen and makes it generate invalid code as can be seen in gentoo's
bug 132667.
https://bugs.gentoo.org/show_bug.cgi?id=132667
and which includes a hacky fix (by using a helper function), but i consider
wasn't worth fixing as the resulting binary won't work at all (as expected).
Carlo
Index: Makefile.target
===
RCS file: /sources/qemu/qemu/Makefile.target,v
retrieving revision 1.113
diff -u -p -r1.113 Makefile.target
--- Makefile.target 30 May 2006 01:48:12 - 1.113
+++ Makefile.target 4 Jun 2006 07:53:52 -
@@ -445,7 +445,7 @@ gen-op.h: op.o $(DYNGEN)
$(DYNGEN) -g -o $@ $<
op.o: op.c
- $(CC) $(OP_CFLAGS) $(DEFINES) -c -o $@ $<
+ $(OP_CC) $(OP_CFLAGS) $(DEFINES) -c -o $@ $<
helper.o: helper.c
$(CC) $(HELPER_CFLAGS) $(DEFINES) -c -o $@ $<
Index: configure
===
RCS file: /sources/qemu/qemu/configure,v
retrieving revision 1.102
diff -u -p -r1.102 configure
--- configure 14 May 2006 11:30:38 - 1.102
+++ configure 4 Jun 2006 07:53:53 -
@@ -23,6 +23,7 @@ static="no"
cross_prefix=""
cc="gcc"
host_cc="gcc"
+op_cc="gcc"
ar="ar"
make="make"
install="install"
@@ -182,6 +183,8 @@ for opt do
;;
--host-cc=*) host_cc="$optarg"
;;
+ --op-cc=*) op_cc="$optarg"
+ ;;
--make=*) make="$optarg"
;;
--install=*) install="$optarg"
@@ -271,6 +274,7 @@ echo " --source-path=PATH path of
echo " --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]"
echo " --cc=CC use C compiler CC [$cc]"
echo " --host-cc=CC use C compiler CC [$host_cc] for dyngen etc."
+echo " --op-cc=CC use C compiler CC [$op_cc] for op.c"
echo " --make=MAKE use specified make [$make]"
echo " --install=INSTALLuse specified install [$install]"
echo " --static enable static build [$static]"
@@ -417,7 +421,7 @@ int main(void) {
EOF
have_gcc3_options="no"
-if $cc -fno-reorder-blocks -fno-optimize-sibling-calls -o $TMPO $TMPC 2>
/dev/null ; then
+if $op_cc -fno-reorder-blocks -fno-optimize-sibling-calls -o $TMPO $TMPC 2>
/dev/null ; then
have_gcc3_options="yes"
fi
@@ -522,6 +526,7 @@ fi
echo "Source path $source_path"
echo "C compiler$cc"
echo "Host C compiler $host_cc"
+echo "OP C compiler $op_cc"
echo "make $make"
echo "install $install"
echo "host CPU $cpu"
@@ -588,6 +593,7 @@ if test "$have_gcc3_options" = "yes" ; t
echo "HAVE_GCC3_OPTIONS=yes" >> $config_mak
fi
echo "HOST_CC=$host_cc" >> $config_mak
+echo "OP_CC=$op_cc" >> $config_mak
echo "AR=$ar" >> $config_mak
echo "STRIP=$strip -s -R .comment -R .note" >> $config_mak
echo "CFLAGS=$CFLAGS" >> $config_mak
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] yet another proposed solution for gcc 4.x
> Why bother? As you say gcc4 has issues other than just op.c, so why not just > compile everything with the old gcc? using the new gcc for the parts that can compile with it, could lead to better performance in some cases, as well to help clean up the code for conformance to newer standards and overall maintainability, while making also clear that there are at least 2 different uses for gcc. 1) to compile the code for qemu to generate working binaries 2) to generate the code to be used for opcode translation this will free us to work in whatever technical solutions are needed to fix 2) in our own schedule without the pressure from all the people that now feel compelled to "port" qemu to a newer gcc, because it is now the default on their corresponding distributions. the long term solution for 2) will be to get the qemu hand written code generator completed, but could also include having an in tree version of gcc that can be used for that simple purpose (like a dependant library/tool) or a reference to an external one as a dependency, and as an intermediate step. Carlo PS. eventhough it wasn't the cleanest build, gcc-4.1.1 when used to build everything but op.c resulted in working binaries on my gentoo 2006.0 amd64 system. ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] yet another proposed solution for gcc 4.x
On Mon, Jun 05, 2006 at 01:21:48PM +0200, Christian MICHON wrote:
> did you get better benchmark results than using gcc-3.x ?
proportionally to native execution, yes (the gcc-4.1.1's compiled sha1-i386
binary from the tests was slower than the one compiled with gcc-3.4.5)
results for running in an amd64 box with native x86 support and no kqemu for a
manually tweaked (as the Makefile won't work for amd64) `make speed` shown
below :
for the gcc-3.4.5 compiled qemu 0.8.1 (vanilla untar in /var/tmp):
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1/tests $ cat ../config-host.mak | grep CC
CC=gcc-3.4.5
HAVE_GCC3_OPTIONS=yes
HOST_CC=gcc-3.4.5
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1/tests $ gcc-3.4.5 -m32 -Wall -O2 -g -o
sha1-i386 sha1.c
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1/tests $ time ./sha1-i386
SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
real0m0.027s
user0m0.025s
sys 0m0.001s
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1/tests $ time ../i386-user/qemu-i386
./sha1-i386
SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
real0m0.255s
user0m0.250s
sys 0m0.005s
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1/tests $ echo | awk '{ print 255/27; }'
9.4
for the patched qemu with hybrid compilers (patched in /var/tmp with
directory name qemu-0.8.1-hybrid)
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1-hybrid/tests $ cat ../config-host.mak |
grep CC
CC=gcc
HAVE_GCC3_OPTIONS=yes
HOST_CC=gcc
OP_CC=gcc-3.4.5
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1-hybrid/tests $ gcc -m32 -Wall -O2 -g -o
sha1-i386 sha1.c
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1-hybrid/tests $ time ./sha1-i386
SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
real0m0.031s
user0m0.030s
sys 0m0.000s
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1-hybrid/tests $ time ../i386-user/qemu-i386
./sha1-i386
SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
real0m0.287s
user0m0.284s
sys 0m0.004s
[EMAIL PROTECTED] /var/tmp/qemu-0.8.1-hybrid/tests $ echo | awk '{ print 287/31;
}'
9.25806
Carlo
PS. is there any "standard" way to benchmark qemu that is all agreed upon as
authoritative other than `make speed`?
PS2. showing here the output of a typical sample, but obviously i run them
several times to eliminate any statistical noise.
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] time for 0.8.3/0.9?
On Tue, Dec 26, 2006 at 10:16:38AM +0100, Werner Dittmann wrote: > When I try to install Opensuse 10.2 as a 64 bit system the Qemu seems to > go into a loop. same thing happens with Fedora Core 6, but still I am not sure that is enough to pinpoint a problem to qemu, as they both share the same version of the kernel (which is the one hanging). FWIW, I have a Gentoo Linux 2006.1 host running a 64bit qemu 0.8.2 that was partialy compiled with gcc 3.4.5 and that runs without problem the following 64bit systems : Ubuntu Linux 6/06 Fedora Core 4 and 5 rPath Linux 1.0 T2 minimal 2.1.0 and desktop 6.0.0 Oracle Enterprise Linux R4 U4 OpenBSD 3.9 and 4.0 (no kqemu) NetBSD 3.0.1 and 3.1 (no kqemu) FreeBSD 6.1 (no kqemu) Solaris 10 (32bit because QEMU CPU isn't detected as amd64 enabled) Carlo ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] DMA timeouts running a FreeBSD guest with last CVS snapshot
Greetings, while testing a snapshot from the last CVS tree with a freshly installed amd64 FreeBSD 6.2 guest I noticed the following errors at boot time : ad0: 2048MB at ata0-master WDMA2 ad0: FAILURE - READ_DMA timed out LBA=4194301 acd0: CDROM at ata1-master WDMA2 acd0: TIMEOUT - READ BIG retrying (1 retry left) after the system finishes booting there are apparently no more timeouts and the disk reports it is using UDMA2 through atacontrol but the IO performance is poor while reads to the virtual disk around the same sector (the disk is 4194304 sectors long) that failed work fine. # atacontrol mode ad0 current mode = WDMA2 # atacontrol cap ad0 Protocol ATA/ATAPI revision 7 device model QEMU HARDDISK serial number QM1 firmware revision 0.8.3 cylinders 4161 heads 16 sectors/track 63 lba supported 4194304 sectors lba48 supported 4194304 sectors dma supported overlap not supported the same can be reproduced with a FreeBSD 6.1 guest and also with i386 guests including an old FreeBSD 5.3 guest and even with an amd64 Debian GNU/kFreeBSD guest but AFAIK not with Linux, OpenBSD or [Open]Solaris. the following are the details as detected by the FreeBSD kernel at boot from dmesg (the relevant stuff) : acpi0: on motherboard pcib0: port 0xcf8-0xcff on acpi0 pci0: on pcib0 atapci0: port 0x1f0-0x1f7,0x3f6,0x170-0x177,0x376,0xc000-0xc00f at device 1.1 on pci0 ata0: on atapci0 ata1: on atapci0 the ata driver is configured to ask the devide for the available modes and seems to be getting DMA2 and PIO4 from it, while defaulting to DMA as configured. the problem goes away if it is forced to run in PIO4 mode which is the only possibility for the FreeBSD 5.3 system which otherwise hungs at boot. the problem doesn't exist in qemu 0.8.2 and is therefore a regression, but i'd yet to bisect the last changes to find the culprit, and since it seems to be a FreeBSD only problem I am first adding debugging to the guest kernel, to track it from the guest side. nonetheless wanted to send this email, so that you guys had a heads up, and to see if anyone had any ideas or some similar problems in other platforms (I am using a Gentoo Linux Host). Carlo ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] [BUG] QEMU x86_64 SSE bug in modf()
On Mon, Jan 15, 2007 at 11:18:01AM +0100, Ludovic Drolez wrote: > > Float to string conversion uses modf() but this function fails under QEMU > and SLES 64, as you can see in this small test program below: pressume you mean running SLES 10 64bit as a guest under QEMU here. which version of qemu for the host? and what platform/arch? > The SLES's glibc uses lots of SSE instructions as you can see in the dump > below (gcc 4.1.0): the gcc that is used for the glibc in the guest should be irrelevant if all the emulated instructions are correctly compiled in the host and there are no bugs on them of course. the host has to be compiled with gcc 3, gcc 4.x won't work even if it compiles. > ===SLES 64 modf()== > 0002ed00 : >2ed00: f2 0f 11 44 24 f8 movsd >%xmm0,0xfff8(%rsp) >2ed06: 48 8b 44 24 f8 mov0xfff8(%rsp),%rax >2ed0b: 66 0f 28 c8 movapd %xmm0,%xmm1 movapd is actually an SSE2 instruction, and that seem to be the main difference between this function and the one from Debian (which uses only SSE) just like the one from my host (Gentoo) > Would someone be able to track down this SSE QEMU bug seen only in SLES's > modf() function ? hopefully you already did. Carlo ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [PATCH] Re: DMA timeouts running a FreeBSD guest with last CVS snapshot
On Mon, Jan 15, 2007 at 03:21:36AM -0600, Carlo Marcelo Arenas Belon wrote:
> FreeBSD 6.2 guest I noticed the following errors at boot time :
>
> ad0: 2048MB at ata0-master WDMA2
> ad0: FAILURE - READ_DMA timed out LBA=4194301
> acd0: CDROM at ata1-master WDMA2
> acd0: TIMEOUT - READ BIG retrying (1 retry left)
>
The problem is that FreeBSD is sending a WIN_READDMA (0xC8) command to the
emulated PIIX3 controller before it sets the DMA address that will be used and
therefore the code in ide_dma_start is not setting bm->cur_addr to the right
value (it is left set to 0) and then it is failing the validation in
dma_buf_rw where the distance between bm->cur_addr and bm->addr is expected to
be smaller than 1 page (4K) and resulting in an EOT error and a timeout which
FreeBSD handles by resetting the IDE controller.
Apparently all other guests I'd tried (Linux, Solaris, OpenBSD and Windows)
set the address first (bmdma_addr_write) and then send the request for a DMA
command and are therefore not affected by this, but even if FreeBSD behavior
is peculiar, it should be OK if implemented against real hardware as per the
spec in :
http://www.intel.com/design/intarch/datashts/290550.htm
The following patch moves the initialization of bm->cur_addr to match FreeBSD
behavior while being also compatible with all other guests but keeping in
sync closely the values of the memory addresses which will be used for the DMA
in a way that better emulates real hardware.
Tested with guests running FreeBSD 6.2 amd64, OpenBSD 4.0 amd64, Ubuntu Linux
6.06 amd64, OpenSolaris b55b amd64, Windows 2000 Professional i386 and Gentoo
Linux 2006.1 i386.
Carlo
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.51
diff -u -r1.51 ide.c
--- hw/ide.c20 Jan 2007 01:12:17 - 1.51
+++ hw/ide.c22 Jan 2007 09:50:20 -
@@ -2230,10 +2230,11 @@
return;
bm->ide_if = s;
bm->dma_cb = dma_cb;
-bm->cur_addr = bm->addr;
bm->cur_prd_last = 0;
bm->cur_prd_addr = 0;
bm->cur_prd_len = 0;
+if (bm->cur_addr != bm->addr)
+bm->cur_addr = bm->addr;
if (bm->status & BM_STATUS_DMAING) {
bm->dma_cb(bm, 0);
}
@@ -2363,6 +2364,7 @@
printf("%s: 0x%08x\n", __func__, val);
#endif
bm->addr = val & ~3;
+bm->cur_addr = bm->addr;
}
static void bmdma_map(PCIDevice *pci_dev, int region_num,
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [PATCH] Add support for 82371FB (Step A1) and Improved support for 82371SB (function 1)
Greetings,
as part of the debugging for the previous problem on FreeBSD and based on the
documentation from Intel from :
http://www.intel.com/design/intarch/datashts/290550.htm
the following patch does some cleanup in the current sources so that a reset
for function 1 (the IDE interface) for PIIX and PIIX3 is supported and PIIX
could be used instead of PIIX3 if needed (by swapping the corresponding
piix?_init and pci_piix?_ide_init functions in hw/pc.c or creating another
cloned emulated "pc").
most of the code is currently not linked, except for the addition of
piix3_reset to the reset for function 1 of PIIX3 to match the spec and the
removal of 2 duplicated lines on the reset functions for PIIX3 and PIIX4, but
it is left behind as it could be useful for a new target for testing of legacy
OSs or embedded.
Carlo
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.51
diff -u -r1.51 ide.c
--- hw/ide.c20 Jan 2007 01:12:17 - 1.51
+++ hw/ide.c22 Jan 2007 09:50:20 -
@@ -2577,6 +2579,55 @@
return 0;
}
+static void piix3_reset(PCIIDEState *d)
+{
+uint8_t *pci_conf = d->dev.config;
+
+pci_conf[0x04] = 0x00;
+pci_conf[0x05] = 0x00;
+pci_conf[0x06] = 0x80; /* FBC */
+pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
+pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+}
+
+void pci_piix_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn)
+{
+PCIIDEState *d;
+uint8_t *pci_conf;
+
+/* register a function 1 of PIIX */
+d = (PCIIDEState *)pci_register_device(bus, "PIIX IDE",
+ sizeof(PCIIDEState),
+ devfn,
+ NULL, NULL);
+d->type = IDE_TYPE_PIIX3;
+
+pci_conf = d->dev.config;
+pci_conf[0x00] = 0x86; // Intel
+pci_conf[0x01] = 0x80;
+pci_conf[0x02] = 0x30;
+pci_conf[0x03] = 0x12;
+pci_conf[0x08] = 0x02; // Step A1
+pci_conf[0x09] = 0x80; // legacy ATA mode
+pci_conf[0x0a] = 0x01; // class_sub = PCI_IDE
+pci_conf[0x0b] = 0x01; // class_base = PCI_mass_storage
+pci_conf[0x0e] = 0x00; // header_type
+
+piix3_reset(d);
+
+pci_register_io_region((PCIDevice *)d, 4, 0x10,
+ PCI_ADDRESS_SPACE_IO, bmdma_map);
+
+ide_init2(&d->ide_if[0], hd_table[0], hd_table[1],
+ pic_set_irq_new, isa_pic, 14);
+ide_init2(&d->ide_if[2], hd_table[2], hd_table[3],
+ pic_set_irq_new, isa_pic, 15);
+ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
+ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
+
+register_savevm("ide", 0, 1, pci_ide_save, pci_ide_load, d);
+}
+
/* hd_table must contain 4 block drivers */
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
void pci_piix3_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn)
@@ -2601,6 +2652,8 @@
pci_conf[0x0b] = 0x01; // class_base = PCI_mass_storage
pci_conf[0x0e] = 0x00; // header_type
+piix3_reset(d);
+
pci_register_io_region((PCIDevice *)d, 4, 0x10,
PCI_ADDRESS_SPACE_IO, bmdma_map);
Index: hw/piix_pci.c
===
RCS file: /sources/qemu/qemu/hw/piix_pci.c,v
retrieving revision 1.8
diff -u -r1.8 piix_pci.c
--- hw/piix_pci.c 15 Jan 2007 17:08:08 - 1.8
+++ hw/piix_pci.c 22 Jan 2007 09:50:20 -
@@ -246,7 +246,6 @@
pci_conf[0x80] = 0x00;
pci_conf[0x82] = 0x00;
pci_conf[0xa0] = 0x08;
-pci_conf[0xa0] = 0x08;
pci_conf[0xa2] = 0x00;
pci_conf[0xa3] = 0x00;
pci_conf[0xa4] = 0x00;
@@ -284,7 +283,6 @@
pci_conf[0x80] = 0x00;
pci_conf[0x82] = 0x00;
pci_conf[0xa0] = 0x08;
-pci_conf[0xa0] = 0x08;
pci_conf[0xa2] = 0x00;
pci_conf[0xa3] = 0x00;
pci_conf[0xa4] = 0x00;
@@ -312,6 +310,31 @@
return pci_device_load(d, f);
}
+int piix_init(PCIBus *bus, int devfn)
+{
+PCIDevice *d;
+uint8_t *pci_conf;
+
+d = pci_register_device(bus, "PIIX", sizeof(PCIDevice),
+devfn, NULL, NULL);
+register_savevm("PIIX", 0, 2, piix_save, piix_load, d);
+
+piix3_dev = d;
+pci_conf = d->config;
+
+pci_conf[0x00] = 0x86; // Intel
+pci_conf[0x01] = 0x80;
+pci_conf[0x02] = 0x2E; // 82371FB PIIX PCI-to-ISA bridge
+pci_conf[0x03] = 0x12;
+pci_conf[0x08] = 0x02; // Step A1
+pci_conf[0x0a] = 0x01; // class_sub = PCI_ISA
+pci_conf[0x0b] = 0x06; // class_base = PCI_bridge
+pci_conf[0x0e] = 0x80; // header_type = PCI_multifunction, generic
+
+piix3_reset(d);
+return d->devfn;
+}
+
int piix3_init(PCIBus *bus, int devfn)
{
PCIDevice *d;
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [PATCH] add a SNAPSHOT flag to be able to generate snapshots from CVS
Greetings, The following patch adds a "SNAPSHOT" variable to the Makefile which can be changed to "yes" (or any other value) so that `make -k tar` generates a snapshot tar (on a configured or not CVS tree) instead of a release source tar (useful when debugging a version of the CVS tree) by using a generated version based on the current date through GNU date. Only the name of the tar is changed so to keep all internal logic that relies in the version unchanged. Carlo Index: Makefile === RCS file: /sources/qemu/qemu/Makefile,v retrieving revision 1.110 diff -u -r1.110 Makefile --- Makefile7 Jan 2007 22:04:40 - 1.110 +++ Makefile22 Jan 2007 09:50:17 - @@ -8,6 +8,8 @@ BASE_CFLAGS= BASE_LDFLAGS= +SNAPSHOT= + BASE_CFLAGS += $(OS_CFLAGS) ifeq ($(ARCH),sparc) BASE_CFLAGS += -mcpu=ultrasparc @@ -128,7 +130,15 @@ html: qemu-doc.html qemu-tech.html -FILE=qemu-$(shell cat VERSION) +ifndef VERSION +VERSION=$(shell cat VERSION) +endif + +ifneq ($(SNAPSHOT),) +SNAPSHOT=.$(shell date -u "+%Y%m%d%H%M") +endif + +FILE=qemu-$(VERSION)$(SNAPSHOT) # tar release (use 'make -k tar' on a checkouted tree) tar: ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] Re: DMA timeouts running a FreeBSD guest with last CVS snapshot
On Mon, Jan 22, 2007 at 11:55:11AM +0100, Aurelien Jarno wrote: > Carlo Marcelo Arenas Belon a écrit : > > > > The following patch moves the initialization of bm->cur_addr to match > > FreeBSD behavior while being also compatible with all other guests > > the following snippet kept the initialization of bm->cur_addr inside the ide_dma_start function as a failback as that is where it conceptually belongs. that is not needed though, as the initialization of bm->addr and also bm->cur_addr is now done when the address for DMA is send to the emulated PIIX. > > Index: hw/ide.c > > === > > RCS file: /sources/qemu/qemu/hw/ide.c,v > > retrieving revision 1.51 > > diff -u -r1.51 ide.c > > --- hw/ide.c20 Jan 2007 01:12:17 - 1.51 > > +++ hw/ide.c22 Jan 2007 09:50:20 - > > @@ -2230,10 +2230,11 @@ > > return; > > bm->ide_if = s; > > bm->dma_cb = dma_cb; > > -bm->cur_addr = bm->addr; > > bm->cur_prd_last = 0; > > bm->cur_prd_addr = 0; > > bm->cur_prd_len = 0; > > +if (bm->cur_addr != bm->addr) > > +bm->cur_addr = bm->addr; > > Why using a condition here? This will probably be slower than doing > bm->cur_addr = bm->addr and the result is the same. as I mentioned before, this initialization is not even needed, but I kept it as a way to keep the original logic that used this function to setup all initial values for the DMA. in retrospective though I can see why it was a bad idea and confusing, so I'd prepared version 2 of the patch that removes this and eliminates your concern as well. thanks, Carlo ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [PATCH V2] DMA timeouts running a FreeBSD guest
Greetings,
the following patch moves the initialization for bm->cur_addr from the
ide_dma_start function to bmdma_addr_writel, so that it is set in sync
with the call to set the destination address for DMA (bm->addr) and to
avoid timeouts in guests that set this address after they have called
the READ_DMA command (FreeBSD).
the final package has been tested with the following guests :
FreeBSD 6.2 (amd64 and i386)
FreeBSD 5.3 (i386)
Fedora Core 6 (i386)
Gentoo Linux 2006.1 (i386)
NexentaOS Alpha 6 (amd64)
OpenSolaris Nevada b55b (amd64)
OpenBSD 4.0 (amd64 and i386)
Windows 2000 (i386)
Carlo
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.51
diff -u -r1.51 ide.c
--- hw/ide.c20 Jan 2007 01:12:17 - 1.51
+++ hw/ide.c23 Jan 2007 09:58:44 -
@@ -2230,7 +2230,6 @@
return;
bm->ide_if = s;
bm->dma_cb = dma_cb;
-bm->cur_addr = bm->addr;
bm->cur_prd_last = 0;
bm->cur_prd_addr = 0;
bm->cur_prd_len = 0;
@@ -2363,6 +2362,7 @@
printf("%s: 0x%08x\n", __func__, val);
#endif
bm->addr = val & ~3;
+bm->cur_addr = bm->addr;
}
static void bmdma_map(PCIDevice *pci_dev, int region_num,
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [PATCH] allow disabling IDE block mode
Greetings,
the following patch changes the logic for the processing of WIN_SETMULT so
that setting it to 0 (off) is a valid operation as shown by (running Linux
on qemu)
# hdparm -m0 /dev/hda
/dev/hda:
setting multcount to 0
multcount= 0 (off)
this is specially visible while running Ubuntu Linux 6.06 (dapper) on qemu as
it by default disables multmode at boot resulting in the following error :
hda: set_multmode: status=0x41 { DriveReady Error }
hda: set_multmode: error=0x04 { DriveStatusError }
ide: failed opcode was: 0xef
Carlo
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.53
diff -u -r1.53 ide.c
--- hw/ide.c24 Jan 2007 21:35:22 - 1.53
+++ hw/ide.c11 Feb 2007 20:32:24 -
@@ -1631,9 +1631,8 @@
ide_set_irq(s);
break;
case WIN_SETMULT:
-if (s->nsector > MAX_MULT_SECTORS ||
-s->nsector == 0 ||
-(s->nsector & (s->nsector - 1)) != 0) {
+if (s->nsector != 0 && (s->nsector > MAX_MULT_SECTORS ||
+(s->nsector & (s->nsector - 1)) != 0)) {
ide_abort_command(s);
} else {
s->mult_sectors = s->nsector;
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [PATCH] add "support" for enable/disable reverting to power-on defaults
Greetings,
the following patch adds subcommands 0xCC and 0x66 for enabling/disabling
reverting to power-on defaults after a soft reset as invoked by the following
command (running under Linux) :
# hdparm -K1 /dev/hda
/dev/hda:
setting drive keep features to 1 (on)
this is specially visible in OpenSolaris that locks the drive configuration
at boot as shown by (line 1366):
http://cvs.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/intel/io/dktp/controller/ata/ata_common.c
and therefore will complain with the following error when booted in qemu :
ata_set_feature: (0x66,0x0) failed
the proposed implementation just ignores the flag but is consistent with the
current behavior for the other IDE feature flags (read look-ahead and write
cache) a complete implementation for all SET_FEATURES subcommands as spelled
in section 8.37 of the ATA/ATAPI 5 (T13/1321D revision 3) standard will be
provided later if the increase in complexity size is worth the added
functionality (to be debated)
Carlo
Index: hw/ide.c
===
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.53
diff -u -r1.53 ide.c
--- hw/ide.c24 Jan 2007 21:35:22 - 1.53
+++ hw/ide.c11 Feb 2007 20:32:24 -
@@ -1729,6 +1728,8 @@
goto abort_cmd;
/* XXX: valid for CDROM ? */
switch(s->feature) {
+case 0xcc: /* reverting to power-on defaults enable */
+case 0x66: /* reverting to power-on defaults disable*/
case 0x02: /* write cache enable */
case 0x82: /* write cache disable */
case 0xaa: /* read look-ahead enable */
___
Qemu-devel mailing list
Qemu-devel@nongnu.org
http://lists.nongnu.org/mailman/listinfo/qemu-devel
