Re: [Qemu-devel] [PATCH] target-arm: Move VLD/VST multiple into helper functions
On 06/03/2011 11:47 PM, Aurelien Jarno wrote: On Mon, May 02, 2011 at 05:01:24PM +0100, Peter Maydell wrote: On 20 April 2011 15:52, Peter Maydell wrote: Move VLD/VST multiple into helper functions, as some cases can generate more TCG ops than the maximum per-instruction limit and certainly more than the recommended 20. I've had a review comment in private email that this patch slows down ffmpeg by a little over 4% (in a linux-user execution run where about 5% of instructions were an affected vld/vst). So perhaps the right approach is to increase the max-ops-per-insn limit and update the guidance in tcg/README instead? Does this patch fixes a real issue (ie most probably a crash), or it is just to make the arm target compliant with the README? Yes, it (or the similar patch in meego qemu) did fix an real issue. IIRC when running pixman code where vld4.8 variant of instruction was used. Riku
Re: [Qemu-devel] [Bug 793317] [NEW] Large amount of write-only variables
On 06.06.2011, at 08:06, David Gibson wrote: > On Mon, Jun 06, 2011 at 01:02:33AM -, Eli wrote: >> Public bug reported: >> >> Whenever I try to compile the source from the git repo, it gets a large >> number of "set but not used" errors, in files such as: >> >> hw/usb_ochi.c (line 526, 1114, and 1108) >> hw/lsi53c895a.c (line 892) >> kvm.c (line 973) >> target-alpha/translate.c (line 1472, 1470) >> linux-user/syscall.c (line 7062, 3754) >> exec.c (line 1211) >> linux-user/linuxload.c (line 60) >> >> Really now, why would anyone create so many write-only variables knowing >> that it's compiled with write-only variables causing errors..? > > It's a new warning in gcc 4.6, which is why these weren't found > before. Yeah, for a quick compile workaround, use ./configure --disable-werror. Otherwise, please send patches that get rid of the write-only variables. People simply didn't get warnings on them before, which is why they slipped through. Alex -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/793317 Title: Large amount of write-only variables Status in QEMU: New Bug description: Whenever I try to compile the source from the git repo, it gets a large number of "set but not used" errors, in files such as: hw/usb_ochi.c (line 526, 1114, and 1108) hw/lsi53c895a.c (line 892) kvm.c (line 973) target-alpha/translate.c (line 1472, 1470) linux-user/syscall.c (line 7062, 3754) exec.c (line 1211) linux-user/linuxload.c (line 60) Really now, why would anyone create so many write-only variables knowing that it's compiled with write-only variables causing errors..? Host: Fedora 15 i686 Gcc: 4.6.0 Guest: None I pulled the code from the git repo earlier today, so I'm pretty sure it did not get fixed in that short timeframe.
[Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
This patch almost rewrites acpi_table_add() function
(but still leaves it using old get_param_value() interface).
The result is that it's now possible to specify whole table
(together with a header) in an external file, instead of just
data portion, with a new file= parameter, but at the same time
it's still possible to specify header fields as before.
Now with the checkpatch.pl formatting fixes, thanks to
Stefan Hajnoczi for suggestions, with changes from
Isaku Yamahata, and with my further refinements.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect (noted by Blue Swirl)
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 292 ---
qemu-options.hx |7 +-
2 files changed, 175 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..4316189 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+uint16_t _length; /* our length, not actual part of the hdr */
+ /* XXX why we have 2 length fields here? */
+char sig[4]; /* ACPI signature (4 ASCII characters) */
uint32_t length; /* Length of table, in bytes, including header */
uint8_t revision; /* ACPI Specification minor version # */
uint8_t checksum; /* To make sum of entire table == 0 */
-char oem_id [6]; /* OEM identification */
-char oem_table_id [8]; /* OEM table identification */
+char oem_id[6]; /* OEM identification */
+char oem_table_id[8]; /* OEM table identification */
uint32_t oem_revision;/* OEM revision number */
-char asl_compiler_id [4]; /* ASL compiler vendor ID */
+char asl_compiler_id[4]; /* ASL compiler vendor ID */
uint32_t asl_compiler_revision; /* ASL compiler revision number */
} __attribute__((packed));
+#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
+#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */
+
+static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] =
+"\0\0" /* fake _length (2) */
+"QEMU\0\0\0\0\1\0" /* sig (4), len(4), revno (1), csum (1) */
+"QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
+"QEMU\1\0\0\0" /* ASL compiler ID (4), version (4) */
+;
+
char *acpi_tables;
size_t acpi_tables_len;
@@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
return (-sum) & 0xff;
}
+/* like strncpy() but zero-fills the tail of destination */
+static void strzcpy(char *dst, const char *src, size_t size)
+{
+size_t len = strlen(src);
+if (len >= size) {
+len = size;
+} else {
+ memset(dst + len, 0, size - len);
+}
+memcpy(dst, src, len);
+}
+
+/* XXX fixme: this function uses obsolete argument parsing interface */
int acpi_table_add(const char *t)
{
-static const char *dfl_id = "QEMUQEMU";
char buf[1024], *p, *f;
-struct acpi_table_header acpi_hdr;
unsigned long val;
-uint32_t length;
-struct acpi_table_header *acpi_hdr_p;
-size_t off;
+size_t len, start, allen;
+bool has_header;
+int changed;
+int r;
+struct acpi_table_header hdr;
+
+r = 0;
+r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0;
+r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0;
+switch (r) {
+case 0:
+buf[0] = '\0';
+case 1:
+has_header = false;
+break;
+case 2:
+has_header = true;
+break;
+default:
+fprintf(stderr, "acpitable: both data and file are specified\n");
+return -1;
+}
+
+if (!acpi_tables) {
+allen = sizeof(uint16_t);
+acpi_tables = qemu_mallocz(allen);
+}
+else {
+allen = acpi_tables_len;
+}
+
+start = allen;
+acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE);
+allen += has_header ? ACPI_TABLE_PFX_SIZE : ACPI_TABLE_HDR_SIZE;
+
+/* now read in the data files, reallocating buffer as needed */
+
+for (f = strtok(buf, ":"); f; f = strtok(NULL, ":")) {
+int fd = open(f, O_RDONLY);
+
+if (fd < 0) {
+fprintf(stderr, "can't open file %s: %s\n", f, strerror(errno));
+return -1;
+}
+
+for (;;) {
+char data[8192];
+r = read(fd, data, sizeof(data));
+if (r == 0) {
+break;
+} else if (r > 0) {
+acpi_tables = qemu_realloc(acpi_tables, allen + r);
+memcpy(acpi_tables + allen, data, r);
+allen += r;
+} else if (errno != EINTR) {
+fprintf(stderr, "can't read file %s: %s\n",
+f, strerror(errno));
+close(fd);
+return -1;
+}
+}
+
+
[Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
This patch almost rewrites acpi_table_add() function
(but still leaves it using old get_param_value() interface).
The result is that it's now possible to specify whole table
(together with a header) in an external file, instead of just
data portion, with a new file= parameter, but at the same time
it's still possible to specify header fields as before.
Now with the checkpatch.pl formatting fixes, thanks to
Stefan Hajnoczi for suggestions, with changes from
Isaku Yamahata, and with my further refinements.
v5: rediffed against current qemu/master.
v6: fix one "} else {" coding style defect
Signed-off-by: Michael Tokarev
---
hw/acpi.c | 291 ---
qemu-options.hx |7 +-
2 files changed, 174 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..b8cd866 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+uint16_t _length; /* our length, not actual part of the hdr */
+ /* XXX why we have 2 length fields here? */
+char sig[4]; /* ACPI signature (4 ASCII characters) */
uint32_t length; /* Length of table, in bytes, including header */
uint8_t revision; /* ACPI Specification minor version # */
uint8_t checksum; /* To make sum of entire table == 0 */
-char oem_id [6]; /* OEM identification */
-char oem_table_id [8]; /* OEM table identification */
+char oem_id[6]; /* OEM identification */
+char oem_table_id[8]; /* OEM table identification */
uint32_t oem_revision;/* OEM revision number */
-char asl_compiler_id [4]; /* ASL compiler vendor ID */
+char asl_compiler_id[4]; /* ASL compiler vendor ID */
uint32_t asl_compiler_revision; /* ASL compiler revision number */
} __attribute__((packed));
+#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
+#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */
+
+static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] =
+"\0\0" /* fake _length (2) */
+"QEMU\0\0\0\0\1\0" /* sig (4), len(4), revno (1), csum (1) */
+"QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
+"QEMU\1\0\0\0" /* ASL compiler ID (4), version (4) */
+;
+
char *acpi_tables;
size_t acpi_tables_len;
@@ -45,158 +57,191 @@ static int acpi_checksum(const uint8_t *data, int len)
return (-sum) & 0xff;
}
+/* like strncpy() but zero-fills the tail of destination */
+static void strzcpy(char *dst, const char *src, size_t size)
+{
+size_t len = strlen(src);
+if (len >= size) {
+len = size;
+} else {
+ memset(dst + len, 0, size - len);
+}
+memcpy(dst, src, len);
+}
+
+/* XXX fixme: this function uses obsolete argument parsing interface */
int acpi_table_add(const char *t)
{
-static const char *dfl_id = "QEMUQEMU";
char buf[1024], *p, *f;
-struct acpi_table_header acpi_hdr;
unsigned long val;
-uint32_t length;
-struct acpi_table_header *acpi_hdr_p;
-size_t off;
+size_t len, start, allen;
+bool has_header;
+int changed;
+int r;
+struct acpi_table_header hdr;
+
+r = 0;
+r |= get_param_value(buf, sizeof(buf), "data", t) ? 1 : 0;
+r |= get_param_value(buf, sizeof(buf), "file", t) ? 2 : 0;
+switch (r) {
+case 0:
+buf[0] = '\0';
+case 1:
+has_header = false;
+break;
+case 2:
+has_header = true;
+break;
+default:
+fprintf(stderr, "acpitable: both data and file are specified\n");
+return -1;
+}
-memset(&acpi_hdr, 0, sizeof(acpi_hdr));
-
-if (get_param_value(buf, sizeof(buf), "sig", t)) {
-strncpy(acpi_hdr.signature, buf, 4);
+if (!acpi_tables) {
+allen = sizeof(uint16_t);
+acpi_tables = qemu_mallocz(allen);
} else {
-strncpy(acpi_hdr.signature, dfl_id, 4);
+allen = acpi_tables_len;
+}
+
+start = allen;
+acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE);
+allen += has_header ? ACPI_TABLE_PFX_SIZE : ACPI_TABLE_HDR_SIZE;
+
+/* now read in the data files, reallocating buffer as needed */
+
+for (f = strtok(buf, ":"); f; f = strtok(NULL, ":")) {
+int fd = open(f, O_RDONLY);
+
+if (fd < 0) {
+fprintf(stderr, "can't open file %s: %s\n", f, strerror(errno));
+return -1;
+}
+
+for (;;) {
+char data[8192];
+r = read(fd, data, sizeof(data));
+if (r == 0) {
+break;
+} else if (r > 0) {
+acpi_tables = qemu_realloc(acpi_tables, allen + r);
+memcpy(acpi_tables + allen, data, r);
+allen += r;
+} else if (errno != EINTR) {
+fpr
Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions
Am 01.06.2011 17:32, schrieb Markus Armbruster: > Kevin Wolf writes: > >> Am 01.06.2011 15:44, schrieb Luiz Capitulino: >>> On Thu, 26 May 2011 18:12:08 -0300 >>> Luiz Capitulino wrote: >>> On Thu, 19 May 2011 14:33:19 +0200 Kevin Wolf wrote: > These printfs aren't really debug messages, but clearly indicate a bug if > they > ever become effective. Then we have a bug somewhere, starting a VM with: # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom /dev/sr0 Where the host's CDROM is empty, triggers one of these asserts: qmp-unstable/hw/ide/pci.c:299: bmdma_cmd_writeb: Assertion `bm->bus->dma->aiocb == ((void *)0)' >>> >>> I found out why this is happening. I'm passing '-snapshot' to the >>> command-line, >>> sorry for not mentioning it (I forgot I was using my devel alias). >> >> And suddenly it's reproducible. :-) >> >> I'll have a look. >> >>> I also found out that /usr/bin/eject in the guest won't work when >>> -snapshot is used. Shouldn't qemu ignore this flag when using cdrom >>> passthrough? >> >> "Won't work" means that it works like with a CD-ROM image? That would be >> what I expect, as you end up having a qcow2 image with -snapshot. > > With a qcow2 stacked onto the host_cdrom. The block layer forwards the > guest's eject only if the top driver has a bdrv_eject() method. Which > qcow2 doesn't have. I guess bdrv_eject() fails with -ENOTSUP, and > cmd_start_stop_unit() reports a funny error to the guest. bdrv_eject ignores -ENOTSUP and only changes the virtual tray in this case. Just the very same thing as with normal ISO images. >> Not sure what's the best way of fixing this. Maybe just ignoring >> -snapshot for read-only block devices? > > Why not, the combination is pointless. It could start making a difference in some obscure combinations. Imagine a read-only image with a backing file, -snapshot and the 'commit' monitor command. Sounds pretty insane, but I wouldn't bet that people aren't using it... >>Or we could try and forward the >> eject request to the backing file if the format driver doesn't handle it. > > For what it's worth, the "raw" driver forwards manually. Yeah, but copying that into each driver probably isn't the right thing to do. For a generic implementation we could probably first try the driver itself, if it returns -ENOTSUP we try the protocol, and if that returns -ENOTSUP, too, we ask the backing file driver. Can't wait to see the requests that with a qcow2 formatted CD with a backing file in another CD drive the other one (or both) should be ejected. :-) Kevin
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
Am 02.06.2011 20:09, schrieb Luiz Capitulino:
> On Thu, 02 Jun 2011 13:00:04 -0500
> Anthony Liguori wrote:
>
>> On 06/02/2011 12:57 PM, Luiz Capitulino wrote:
>>> On Wed, 01 Jun 2011 16:35:03 -0500
>>> Anthony Liguori wrote:
>>>
On 06/01/2011 04:12 PM, Luiz Capitulino wrote:
> Hi there,
>
> There are people who want to use QMP for thin provisioning. That's, the
> VM is
> started with a small storage and when a no space error is triggered, more
> space
> is allocated and the VM is put to run again.
>
> QMP has two limitations that prevent people from doing this today:
>
> 1. The BLOCK_IO_ERROR doesn't contain error information
>
> 2. Considering we solve item 1, we still have to provide a way for clients
> to query why a VM stopped. This is needed because clients may miss
> the
> BLOCK_IO_ERROR event or may connect to the VM while it's already
> stopped
>
> A proposal to solve both problems follow.
>
> A. BLOCK_IO_ERROR information
> -
>
> We already have discussed this a lot, but didn't reach a consensus. My
> solution
> is quite simple: to add a stringfied errno name to the BLOCK_IO_ERROR
> event,
> for example (see the "reason" key):
>
> { "event": "BLOCK_IO_ERROR",
> "data": { "device": "ide0-hd1",
>"operation": "write",
>"action": "stop",
>"reason": "enospc", }
you can call the reason whatever you want, but don't call it stringfied
errno name :-)
In fact, just make reason "no space".
>>>
>>> You mean, we should do:
>>>
>>>"reason": "no space"
>>>
>>> Or that we should make it a boolean, like:
>>>
>>> "no space": true
>>
>>
>> Do we need reason in BLOCK_IO_ERROR if query-block returns this information?
>
> True, no.
>
>>> I'm ok with either way. But in case you meant the second one, I guess
>>> we should make "reason" a dictionary so that we can group related
>>> information when we extend the field, for example:
>>>
>>> "reason": { "no space": false, "no permission": true }
Splitting up enums into a number of booleans looks like a bad idea to
me. It makes things more verbose than they should be, and even worse, it
implies that more than one field could be true.
If this new schema thing doesn't support proper enums, that's something
that should be changed.
>>
>> Why would we ever have "no permission"?
>
> It's an I/O error. I have a report from a developer who was getting
> the BLOCK_IO_ERROR event and had to debug qemu to know the error cause,
> it turned out to be no permission.
And I want to add that it's a PITA to handle bug report when the only
message you get from qemu is "something went wrong". Sorry, that's not
useful at all. I want to see the real error reason (and at least for
debugging this means, I want to see the errno value/string).
Finding out that it was -EACCES in fact cost me (and QA who ran into the
problem) much more time than it should have. It's simply too much that
you need to attach gdb to find out what really happened.
Kevin
Re: [Qemu-devel] [0/7] USB UVC updates
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Hi Brad, El 02/06/2011, a las 06:41, Brad Hards escribió: > Hi Natalia, > > As you suggested, I've stopped my nonsense and worked from your > UVC patches (http://patchwork.ozlabs.org/patch/55001 and > http://patchwork.ozlabs.org/patch/55000). These changes > are relative to your patches (applied on top of trunk). > > I've mostly just incorporated some changes requested by Blue > Swirl in his review. Fixing the checkpatch stuff changed a lot > of whitespace (tab -> spaces), so I'd like to make sure this is > OK before making more changes. I see it ok, have you tested nothing broken? (specially with the &s patch) I should take more care with 's's in typos next time > The next change I propose to make is to rework the descriptors > and add constants / defines for magic numbers. Yeah, there are a couple of magic numbers that can be added. Please note that most of the constants used in the descriptors should go in usb.h Also take special care with the descriptors, the minimal change can break it very easily. The ideal solution would be to make the descriptors be dynamic depending on the host webcam capabilities. Bad thing the USB IF put the resolutions and formats in the descriptor, instead of in a query command. -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) Comment: GPGTools - http://gpgtools.org iF0EAREIAAYFAk3snrEACgkQv/wfOsykIRRbngD492YNOzZGwRgwwVqok+AewO6H kXvTClUJibR4fTVaDgEArEqcnm/xLF8gvANDc7DX6l8z7jOY8pagK4V488PMOhI= =gzkf -END PGP SIGNATURE-
Re: [Qemu-devel] [PATCH] Correct and standardize header guards in all header files
Alexandre Raymond writes: > As per ISO/IEC 9899, Section 7.1.3 on reserved identifiers, > identifiers starting with an underscore are reserved. > > Therefore: > 1) Rename all incorrect header guards > > While I'm at it: > 2) Standardize header guards to "#ifndef SOMETHING_H" Consistency is good. > 3) Repeat the header guard identifier after the closing "#endif" Such comments can help readers to navigate #if mazes. Not the case for header guards. Just noise.
Re: [Qemu-devel] [0/7] USB UVC updates
On Mon, 6 Jun 2011 07:32:32 PM Natalia Portillo wrote: > I see it ok, have you tested nothing broken? (specially with the &s patch) It doesn't work for me (before or after). The descriptor part (which I've now implemented) works OK (i.e. the guest can dump it), cheese runs in the guest but doesn't show images. luvcview opens but won't capture data. So I have more work to do. > I should take more care with 's's in typos next time I don't think this is the biggest problem :-) > > The next change I propose to make is to rework the descriptors > > and add constants / defines for magic numbers. > > Yeah, there are a couple of magic numbers that can be added. > Please note that most of the constants used in the descriptors should go > in usb.h I was planning to do the ones that are specific to usb-uvc.c in that file, and only (potentially) shared things like the interface descriptor stuff in usb.h > Also take special care with the descriptors, the minimal change can break > it very easily. The ideal solution would be to make the descriptors be > dynamic depending on the host webcam capabilities. I believe that part of the descriptor change may be associated with not reading data. I could easily have introduced a problem. I haven't finished debugging it, but I never see transactions to EP 2 (the isoc data endpoint). > Bad thing the USB IF put the resolutions and formats in the descriptor, > instead of in a query command. Indeed, but like everything in USB device space, its designed down to minimise cost on (real) hardware. Thanks. Brad
Re: [Qemu-devel] VMDK development plan for Summer of Code 2011
Am 02.06.2011 00:11, schrieb Stefan Hajnoczi: > On Wed, Jun 1, 2011 at 10:13 AM, Alexander Graf wrote: >> >> On 01.06.2011, at 11:11, Kevin Wolf wrote: >> >>> Am 01.06.2011 10:49, schrieb Alexander Graf: On 01.06.2011, at 06:29, Stefan Hajnoczi wrote: > On Sun, May 29, 2011 at 2:19 PM, Fam Zheng wrote: >> As a project of Google Summer of Code 2011, I'm now working on >> improving VMDK image support. There are many subformats of VMDK >> virtual disk, some of which have separate descriptor file and others >> don't, some allocate space at once and some others grow dynamically, >> some have optional data compression. The current support of VMDK >> format is very limited, i.e. qemu now supports single file images, but >> couldn't recognize the widely used multi-file types. We have planned >> to add such support to VMDK block driver and enable more image types, >> and the working timeline is set in weeks (#1 to #7) as: >> >> [#1] Monolithic flat layout support >> [#2] Implement compression and Stream-Optimized Compressed Sparse >> Extents support. >> [#3] Improve ESX Server Sparse Extents support. >> [#4] Debug and test. Collect virtual disks with various versions and >> options, test qemu-img with them. By now some patches may be ready to >> deliver. >> [#5, 6] Add multi-file support (2GB extent formats) >> [#7] Clean up and midterm evaluation. > > Thanks to Fam's work, we'll hopefully support the latest real-world > VMDK files in qemu-img convert within the next few months. > > If anyone has had particular VMDK "problem files" which qemu-img > cannot handle, please reply, they would make interesting test cases. There is one very useful use-case of VMDK files that we currently don't support: remapping. A vmdk file can specify that it really is backed by a raw block device, but only for certain chunks, while other chunks of it can be mapped read-only or zero. That is very useful when passing in a host disk to the guest and you want to be sure that you don't break other partitions than the one you're playing with. It can also shadow map those chunks. For example on the case above, the MBR is COW (IIRC) for the image, so you can install a bootloader in there. >>> >>> Hm, wondering if that's something to consider for qcow2v3, too... Do you >>> think it's still useful when doing this on a cluster granularity? It >>> would only work for well-aligned partitions then, but I think that >>> shouldn't be a problem for current OSes. >> >> Well, we could always just hack around for bits where it overlaps. When >> passing in a differently aligned partition for example, we could just >> declare the odd sector as COW sector and copy the contents over :). Though >> that might not be what the user really wants. Hrm. >> >>> Basically, additionally to the three cluster types "read from this >>> image", "COW from backing file" and "zero cluster" we could introduce a >>> fourth one "read/write to backing file". >> >> Yup, sounds very much straight forward! Then all we need is some tool to >> create such a qcow file :) > > If we want to implement mini-device mapper why not do it as a separate > BlockDriver? This could be useful for non-qcow2 cases like *safely* > passing through a physical disk with a guarantee that you won't > destroy the MBR. Also if we do it outside of an image format we don't > need to worry about clusters and can do sector-granularity mapping. > > In fact, if we want mini-device mapper, that could be used to > implement the VMDK multi-file support too. So if Fam writes a generic > BlockDriver extent mapper we can use it from VMDK but also from > command-line options that tie together qcow2, qed, raw, etc images. Does it really work for Alex' case, where you have some parts of an image file that you want to be COW and other parts that write directly to the backing file? Or to put it in a more general way: Does it work when you reference an image more than once? Wouldn't you have to open the same image twice? Kevin
Re: [Qemu-devel] [PATCH] bdrv_img_create: Fix segfault
Am 02.06.2011 00:34, schrieb Stefan Hajnoczi: > On Wed, Jun 1, 2011 at 1:05 PM, Kevin Wolf wrote: >> Block drivers that don't support creating images don't have a size option. >> Fail >> gracefully instead of segfaulting when trying to access the option's value. >> >> Signed-off-by: Kevin Wolf >> --- >> block.c |5 +++-- >> 1 files changed, 3 insertions(+), 2 deletions(-) > > A command-line to reproduce the crash would be nice. qemu-img create -f bochs nbd:foo 32M It doesn't happen with a file protocol any more since we merge the create options of the protocol with those of the format (was introduced with Sheepdog). > I noticed this line above your fix: > set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); > > If I follow correctly there should be an "Unknown option 'size'" error > message before set_option_parameter_int() returns -1 (which we ignore) > and then crash. Right, this is what happens. > Perhaps we should just catch the error when set_option_parameter_int() fails? We could do that, but the segfault isn't really related to a failing set_option_parameter_int() but to the failing get_option_parameter(). I think it's good style not to rely on the error handling of an unrelated action some lines above. Adding some error handling there, too, wouldn't hurt, though. Kevin
Re: [Qemu-devel] [PATCH 07/14] usb-linux: If opening a device fails remove it from our filter list
On 06/01/11 16:37, Hans de Goede wrote: Hi, On 06/01/2011 02:32 PM, Gerd Hoffmann wrote: On 05/31/11 11:35, Hans de Goede wrote: So that we don't retry to open it every 2 seconds flooding stderr with error messages. The polling here is done intentionally, so the devices catched by the filter show up in the guest automagically as soon as they are plugged in. Just zapping the filter on failure isn't the right thing to do here. Note I'm zapping the filter when we fail to open the device, not when it is not present. This can happen for example when the qemu user does not have rights on the usbfs device node. It seems better to me to print the relevant error once, and then require the user to redo the usb_add / device_add if necessary, then to flood the monitor with repeating the same error every 2 seconds. Devices magically disappearing is a problem for the management stack (such as libvirt) though. qemu does stuff like that in too many places already and we are trying to get rid of it. Flooding the monitor (or stderr) indeed isn't nice though. cheers, Gerd
[Qemu-devel] [Bug 793317] Re: Large amount of write-only variables
Patches were already sent to the ML, see the "[PATCH 00/14] More gcc 4.6 warnings fixes" and "[PATCH 0/3] Fix unused-but-set-variable warnings" threads -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/793317 Title: Large amount of write-only variables Status in QEMU: New Bug description: Whenever I try to compile the source from the git repo, it gets a large number of "set but not used" errors, in files such as: hw/usb_ochi.c (line 526, 1114, and 1108) hw/lsi53c895a.c (line 892) kvm.c (line 973) target-alpha/translate.c (line 1472, 1470) linux-user/syscall.c (line 7062, 3754) exec.c (line 1211) linux-user/linuxload.c (line 60) Really now, why would anyone create so many write-only variables knowing that it's compiled with write-only variables causing errors..? Host: Fedora 15 i686 Gcc: 4.6.0 Guest: None I pulled the code from the git repo earlier today, so I'm pretty sure it did not get fixed in that short timeframe.
Re: [Qemu-devel] [PATCH 14/14] usb: Proper error propagation for usb_device_attach errors
Hi, Let me know if you'd rather have me resend 12 + 14 to the list. I'd suggest to look at the leftover stuff after the next usb merge. cheers, Gerd PS: /me is busy doing tests, preparing the next usb pull which hopefully goes out later today.
Re: [Qemu-devel] [PATCH] Correct and standardize header guards in all header files
On Sun, Jun 5, 2011 at 4:57 AM, Alexandre Raymond wrote: > diff --git a/balloon.h b/balloon.h > index d478e28..4602d85 100644 > --- a/balloon.h > +++ b/balloon.h > @@ -11,8 +11,8 @@ > * > */ > > -#ifndef _QEMU_BALLOON_H > -#define _QEMU_BALLOON_H > +#ifndef QEMU_BALLOON_H > +#define QEMU_BALLOON_H > > #include "monitor.h" > > @@ -30,4 +30,4 @@ int do_info_balloon(Monitor *mon, MonitorCompletion cb, > void *opaque); > int do_balloon(Monitor *mon, const QDict *params, > MonitorCompletion cb, void *opaque); > > -#endif > +#endif /* QEMU_BALOON_H */ BALOON? > diff --git a/darwin-user/qemu.h b/darwin-user/qemu.h > index b6d3e6c..a426381 100644 > --- a/darwin-user/qemu.h > +++ b/darwin-user/qemu.h > @@ -175,4 +175,4 @@ static inline uint64_t get_int64_arg(int *i, CPUPPCState > *cpu_env) > } > #endif > > -#endif > +#endif /* GEMU_H */ GEMU, looks like darwin-user/qemu.h typoed "QEMU"? If you really want to do this, please use a script to convert the code consistently without errors. I don't have time to review long error-prone patches that have no external benefit for QEMU, sorry. Stefan
Re: [Qemu-devel] VMDK development plan for Summer of Code 2011
On Mon, Jun 6, 2011 at 10:50 AM, Kevin Wolf wrote: > Am 02.06.2011 00:11, schrieb Stefan Hajnoczi: >> On Wed, Jun 1, 2011 at 10:13 AM, Alexander Graf wrote: >>> >>> On 01.06.2011, at 11:11, Kevin Wolf wrote: >>> Am 01.06.2011 10:49, schrieb Alexander Graf: > > On 01.06.2011, at 06:29, Stefan Hajnoczi wrote: > >> On Sun, May 29, 2011 at 2:19 PM, Fam Zheng wrote: >>> As a project of Google Summer of Code 2011, I'm now working on >>> improving VMDK image support. There are many subformats of VMDK >>> virtual disk, some of which have separate descriptor file and others >>> don't, some allocate space at once and some others grow dynamically, >>> some have optional data compression. The current support of VMDK >>> format is very limited, i.e. qemu now supports single file images, but >>> couldn't recognize the widely used multi-file types. We have planned >>> to add such support to VMDK block driver and enable more image types, >>> and the working timeline is set in weeks (#1 to #7) as: >>> >>> [#1] Monolithic flat layout support >>> [#2] Implement compression and Stream-Optimized Compressed Sparse >>> Extents support. >>> [#3] Improve ESX Server Sparse Extents support. >>> [#4] Debug and test. Collect virtual disks with various versions and >>> options, test qemu-img with them. By now some patches may be ready to >>> deliver. >>> [#5, 6] Add multi-file support (2GB extent formats) >>> [#7] Clean up and midterm evaluation. >> >> Thanks to Fam's work, we'll hopefully support the latest real-world >> VMDK files in qemu-img convert within the next few months. >> >> If anyone has had particular VMDK "problem files" which qemu-img >> cannot handle, please reply, they would make interesting test cases. > > There is one very useful use-case of VMDK files that we currently don't > support: remapping. > > A vmdk file can specify that it really is backed by a raw block device, > but only for certain chunks, while other chunks of it can be mapped > read-only or zero. That is very useful when passing in a host disk to the > guest and you want to be sure that you don't break other partitions than > the one you're playing with. > > It can also shadow map those chunks. For example on the case above, the > MBR is COW (IIRC) for the image, so you can install a bootloader in there. Hm, wondering if that's something to consider for qcow2v3, too... Do you think it's still useful when doing this on a cluster granularity? It would only work for well-aligned partitions then, but I think that shouldn't be a problem for current OSes. >>> >>> Well, we could always just hack around for bits where it overlaps. When >>> passing in a differently aligned partition for example, we could just >>> declare the odd sector as COW sector and copy the contents over :). Though >>> that might not be what the user really wants. Hrm. >>> Basically, additionally to the three cluster types "read from this image", "COW from backing file" and "zero cluster" we could introduce a fourth one "read/write to backing file". >>> >>> Yup, sounds very much straight forward! Then all we need is some tool to >>> create such a qcow file :) >> >> If we want to implement mini-device mapper why not do it as a separate >> BlockDriver? This could be useful for non-qcow2 cases like *safely* >> passing through a physical disk with a guarantee that you won't >> destroy the MBR. Also if we do it outside of an image format we don't >> need to worry about clusters and can do sector-granularity mapping. >> >> In fact, if we want mini-device mapper, that could be used to >> implement the VMDK multi-file support too. So if Fam writes a generic >> BlockDriver extent mapper we can use it from VMDK but also from >> command-line options that tie together qcow2, qed, raw, etc images. > > Does it really work for Alex' case, where you have some parts of an > image file that you want to be COW and other parts that write directly > to the backing file? > > Or to put it in a more general way: Does it work when you reference an > image more than once? Wouldn't you have to open the same image twice? Here is an example of booting from a physical disk: [mbr][/dev/zero][/dev/sda] mbr is a COW image based on /dev/sda. /dev/zero is used to protect the first partition would be. The guest only sees zeroes and writes are ignored because the guest should never access this region. /dev/sda is the extent containing the second partition (actually we could just open /dev/sda2). Here we have the case that you mentioned with /dev/sda open as the read-only backing file for mbr and as read-write for the second partition. The question is are raw images safe for multiple opens when at least one is read-write? I think the answer for raw is yes. It is not safe to open non-raw image file
Re: [Qemu-devel] VMDK development plan for Summer of Code 2011
Am 06.06.2011 12:53, schrieb Stefan Hajnoczi: > On Mon, Jun 6, 2011 at 10:50 AM, Kevin Wolf wrote: >> Am 02.06.2011 00:11, schrieb Stefan Hajnoczi: >>> On Wed, Jun 1, 2011 at 10:13 AM, Alexander Graf wrote: On 01.06.2011, at 11:11, Kevin Wolf wrote: > Am 01.06.2011 10:49, schrieb Alexander Graf: >> There is one very useful use-case of VMDK files that we currently don't >> support: remapping. >> >> A vmdk file can specify that it really is backed by a raw block device, >> but only for certain chunks, while other chunks of it can be mapped >> read-only or zero. That is very useful when passing in a host disk to >> the guest and you want to be sure that you don't break other partitions >> than the one you're playing with. >> >> It can also shadow map those chunks. For example on the case above, the >> MBR is COW (IIRC) for the image, so you can install a bootloader in >> there. > > Hm, wondering if that's something to consider for qcow2v3, too... Do you > think it's still useful when doing this on a cluster granularity? It > would only work for well-aligned partitions then, but I think that > shouldn't be a problem for current OSes. Well, we could always just hack around for bits where it overlaps. When passing in a differently aligned partition for example, we could just declare the odd sector as COW sector and copy the contents over :). Though that might not be what the user really wants. Hrm. > Basically, additionally to the three cluster types "read from this > image", "COW from backing file" and "zero cluster" we could introduce a > fourth one "read/write to backing file". Yup, sounds very much straight forward! Then all we need is some tool to create such a qcow file :) >>> >>> If we want to implement mini-device mapper why not do it as a separate >>> BlockDriver? This could be useful for non-qcow2 cases like *safely* >>> passing through a physical disk with a guarantee that you won't >>> destroy the MBR. Also if we do it outside of an image format we don't >>> need to worry about clusters and can do sector-granularity mapping. >>> >>> In fact, if we want mini-device mapper, that could be used to >>> implement the VMDK multi-file support too. So if Fam writes a generic >>> BlockDriver extent mapper we can use it from VMDK but also from >>> command-line options that tie together qcow2, qed, raw, etc images. >> >> Does it really work for Alex' case, where you have some parts of an >> image file that you want to be COW and other parts that write directly >> to the backing file? >> >> Or to put it in a more general way: Does it work when you reference an >> image more than once? Wouldn't you have to open the same image twice? > > Here is an example of booting from a physical disk: > > [mbr][/dev/zero][/dev/sda] > > mbr is a COW image based on /dev/sda. > > /dev/zero is used to protect the first partition would be. The guest > only sees zeroes and writes are ignored because the guest should never > access this region. > > /dev/sda is the extent containing the second partition (actually we > could just open /dev/sda2). > > Here we have the case that you mentioned with /dev/sda open as the > read-only backing file for mbr and as read-write for the second > partition. The question is are raw images safe for multiple opens > when at least one is read-write? I think the answer for raw is yes. > It is not safe to open non-raw image files multiple times. Yes, it would work for raw images. But should we really introduce concepts that only work with raw images? > I'm also wondering if the -blockdev backing_file= option that > has been discussed could be used in non-raw cases. Instead of opening > backing files by name, specify the backing file block device on the > command-line so that the same BlockDriverState is shared, avoiding > inconsistencies. > > The multiple opener issue is orthogonal to device mapper support. Well, no. If the only use case for the device mapper thing means that we need to open images twice, there's no point in implementing it without solving the multiple opener problem first. Kevin
Re: [Qemu-devel] [PATCH] bdrv_img_create: Fix segfault
On Mon, Jun 6, 2011 at 11:00 AM, Kevin Wolf wrote: > Am 02.06.2011 00:34, schrieb Stefan Hajnoczi: >> On Wed, Jun 1, 2011 at 1:05 PM, Kevin Wolf wrote: >>> Block drivers that don't support creating images don't have a size option. >>> Fail >>> gracefully instead of segfaulting when trying to access the option's value. >>> >>> Signed-off-by: Kevin Wolf >>> --- >>> block.c | 5 +++-- >>> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> A command-line to reproduce the crash would be nice. > > qemu-img create -f bochs nbd:foo 32M > > It doesn't happen with a file protocol any more since we merge the > create options of the protocol with those of the format (was introduced > with Sheepdog). Interesting, I was just looking at the format + protocol create_options merging because I tried to replace QEMUOptionParameter with QemuOpts. The idea was to make the block layer use QemuOpts and then introduce a BlockDriver open_options QemuOptsList so that image format parameters like image streaming or copy-on-read can be specified on launch. Here is where I got to: http://repo.or.cz/w/qemu/stefanha.git/commitdiff/b49babb2c8b476a36357cfd7276ca45a11039ca5 The main thing stopping me from dropping QEMUOptionParameter completely is this merging behavior. Any suggestions? >> I noticed this line above your fix: >> set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); >> >> If I follow correctly there should be an "Unknown option 'size'" error >> message before set_option_parameter_int() returns -1 (which we ignore) >> and then crash. > > Right, this is what happens. > >> Perhaps we should just catch the error when set_option_parameter_int() fails? > > We could do that, but the segfault isn't really related to a failing > set_option_parameter_int() but to the failing get_option_parameter(). I > think it's good style not to rely on the error handling of an unrelated > action some lines above. It's also bad to keep executing code after something has broken, which is the case when set_option_parameter_int() fails but we ignore the return value. Thanks for explaining, I now understand your fix better and am happy with it. Stefan
Re: [Qemu-devel] VMDK development plan for Summer of Code 2011
On Mon, Jun 6, 2011 at 12:08 PM, Kevin Wolf wrote: > Am 06.06.2011 12:53, schrieb Stefan Hajnoczi: >> On Mon, Jun 6, 2011 at 10:50 AM, Kevin Wolf wrote: >>> Am 02.06.2011 00:11, schrieb Stefan Hajnoczi: On Wed, Jun 1, 2011 at 10:13 AM, Alexander Graf wrote: > > On 01.06.2011, at 11:11, Kevin Wolf wrote: > >> Am 01.06.2011 10:49, schrieb Alexander Graf: >>> There is one very useful use-case of VMDK files that we currently don't >>> support: remapping. >>> >>> A vmdk file can specify that it really is backed by a raw block device, >>> but only for certain chunks, while other chunks of it can be mapped >>> read-only or zero. That is very useful when passing in a host disk to >>> the guest and you want to be sure that you don't break other partitions >>> than the one you're playing with. >>> >>> It can also shadow map those chunks. For example on the case above, the >>> MBR is COW (IIRC) for the image, so you can install a bootloader in >>> there. >> >> Hm, wondering if that's something to consider for qcow2v3, too... Do you >> think it's still useful when doing this on a cluster granularity? It >> would only work for well-aligned partitions then, but I think that >> shouldn't be a problem for current OSes. > > Well, we could always just hack around for bits where it overlaps. When > passing in a differently aligned partition for example, we could just > declare the odd sector as COW sector and copy the contents over :). > Though that might not be what the user really wants. Hrm. > >> Basically, additionally to the three cluster types "read from this >> image", "COW from backing file" and "zero cluster" we could introduce a >> fourth one "read/write to backing file". > > Yup, sounds very much straight forward! Then all we need is some tool to > create such a qcow file :) If we want to implement mini-device mapper why not do it as a separate BlockDriver? This could be useful for non-qcow2 cases like *safely* passing through a physical disk with a guarantee that you won't destroy the MBR. Also if we do it outside of an image format we don't need to worry about clusters and can do sector-granularity mapping. In fact, if we want mini-device mapper, that could be used to implement the VMDK multi-file support too. So if Fam writes a generic BlockDriver extent mapper we can use it from VMDK but also from command-line options that tie together qcow2, qed, raw, etc images. >>> >>> Does it really work for Alex' case, where you have some parts of an >>> image file that you want to be COW and other parts that write directly >>> to the backing file? >>> >>> Or to put it in a more general way: Does it work when you reference an >>> image more than once? Wouldn't you have to open the same image twice? >> >> Here is an example of booting from a physical disk: >> >> [mbr][/dev/zero][/dev/sda] >> >> mbr is a COW image based on /dev/sda. >> >> /dev/zero is used to protect the first partition would be. The guest >> only sees zeroes and writes are ignored because the guest should never >> access this region. >> >> /dev/sda is the extent containing the second partition (actually we >> could just open /dev/sda2). >> >> Here we have the case that you mentioned with /dev/sda open as the >> read-only backing file for mbr and as read-write for the second >> partition. The question is are raw images safe for multiple opens >> when at least one is read-write? I think the answer for raw is yes. >> It is not safe to open non-raw image files multiple times. > > Yes, it would work for raw images. But should we really introduce > concepts that only work with raw images? > >> I'm also wondering if the -blockdev backing_file= option that >> has been discussed could be used in non-raw cases. Instead of opening >> backing files by name, specify the backing file block device on the >> command-line so that the same BlockDriverState is shared, avoiding >> inconsistencies. >> >> The multiple opener issue is orthogonal to device mapper support. > > Well, no. If the only use case for the device mapper thing means that we > need to open images twice, there's no point in implementing it without > solving the multiple opener problem first. Protecting physical disk partitions is one use case, the other use case was for implementing VMDK multi-file support, which is why I mentioned Fam. Stefan
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
Anthony Liguori writes:
> On 06/01/2011 04:12 PM, Luiz Capitulino wrote:
>> Hi there,
>>
>> There are people who want to use QMP for thin provisioning. That's, the VM is
>> started with a small storage and when a no space error is triggered, more
>> space
>> is allocated and the VM is put to run again.
>>
>> QMP has two limitations that prevent people from doing this today:
>>
>> 1. The BLOCK_IO_ERROR doesn't contain error information
>>
>> 2. Considering we solve item 1, we still have to provide a way for clients
>> to query why a VM stopped. This is needed because clients may miss the
>> BLOCK_IO_ERROR event or may connect to the VM while it's already stopped
>>
>> A proposal to solve both problems follow.
>>
>> A. BLOCK_IO_ERROR information
>> -
>>
>> We already have discussed this a lot, but didn't reach a consensus. My
>> solution
>> is quite simple: to add a stringfied errno name to the BLOCK_IO_ERROR event,
>> for example (see the "reason" key):
>>
>> { "event": "BLOCK_IO_ERROR",
>> "data": { "device": "ide0-hd1",
>> "operation": "write",
>> "action": "stop",
>> "reason": "enospc", }
>
> you can call the reason whatever you want, but don't call it
> stringfied errno name :-)
Care to explain why?
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
Anthony Liguori writes:
> On 06/02/2011 08:24 AM, Jiri Denemark wrote:
>> On Thu, Jun 02, 2011 at 08:08:35 -0500, Anthony Liguori wrote:
>>> On 06/02/2011 04:06 AM, Daniel P. Berrange wrote:
>> B. query-stop-reason
>>
>>
>> I also have a simple solution for item 2. The vm_stop() accepts a reason
>> argument, so we could store it somewhere and return it as a string, like:
>>
>> ->{ "execute": "query-stop-reason" }
>> <- { "return": { "reason": "user" } }
>>
>> Valid reasons could be: "user", "debug", "shutdown", "diskfull" (hey,
>> this should be "ioerror", no?), "watchdog", "panic", "savevm", "loadvm",
>> "migrate".
>>
>> Also note that we have a STOP event. It should be extended with the
>> stop reason too, for completeness.
>
>
> Can we just extend query-block?
Primarily we want 'query-stop-reason' to tell us what caused the VM
CPUs to stop. If that reason was 'ioerror', then 'query-block' could
be used to find out which particular block device(s) caused the IO
error to occurr& get the "reason" that was in the BLOCK_IO_ERROR
event.
>>>
>>> My concern is that we're over abstracting here. We're not going to add
>>> additional stop reasons in the future.
>>>
>>> Maybe just add an 'io-error': True to query-state.
>>
>> Sure, adding a new field to query-state response would work as well. And it
>> seems like a good idea to me since one already needs to call query-status to
>> check if CPUs are stopped or not so it makes sense to incorporate the
>> additional information there as well. And if you want to be safe for the
>> future, the new field doesn't have to be boolean 'io-error' but it can be the
>> string 'reason' which Luiz suggested above.
>
>
> String enumerations are a Bad Thing. It's impossible to figure out
> what strings are valid and it lacks type safety.
>
> Adding more booleans provides better type safety, and when we move to
> QAPI with a queryable schema, provides a way to figure out exactly
> what combinations are supported by QEMU.
Faking enumerations with strings has its drawbacks. Doesn't mean we
should fake them with a bunch of booleans. What about having the real
thing instead?
I'm not claiming the proper solution to the problem at hand is an
enumeration, just challenging the idea that we should use booleans
instead of enumerations (string or otherwise).
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
Kevin Wolf writes:
> Am 02.06.2011 20:09, schrieb Luiz Capitulino:
>> On Thu, 02 Jun 2011 13:00:04 -0500
>> Anthony Liguori wrote:
>>
>>> On 06/02/2011 12:57 PM, Luiz Capitulino wrote:
On Wed, 01 Jun 2011 16:35:03 -0500
Anthony Liguori wrote:
> On 06/01/2011 04:12 PM, Luiz Capitulino wrote:
>> Hi there,
>>
>> There are people who want to use QMP for thin provisioning. That's, the
>> VM is
>> started with a small storage and when a no space error is triggered,
>> more space
>> is allocated and the VM is put to run again.
>>
>> QMP has two limitations that prevent people from doing this today:
>>
>> 1. The BLOCK_IO_ERROR doesn't contain error information
>>
>> 2. Considering we solve item 1, we still have to provide a way for
>> clients
>> to query why a VM stopped. This is needed because clients may miss
>> the
>> BLOCK_IO_ERROR event or may connect to the VM while it's already
>> stopped
>>
>> A proposal to solve both problems follow.
>>
>> A. BLOCK_IO_ERROR information
>> -
>>
>> We already have discussed this a lot, but didn't reach a consensus. My
>> solution
>> is quite simple: to add a stringfied errno name to the BLOCK_IO_ERROR
>> event,
>> for example (see the "reason" key):
>>
>> { "event": "BLOCK_IO_ERROR",
>> "data": { "device": "ide0-hd1",
>>"operation": "write",
>>"action": "stop",
>>"reason": "enospc", }
>
> you can call the reason whatever you want, but don't call it stringfied
> errno name :-)
>
> In fact, just make reason "no space".
You mean, we should do:
"reason": "no space"
Or that we should make it a boolean, like:
"no space": true
>>>
>>>
>>> Do we need reason in BLOCK_IO_ERROR if query-block returns this information?
>>
>> True, no.
>>
I'm ok with either way. But in case you meant the second one, I guess
we should make "reason" a dictionary so that we can group related
information when we extend the field, for example:
"reason": { "no space": false, "no permission": true }
>
> Splitting up enums into a number of booleans looks like a bad idea to
> me. It makes things more verbose than they should be, and even worse, it
> implies that more than one field could be true.
>
> If this new schema thing doesn't support proper enums, that's something
> that should be changed.
>
>>>
>>> Why would we ever have "no permission"?
>>
>> It's an I/O error. I have a report from a developer who was getting
>> the BLOCK_IO_ERROR event and had to debug qemu to know the error cause,
>> it turned out to be no permission.
>
> And I want to add that it's a PITA to handle bug report when the only
> message you get from qemu is "something went wrong". Sorry, that's not
> useful at all. I want to see the real error reason (and at least for
> debugging this means, I want to see the errno value/string).
And I want it straight, not wrapped in a pile of pseudo-abstractions
that make me go to the source code to figure out how to unwrap them.
[...]
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
Markus Armbruster writes: > Anthony Liguori writes: [...] >> you can call the reason whatever you want, but don't call it >> stringfied errno name :-) > > Care to explain why? Found a bit of explanation downthread. Sorry for the noise.
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
On (Fri) 03 Jun 2011 [16:03:57], Luiz Capitulino wrote:
> +static int tray_open(const char *device, int remove, int force)
> +{
> +BlockDriverState *bs;
> +
> +bs = bdrv_removable_find(device);
> +if (!bs) {
> +return -1;
> +}
> +
> +if (bdrv_eject(bs, 1, force) < 0) {
> +/* FIXME: will report undefined error in QMP */
> +return -1;
> +}
> +
> +if (remove) {
> +bdrv_close(bs);
> +}
> +
> +return 0;
> +}
What's the reason to tie the 'remove' with tray open? Won't it be
simpler to have it separated out, perhaps a 'change' event instead of
'insert' that can accept NULL which means just remove medium?
Amit
Re: [Qemu-devel] [RFC 07/10] QMP: Introduce the blockdev-media-insert command
On (Fri) 03 Jun 2011 [16:03:59], Luiz Capitulino wrote:
>
> +static int media_insert(const char *device, const char *mediafile,
> +const char *format)
> +{
> +BlockDriver *drv = NULL;
> +BlockDriverState *bs;
> +int bdrv_flags;
> +
> +bs = bdrv_removable_find(device);
> +if (!bs) {
> +return -1;
> +}
> +
> +if (bdrv_is_locked(bs)) {
> +qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
> +return -1;
> +}
> +
> +if (bdrv_is_inserted(bs)) {
> +/* FIXME: will report undefined error in QMP */
> +return -1;
> +}
> +
> +if (!bs->tray_open) {
> +/* FIXME: will report undefined error in QMP */
> +return 1;
> +}
Yes, these should be fixed.
Amit
Re: [Qemu-devel] [RFC 10/10] QMP/HMP: change: Use QMP tray commands
On (Fri) 03 Jun 2011 [16:04:02], Luiz Capitulino wrote:
> This commit rewrites change in terms of blockdev-tray-open,
> blockdev-media-insert and blockdev-tray-close.
>
> There should be no visible changes in HMP or QMP, except that
> the use of this command causes the BLOCK_TRAY_OPEN *and*
> BLOCK_TRAY_CLOSE events to emitted.
>
> Signed-off-by: Luiz Capitulino
> ---
> blockdev.c | 43 +--
> 1 files changed, 9 insertions(+), 34 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 36c56fd..e9edcb3 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -648,23 +648,6 @@ out:
> return ret;
> }
>
> -static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
> -{
> -if (!force) {
> -if (!bdrv_is_removable(bs)) {
> -qerror_report(QERR_DEVICE_NOT_REMOVABLE,
> - bdrv_get_device_name(bs));
> -return -1;
> -}
> -if (bdrv_is_locked(bs)) {
> -qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
> -return -1;
> -}
> -}
> -bdrv_close(bs);
> -return 0;
> -}
> -
> static BlockDriverState *bdrv_removable_find(const char *name)
> {
> BlockDriverState *bs;
> @@ -819,30 +802,22 @@ int do_change_block(Monitor *mon, const char *device,
> const char *filename, const char *fmt)
> {
> BlockDriverState *bs;
> -BlockDriver *drv = NULL;
> -int bdrv_flags;
>
> -bs = bdrv_find(device);
> -if (!bs) {
> -qerror_report(QERR_DEVICE_NOT_FOUND, device);
> +if (tray_open(device, 1, 0) < 0) { /* XXX: should we force? */
> return -1;
> }
> -if (fmt) {
> -drv = bdrv_find_whitelisted_format(fmt);
> -if (!drv) {
> -qerror_report(QERR_INVALID_BLOCK_FORMAT, fmt);
> -return -1;
> -}
> -}
> -if (eject_device(mon, bs, 0) < 0) {
> +
> +if (media_insert(device, filename, fmt) < 0) {
> return -1;
> }
> -bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> -bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> -if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> -qerror_report(QERR_OPEN_FILE_FAILED, filename);
> +
> +if (tray_close(device) < 0) {
> +/* XXX: open the tray and remove the media? */
> return -1;
> }
So if the new medium points to a non-existent file, close will fail,
and leave the tray in the open state. A next change command will fail
because the tray is already open. Not a good thing.
Amit
Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions
Kevin Wolf writes: > Am 01.06.2011 17:32, schrieb Markus Armbruster: >> Kevin Wolf writes: >> >>> Am 01.06.2011 15:44, schrieb Luiz Capitulino: On Thu, 26 May 2011 18:12:08 -0300 Luiz Capitulino wrote: > On Thu, 19 May 2011 14:33:19 +0200 > Kevin Wolf wrote: > >> These printfs aren't really debug messages, but clearly indicate a bug >> if they >> ever become effective. > > Then we have a bug somewhere, starting a VM with: > > # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom /dev/sr0 > > Where the host's CDROM is empty, triggers one of these asserts: > > qmp-unstable/hw/ide/pci.c:299: bmdma_cmd_writeb: Assertion > `bm->bus->dma->aiocb == ((void *)0)' I found out why this is happening. I'm passing '-snapshot' to the command-line, sorry for not mentioning it (I forgot I was using my devel alias). >>> >>> And suddenly it's reproducible. :-) >>> >>> I'll have a look. >>> I also found out that /usr/bin/eject in the guest won't work when -snapshot is used. Shouldn't qemu ignore this flag when using cdrom passthrough? >>> >>> "Won't work" means that it works like with a CD-ROM image? That would be >>> what I expect, as you end up having a qcow2 image with -snapshot. >> >> With a qcow2 stacked onto the host_cdrom. The block layer forwards the >> guest's eject only if the top driver has a bdrv_eject() method. Which >> qcow2 doesn't have. I guess bdrv_eject() fails with -ENOTSUP, and >> cmd_start_stop_unit() reports a funny error to the guest. > > bdrv_eject ignores -ENOTSUP and only changes the virtual tray in this > case. Just the very same thing as with normal ISO images. Missed that. Thanks. >>> Not sure what's the best way of fixing this. Maybe just ignoring >>> -snapshot for read-only block devices? >> >> Why not, the combination is pointless. > > It could start making a difference in some obscure combinations. Imagine > a read-only image with a backing file, -snapshot and the 'commit' > monitor command. > > Sounds pretty insane, but I wouldn't bet that people aren't using it... People try all kinds of insane things. The question is whether we can change it anyway. >>>Or we could try and forward the >>> eject request to the backing file if the format driver doesn't handle it. >> >> For what it's worth, the "raw" driver forwards manually. > > Yeah, but copying that into each driver probably isn't the right thing > to do. I didn't mean to hold up the "raw" driver as a shining example of how to do stuff. >For a generic implementation we could probably first try the > driver itself, if it returns -ENOTSUP we try the protocol, and if that > returns -ENOTSUP, too, we ask the backing file driver. I don't want to start another "format vs. protocol" semantic war, just point out that the general case is a tree of block drivers, and a general rule for passing eject up the tree better covers that. The current block.c provides for binary trees (bs->file and bs->backing_hd). When we discussed blockdev_add, we came up with much wilder trees. > Can't wait to see the requests that with a qcow2 formatted CD with a > backing file in another CD drive the other one (or both) should be > ejected. :-) Requests like that can make you wish for a way to eject users ;)
[Qemu-devel] [PATCH] Initial support for loading bootrom for OMAP3 from file (with "-bios" option)
Initial support for loading bootrom for OMAP3 from file (with "-bios" option) Signed-off-by: Anton Kochkov This patch adds support of loading bootrom for OMAP3 platform from file, for booting. Here is example, how-to use qemu with it: qemu-system-arm -M n900 -m 256 -L . -bios bootrom.bin -mtdblock bootloader.raw -d in_asm,cpu,exec -nographic Right now it only placed in GPMC memory region. But need to be implemented with independend OCM (On-Chip Memory) controller. Because, when bootrom call GPMC init function (there is place, where it filling by zeroes, so rewrite bootrom, in memory). This need to be fixed. Right now it can be easy skipped with gdb. Here is use case https://www.droid-developers.org/wiki/QEMU Best regards, Anton Kochkov. 0001-Initial-support-for-loading-bootrom-for-OMAP3-from-f.patch Description: Binary data
Re: [Qemu-devel] [PATCH 0/4] introduce cpu_physical_memory_map_fast
On 05/31/2011 11:16 AM, Paolo Bonzini wrote: On 05/12/2011 04:51 PM, Paolo Bonzini wrote: On 05/03/2011 06:49 PM, Paolo Bonzini wrote: Paravirtualized devices (and also some real devices) can assume they are going to access RAM. For this reason, provide a fast-path function with the following properties: 1) it will never allocate a bounce buffer 2) it can be used for read-modify-write operations 3) unlike qemu_get_ram_ptr, it is safe because it recognizes "short" blocks Patches 3 and 4 use this function for virtio devices and the milkymist GPU. The latter is only compile-tested. Another function checks if it is possible to split a contiguous physical address range into multiple subranges, all of which use the fast path. I will introduce later a use for this function. Paolo Bonzini (4): exec: extract cpu_physical_memory_map_internal exec: introduce cpu_physical_memory_map_fast and cpu_physical_memory_map_check virtio: use cpu_physical_memory_map_fast milkymist: use cpu_physical_memory_map_fast cpu-common.h | 4 ++ exec.c | 108 +- hw/milkymist-tmu2.c | 39 ++ hw/vhost.c | 10 ++-- hw/virtio.c | 2 +- 5 files changed, 111 insertions(+), 52 deletions(-) Ping? Ping^2? Ping^3? Paolo
Re: [Qemu-devel] [RFC][PATCH] ide: Break migration by splitting error status from status register
On 05/31/2011 12:09 PM, Kevin Wolf wrote: When adding the werror=stop mode, some flags were added to s->status which are used to determine what kind of operation should be restarted when the VM is continued. Unfortunately, it turns out that s->status is in fact a device register and as such is visible to the guest (some of the abused bits are even writable for the guest). Splitting the internal status and the status register into two different variables is easy enough, but this will break migration: We must have a way to detect what s->status really means. Is it only the status register (as used by new versions) or do we have to extract internal error status flags? Here we seem to be lacking some kind of optional subsection that would be simply ignored by older versions, but can contain information for new versions. Is there any precedence on how to solve this? You need to stop writing either status field to the migration stream; instead you recreate the "wrong" status field before saving, and set the "right" status fields from the saved data after loading. On top of this, you use a subsection to save bits 3-7 of the "real" IDE status registers. These had been hijacked, so there is no room for them in the migration stream. Of course, the subsection is needed only if any of those bits is set. Paolo
[Qemu-devel] [PATCH 04/31] usb-ehci: trace port state
Trace usb port operations (attach, detach, reset),
drop a few obsolete DPRINTF's.
No change in behavior.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 10 --
trace-events |3 +++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 40a447b..85e5ed9 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -604,8 +604,7 @@ static void ehci_attach(USBPort *port)
EHCIState *s = port->opaque;
uint32_t *portsc = &s->portsc[port->index];
-DPRINTF("ehci_attach invoked for index %d, portsc 0x%x, desc %s\n",
- port->index, *portsc, port->dev->product_desc);
+trace_usb_ehci_port_attach(port->index, port->dev->product_desc);
*portsc |= PORTSC_CONNECT;
*portsc |= PORTSC_CSC;
@@ -625,8 +624,7 @@ static void ehci_detach(USBPort *port)
EHCIState *s = port->opaque;
uint32_t *portsc = &s->portsc[port->index];
-DPRINTF("ehci_attach invoked for index %d, portsc 0x%x\n",
- port->index, *portsc);
+trace_usb_ehci_port_detach(port->index);
*portsc &= ~PORTSC_CONNECT;
*portsc |= PORTSC_CSC;
@@ -733,11 +731,11 @@ static void handle_port_status_write(EHCIState *s, int
port, uint32_t val)
*portsc &= ~rwc;
if ((val & PORTSC_PRESET) && !(*portsc & PORTSC_PRESET)) {
-DPRINTF("port_status_write: USBTRAN Port %d reset begin\n", port);
+trace_usb_ehci_port_reset(port, 1);
}
if (!(val & PORTSC_PRESET) &&(*portsc & PORTSC_PRESET)) {
-DPRINTF("port_status_write: USBTRAN Port %d reset done\n", port);
+trace_usb_ehci_port_reset(port, 0);
usb_attach(&s->ports[port], dev);
// TODO how to handle reset of ports with no device
diff --git a/trace-events b/trace-events
index 2b89d0f..38c8939 100644
--- a/trace-events
+++ b/trace-events
@@ -203,6 +203,9 @@ disable usb_ehci_state(const char *schedule, const char
*state) "%s schedule %s"
disable usb_ehci_qh(uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t
n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c,
int h, int dtc, int i) "QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen
%d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
disable usb_ehci_qtd(uint32_t addr, uint32_t next, uint32_t altnext, int
tbytes, int cpage, int cerr, int pid, int ioc, int active, int halt, int
babble, int xacterr) "QH @ %08x: next %08x altnext %08x - tbytes %d, cpage %d,
cerr %d, pid %d, ioc %d, active %d, halt %d, babble %d, xacterr %d"
disable usb_ehci_itd(uint32_t addr, uint32_t next) "ITD @ %08x: next %08x"
+disable usb_ehci_port_attach(uint32_t port, const char *device) "attach port
#%d - %s"
+disable usb_ehci_port_detach(uint32_t port) "detach port #%d"
+disable usb_ehci_port_reset(uint32_t port, int enable) "reset port #%d - %d"
# hw/usb-desc.c
disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len
%d, ret %d"
--
1.7.1
[Qemu-devel] [PATCH 01/31] usb-linux: catch ENODEV in more places.
Factor out disconnect code (called when a device disappears) to a
separate function. Add a check for ENODEV errno to a few more places
to make sure we notice disconnects.
Signed-off-by: Gerd Hoffmann
---
usb-linux.c | 27 ---
1 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index baa6574..b195e38 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -268,6 +268,14 @@ static void async_free(AsyncURB *aurb)
qemu_free(aurb);
}
+static void do_disconnect(USBHostDevice *s)
+{
+printf("husb: device %d.%d disconnected\n",
+ s->bus_num, s->addr);
+usb_host_close(s);
+usb_host_auto_check(NULL);
+}
+
static void async_complete(void *opaque)
{
USBHostDevice *s = opaque;
@@ -282,10 +290,7 @@ static void async_complete(void *opaque)
return;
}
if (errno == ENODEV && !s->closing) {
-printf("husb: device %d.%d disconnected\n",
- s->bus_num, s->addr);
-usb_host_close(s);
-usb_host_auto_check(NULL);
+do_disconnect(s);
return;
}
@@ -359,6 +364,7 @@ static void usb_host_async_cancel(USBDevice *dev, USBPacket
*p)
static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
{
+const char *op = NULL;
int dev_descr_len, config_descr_len;
int interface, nb_interfaces;
int ret, i;
@@ -411,9 +417,9 @@ static int usb_host_claim_interfaces(USBHostDevice *dev,
int configuration)
ctrl.ioctl_code = USBDEVFS_DISCONNECT;
ctrl.ifno = interface;
ctrl.data = 0;
+op = "USBDEVFS_DISCONNECT";
ret = ioctl(dev->fd, USBDEVFS_IOCTL, &ctrl);
if (ret < 0 && errno != ENODATA) {
-perror("USBDEVFS_DISCONNECT");
goto fail;
}
}
@@ -422,6 +428,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev,
int configuration)
/* XXX: only grab if all interfaces are free */
for (interface = 0; interface < nb_interfaces; interface++) {
+op = "USBDEVFS_CLAIMINTERFACE";
ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE, &interface);
if (ret < 0) {
if (errno == EBUSY) {
@@ -429,8 +436,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev,
int configuration)
} else {
perror("husb: failed to claim interface");
}
-fail:
-return 0;
+goto fail;
}
}
@@ -440,6 +446,13 @@ static int usb_host_claim_interfaces(USBHostDevice *dev,
int configuration)
dev->ninterfaces = nb_interfaces;
dev->configuration = configuration;
return 1;
+
+fail:
+if (errno == ENODEV) {
+do_disconnect(dev);
+}
+perror(op);
+return 0;
}
static int usb_host_release_interfaces(USBHostDevice *s)
--
1.7.1
[Qemu-devel] [PATCH 06/31] usb-ehci: trace buffer copy
Add a trace point for buffer copies and drop the DPRINTF's.
No change in behavior.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c |8 +---
trace-events |1 +
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index b9204ab..d89cb28 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -901,8 +901,6 @@ static int ehci_qh_do_overlay(EHCIState *ehci, EHCIqh *qh,
EHCIqtd *qtd)
dtoggle = qh->token & QTD_TOKEN_DTOGGLE;
ping= qh->token & QTD_TOKEN_PING;
-DPRINTF("setting qh.current from %08X to 0x%08X\n", qh->current_qtd,
-ehci->qtdaddr);
qh->current_qtd = ehci->qtdaddr;
qh->next_qtd= qtd->next;
qh->altnext_qtd = qtd->altnext;
@@ -955,8 +953,6 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int
bytes, int rw)
}
offset = qh->bufptr[0] & ~QTD_BUFPTR_MASK;
-DPRINTF("ehci_buffer_rw: %sing %d bytes %08x cpage %d offset %d\n",
- rw ? "writ" : "read", bytes, qh->bufptr[0], cpage, offset);
do {
/* start and end of this page */
@@ -969,9 +965,7 @@ static int ehci_buffer_rw(uint8_t *buffer, EHCIqh *qh, int
bytes, int rw)
tail = head + bytes;
}
-DPRINTF("DATA %s cpage:%d head:%08X tail:%08X target:%08X\n",
-rw ? "WRITE" : "READ ", cpage, head, tail, bufpos);
-
+trace_usb_ehci_data(rw, cpage, offset, head, tail-head, bufpos);
cpu_physical_memory_rw(head, &buffer[bufpos], tail - head, rw);
bufpos += (tail - head);
diff --git a/trace-events b/trace-events
index 1ee711e..3426e14 100644
--- a/trace-events
+++ b/trace-events
@@ -207,6 +207,7 @@ disable usb_ehci_itd(uint32_t addr, uint32_t next) "ITD @
%08x: next %08x"
disable usb_ehci_port_attach(uint32_t port, const char *device) "attach port
#%d - %s"
disable usb_ehci_port_detach(uint32_t port) "detach port #%d"
disable usb_ehci_port_reset(uint32_t port, int enable) "reset port #%d - %d"
+disable usb_ehci_data(int rw, uint32_t cpage, uint32_t offset, uint32_t addr,
uint32_t len, uint32_t bufpos) "write %d, cpage %d, offset 0x%03x, addr 0x%08x,
len %d, bufpos %d"
# hw/usb-desc.c
disable usb_desc_device(int addr, int len, int ret) "dev %d query device, len
%d, ret %d"
--
1.7.1
[Qemu-devel] [PATCH 00/31] usb patch queue
Hi, Here is the current usb patch queue. It brings a bunch of small fixes and cleanups, especially for EHCI and usb-linux. Additionally EHCI gets trace points and support for multiple async requests at the same time. please pull, Gerd The following changes since commit d800040fb47fe4500d1f8bf604b9fd129bda9419: scsi: fix tracing of scsi requests with simple backend (2011-06-05 15:05:35 +) are available in the git repository at: git://git.kraxel.org/qemu usb.15 Brad Hards (3): usb: Add defines for USB Serial Bus Release Number register usb: Use defines for serial bus release number register for UHCI usb: Use defines for serial bus release number register for EHCI Gerd Hoffmann (17): usb-linux: catch ENODEV in more places. usb-ehci: trace mmio and usbsts usb-ehci: trace state machine changes usb-ehci: trace port state usb-ehci: improve mmio tracing usb-ehci: trace buffer copy usb-ehci: add queue data struct usb-ehci: multiqueue support usb-ehci: fix offset writeback in ehci_buffer_rw usb-ehci: fix error handling. usb: cancel async packets on unplug usb-ehci: drop EXECUTING checks. usb-ehci: itd handling fixes. usb-ehci: split trace calls to handle arg count limits usb: documentation update usb-linux: only cleanup in host_close when host_open was successful. usb: don't call usb_host_device_open from vl.c Hans de Goede (9): ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6) usb-linux: Get speed from sysfs rather then from the connectinfo ioctl usb-linux: Teach about super speed usb-linux: Don't do perror when errno is not set usb-linux: Ensure devep != 0 usb-linux: Don't try to open the same device twice usb-linux: Enlarge buffer for descriptors to 8192 bytes usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper usb-bus: Don't detach non attached devices on device exit Kevin O'Connor (2): Fix USB mouse Set_Protocol behavior The USB tablet should not claim boot protocol support. docs/usb2.txt | 85 hw/milkymist-softusb.c | 10 +- hw/usb-bus.c | 10 +- hw/usb-ehci.c | 1198 hw/usb-hid.c |5 +- hw/usb-musb.c | 23 +- hw/usb-ohci.c | 16 +- hw/usb-uhci.c | 28 +- hw/usb.h | 14 +- trace-events | 20 + usb-linux.c| 96 +++-- vl.c |6 +- 12 files changed, 967 insertions(+), 544 deletions(-)
[Qemu-devel] [PATCH 02/31] usb-ehci: trace mmio and usbsts
This patch starts adding trace support to ehci. It traces
updates of the status register (USBSTS), mmio access and
controller reset.
It also adds functions to set and clear status register bits
and puts them in use everywhere.
Some DPRINTF's are dropped in favor of the new tracepoints.
No change in behavior.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 156 ++---
trace-events |6 ++
2 files changed, 89 insertions(+), 73 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index f63519e..16fbca6 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -30,6 +30,7 @@
#include "usb.h"
#include "pci.h"
#include "monitor.h"
+#include "trace.h"
#define EHCI_DEBUG 0
#define STATE_DEBUG 0 /* state transitions */
@@ -417,24 +418,23 @@ typedef struct {
} while(0)
-#if EHCI_DEBUG
-static const char *addr2str(unsigned addr)
+static const char *addr2str(target_phys_addr_t addr)
{
-const char *r= " unknown";
+const char *r= "unknown";
const char *n[] = {
-[ CAPLENGTH ]= " CAPLENGTH",
+[ CAPLENGTH ]= "CAPLENGTH",
[ HCIVERSION ] = "HCIVERSION",
-[ HCSPARAMS ]= " HCSPARAMS",
-[ HCCPARAMS ]= " HCCPARAMS",
-[ USBCMD ] = " COMMAND",
-[ USBSTS ] = "STATUS",
-[ USBINTR ] = " INTERRUPT",
-[ FRINDEX ] = " FRAME IDX",
+[ HCSPARAMS ]= "HCSPARAMS",
+[ HCCPARAMS ]= "HCCPARAMS",
+[ USBCMD ] = "USBCMD",
+[ USBSTS ] = "USBSTS",
+[ USBINTR ] = "USBINTR",
+[ FRINDEX ] = "FRINDEX",
[ PERIODICLISTBASE ] = "P-LIST BASE",
[ ASYNCLISTADDR ]= "A-LIST ADDR",
[ PORTSC_BEGIN ...
- PORTSC_END ] = "PORT STATUS",
-[ CONFIGFLAG ] = "CONFIG FLAG",
+ PORTSC_END ] = "PORTSC",
+[ CONFIGFLAG ] = "CONFIGFLAG",
};
if (addr < ARRAY_SIZE(n) && n[addr] != NULL) {
@@ -443,8 +443,61 @@ static const char *addr2str(unsigned addr)
return r;
}
}
-#endif
+static void ehci_trace_usbsts(uint32_t mask, int state)
+{
+/* interrupts */
+if (mask & USBSTS_INT) {
+trace_usb_ehci_usbsts("INT", state);
+}
+if (mask & USBSTS_ERRINT) {
+trace_usb_ehci_usbsts("ERRINT", state);
+}
+if (mask & USBSTS_PCD) {
+trace_usb_ehci_usbsts("PCD", state);
+}
+if (mask & USBSTS_FLR) {
+trace_usb_ehci_usbsts("FLR", state);
+}
+if (mask & USBSTS_HSE) {
+trace_usb_ehci_usbsts("HSE", state);
+}
+if (mask & USBSTS_IAA) {
+trace_usb_ehci_usbsts("IAA", state);
+}
+
+/* status */
+if (mask & USBSTS_HALT) {
+trace_usb_ehci_usbsts("HALT", state);
+}
+if (mask & USBSTS_REC) {
+trace_usb_ehci_usbsts("REC", state);
+}
+if (mask & USBSTS_PSS) {
+trace_usb_ehci_usbsts("PSS", state);
+}
+if (mask & USBSTS_ASS) {
+trace_usb_ehci_usbsts("ASS", state);
+}
+}
+
+static inline void ehci_set_usbsts(EHCIState *s, int mask)
+{
+if ((s->usbsts & mask) == mask) {
+return;
+}
+ehci_trace_usbsts(mask, 1);
+s->usbsts |= mask;
+}
+
+static inline void ehci_clear_usbsts(EHCIState *s, int mask)
+{
+if ((s->usbsts & mask) == 0) {
+return;
+}
+ehci_trace_usbsts(mask, 0);
+s->usbsts &= ~mask;
+}
static inline void ehci_set_interrupt(EHCIState *s, int intr)
{
@@ -452,7 +505,7 @@ static inline void ehci_set_interrupt(EHCIState *s, int
intr)
// TODO honour interrupt threshold requests
-s->usbsts |= intr;
+ehci_set_usbsts(s, intr);
if ((s->usbsts & USBINTR_MASK) & s->usbintr) {
level = 1;
@@ -526,6 +579,7 @@ static void ehci_reset(void *opaque)
uint8_t *pci_conf;
int i;
+trace_usb_ehci_reset();
pci_conf = s->dev.config;
memset(&s->mmio[OPREGBASE], 0x00, MMIO_SIZE - OPREGBASE);
@@ -576,6 +630,7 @@ static uint32_t ehci_mem_readl(void *ptr,
target_phys_addr_t addr)
val = s->mmio[addr] | (s->mmio[addr+1] << 8) |
(s->mmio[addr+2] << 16) | (s->mmio[addr+3] << 24);
+trace_usb_ehci_mmio_readl(addr, addr2str(addr), val);
return val;
}
@@ -645,9 +700,9 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t
addr, uint32_t val)
{
EHCIState *s = ptr;
int i;
-#if EHCI_DEBUG
-const char *str;
-#endif
+
+trace_usb_ehci_mmio_writel(addr, addr2str(addr), val,
+ *(uint32_t *)(&s->mmio[addr]));
/* Only aligned reads are allowed on OHCI */
if (addr & 3) {
@@ -669,30 +724,21 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t
addr, uint32_t val)
/* Do any register specific pre-write processing here. */
-#if EHCI_DEBUG
-str = addr2str((unsigned) addr);
-#endif
[Qemu-devel] [PATCH 12/31] usb: cancel async packets on unplug
This patch adds USBBusOps struct with (for now) only a single callback
which is called when a device is about to be destroyed. The USB Host
adapters are implementing this callback and use it to cancel any async
requests which might be in flight before the device actually goes away.
Signed-off-by: Gerd Hoffmann
---
hw/milkymist-softusb.c | 10 +-
hw/usb-bus.c |5 -
hw/usb-ehci.c | 25 -
hw/usb-musb.c | 23 ++-
hw/usb-ohci.c | 16 +++-
hw/usb-uhci.c | 26 +-
hw/usb.h |8 +++-
7 files changed, 106 insertions(+), 7 deletions(-)
diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c
index 1565260..028f3b7 100644
--- a/hw/milkymist-softusb.c
+++ b/hw/milkymist-softusb.c
@@ -247,10 +247,18 @@ static void softusb_attach(USBPort *port)
{
}
+static void softusb_device_destroy(USBBus *bus, USBDevice *dev)
+{
+}
+
static USBPortOps softusb_ops = {
.attach = softusb_attach,
};
+static USBBusOps softusb_bus_ops = {
+.device_destroy = softusb_device_destroy,
+};
+
static void milkymist_softusb_reset(DeviceState *d)
{
MilkymistSoftUsbState *s =
@@ -294,7 +302,7 @@ static int milkymist_softusb_init(SysBusDevice *dev)
qemu_add_mouse_event_handler(softusb_mouse_event, s, 0, "Milkymist Mouse");
/* create our usb bus */
-usb_bus_new(&s->usbbus, NULL);
+usb_bus_new(&s->usbbus, &softusb_bus_ops, NULL);
/* our two ports */
usb_register_port(&s->usbbus, &s->usbport[0], NULL, 0, &softusb_ops,
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index abc7e61..874c253 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -39,9 +39,10 @@ const VMStateDescription vmstate_usb_device = {
}
};
-void usb_bus_new(USBBus *bus, DeviceState *host)
+void usb_bus_new(USBBus *bus, USBBusOps *ops, DeviceState *host)
{
qbus_create_inplace(&bus->qbus, &usb_bus_info, host, NULL);
+bus->ops = ops;
bus->busnr = next_usb_bus++;
bus->qbus.allow_hotplug = 1; /* Yes, we can */
QTAILQ_INIT(&bus->free);
@@ -81,8 +82,10 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo *base)
static int usb_qdev_exit(DeviceState *qdev)
{
USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
+USBBus *bus = usb_bus_from_device(dev);
usb_device_detach(dev);
+bus->ops->device_destroy(bus, dev);
if (dev->info->handle_destroy) {
dev->info->handle_destroy(dev);
}
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index e368395..e345a1c 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -685,6 +685,18 @@ static void ehci_queues_rip_unused(EHCIState *ehci)
}
}
+static void ehci_queues_rip_device(EHCIState *ehci, USBDevice *dev)
+{
+EHCIQueue *q, *tmp;
+
+QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+if (q->packet.owner != dev) {
+continue;
+}
+ehci_free_queue(q);
+}
+}
+
static void ehci_queues_rip_all(EHCIState *ehci)
{
EHCIQueue *q, *tmp;
@@ -2100,6 +2112,13 @@ static void ehci_map(PCIDevice *pci_dev, int region_num,
cpu_register_physical_memory(addr, size, s->mem);
}
+static void ehci_device_destroy(USBBus *bus, USBDevice *dev)
+{
+EHCIState *s = container_of(bus, EHCIState, bus);
+
+ehci_queues_rip_device(s, dev);
+}
+
static int usb_ehci_initfn(PCIDevice *dev);
static USBPortOps ehci_port_ops = {
@@ -2108,6 +2127,10 @@ static USBPortOps ehci_port_ops = {
.complete = ehci_async_complete_packet,
};
+static USBBusOps ehci_bus_ops = {
+.device_destroy = ehci_device_destroy,
+};
+
static PCIDeviceInfo ehci_info = {
.qdev.name= "usb-ehci",
.qdev.size= sizeof(EHCIState),
@@ -2170,7 +2193,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
s->irq = s->dev.irq[3];
-usb_bus_new(&s->bus, &s->dev.qdev);
+usb_bus_new(&s->bus, &ehci_bus_ops, &s->dev.qdev);
for(i = 0; i < NB_PORTS; i++) {
usb_register_port(&s->bus, &s->ports[i], s, i, &ehci_port_ops,
USB_SPEED_MASK_HIGH);
diff --git a/hw/usb-musb.c b/hw/usb-musb.c
index 6037193..21f35af 100644
--- a/hw/usb-musb.c
+++ b/hw/usb-musb.c
@@ -262,6 +262,7 @@
static void musb_attach(USBPort *port);
static void musb_detach(USBPort *port);
static void musb_schedule_cb(USBDevice *dev, USBPacket *p);
+static void musb_device_destroy(USBBus *bus, USBDevice *dev);
static USBPortOps musb_port_ops = {
.attach = musb_attach,
@@ -269,6 +270,10 @@ static USBPortOps musb_port_ops = {
.complete = musb_schedule_cb,
};
+static USBBusOps musb_bus_ops = {
+.device_destroy = musb_device_destroy,
+};
+
typedef struct MUSBPacket MUSBPacket;
typedef struct MUSBEndPoint MUSBEndPoint;
@@ -361,7 +366,7 @@ struct MUSBState *musb_init(qemu_irq *irqs)
s->ep[i].epnum = i;
}
-usb_bus_new(&s->bus, NULL /* FIXME */);
+usb_bus_new(&s->bus, &musb_bus_ops, NULL /* FIXME */);
[Qemu-devel] [PATCH 18/31] usb: documentation update
Add some more informations to docs/usb2.txt about using usb2 (also usb1)
devices.
Signed-off-by: Gerd Hoffmann
---
docs/usb2.txt | 85 +
1 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/docs/usb2.txt b/docs/usb2.txt
index b283c13..5950c71 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -31,6 +31,91 @@ a complete example:
This attaches a usb tablet to the UHCI adapter and a usb mass storage
device to the EHCI adapter.
+
+More USB tips & tricks
+==
+
+Recently the usb pass through driver (also known as usb-host) and the
+qemu usb subsystem gained a few capabilities which are available only
+via qdev properties, i,e. when using '-device'.
+
+
+physical port addressing
+
+
+First you can (for all usb devices) specify the physical port where
+the device will show up in the guest. This can be done using the
+"port" property. UHCI has two root ports (1,2). EHCI has four root
+ports (1-4), the emulated (1.1) USB hub has eight ports.
+
+Plugging a tablet into UHCI port 1 works like this:
+
+-device usb-tablet,bus=usb.0,port=1
+
+Plugging a hub into UHCI port 2 works like this:
+
+-device usb-hub,bus=usb.0,port=2
+
+Plugging a virtual usb stick into port 4 of the hub just plugged works
+this way:
+
+-device usb-storage,bus=usb.0,port=2.4,drive=...
+
+You can do basically the same in the monitor using the device_add
+command. If you want to unplug devices too you should specify some
+unique id which you can use to refer to the device ...
+
+(qemu) device_add usb-tablet,bus=usb.0,port=1,id=my-tablet
+(qemu) device_del my-tablet
+
+... when unplugging it with device_del.
+
+
+USB pass through hints
+--
+
+The usb-host driver has a bunch of properties to specify the device
+which should be passed to the guest:
+
+ hostbus= -- Specifies the bus number the device must be attached
+ to.
+
+ hostaddr= -- Specifies the device address the device got
+ assigned by the guest os.
+
+ hostport= -- Specifies the physical port the device is attached
+ to.
+
+ vendorid= -- Specifies the vendor ID of the device.
+ productid= -- Specifies the product ID of the device.
+
+In theory you can combine all these properties as you like. In
+practice only a few combinations are useful:
+
+ (1) vendorid+productid -- match for a specific device, pass it to
+ the guest when it shows up somewhere in the host.
+
+ (2) hostbus+hostport -- match for a specific physical port in the
+ host, any device which is plugged in there gets passed to the
+ guest.
+
+ (3) hostbus+hostaddr -- most useful for ad-hoc pass through as the
+ hostaddr isn't stable, the next time you plug in the device it
+ gets a new one ...
+
+Note that USB 1.1 devices are handled by UHCI/OHCI and USB 2.0 by
+EHCI. That means a device plugged into the very same physical port
+may show up on different busses depending on the speed. The port I'm
+using for testing is bus 1 + port 1 for 2.0 devices and bus 3 + port 1
+for 1.1 devices. Passing through any device plugged into that port
+and also assign them to the correct bus can be done this way:
+
+qemu -M pc ${otheroptions} \
+-usb \
+-device usb-ehci,id=ehci \
+-device usb-host,bus=usb.0,hostbus=3,hostport=1 \
+-device usb-host,bus=ehci.0,hostbus=1,hostport=1
+
enjoy,
Gerd
--
1.7.1
[Qemu-devel] [PATCH 03/31] usb-ehci: trace state machine changes
Add functions to get and set the current state of the state machine,
add tracepoints there to trace state transitions. Add support for
traceing the queue heads and transfer descriptors as we look at them.
Drop a few DPRINTFs and all DPRINTF_ST lines, they are obsolete now.
No change in behavior.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 307 +++-
trace-events |4 +
2 files changed, 174 insertions(+), 137 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 16fbca6..40a447b 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -33,20 +33,13 @@
#include "trace.h"
#define EHCI_DEBUG 0
-#define STATE_DEBUG 0 /* state transitions */
-#if EHCI_DEBUG || STATE_DEBUG
+#if EHCI_DEBUG
#define DPRINTF printf
#else
#define DPRINTF(...)
#endif
-#if STATE_DEBUG
-#define DPRINTF_ST DPRINTF
-#else
-#define DPRINTF_ST(...)
-#endif
-
/* internal processing - reset HC to try and recover */
#define USB_RET_PROCERR (-99)
@@ -417,33 +410,59 @@ typedef struct {
*data = val; \
} while(0)
+static const char *ehci_state_names[] = {
+[ EST_INACTIVE ] = "INACTIVE",
+[ EST_ACTIVE ] = "ACTIVE",
+[ EST_EXECUTING ]= "EXECUTING",
+[ EST_SLEEPING ] = "SLEEPING",
+[ EST_WAITLISTHEAD ] = "WAITLISTHEAD",
+[ EST_FETCHENTRY ] = "FETCH ENTRY",
+[ EST_FETCHQH ] = "FETCH QH",
+[ EST_FETCHITD ] = "FETCH ITD",
+[ EST_ADVANCEQUEUE ] = "ADVANCEQUEUE",
+[ EST_FETCHQTD ] = "FETCH QTD",
+[ EST_EXECUTE ] = "EXECUTE",
+[ EST_WRITEBACK ]= "WRITEBACK",
+[ EST_HORIZONTALQH ] = "HORIZONTALQH",
+};
+
+static const char *ehci_mmio_names[] = {
+[ CAPLENGTH ]= "CAPLENGTH",
+[ HCIVERSION ] = "HCIVERSION",
+[ HCSPARAMS ]= "HCSPARAMS",
+[ HCCPARAMS ]= "HCCPARAMS",
+[ USBCMD ] = "USBCMD",
+[ USBSTS ] = "USBSTS",
+[ USBINTR ] = "USBINTR",
+[ FRINDEX ] = "FRINDEX",
+[ PERIODICLISTBASE ] = "P-LIST BASE",
+[ ASYNCLISTADDR ]= "A-LIST ADDR",
+[ PORTSC_BEGIN ] = "PORTSC #0",
+[ PORTSC_BEGIN + 4] = "PORTSC #1",
+[ PORTSC_BEGIN + 8] = "PORTSC #2",
+[ PORTSC_BEGIN + 12] = "PORTSC #3",
+[ CONFIGFLAG ] = "CONFIGFLAG",
+};
-static const char *addr2str(target_phys_addr_t addr)
+static const char *nr2str(const char **n, size_t len, uint32_t nr)
{
-const char *r= "unknown";
-const char *n[] = {
-[ CAPLENGTH ]= "CAPLENGTH",
-[ HCIVERSION ] = "HCIVERSION",
-[ HCSPARAMS ]= "HCSPARAMS",
-[ HCCPARAMS ]= "HCCPARAMS",
-[ USBCMD ] = "USBCMD",
-[ USBSTS ] = "USBSTS",
-[ USBINTR ] = "USBINTR",
-[ FRINDEX ] = "FRINDEX",
-[ PERIODICLISTBASE ] = "P-LIST BASE",
-[ ASYNCLISTADDR ]= "A-LIST ADDR",
-[ PORTSC_BEGIN ...
- PORTSC_END ] = "PORTSC",
-[ CONFIGFLAG ] = "CONFIGFLAG",
-};
-
-if (addr < ARRAY_SIZE(n) && n[addr] != NULL) {
-return n[addr];
+if (nr < len && n[nr] != NULL) {
+return n[nr];
} else {
-return r;
+return "unknown";
}
}
+static const char *state2str(uint32_t state)
+{
+return nr2str(ehci_state_names, ARRAY_SIZE(ehci_state_names), state);
+}
+
+static const char *addr2str(target_phys_addr_t addr)
+{
+return nr2str(ehci_mmio_names, ARRAY_SIZE(ehci_mmio_names), addr);
+}
+
static void ehci_trace_usbsts(uint32_t mask, int state)
{
/* interrupts */
@@ -528,6 +547,56 @@ static inline void ehci_commit_interrupt(EHCIState *s)
s->usbsts_pending = 0;
}
+static void ehci_set_state(EHCIState *s, int async, int state)
+{
+if (async) {
+trace_usb_ehci_state("async", state2str(state));
+s->astate = state;
+} else {
+trace_usb_ehci_state("periodic", state2str(state));
+s->pstate = state;
+}
+}
+
+static int ehci_get_state(EHCIState *s, int async)
+{
+return async ? s->astate : s->pstate;
+}
+
+static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
+{
+trace_usb_ehci_qh(addr, qh->next,
+ qh->current_qtd, qh->next_qtd, qh->altnext_qtd,
+ get_field(qh->epchar, QH_EPCHAR_RL),
+ get_field(qh->epchar, QH_EPCHAR_MPLEN),
+ get_field(qh->epchar, QH_EPCHAR_EPS),
+ get_field(qh->epchar, QH_EPCHAR_EP),
+ get_field(qh->epchar, QH_EPCHAR_DEVADDR),
+ (bool)(qh->epchar & QH_EPCHAR_C),
+ (bool)(qh->epchar & QH_EPCHAR_H),
+ (bool)(qh->epchar & QH_EPCHAR_DTC),
+ (bool)(qh->epchar & QH_EPCHAR_I));
+}
+
+static void ehci_trace_qtd(EHCIState *s, target_phys_addr_t addr, EHCIqtd *qtd)
+{
+trace_usb_ehci_qtd
[Qemu-devel] [PATCH 11/31] ehci: fix a number of unused-but-set-variable warnings (new with gcc-4.6)
From: Hans de Goede
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 2bbb5e4..e368395 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -740,11 +740,9 @@ static void ehci_detach(USBPort *port)
static void ehci_reset(void *opaque)
{
EHCIState *s = opaque;
-uint8_t *pci_conf;
int i;
trace_usb_ehci_reset();
-pci_conf = s->dev.config;
memset(&s->mmio[OPREGBASE], 0x00, MMIO_SIZE - OPREGBASE);
@@ -1268,12 +1266,11 @@ static int ehci_process_itd(EHCIState *ehci,
int dir;
int devadr;
int endp;
-int maxpkt;
dir =(itd->bufptr[1] & ITD_BUFPTR_DIRECTION);
devadr = get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR);
endp = get_field(itd->bufptr[0], ITD_BUFPTR_EP);
-maxpkt = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT);
+/* maxpkt = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT); */
for(i = 0; i < 8; i++) {
if (itd->transact[i] & ITD_XACT_ACTIVE) {
--
1.7.1
[Qemu-devel] [PATCH 23/31] usb-linux: Don't try to open the same device twice
From: Hans de Goede
If a user wants to redirect 2 identical usb sticks, in theory this is
possible by doing:
usb_add host:1234:5678
usb_add host:1234:5678
But this will lead to us trying to open the first stick twice, since we
don't break the loop after having found a match in our filter list, so the next'
filter list entry will result in us trying to open the same device again.
Fix this by adding the missing break.
Signed-off-by: Gerd Hoffmann
---
usb-linux.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 82c1e7d..1208a97 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1672,6 +1672,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num,
int addr, char *port,
DPRINTF("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
usb_host_open(s, bus_num, addr, port, product_name, speed);
+break;
}
return 0;
--
1.7.1
[Qemu-devel] [PATCH 05/31] usb-ehci: improve mmio tracing
Add a separate tracepoint to log how register values change in response
to a mmio write. Especially useful for registers which have read-only
or clear-on-write bits in them.
No change in behavior.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 16 ++--
trace-events |3 ++-
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 85e5ed9..b9204ab 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -719,10 +719,6 @@ static void handle_port_status_write(EHCIState *s, int
port, uint32_t val)
int rwc;
USBDevice *dev = s->ports[port].dev;
-DPRINTF("port_status_write: "
-"PORTSC (port %d) curr %08X new %08X rw-clear %08X rw %08X\n",
-port, *portsc, val, (val & PORTSC_RWC_MASK), val & PORTSC_RO_MASK);
-
rwc = val & PORTSC_RWC_MASK;
val &= PORTSC_RO_MASK;
@@ -744,8 +740,6 @@ static void handle_port_status_write(EHCIState *s, int
port, uint32_t val)
}
if (s->ports[port].dev) {
-DPRINTF("port_status_write: "
-"Device was connected before reset, clearing CSC bit\n");
*portsc &= ~PORTSC_CSC;
}
@@ -760,16 +754,16 @@ static void handle_port_status_write(EHCIState *s, int
port, uint32_t val)
*portsc &= ~PORTSC_RO_MASK;
*portsc |= val;
-DPRINTF("port_status_write: Port %d status set to 0x%08x\n", port,
*portsc);
}
static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
{
EHCIState *s = ptr;
+uint32_t *mmio = (uint32_t *)(&s->mmio[addr]);
+uint32_t old = *mmio;
int i;
-trace_usb_ehci_mmio_writel(addr, addr2str(addr), val,
- *(uint32_t *)(&s->mmio[addr]));
+trace_usb_ehci_mmio_writel(addr, addr2str(addr), val);
/* Only aligned reads are allowed on OHCI */
if (addr & 3) {
@@ -780,6 +774,7 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t
addr, uint32_t val)
if (addr >= PORTSC && addr < PORTSC + 4 * NB_PORTS) {
handle_port_status_write(s, (addr-PORTSC)/4, val);
+trace_usb_ehci_mmio_change(addr, addr2str(addr), *mmio, old);
return;
}
@@ -858,7 +853,8 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t
addr, uint32_t val)
break;
}
-*(uint32_t *)(&s->mmio[addr]) = val;
+*mmio = val;
+trace_usb_ehci_mmio_change(addr, addr2str(addr), *mmio, old);
}
diff --git a/trace-events b/trace-events
index 38c8939..1ee711e 100644
--- a/trace-events
+++ b/trace-events
@@ -197,7 +197,8 @@ disable sun4m_iommu_bad_addr(uint64_t addr) "bad addr
%"PRIx64""
# hw/usb-ehci.c
disable usb_ehci_reset(void) "=== RESET ==="
disable usb_ehci_mmio_readl(uint32_t addr, const char *str, uint32_t val) "rd
mmio %04x [%s] = %x"
-disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val,
uint32_t oldval) "wr mmio %04x [%s] = %x (old: %x)"
+disable usb_ehci_mmio_writel(uint32_t addr, const char *str, uint32_t val) "wr
mmio %04x [%s] = %x"
+disable usb_ehci_mmio_change(uint32_t addr, const char *str, uint32_t new,
uint32_t old) "ch mmio %04x [%s] = %x (old: %x)"
disable usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
disable usb_ehci_state(const char *schedule, const char *state) "%s schedule
%s"
disable usb_ehci_qh(uint32_t addr, uint32_t next, uint32_t c_qtd, uint32_t
n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int devaddr, int c,
int h, int dtc, int i) "QH @ %08x: next %08x qtds %08x,%08x,%08x - rl %d, mplen
%d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i %d"
--
1.7.1
[Qemu-devel] [PATCH 14/31] Fix USB mouse Set_Protocol behavior
From: Kevin O'Connor The QEMU USB mouse claims to support the "boot" protocol (bInterfaceSubClass is 1). However, the mouse rejects the Set_Protocol command. The qemu mouse does support the "boot" protocol specification, so a simple fix is to enable the Set_Protocol request. Signed-off-by: Kevin O'Connor Signed-off-by: Gerd Hoffmann --- hw/usb-hid.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb-hid.c b/hw/usb-hid.c index 53b261c..8197a86 100644 --- a/hw/usb-hid.c +++ b/hw/usb-hid.c @@ -782,13 +782,13 @@ static int usb_hid_handle_control(USBDevice *dev, USBPacket *p, goto fail; break; case GET_PROTOCOL: -if (s->kind != USB_KEYBOARD) +if (s->kind != USB_KEYBOARD && s->kind != USB_MOUSE) goto fail; ret = 1; data[0] = s->protocol; break; case SET_PROTOCOL: -if (s->kind != USB_KEYBOARD) +if (s->kind != USB_KEYBOARD && s->kind != USB_MOUSE) goto fail; ret = 0; s->protocol = value; -- 1.7.1
[Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
The state machine doesn't stop in EXECUTING state any more when async
packets are in flight, so the checks are not needed any more and can
be dropped.
Also kick out the check for the frame timer. As we don't stop & sleep
any more on async packets this is obsolete.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 32 ++--
1 files changed, 2 insertions(+), 30 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index e345a1c..8b1ae3a 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int
async)
int again = 0;
uint32_t entry = ehci_get_fetch_addr(ehci, async);
-#if EHCI_DEBUG == 0
-if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) {
-if (async) {
-DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
-goto out;
-} else {
-DPRINTF("FETCHENTRY: WARNING "
-"- frame timer elapsed during periodic\n");
-}
-}
-#endif
if (entry < 0x1000) {
DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
ehci_set_state(ehci, async, EST_ACTIVE);
@@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
}
ehci_set_state(ehci, async, EST_WAITLISTHEAD);
-/* fall through */
-
-case EST_FETCHENTRY:
-/* fall through */
-
-case EST_EXECUTING:
ehci_advance_state(ehci, async);
break;
@@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
ehci_advance_state(ehci, async);
break;
-case EST_EXECUTING:
-DPRINTF("PERIODIC state adv for executing\n");
-ehci_advance_state(ehci, async);
-break;
-
default:
/* this should only be due to a developer mistake */
fprintf(stderr, "ehci: Bad periodic state %d. "
@@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque)
if (frames - i > 10) {
skipped_frames++;
} else {
-// TODO could this cause periodic frames to get skipped if async
-// active?
-if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
-ehci_advance_periodic_state(ehci);
-}
+ehci_advance_periodic_state(ehci);
}
ehci->last_run_usec += FRAME_TIMER_USEC;
@@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque)
/* Async is not inside loop since it executes everything it can once
* called
*/
-if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
-ehci_advance_async_state(ehci);
-}
+ehci_advance_async_state(ehci);
qemu_mod_timer(ehci->frame_timer, expire_time);
}
--
1.7.1
[Qemu-devel] [PATCH 22/31] usb-linux: Ensure devep != 0
From: Hans de Goede
So that we don't index endp_table with a negative index.
Signed-off-by: Gerd Hoffmann
---
usb-linux.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 4c6c284..82c1e7d 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1030,6 +1030,11 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
}
devep = descriptors[i + 2];
+if ((devep & 0x0f) == 0) {
+fprintf(stderr, "usb-linux: invalid ep descriptor, ep == 0\n");
+return 1;
+}
+
switch (descriptors[i + 3] & 0x3) {
case 0x00:
type = USBDEVFS_URB_TYPE_CONTROL;
--
1.7.1
[Qemu-devel] [PATCH 08/31] usb-ehci: multiqueue support
This patch adds support for keeping multiple queues going at the same
time. One slow device will not affect other devices any more.
The patch adds code to manage EHCIQueue structs. It also does a number
of changes to the state machine:
* The state machine will never ever stop in EXECUTING any more.
Instead it will continue with the next queue (aka HORIZONTALQH) when
the usb device returns USB_RET_ASYNC.
* The state machine will stop processing when it figures it walks in
circles (easy to figure now that we have a EHCIQueue struct for each
QH we've processed). The bailout logic should not be needed any
more. For now it is still in, but will assert() in case it triggers.
* The state machine will just skip queues with a async USBPacket in
flight.
* The state machine will resume processing as soon as the async
USBPacket is finished.
The patch also takes care to flush the QH struct back to guest memory
when needed, so we don't get stale data when (re-)loading it from guest
memory in FETCHQH state.
It also makes the writeback code to not touch the first three dwords of
the QH struct as the EHCI must not write them. This actually fixes a
bug where QH chaining changes (next ptr) by the linux ehci driver where
overwritten by the emulated EHCI.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 171 ++---
trace-events |5 +-
2 files changed, 142 insertions(+), 34 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 3656a35..5cbb675 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -345,6 +345,9 @@ enum async_state {
struct EHCIQueue {
EHCIState *ehci;
+QTAILQ_ENTRY(EHCIQueue) next;
+bool async_schedule;
+uint32_t seen, ts;
/* cached data from guest - needs to be flushed
* when guest removes an entry (doorbell, handshake sequence)
@@ -400,7 +403,7 @@ struct EHCIState {
int pstate;// Current state in periodic schedule
USBPort ports[NB_PORTS];
uint32_t usbsts_pending;
-EHCIQueue queue;
+QTAILQ_HEAD(, EHCIQueue) queues;
uint32_t a_fetch_addr; // which address to look at next
uint32_t p_fetch_addr; // which address to look at next
@@ -594,9 +597,9 @@ static int ehci_get_fetch_addr(EHCIState *s, int async)
return async ? s->a_fetch_addr : s->p_fetch_addr;
}
-static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
+static void ehci_trace_qh(EHCIQueue *q, target_phys_addr_t addr, EHCIqh *qh)
{
-trace_usb_ehci_qh(addr, qh->next,
+trace_usb_ehci_qh(q, addr, qh->next,
qh->current_qtd, qh->next_qtd, qh->altnext_qtd,
get_field(qh->epchar, QH_EPCHAR_RL),
get_field(qh->epchar, QH_EPCHAR_MPLEN),
@@ -609,9 +612,9 @@ static void ehci_trace_qh(EHCIState *s, target_phys_addr_t
addr, EHCIqh *qh)
(bool)(qh->epchar & QH_EPCHAR_I));
}
-static void ehci_trace_qtd(EHCIState *s, target_phys_addr_t addr, EHCIqtd *qtd)
+static void ehci_trace_qtd(EHCIQueue *q, target_phys_addr_t addr, EHCIqtd *qtd)
{
-trace_usb_ehci_qtd(addr, qtd->next, qtd->altnext,
+trace_usb_ehci_qtd(q, addr, qtd->next, qtd->altnext,
get_field(qtd->token, QTD_TOKEN_TBYTES),
get_field(qtd->token, QTD_TOKEN_CPAGE),
get_field(qtd->token, QTD_TOKEN_CERR),
@@ -628,6 +631,69 @@ static void ehci_trace_itd(EHCIState *s,
target_phys_addr_t addr, EHCIitd *itd)
trace_usb_ehci_itd(addr, itd->next);
}
+/* queue management */
+
+static EHCIQueue *ehci_alloc_queue(EHCIState *ehci, int async)
+{
+EHCIQueue *q;
+
+q = qemu_mallocz(sizeof(*q));
+q->ehci = ehci;
+q->async_schedule = async;
+QTAILQ_INSERT_HEAD(&ehci->queues, q, next);
+trace_usb_ehci_queue_action(q, "alloc");
+return q;
+}
+
+static void ehci_free_queue(EHCIQueue *q)
+{
+trace_usb_ehci_queue_action(q, "free");
+if (q->async == EHCI_ASYNC_INFLIGHT) {
+usb_cancel_packet(&q->packet);
+}
+QTAILQ_REMOVE(&q->ehci->queues, q, next);
+qemu_free(q);
+}
+
+static EHCIQueue *ehci_find_queue_by_qh(EHCIState *ehci, uint32_t addr)
+{
+EHCIQueue *q;
+
+QTAILQ_FOREACH(q, &ehci->queues, next) {
+if (addr == q->qhaddr) {
+return q;
+}
+}
+return NULL;
+}
+
+static void ehci_queues_rip_unused(EHCIState *ehci)
+{
+EHCIQueue *q, *tmp;
+
+QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+if (q->seen) {
+q->seen = 0;
+q->ts = ehci->last_run_usec;
+continue;
+}
+if (ehci->last_run_usec < q->ts + 25) {
+/* allow 0.25 sec idle */
+continue;
+}
+ehci_free_queue(q);
+}
+}
+
+static void ehci_queues_rip_all(EHCIState *ehci)
+{
+EHCIQueue *q, *tmp;
+
+QTAILQ_FOREACH_SAFE(q, &ehci->queues, next, tmp) {
+ehci_free_
[Qemu-devel] [PATCH 10/31] usb-ehci: fix error handling.
Set the correct bits for nodev, stall and babble errors.
Raise errint irq. Fix state transition from WRITEBACK
to the next state.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 25 +++--
1 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index cf10dfc..2bbb5e4 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1114,10 +1114,10 @@ err:
switch(q->usb_status) {
case USB_RET_NODEV:
-fprintf(stderr, "USB no device\n");
+q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_XACTERR);
+ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
break;
case USB_RET_STALL:
-fprintf(stderr, "USB stall\n");
q->qh.token |= QTD_TOKEN_HALT;
ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
break;
@@ -1133,8 +1133,7 @@ err:
}
break;
case USB_RET_BABBLE:
-fprintf(stderr, "USB babble TODO\n");
-q->qh.token |= QTD_TOKEN_BABBLE;
+q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
break;
default:
@@ -1792,15 +1791,21 @@ static int ehci_state_writeback(EHCIQueue *q, int async)
put_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qh.next_qtd,
sizeof(EHCIqtd) >> 2);
-/* TODO confirm next state. For now, keep going if async
- * but stop after one qtd if periodic
+/*
+ * EHCI specs say go horizontal here.
+ *
+ * We can also advance the queue here for performance reasons. We
+ * need to take care to only take that shortcut in case we've
+ * processed the qtd just written back without errors, i.e. halt
+ * bit is clear.
*/
-//if (async) {
+if (q->qh.token & QTD_TOKEN_HALT) {
+ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
+again = 1;
+} else {
ehci_set_state(q->ehci, async, EST_ADVANCEQUEUE);
again = 1;
-//} else {
-//ehci_set_state(ehci, async, EST_ACTIVE);
-//}
+}
return again;
}
--
1.7.1
[Qemu-devel] [PATCH 09/31] usb-ehci: fix offset writeback in ehci_buffer_rw
Two bugs at once:
First the mask is backwards, so the it used to keeps the offset and
clears the page address, which is not what we need when we update the
offset.
Second the offset calculation is wrong in case head isn't page aligned.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 5cbb675..cf10dfc 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1066,6 +1066,7 @@ static int ehci_buffer_rw(EHCIQueue *q, int bytes, int rw)
cpu_physical_memory_rw(head, q->buffer + bufpos, tail - head, rw);
bufpos += (tail - head);
+offset += (tail - head);
bytes -= (tail - head);
if (bytes > 0) {
@@ -1078,8 +1079,7 @@ static int ehci_buffer_rw(EHCIQueue *q, int bytes, int rw)
set_field(&q->qh.token, cpage, QTD_TOKEN_CPAGE);
/* save offset into cpage */
-offset = tail - head;
-q->qh.bufptr[0] &= ~QTD_BUFPTR_MASK;
+q->qh.bufptr[0] &= QTD_BUFPTR_MASK;
q->qh.bufptr[0] |= offset;
return 0;
--
1.7.1
[Qemu-devel] [PATCH 20/31] usb-linux: Teach about super speed
From: Hans de Goede
Signed-off-by: Gerd Hoffmann
---
usb-linux.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 32e6ecb..03e43c0 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1379,7 +1379,9 @@ static int usb_host_scan_dev(void *opaque, USBScanFunc
*func)
if (get_tag_value(buf, sizeof(buf), line, "Spd=", " ") < 0) {
goto fail;
}
-if (!strcmp(buf, "480")) {
+if (!strcmp(buf, "5000")) {
+speed = USB_SPEED_SUPER;
+} else if (!strcmp(buf, "480")) {
speed = USB_SPEED_HIGH;
} else if (!strcmp(buf, "1.5")) {
speed = USB_SPEED_LOW;
@@ -1523,7 +1525,9 @@ static int usb_host_scan_sys(void *opaque, USBScanFunc
*func)
if (!usb_host_read_file(line, sizeof(line), "speed", de->d_name)) {
goto the_end;
}
-if (!strcmp(line, "480\n")) {
+if (!strcmp(line, "5000\n")) {
+speed = USB_SPEED_SUPER;
+} else if (!strcmp(line, "480\n")) {
speed = USB_SPEED_HIGH;
} else if (!strcmp(line, "1.5\n")) {
speed = USB_SPEED_LOW;
@@ -1800,6 +1804,9 @@ static void usb_info_device(Monitor *mon, int bus_num,
int addr, char *port,
case USB_SPEED_HIGH:
speed_str = "480";
break;
+case USB_SPEED_SUPER:
+speed_str = "5000";
+break;
default:
speed_str = "?";
break;
--
1.7.1
[Qemu-devel] [PATCH 19/31] usb-linux: Get speed from sysfs rather then from the connectinfo ioctl
From: Hans de Goede
The connectinfo ioctl only differentiates between lo speed devices, and
all other speeds, where as we would like to know the real speed. The real
speed is available in sysfs so use that when available.
Signed-off-by: Gerd Hoffmann
---
usb-linux.c | 37 +
1 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index b195e38..32e6ecb 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1057,10 +1057,9 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
}
static int usb_host_open(USBHostDevice *dev, int bus_num,
- int addr, char *port, const char *prod_name)
+int addr, char *port, const char *prod_name, int speed)
{
int fd = -1, ret;
-struct usbdevfs_connectinfo ci;
char buf[1024];
if (dev->fd != -1) {
@@ -1115,24 +1114,29 @@ static int usb_host_open(USBHostDevice *dev, int
bus_num,
goto fail;
}
-ret = ioctl(fd, USBDEVFS_CONNECTINFO, &ci);
-if (ret < 0) {
-perror("usb_host_device_open: USBDEVFS_CONNECTINFO");
-goto fail;
-}
-
-printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
-
ret = usb_linux_update_endp_table(dev);
if (ret) {
goto fail;
}
-if (ci.slow) {
-dev->dev.speed = USB_SPEED_LOW;
-} else {
-dev->dev.speed = USB_SPEED_HIGH;
+if (speed == -1) {
+struct usbdevfs_connectinfo ci;
+
+ret = ioctl(fd, USBDEVFS_CONNECTINFO, &ci);
+if (ret < 0) {
+perror("usb_host_device_open: USBDEVFS_CONNECTINFO");
+goto fail;
+}
+
+if (ci.slow) {
+speed = USB_SPEED_LOW;
+} else {
+speed = USB_SPEED_HIGH;
+}
}
+dev->dev.speed = speed;
+
+printf("husb: grabbed usb device %d.%d\n", bus_num, addr);
if (!prod_name || prod_name[0] == '\0') {
snprintf(dev->dev.product_desc, sizeof(dev->dev.product_desc),
@@ -1346,7 +1350,8 @@ static int usb_host_scan_dev(void *opaque, USBScanFunc
*func)
}
device_count = 0;
-bus_num = addr = speed = class_id = product_id = vendor_id = 0;
+bus_num = addr = class_id = product_id = vendor_id = 0;
+speed = -1; /* Can't get the speed from /[proc|dev]/bus/usb/devices */
for(;;) {
if (fgets(line, sizeof(line), f) == NULL) {
break;
@@ -1656,7 +1661,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num,
int addr, char *port,
}
DPRINTF("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
-usb_host_open(s, bus_num, addr, port, product_name);
+usb_host_open(s, bus_num, addr, port, product_name, speed);
}
return 0;
--
1.7.1
[Qemu-devel] [PATCH 21/31] usb-linux: Don't do perror when errno is not set
From: Hans de Goede
Note that "op" also is not set, so before this change these error paths
would feed NULL to perror.
Signed-off-by: Gerd Hoffmann
---
usb-linux.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 03e43c0..4c6c284 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -377,7 +377,8 @@ static int usb_host_claim_interfaces(USBHostDevice *dev,
int configuration)
i = 0;
dev_descr_len = dev->descr[0];
if (dev_descr_len > dev->descr_len) {
-goto fail;
+fprintf(stderr, "husb: update iface failed. descr too short\n");
+return 0;
}
i += dev_descr_len;
@@ -405,7 +406,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev,
int configuration)
if (i >= dev->descr_len) {
fprintf(stderr,
"husb: update iface failed. no matching configuration\n");
-goto fail;
+return 0;
}
nb_interfaces = dev->descr[i + 4];
--
1.7.1
[Qemu-devel] [PATCH 16/31] usb-ehci: itd handling fixes.
This patch fixes a bunch of issues in the itd descriptor handling.
Most important fix is to handle transfers which cross page borders
correctly by looking up the address of the next page. Luckily the
linux uses physically contigous memory so the data used to hits the
correct location even with this bug instead of corrupting guest
memory. Also the transfer length updates for outgoing transfers wasn't
correct.
While being at it DPRINTFs have been replaced by tracepoints.
The isoch_pause logic has been disabled. Not clear to me which propose
this serves and I think it is incorrect too as we just skip processing
itds. Even when no xfer happens we have to clear the active bit.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 101
trace-events |2 +-
2 files changed, 66 insertions(+), 37 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index 8b1ae3a..d807245 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -198,6 +198,7 @@ typedef struct EHCIitd {
#define ITD_BUFPTR_MAXPKT_MASK 0x07ff
#define ITD_BUFPTR_MAXPKT_SH 0
#define ITD_BUFPTR_MULT_MASK 0x0003
+#define ITD_BUFPTR_MULT_SH 0
} EHCIitd;
/* EHCI spec version 1.0 Section 3.4
@@ -628,7 +629,11 @@ static void ehci_trace_qtd(EHCIQueue *q,
target_phys_addr_t addr, EHCIqtd *qtd)
static void ehci_trace_itd(EHCIState *s, target_phys_addr_t addr, EHCIitd *itd)
{
-trace_usb_ehci_itd(addr, itd->next);
+trace_usb_ehci_itd(addr, itd->next,
+ get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT),
+ get_field(itd->bufptr[2], ITD_BUFPTR_MULT),
+ get_field(itd->bufptr[0], ITD_BUFPTR_EP),
+ get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR));
}
/* queue management */
@@ -1270,41 +1275,51 @@ static int ehci_process_itd(EHCIState *ehci,
USBPort *port;
USBDevice *dev;
int ret;
-int i, j;
-int ptr;
-int pid;
-int pg;
-int len;
-int dir;
-int devadr;
-int endp;
+uint32_t i, j, len, len1, len2, pid, dir, devaddr, endp;
+uint32_t pg, off, ptr1, ptr2, max, mult;
dir =(itd->bufptr[1] & ITD_BUFPTR_DIRECTION);
-devadr = get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR);
+devaddr = get_field(itd->bufptr[0], ITD_BUFPTR_DEVADDR);
endp = get_field(itd->bufptr[0], ITD_BUFPTR_EP);
-/* maxpkt = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT); */
+max = get_field(itd->bufptr[1], ITD_BUFPTR_MAXPKT);
+mult = get_field(itd->bufptr[2], ITD_BUFPTR_MULT);
for(i = 0; i < 8; i++) {
if (itd->transact[i] & ITD_XACT_ACTIVE) {
-DPRINTF("ISOCHRONOUS active for frame %d, interval %d\n",
-ehci->frindex >> 3, i);
-
-pg = get_field(itd->transact[i], ITD_XACT_PGSEL);
-ptr = (itd->bufptr[pg] & ITD_BUFPTR_MASK) |
-(itd->transact[i] & ITD_XACT_OFFSET_MASK);
-len = get_field(itd->transact[i], ITD_XACT_LENGTH);
+pg = get_field(itd->transact[i], ITD_XACT_PGSEL);
+off = itd->transact[i] & ITD_XACT_OFFSET_MASK;
+ptr1 = (itd->bufptr[pg] & ITD_BUFPTR_MASK);
+ptr2 = (itd->bufptr[pg+1] & ITD_BUFPTR_MASK);
+len = get_field(itd->transact[i], ITD_XACT_LENGTH);
+
+if (len > max * mult) {
+len = max * mult;
+}
if (len > BUFF_SIZE) {
return USB_RET_PROCERR;
}
-DPRINTF("ISOCH: buffer %08X len %d\n", ptr, len);
+if (off + len > 4096) {
+/* transfer crosses page border */
+len2 = off + len - 4096;
+len1 = len - len2;
+} else {
+len1 = len;
+len2 = 0;
+}
if (!dir) {
-cpu_physical_memory_rw(ptr, &ehci->ibuffer[0], len, 0);
pid = USB_TOKEN_OUT;
-} else
+trace_usb_ehci_data(0, pg, off, ptr1 + off, len1, 0);
+cpu_physical_memory_rw(ptr1 + off, &ehci->ibuffer[0], len1, 0);
+if (len2) {
+trace_usb_ehci_data(0, pg+1, 0, ptr2, len2, len1);
+cpu_physical_memory_rw(ptr2, &ehci->ibuffer[len1], len2,
0);
+}
+} else {
pid = USB_TOKEN_IN;
+}
ret = USB_RET_NODEV;
@@ -1315,18 +1330,15 @@ static int ehci_process_itd(EHCIState *ehci,
// TODO sometime we will also need to check if we are the port
owner
if (!(ehci->portsc[j] &(PORTSC_CONNECT))) {
-DPRINTF("Port %d, no exec, not connected(%08X)\n",
-j, ehci->portsc[j]);
continue;
}
ehci->ipacket.pid = pid;
-ehci->ipacket.devaddr = devadr;
+ehci->ipacket.devaddr = de
[Qemu-devel] [PULL] spice patch queue
Hi, Resending pull request for the spice patch queue. Almost nothing changed, the queue gained one additional fix from Alon and was rebased to latest master. please pull, Gerd The following changes since commit d800040fb47fe4500d1f8bf604b9fd129bda9419: scsi: fix tracing of scsi requests with simple backend (2011-06-05 15:05:35 +) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu spice.v37 Alon Levy (1): qxl: fix cmdlog for vga Gerd Hoffmann (3): qxl: add to the list of devices which disable the default vga qemu-config: comment spell fix spice: require spice 0.6.0 or newer. Hans de Goede (2): spice-qemu-char: Fix flow control in client -> guest direction spice: add option for disabling copy paste support Marc-André Lureau (1): spice: add SASL support configure |2 +- hw/qxl.c |4 +++- qemu-config.c | 12 +--- qemu-options.hx | 16 spice-qemu-char.c | 11 +-- ui/spice-core.c | 26 ++ vl.c |1 + 7 files changed, 53 insertions(+), 19 deletions(-)
[Qemu-devel] [PATCH 07/31] usb-ehci: add queue data struct
Add EHCIQueue struct, move the fields needed to track the queue state
into that struct. Pass the new struct instead of ehci state down to
functions which handle the queue state. Lot of variable references have
changed due to that without an actual functional change.
Replace fetch_addr with two variables, one for async and one for
periodic schedule. Add functions to get and set the fetch address.
Use EHCIQueue->usb_status (old name: EHCIState->exec_status) directly in
ehci_execute_complete instead of passing around the status using a
parameters and the return value.
ehci_state_fetchqh returns a EHCIQueue struct now.
No change in behavior.
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 486 ++---
1 files changed, 257 insertions(+), 229 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d89cb28..3656a35 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -334,8 +334,37 @@ typedef struct EHCIfstn {
uint32_t backptr; // Standard next link pointer
} EHCIfstn;
-typedef struct {
+typedef struct EHCIQueue EHCIQueue;
+typedef struct EHCIState EHCIState;
+
+enum async_state {
+EHCI_ASYNC_NONE = 0,
+EHCI_ASYNC_INFLIGHT,
+EHCI_ASYNC_FINISHED,
+};
+
+struct EHCIQueue {
+EHCIState *ehci;
+
+/* cached data from guest - needs to be flushed
+ * when guest removes an entry (doorbell, handshake sequence)
+ */
+EHCIqh qh; // copy of current QH (being worked on)
+uint32_t qhaddr; // address QH read from
+EHCIqtd qtd; // copy of current QTD (being worked on)
+uint32_t qtdaddr; // address QTD read from
+
+USBPacket packet;
+uint8_t buffer[BUFF_SIZE];
+int pid;
+uint32_t tbytes;
+enum async_state async;
+int usb_status;
+};
+
+struct EHCIState {
PCIDevice dev;
+USBBus bus;
qemu_irq irq;
target_phys_addr_t mem_base;
int mem;
@@ -360,6 +389,7 @@ typedef struct {
uint32_t portsc[NB_PORTS];
};
};
+
/*
* Internal states, shadow registers, etc
*/
@@ -369,32 +399,19 @@ typedef struct {
int astate;// Current state in asynchronous
schedule
int pstate;// Current state in periodic schedule
USBPort ports[NB_PORTS];
-uint8_t buffer[BUFF_SIZE];
uint32_t usbsts_pending;
+EHCIQueue queue;
-/* cached data from guest - needs to be flushed
- * when guest removes an entry (doorbell, handshake sequence)
- */
-EHCIqh qh; // copy of current QH (being worked on)
-uint32_t qhaddr; // address QH read from
-
-EHCIqtd qtd; // copy of current QTD (being worked on)
-uint32_t qtdaddr; // address QTD read from
-
-uint32_t itdaddr; // current ITD
-
-uint32_t fetch_addr; // which address to look at next
+uint32_t a_fetch_addr; // which address to look at next
+uint32_t p_fetch_addr; // which address to look at next
-USBBus bus;
-USBPacket usb_packet;
-int async_complete;
-uint32_t tbytes;
-int pid;
-int exec_status;
+USBPacket ipacket;
+uint8_t ibuffer[BUFF_SIZE];
int isoch_pause;
+
uint32_t last_run_usec;
uint32_t frame_end_usec;
-} EHCIState;
+};
#define SET_LAST_RUN_CLOCK(s) \
(s)->last_run_usec = qemu_get_clock_ns(vm_clock) / 1000;
@@ -563,6 +580,20 @@ static int ehci_get_state(EHCIState *s, int async)
return async ? s->astate : s->pstate;
}
+static void ehci_set_fetch_addr(EHCIState *s, int async, uint32_t addr)
+{
+if (async) {
+s->a_fetch_addr = addr;
+} else {
+s->p_fetch_addr = addr;
+}
+}
+
+static int ehci_get_fetch_addr(EHCIState *s, int async)
+{
+return async ? s->a_fetch_addr : s->p_fetch_addr;
+}
+
static void ehci_trace_qh(EHCIState *s, target_phys_addr_t addr, EHCIqh *qh)
{
trace_usb_ehci_qh(addr, qh->next,
@@ -656,7 +687,6 @@ static void ehci_reset(void *opaque)
s->astate = EST_INACTIVE;
s->pstate = EST_INACTIVE;
-s->async_complete = 0;
s->isoch_pause = -1;
s->attach_poll_counter = 0;
@@ -888,7 +918,7 @@ static inline int put_dwords(uint32_t addr, uint32_t *buf,
int num)
// 4.10.2
-static int ehci_qh_do_overlay(EHCIState *ehci, EHCIqh *qh, EHCIqtd *qtd)
+static int ehci_qh_do_overlay(EHCIQueue *q)
{
int i;
int dtoggle;
@@ -898,43 +928,43 @@ static int ehci_qh_do_overlay(EHCIState *ehci, EHCIqh
*qh, EHCIqtd *qtd)
// remember values in fields to preserve in qh after overlay
-dtoggle = qh->token & QTD_TOKEN_DTOGGLE;
-ping= qh->token & QTD_TOKEN_PING;
+dtoggle = q->qh.token & QTD_TOKEN_DTOGGLE;
+ping= q->qh.token & QTD_TOKEN_PING;
-qh->current_qtd = ehci->qtdaddr;
-qh->next_qtd= qtd->next;
-qh->altnext_qtd = qtd->altnext;
-qh->token = qtd->token;
+q->qh.current_qtd = q->qtdaddr;
+q->qh.next_qtd= q->qtd.
[Qemu-devel] [PATCH 28/31] usb-bus: Don't detach non attached devices on device exit
From: Hans de Goede
This causes an "Error: tried to detach unattached usb device " to be printed,
this can happen when deleting ie a usb host qdev, which did not
get attached (because a device matching the filter never got plugged in).
Signed-off-by: Gerd Hoffmann
---
hw/usb-bus.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 91f2083..480956d 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -84,7 +84,9 @@ static int usb_qdev_exit(DeviceState *qdev)
USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
USBBus *bus = usb_bus_from_device(dev);
-usb_device_detach(dev);
+if (dev->attached) {
+usb_device_detach(dev);
+}
bus->ops->device_destroy(bus, dev);
if (dev->info->handle_destroy) {
dev->info->handle_destroy(dev);
--
1.7.1
[Qemu-devel] [PATCH 27/31] usb-bus: Add knowledge of USB_SPEED_SUPER to usb_speed helper
From: Hans de Goede Signed-off-by: Gerd Hoffmann --- hw/usb-bus.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/usb-bus.c b/hw/usb-bus.c index 874c253..91f2083 100644 --- a/hw/usb-bus.c +++ b/hw/usb-bus.c @@ -273,6 +273,7 @@ static const char *usb_speed(unsigned int speed) [ USB_SPEED_LOW ] = "1.5", [ USB_SPEED_FULL ] = "12", [ USB_SPEED_HIGH ] = "480", +[ USB_SPEED_SUPER ] = "5000", }; if (speed >= ARRAY_SIZE(txt)) return "?"; -- 1.7.1
[Qemu-devel] [PATCH 5/7] qemu-config: comment spell fix
Signed-off-by: Gerd Hoffmann
---
qemu-config.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index b00aa3a..c63741c 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -306,7 +306,7 @@ static QemuOptsList qemu_trace_opts = {
.name = "file",
.type = QEMU_OPT_STRING,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
#endif
@@ -436,7 +436,7 @@ QemuOptsList qemu_spice_opts = {
.name = "playback-compression",
.type = QEMU_OPT_BOOL,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
@@ -452,7 +452,7 @@ QemuOptsList qemu_option_rom_opts = {
.name = "romfile",
.type = QEMU_OPT_STRING,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
--
1.7.1
[Qemu-devel] [PATCH 25/31] usb: don't call usb_host_device_open from vl.c
Not needed any more, usb-host is qdev-ified these days.
Well, at least the linux version ...
Signed-off-by: Gerd Hoffmann
---
vl.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index b362871..b432421 100644
--- a/vl.c
+++ b/vl.c
@@ -923,9 +923,13 @@ static int usb_device_add(const char *devname)
goto done;
/* the other ones */
+#ifndef CONFIG_LINUX
+/* only the linux version is qdev-ified, usb-bsd still needs this */
if (strstart(devname, "host:", &p)) {
dev = usb_host_device_open(p);
-} else if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
+} else
+#endif
+if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
dev = usb_bt_init(devname[2] ? hci_init(p) :
bt_new_hci(qemu_find_bt_vlan(0)));
} else {
--
1.7.1
[Qemu-devel] [PATCH 26/31] usb-linux: Enlarge buffer for descriptors to 8192 bytes
From: Hans de Goede
1024 bytes is way to small, one hd UVC webcam I have over here has so
many resolutions its descriptors take op close to 4k. Hopefully 8k will
be enough for all devices.
Signed-off-by: Gerd Hoffmann
---
usb-linux.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index be667f0..600ec2d 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -116,7 +116,7 @@ typedef struct USBHostDevice {
USBDevice dev;
int fd;
-uint8_t descr[1024];
+uint8_t descr[8192];
int descr_len;
int configuration;
int ninterfaces;
--
1.7.1
[Qemu-devel] [PATCH 15/31] The USB tablet should not claim boot protocol support.
From: Kevin O'Connor
The USB tablet advertises that it supports the "boot" protocol.
However, its reports aren't "boot" protocol compatible. So, it
shouldn't claim that.
Signed-off-by: Kevin O'Connor
Signed-off-by: Gerd Hoffmann
---
hw/usb-hid.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index 8197a86..d711b5c 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -142,7 +142,6 @@ static const USBDescIface desc_iface_tablet = {
.bInterfaceNumber = 0,
.bNumEndpoints = 1,
.bInterfaceClass = USB_CLASS_HID,
-.bInterfaceSubClass= 0x01, /* boot */
.bInterfaceProtocol= 0x02,
.ndesc = 1,
.descs = (USBDescOther[]) {
--
1.7.1
[Qemu-devel] [PATCH 6/7] spice: require spice 0.6.0 or newer.
This patch raises the minimum required spice version to 0.6.0 and drops
a few ifdefs.
0.6.0 is the first stable release with the current libspice-server API,
there shouldn't be any 0.5.x development versions deployed any more.
Signed-off-by: Gerd Hoffmann
---
configure |2 +-
ui/spice-core.c |8
2 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index d38b952..11f85bf 100755
--- a/configure
+++ b/configure
@@ -2431,7 +2431,7 @@ int main(void) { spice_server_new(); return 0; }
EOF
spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
- if $pkg_config --atleast-version=0.5.3 spice-server >/dev/null 2>&1 && \
+ if $pkg_config --atleast-version=0.6.0 spice-server >/dev/null 2>&1 && \
compile_prog "$spice_cflags" "$spice_libs" ; then
spice="yes"
libs_softmmu="$libs_softmmu $spice_libs"
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 457d34d..dd9905b 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -299,8 +299,6 @@ static int parse_name(const char *string, const char
*optname,
exit(1);
}
-#if SPICE_SERVER_VERSION >= 0x000600 /* 0.6.0 */
-
static const char *stream_video_names[] = {
[ SPICE_STREAM_VIDEO_OFF ]= "off",
[ SPICE_STREAM_VIDEO_ALL ]= "all",
@@ -309,8 +307,6 @@ static const char *stream_video_names[] = {
#define parse_stream_video(_name) \
name2enum(_name, stream_video_names, ARRAY_SIZE(stream_video_names))
-#endif /* >= 0.6.0 */
-
static const char *compression_names[] = {
[ SPICE_IMAGE_COMPRESS_OFF ] = "off",
[ SPICE_IMAGE_COMPRESS_AUTO_GLZ ] = "auto_glz",
@@ -593,8 +589,6 @@ void qemu_spice_init(void)
}
spice_server_set_zlib_glz_compression(spice_server, wan_compr);
-#if SPICE_SERVER_VERSION >= 0x000600 /* 0.6.0 */
-
str = qemu_opt_get(opts, "streaming-video");
if (str) {
int streaming_video = parse_stream_video(str);
@@ -606,8 +600,6 @@ void qemu_spice_init(void)
spice_server_set_playback_compression
(spice_server, qemu_opt_get_bool(opts, "playback-compression", 1));
-#endif /* >= 0.6.0 */
-
qemu_opt_foreach(opts, add_channel, NULL, 0);
spice_server_init(spice_server, &core_interface);
--
1.7.1
[Qemu-devel] [PATCH 24/31] usb-linux: only cleanup in host_close when host_open was successful.
---
usb-linux.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 1208a97..be667f0 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1159,9 +1159,9 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
return 0;
fail:
-dev->fd = -1;
-if (fd != -1) {
-close(fd);
+if (dev->fd != -1) {
+close(dev->fd);
+dev->fd = -1;
}
return -1;
}
@@ -1170,7 +1170,7 @@ static int usb_host_close(USBHostDevice *dev)
{
int i;
-if (dev->fd == -1) {
+if (dev->fd == -1 || !dev->dev.attached) {
return -1;
}
--
1.7.1
[Qemu-devel] [PATCH 17/31] usb-ehci: split trace calls to handle arg count limits
Signed-off-by: Gerd Hoffmann
---
hw/usb-ehci.c | 48 +++-
trace-events |8 ++--
2 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index d807245..b151b48 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -600,31 +600,37 @@ static int ehci_get_fetch_addr(EHCIState *s, int async)
static void ehci_trace_qh(EHCIQueue *q, target_phys_addr_t addr, EHCIqh *qh)
{
-trace_usb_ehci_qh(q, addr, qh->next,
- qh->current_qtd, qh->next_qtd, qh->altnext_qtd,
- get_field(qh->epchar, QH_EPCHAR_RL),
- get_field(qh->epchar, QH_EPCHAR_MPLEN),
- get_field(qh->epchar, QH_EPCHAR_EPS),
- get_field(qh->epchar, QH_EPCHAR_EP),
- get_field(qh->epchar, QH_EPCHAR_DEVADDR),
- (bool)(qh->epchar & QH_EPCHAR_C),
- (bool)(qh->epchar & QH_EPCHAR_H),
- (bool)(qh->epchar & QH_EPCHAR_DTC),
- (bool)(qh->epchar & QH_EPCHAR_I));
+/* need three here due to argument count limits */
+trace_usb_ehci_qh_ptrs(q, addr, qh->next,
+ qh->current_qtd, qh->next_qtd, qh->altnext_qtd);
+trace_usb_ehci_qh_fields(addr,
+ get_field(qh->epchar, QH_EPCHAR_RL),
+ get_field(qh->epchar, QH_EPCHAR_MPLEN),
+ get_field(qh->epchar, QH_EPCHAR_EPS),
+ get_field(qh->epchar, QH_EPCHAR_EP),
+ get_field(qh->epchar, QH_EPCHAR_DEVADDR));
+trace_usb_ehci_qh_bits(addr,
+ (bool)(qh->epchar & QH_EPCHAR_C),
+ (bool)(qh->epchar & QH_EPCHAR_H),
+ (bool)(qh->epchar & QH_EPCHAR_DTC),
+ (bool)(qh->epchar & QH_EPCHAR_I));
}
static void ehci_trace_qtd(EHCIQueue *q, target_phys_addr_t addr, EHCIqtd *qtd)
{
-trace_usb_ehci_qtd(q, addr, qtd->next, qtd->altnext,
- get_field(qtd->token, QTD_TOKEN_TBYTES),
- get_field(qtd->token, QTD_TOKEN_CPAGE),
- get_field(qtd->token, QTD_TOKEN_CERR),
- get_field(qtd->token, QTD_TOKEN_PID),
- (bool)(qtd->token & QTD_TOKEN_IOC),
- (bool)(qtd->token & QTD_TOKEN_ACTIVE),
- (bool)(qtd->token & QTD_TOKEN_HALT),
- (bool)(qtd->token & QTD_TOKEN_BABBLE),
- (bool)(qtd->token & QTD_TOKEN_XACTERR));
+/* need three here due to argument count limits */
+trace_usb_ehci_qtd_ptrs(q, addr, qtd->next, qtd->altnext);
+trace_usb_ehci_qtd_fields(addr,
+ get_field(qtd->token, QTD_TOKEN_TBYTES),
+ get_field(qtd->token, QTD_TOKEN_CPAGE),
+ get_field(qtd->token, QTD_TOKEN_CERR),
+ get_field(qtd->token, QTD_TOKEN_PID));
+trace_usb_ehci_qtd_bits(addr,
+(bool)(qtd->token & QTD_TOKEN_IOC),
+(bool)(qtd->token & QTD_TOKEN_ACTIVE),
+(bool)(qtd->token & QTD_TOKEN_HALT),
+(bool)(qtd->token & QTD_TOKEN_BABBLE),
+(bool)(qtd->token & QTD_TOKEN_XACTERR));
}
static void ehci_trace_itd(EHCIState *s, target_phys_addr_t addr, EHCIitd *itd)
diff --git a/trace-events b/trace-events
index dd69702..f1230f1 100644
--- a/trace-events
+++ b/trace-events
@@ -201,8 +201,12 @@ disable usb_ehci_mmio_writel(uint32_t addr, const char
*str, uint32_t val) "wr m
disable usb_ehci_mmio_change(uint32_t addr, const char *str, uint32_t new,
uint32_t old) "ch mmio %04x [%s] = %x (old: %x)"
disable usb_ehci_usbsts(const char *sts, int state) "usbsts %s %d"
disable usb_ehci_state(const char *schedule, const char *state) "%s schedule
%s"
-disable usb_ehci_qh(void *q, uint32_t addr, uint32_t next, uint32_t c_qtd,
uint32_t n_qtd, uint32_t a_qtd, int rl, int mplen, int eps, int ep, int
devaddr, int c, int h, int dtc, int i) "q %p - QH @ %08x: next %08x qtds
%08x,%08x,%08x - rl %d, mplen %d, eps %d, ep %d, dev %d, c %d, h %d, dtc %d, i
%d"
-disable usb_ehci_qtd(void *q, uint32_t addr, uint32_t next, uint32_t altnext,
int tbytes, int cpage, int cerr, int pid, int ioc, int active, int halt, int
babble, int xacterr) "q %p - QTD @ %08x: next %08x altnext %08x - tbytes %d,
cpage %d, cerr %d, pid %d, ioc %d, active %d, halt %d, babble %d, xacterr %d"
+disable usb_ehci_qh_ptrs(void *q, uint32_t addr, uint32_t next, uint32_t
c_qtd, uint32_t n_qtd, uint32_t a_qtd) "q %p - QH @ %08x: next %08x qtds
%08x,%08x,%08x"
+disable usb_ehci_qh_fields(uint32_t addr, int rl, int mplen, int eps, int ep,
int devaddr) "QH @ %08x - rl %d, mplen %d, eps %d, ep %d, dev %d"
+dis
[Qemu-devel] [PATCH 29/31] usb: Add defines for USB Serial Bus Release Number register
From: Brad Hards Signed-off-by: Brad Hards Signed-off-by: Gerd Hoffmann --- hw/usb.h |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/hw/usb.h b/hw/usb.h index 6097208..06ce058 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -26,6 +26,12 @@ #include "qdev.h" #include "qemu-queue.h" +/* Constants related to the USB / PCI interaction */ +#define USB_SBRN0x60 /* Serial Bus Release Number Register */ +#define USB_RELEASE_1 0x10 /* USB 1.0 */ +#define USB_RELEASE_2 0x20 /* USB 2.0 */ +#define USB_RELEASE_3 0x30 /* USB 3.0 */ + #define USB_TOKEN_SETUP 0x2d #define USB_TOKEN_IN0x69 /* device -> host */ #define USB_TOKEN_OUT 0xe1 /* host -> device */ -- 1.7.1
[Qemu-devel] KVM call agendo for June 7th
Please send in any agenda items you are interested in covering. Thanks, Juan.
[Qemu-devel] [PATCH 31/31] usb: Use defines for serial bus release number register for EHCI
From: Brad Hards Signed-off-by: Brad Hards Signed-off-by: Gerd Hoffmann --- hw/usb-ehci.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index b151b48..36ba41e 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -2167,7 +2167,7 @@ static int usb_ehci_initfn(PCIDevice *dev) // pci_conf[0x50] = 0x01; // power management caps -pci_set_byte(&pci_conf[0x60], 0x20); // spec release number (2.1.4) +pci_set_byte(&pci_conf[USB_SBRN], USB_RELEASE_2); // release number (2.1.4) pci_set_byte(&pci_conf[0x61], 0x20); // frame length adjustment (2.1.5) pci_set_word(&pci_conf[0x62], 0x00); // port wake up capability (2.1.6) -- 1.7.1
[Qemu-devel] [PATCH 30/31] usb: Use defines for serial bus release number register for UHCI
From: Brad Hards
Signed-off-by: Brad Hards
Signed-off-by: Gerd Hoffmann
---
hw/usb-uhci.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 8f504d1..1503373 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -1122,7 +1122,7 @@ static int usb_uhci_common_initfn(UHCIState *s)
pci_config_set_class(pci_conf, PCI_CLASS_SERIAL_USB);
/* TODO: reset value should be 0. */
pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
-pci_conf[0x60] = 0x10; // release number
+pci_conf[USB_SBRN] = USB_RELEASE_1; // release number
usb_bus_new(&s->bus, &uhci_bus_ops, &s->dev.qdev);
for(i = 0; i < NB_PORTS; i++) {
--
1.7.1
Re: [Qemu-devel] [PATCH 0/4] introduce cpu_physical_memory_map_fast
On 06/06/2011 07:27 AM, Paolo Bonzini wrote: On 05/31/2011 11:16 AM, Paolo Bonzini wrote: On 05/12/2011 04:51 PM, Paolo Bonzini wrote: On 05/03/2011 06:49 PM, Paolo Bonzini wrote: Paravirtualized devices (and also some real devices) can assume they are going to access RAM. For this reason, provide a fast-path function with the following properties: 1) it will never allocate a bounce buffer 2) it can be used for read-modify-write operations 3) unlike qemu_get_ram_ptr, it is safe because it recognizes "short" blocks Patches 3 and 4 use this function for virtio devices and the milkymist GPU. The latter is only compile-tested. Another function checks if it is possible to split a contiguous physical address range into multiple subranges, all of which use the fast path. I will introduce later a use for this function. Paolo Bonzini (4): exec: extract cpu_physical_memory_map_internal exec: introduce cpu_physical_memory_map_fast and cpu_physical_memory_map_check virtio: use cpu_physical_memory_map_fast milkymist: use cpu_physical_memory_map_fast cpu-common.h | 4 ++ exec.c | 108 +- hw/milkymist-tmu2.c | 39 ++ hw/vhost.c | 10 ++-- hw/virtio.c | 2 +- 5 files changed, 111 insertions(+), 52 deletions(-) Ping? Ping^2? Ping^3? Oh, the patch series basically died for me when I saw: Avi> What performance benefit does this bring? Paolo> Zero Especially given Avi's efforts to introduce a new RAM API, I don't want yet another special case to handle. You're just trying to avoid having to handle map failures, right? So it's not really cpu_physical_memory_map_fast, it's really cpu_physical_memory_map_simple? I'd prefer that a device just treat failures as fatal vs. using a new API. Regards, Anthony Liguori Paolo
[Qemu-devel] [PATCH 3/7] qxl: add to the list of devices which disable the default vga
Signed-off-by: Gerd Hoffmann
---
vl.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/vl.c b/vl.c
index b362871..2021bbb 100644
--- a/vl.c
+++ b/vl.c
@@ -289,6 +289,7 @@ static struct {
{ .driver = "VGA", .flag = &default_vga },
{ .driver = "cirrus-vga", .flag = &default_vga },
{ .driver = "vmware-svga", .flag = &default_vga },
+{ .driver = "qxl-vga", .flag = &default_vga },
};
static int default_driver_check(QemuOpts *opts, void *opaque)
--
1.7.1
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
On 06/06/2011 06:27 AM, Markus Armbruster wrote:
Kevin Wolf writes:
Am 02.06.2011 20:09, schrieb Luiz Capitulino:
On Thu, 02 Jun 2011 13:00:04 -0500
Anthony Liguori wrote:
On 06/02/2011 12:57 PM, Luiz Capitulino wrote:
On Wed, 01 Jun 2011 16:35:03 -0500
Anthony Liguori wrote:
On 06/01/2011 04:12 PM, Luiz Capitulino wrote:
Hi there,
There are people who want to use QMP for thin provisioning. That's, the VM is
started with a small storage and when a no space error is triggered, more space
is allocated and the VM is put to run again.
QMP has two limitations that prevent people from doing this today:
1. The BLOCK_IO_ERROR doesn't contain error information
2. Considering we solve item 1, we still have to provide a way for clients
to query why a VM stopped. This is needed because clients may miss the
BLOCK_IO_ERROR event or may connect to the VM while it's already stopped
A proposal to solve both problems follow.
A. BLOCK_IO_ERROR information
-
We already have discussed this a lot, but didn't reach a consensus. My solution
is quite simple: to add a stringfied errno name to the BLOCK_IO_ERROR event,
for example (see the "reason" key):
{ "event": "BLOCK_IO_ERROR",
"data": { "device": "ide0-hd1",
"operation": "write",
"action": "stop",
"reason": "enospc", }
you can call the reason whatever you want, but don't call it stringfied
errno name :-)
In fact, just make reason "no space".
You mean, we should do:
"reason": "no space"
Or that we should make it a boolean, like:
"no space": true
Do we need reason in BLOCK_IO_ERROR if query-block returns this information?
True, no.
I'm ok with either way. But in case you meant the second one, I guess
we should make "reason" a dictionary so that we can group related
information when we extend the field, for example:
"reason": { "no space": false, "no permission": true }
Splitting up enums into a number of booleans looks like a bad idea to
me. It makes things more verbose than they should be, and even worse, it
implies that more than one field could be true.
If this new schema thing doesn't support proper enums, that's something
that should be changed.
Why would we ever have "no permission"?
It's an I/O error. I have a report from a developer who was getting
the BLOCK_IO_ERROR event and had to debug qemu to know the error cause,
it turned out to be no permission.
And I want to add that it's a PITA to handle bug report when the only
message you get from qemu is "something went wrong". Sorry, that's not
useful at all. I want to see the real error reason (and at least for
debugging this means, I want to see the errno value/string).
And I want it straight, not wrapped in a pile of pseudo-abstractions
that make me go to the source code to figure out how to unwrap them.
A set of default logged trace points would be perfect, no?
Regards,
Anthony Liguori
[...]
[Qemu-devel] [PATCH 2/7] spice: add option for disabling copy paste support
From: Hans de Goede
Some people want to be able disable spice's guest <-> client copy paste support
because of security considerations.
[ kraxel: drop old-version error message ]
Signed-off-by: Gerd Hoffmann
---
qemu-config.c |3 +++
qemu-options.hx |3 +++
ui/spice-core.c |6 ++
3 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 5d7ffa2..04c97e5 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -385,6 +385,9 @@ QemuOptsList qemu_spice_opts = {
.name = "disable-ticketing",
.type = QEMU_OPT_BOOL,
},{
+.name = "disable-copy-paste",
+.type = QEMU_OPT_BOOL,
+},{
.name = "x509-dir",
.type = QEMU_OPT_STRING,
},{
diff --git a/qemu-options.hx b/qemu-options.hx
index 82e085a..63e8cb0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -717,6 +717,9 @@ Set the password you need to authenticate.
@item disable-ticketing
Allow client connects without authentication.
+@item disable-copy-paste
+Disable copy paste between the client and the guest.
+
@item tls-port=
Set the TCP port spice is listening on for encrypted channels.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ef56ed6..a3351f3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -554,6 +554,12 @@ void qemu_spice_init(void)
spice_server_set_noauth(spice_server);
}
+#if SPICE_SERVER_VERSION >= 0x000801
+if (qemu_opt_get_bool(opts, "disable-copy-paste", 0)) {
+spice_server_set_agent_copypaste(spice_server, false);
+}
+#endif
+
compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
str = qemu_opt_get(opts, "image-compression");
if (str) {
--
1.7.1
[Qemu-devel] [PATCH 1/7] spice-qemu-char: Fix flow control in client -> guest direction
From: Hans de Goede
In the old spice-vmc device we used to have:
last_out = virtio_serial_write(&svc->port, p, MIN(len, VMC_MAX_HOST_WRITE));
if (last_out > 0)
...
Now in the chardev backend we have:
last_out = MIN(len, VMC_MAX_HOST_WRITE);
qemu_chr_read(scd->chr, p, last_out);
if (last_out > 0) {
...
Which causes us to no longer detect if the virtio port is not ready
to receive data from us. chardev actually has a mechanism to detect this,
but it requires a separate call to qemu_chr_can_read, before calling
qemu_chr_read (which return void).
This patch uses qemu_chr_can_read to fix the flow control from client to
guest.
Signed-off-by: Hans de Goede
Signed-off-by: Gerd Hoffmann
---
spice-qemu-char.c | 11 +--
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index fa15a71..605c241 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -36,14 +36,13 @@ static int vmc_write(SpiceCharDeviceInstance *sin, const
uint8_t *buf, int len)
while (len > 0) {
last_out = MIN(len, VMC_MAX_HOST_WRITE);
-qemu_chr_read(scd->chr, p, last_out);
-if (last_out > 0) {
-out += last_out;
-len -= last_out;
-p += last_out;
-} else {
+if (qemu_chr_can_read(scd->chr) < last_out) {
break;
}
+qemu_chr_read(scd->chr, p, last_out);
+out += last_out;
+len -= last_out;
+p += last_out;
}
dprintf(scd, 3, "%s: %lu/%zd\n", __func__, out, len + out);
--
1.7.1
Re: [Qemu-devel] [RFC 00/10]: QMP/HMP: Introduce tray handling commands
Luiz Capitulino writes: > In a recent discussion on the mailing list regarding the introduction of > the BLOCK_TRAY_OPEN and BLOCK_TRAY_CLOSE events[1], it was mentioned that > we need to fix the eject command and maybe introduce new commands first. > > Here's a my proposal. > > This series introduces three new commands: > > o blockdev-tray-open: opens the drive tray. Also Supports removing the > inserted >media. The BLOCK_TRAY_OPEN event is emitted if this command succeeds. Conflates tray control with media control. Hmm. > o blockdev-tray-close: closes a drive tray. The BLOCK_TRAY_CLOSE event is >emitted. > o blockdev-media-insert: Inserts a media in the tray. The tray must empty >and already opened. No event is emitted. What about separating tray control and media control like this: * Tray control: either blockdev-tray-open and blockdev-tray-close, or a single blockdev-tray with a boolean argument. * Media control: blockdev-media to change media. Tray must be open, may or may not contain media. Arguments specify new media, or no media.
Re: [Qemu-devel] [RFC 04/10] HMP: info block: Print the 'tray-open' key
Luiz Capitulino writes: > It's printed before the "[not inserted]" or the "file" fields, like this: > > (qemu) info block > ide0-hd0: removable=0 file=disks/test.img ro=0 drv=qcow2 encrypted=0 > ide1-cd0: removable=1 locked=0 tray-open=0 file=/Fedora-14-x86_64-DVD.iso > ro=1 drv=raw encrypted=0 > floppy0: removable=1 locked=0 tray-open=0 [not inserted] > sd0: removable=1 locked=0 tray-open=0 [not inserted] > (qemu) > > TODO: Confirm this won't break libvirt. As far as I can see, libvirt uses neither "info block" nor "query-block". I'd squash this into the previous commit.
Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions
Am 06.06.2011 13:57, schrieb Markus Armbruster: Not sure what's the best way of fixing this. Maybe just ignoring -snapshot for read-only block devices? >>> >>> Why not, the combination is pointless. >> >> It could start making a difference in some obscure combinations. Imagine >> a read-only image with a backing file, -snapshot and the 'commit' >> monitor command. >> >> Sounds pretty insane, but I wouldn't bet that people aren't using it... > > People try all kinds of insane things. The question is whether we can > change it anyway. We have a backing file chain like base <- cow [<- tmp], and the drive is read-only. Currently, 'commit' means that tmp is committed to cow (i.e. nothing happens because it's read-only). After changing it, we would commit the content of cow to base and possibly corrupt other images that are based on base. We can hope that nobody would be hit by it in practice, but it's not a change I'd feel very comfortable about. Or we could try and forward the eject request to the backing file if the format driver doesn't handle it. >>> >>> For what it's worth, the "raw" driver forwards manually. >> >> Yeah, but copying that into each driver probably isn't the right thing >> to do. > > I didn't mean to hold up the "raw" driver as a shining example of how to > do stuff. > >>For a generic implementation we could probably first try the >> driver itself, if it returns -ENOTSUP we try the protocol, and if that >> returns -ENOTSUP, too, we ask the backing file driver. > > I don't want to start another "format vs. protocol" semantic war, just > point out that the general case is a tree of block drivers, and a > general rule for passing eject up the tree better covers that. > > The current block.c provides for binary trees (bs->file and > bs->backing_hd). When we discussed blockdev_add, we came up with much > wilder trees. I didn't mean to imply that you suggested raw was a shining example. However, if we're going for the wild trees (or even graphs), there's no simple option of doing it in generic code any more. Backing files and protocols (for lack of a better term) are special in the tree because they are a concept that the block layer knows. For everything else the least we need is a hint from the block driver. In any case passing up the tree means that you need to decide which child is the one to try first, or if there are children to which it shouldn't be passed at all. Kevin
[Qemu-devel] [PATCH 7/7] qxl: fix cmdlog for vga
From: Alon Levy
Signed-off-by: Alon Levy
Signed-off-by: Gerd Hoffmann
---
hw/qxl.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 2bb36c6..1906e84 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -357,7 +357,9 @@ static int interface_get_command(QXLInstance *sin, struct
QXLCommandExt *ext)
ret = true;
}
qemu_mutex_unlock(&qxl->ssd.lock);
-qxl_log_command(qxl, "vga", ext);
+if (ret) {
+qxl_log_command(qxl, "vga", ext);
+}
return ret;
case QXL_MODE_COMPAT:
case QXL_MODE_NATIVE:
--
1.7.1
[Qemu-devel] [PATCH 4/7] spice: add SASL support
From: Marc-André Lureau
Turn on SASL support by appending "sasl" to the spice arguments, which
requires that the client use SASL to authenticate with the spice. The
exact choice of authentication method used is controlled from the
system / user's SASL configuration file for the 'qemu' service. This
is typically found in /etc/sasl2/qemu.conf. If running QEMU as an
unprivileged user, an environment variable SASL_CONF_PATH can be used
to make it search alternate locations for the service config. While
some SASL auth methods can also provide data encryption (eg GSSAPI),
it is recommended that SASL always be combined with the 'tls' and
'x509' settings to enable use of SSL and server certificates. This
ensures a data encryption preventing compromise of authentication
credentials.
It requires support from spice 0.8.1.
[ kraxel: moved spell fix to separate commit ]
Signed-off-by: Gerd Hoffmann
---
qemu-config.c |3 +++
qemu-options.hx | 13 +
ui/spice-core.c | 12
3 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 04c97e5..b00aa3a 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -388,6 +388,9 @@ QemuOptsList qemu_spice_opts = {
.name = "disable-copy-paste",
.type = QEMU_OPT_BOOL,
},{
+.name = "sasl",
+.type = QEMU_OPT_BOOL,
+},{
.name = "x509-dir",
.type = QEMU_OPT_STRING,
},{
diff --git a/qemu-options.hx b/qemu-options.hx
index 63e8cb0..d9edff7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -714,6 +714,19 @@ Force using the specified IP version.
@item password=
Set the password you need to authenticate.
+@item sasl
+Require that the client use SASL to authenticate with the spice.
+The exact choice of authentication method used is controlled from the
+system / user's SASL configuration file for the 'qemu' service. This
+is typically found in /etc/sasl2/qemu.conf. If running QEMU as an
+unprivileged user, an environment variable SASL_CONF_PATH can be used
+to make it search alternate locations for the service config.
+While some SASL auth methods can also provide data encryption (eg GSSAPI),
+it is recommended that SASL always be combined with the 'tls' and
+'x509' settings to enable use of SSL and server certificates. This
+ensures a data encryption preventing compromise of authentication
+credentials.
+
@item disable-ticketing
Allow client connects without authentication.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a3351f3..457d34d 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -549,6 +549,18 @@ void qemu_spice_init(void)
if (password) {
spice_server_set_ticket(spice_server, password, 0, 0, 0);
}
+if (qemu_opt_get_bool(opts, "sasl", 0)) {
+#if SPICE_SERVER_VERSION >= 0x000900 /* 0.9.0 */
+if (spice_server_set_sasl_appname(spice_server, "qemu") == -1 ||
+spice_server_set_sasl(spice_server, 1) == -1) {
+fprintf(stderr, "spice: failed to enable sasl\n");
+exit(1);
+}
+#else
+fprintf(stderr, "spice: sasl is not available (spice >= 0.9
required)\n");
+exit(1);
+#endif
+}
if (qemu_opt_get_bool(opts, "disable-ticketing", 0)) {
auth = "none";
spice_server_set_noauth(spice_server);
--
1.7.1
[Qemu-devel] [PATCH v3 0/3] spapr qdevification
This series fixes some problems with spapr's qdev interface. Patch 1 is the important one, which makes it possible to use -device to create vio devices. The other two are cosmetic. v1->v2: abstracted the call to xics_find_qirq behind spapr_find_qirq v2->v3: undid v1->v2 change, introduced spapr_allocate_irq Paolo Bonzini (3): spapr: proper qdevification spapr: prepare for qdevification of irq spapr: make irq customizable via qdev hw/spapr.c | 16 ++-- hw/spapr.h |6 ++ hw/spapr_llan.c | 11 ++- hw/spapr_vio.c |9 + hw/spapr_vio.h | 18 +- hw/spapr_vscsi.c | 12 ++-- hw/spapr_vty.c | 10 ++ 7 files changed, 36 insertions(+), 46 deletions(-) -- 1.7.4.4
Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
Hi,
-#if EHCI_DEBUG == 0
-if (qemu_get_clock_ns(vm_clock) / 1000>= ehci->frame_end_usec) {
-if (async) {
-DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
-goto out;
-} else {
-DPRINTF("FETCHENTRY: WARNING "
-"- frame timer elapsed during periodic\n");
-}
-}
-#endif
This check was added to per Section 4 of the EHCI spec -- the HC should
not start transactions that will not be completed before the end of the
micro-frame. If you remove this what causes the EHCI model to take a
breather?
Look at patch #8. That brings a number of state machine changes. One
of them is that the async schedule stops as soon as it notices it walks
in circles (i.e. sees a QH the second time).
-case EST_FETCHENTRY:
-/* fall through */
Why drop this case too? As I recall that is needed for proper execution.
The async schedule doesn't pause in fetchentry state any more (also done
by patch #8).
cheers,
Gerd
Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
On 06/06/2011 04:25 AM, Kevin Wolf wrote:
Am 02.06.2011 20:09, schrieb Luiz Capitulino:
I'm ok with either way. But in case you meant the second one, I guess
we should make "reason" a dictionary so that we can group related
information when we extend the field, for example:
"reason": { "no space": false, "no permission": true }
Splitting up enums into a number of booleans looks like a bad idea to
me. It makes things more verbose than they should be, and even worse, it
implies that more than one field could be true.
I agree. What I had suggested was to not have a reason at all.
If this new schema thing doesn't support proper enums, that's something
that should be changed.
It does BTW.
Why would we ever have "no permission"?
It's an I/O error. I have a report from a developer who was getting
the BLOCK_IO_ERROR event and had to debug qemu to know the error cause,
it turned out to be no permission.
And I want to add that it's a PITA to handle bug report when the only
message you get from qemu is "something went wrong". Sorry, that's not
useful at all. I want to see the real error reason (and at least for
debugging this means, I want to see the errno value/string).
Finding out that it was -EACCES in fact cost me (and QA who ran into the
problem) much more time than it should have. It's simply too much that
you need to attach gdb to find out what really happened.
You want a log file and you want libvirt to actually let QEMU write to a
log file.
Since we already have trace points, could we have a white list of trace
points that are written to a log file by default?
QMP is a stable interface that we have to maintain forever. We can
write whatever we want to a log file and change what gets written at will.
Regards,
Anthony Liguori
Kevin
[Qemu-devel] [PATCH v3 1/3] spapr: proper qdevification
Right now the spapr devices cannot be instantiated with -device,
because the IRQs need to be passed to the spapr_*_create functions.
Do this instead in the bus's init wrapper.
This is particularly important with the conversion from scsi-disk
to scsi-{cd,hd} that Markus made. After his patches, if you
specify a scsi-cd device attached to an if=none drive, the default
VSCSI controller will not be created and, without qdevification,
you will not be able to add yours.
Signed-off-by: Paolo Bonzini
---
hw/spapr.c | 16 ++--
hw/spapr.h |6 ++
hw/spapr_llan.c |7 +--
hw/spapr_vio.c |3 +++
hw/spapr_vio.h | 13 -
hw/spapr_vscsi.c |8 +---
hw/spapr_vty.c |8 +---
7 files changed, 22 insertions(+), 39 deletions(-)
diff --git a/hw/spapr.c b/hw/spapr.c
index 109b774..575dcaf 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -298,7 +298,6 @@ static void ppc_spapr_init(ram_addr_t ram_size,
long kernel_size, initrd_size, fw_size;
long pteg_shift = 17;
char *filename;
-int irq = 16;
spapr = qemu_malloc(sizeof(*spapr));
cpu_ppc_hypercall = emulate_spapr_hypercall;
@@ -308,6 +307,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
* necessary */
spapr->fdt_addr = MIN(ram_size, 0x8000) - FDT_MAX_SIZE;
spapr->rtas_addr = spapr->fdt_addr - RTAS_MAX_SIZE;
+spapr->next_irq = 16;
/* init CPUs */
if (cpu_model == NULL) {
@@ -360,15 +360,14 @@ static void ppc_spapr_init(ram_addr_t ram_size,
/* Set up VIO bus */
spapr->vio_bus = spapr_vio_bus_init();
-for (i = 0; i < MAX_SERIAL_PORTS; i++, irq++) {
+for (i = 0; i < MAX_SERIAL_PORTS; i++) {
if (serial_hds[i]) {
spapr_vty_create(spapr->vio_bus, SPAPR_VTY_BASE_ADDRESS + i,
- serial_hds[i], xics_find_qirq(spapr->icp, irq),
- irq);
+ serial_hds[i]);
}
}
-for (i = 0; i < nb_nics; i++, irq++) {
+for (i = 0; i < nb_nics; i++) {
NICInfo *nd = &nd_table[i];
if (!nd->model) {
@@ -376,8 +375,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
}
if (strcmp(nd->model, "ibmveth") == 0) {
-spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd,
- xics_find_qirq(spapr->icp, irq), irq);
+spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd);
} else {
fprintf(stderr, "pSeries (sPAPR) platform does not support "
"NIC model '%s' (only ibmveth is supported)\n",
@@ -387,9 +385,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
}
for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) {
-spapr_vscsi_create(spapr->vio_bus, 0x2000 + i,
- xics_find_qirq(spapr->icp, irq), irq);
-irq++;
+spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
}
if (kernel_filename) {
diff --git a/hw/spapr.h b/hw/spapr.h
index b52133a..eb9e5a9 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -8,6 +8,7 @@ typedef struct sPAPREnvironment {
struct VIOsPAPRBus *vio_bus;
struct icp_state *icp;
+int next_irq;
void *htab;
long htab_size;
target_phys_addr_t fdt_addr, rtas_addr;
@@ -278,6 +279,11 @@ void spapr_register_hypercall(target_ulong opcode,
spapr_hcall_fn fn);
target_ulong spapr_hypercall(CPUState *env, target_ulong opcode,
target_ulong *args);
+static inline int spapr_allocate_irq(sPAPREnvironment *spapr)
+{
+return spapr->next_irq++;
+}
+
static inline uint32_t rtas_ld(target_ulong phys, int n)
{
return ldl_phys(phys + 4*n);
diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index c18efc7..2597748 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -195,11 +195,9 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
return 0;
}
-void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd,
- qemu_irq qirq, uint32_t vio_irq_num)
+void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd)
{
DeviceState *dev;
-VIOsPAPRDevice *sdev;
dev = qdev_create(&bus->bus, "spapr-vlan");
qdev_prop_set_uint32(dev, "reg", reg);
@@ -207,9 +205,6 @@ void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg,
NICInfo *nd,
qdev_set_nic_properties(dev, nd);
qdev_init_nofail(dev);
-sdev = (VIOsPAPRDevice *)dev;
-sdev->qirq = qirq;
-sdev->vio_irq_num = vio_irq_num;
}
static int spapr_vlan_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 481a804..6f34159 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -32,6 +32,7 @@
#include "hw/spapr.h"
#include "hw/spapr_vio.h"
+#include "hw/xics.h"
#ifdef CONFIG_FDT
#include
@@ -602,6 +603,8 @@ static int spapr_vio_busdev_init(DeviceState *qdev,
DeviceInfo *qinfo)
}
dev->qdev.id = id;
+dev->vio_ir
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
Luiz Capitulino writes: > This command opens a removable media drive's tray. It's only available > in QMP. > > The do_tray_open() function is split into two smaller functions because > next commits will also use them. > > Please, check the command's documentation (being introduced in this > commit) for a detailed description. > > XXX: Should we return an error if the tray is already open? Use case? For what it's worth, Linux ioctl CDROMCLOSETRAY appears to return success when it does nothing.
[Qemu-devel] [PATCH v2 1/3] move WORDS_ALIGNED to qemu-common.h
This is not a CPU interface. Signed-off-by: Paolo Bonzini --- cpu-common.h |4 qemu-common.h |4 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cpu-common.h b/cpu-common.h index 7a86d63..7979a18 100644 --- a/cpu-common.h +++ b/cpu-common.h @@ -3,10 +3,6 @@ /* CPU interfaces that are target indpendent. */ -#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__) -#define WORDS_ALIGNED -#endif - #ifdef TARGET_PHYS_ADDR_BITS #include "targphys.h" #endif diff --git a/qemu-common.h b/qemu-common.h index b851b20..7484ef8 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -4,6 +4,10 @@ #include "config-host.h" +#if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__) +#define WORDS_ALIGNED +#endif + #define QEMU_NORETURN __attribute__ ((__noreturn__)) #ifdef CONFIG_GCC_ATTRIBUTE_WARN_UNUSED_RESULT #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) -- 1.7.4.4
Re: [Qemu-devel] [PATCH 0/4] introduce cpu_physical_memory_map_fast
On 06/06/2011 02:56 PM, Anthony Liguori wrote: Oh, the patch series basically died for me when I saw: Avi> What performance benefit does this bring? Paolo> Zero :) Especially given Avi's efforts to introduce a new RAM API, I don't want yet another special case to handle. This is not a special case, the existing functions are all mapped onto the new cpu_physical_memory_map_internal. I don't think this is in any way related to Avi's RAM API which is (mostly) for MMIO. You're just trying to avoid having to handle map failures, right? Not just that. If you had a memory block at say 1 GB - 2 GB, and another at 2 GB - 3 GB, a DMA operation that crosses the two could be implemented with cpu_physical_memory_map_fast; you would simply build a two-element iovec in two steps, something the current API does not allow. The patch does not change virtio to do the split, but it is possible to do that. The reason I'm not doing the virtio change, is that I know mst has pending changes to virtio and I'd rather avoid the conflicts for now. However, for vmw_pvscsi I'm going to handle it using the new functions. Paolo
Re: [Qemu-devel] [RFC 07/10] QMP: Introduce the blockdev-media-insert command
Luiz Capitulino writes:
> This command inserts a new media in an already opened tray. It's only
> available in QMP.
>
> Please, check the command's documentation (being introduced in this
> commit) for a detailed description.
>
> Signed-off-by: Luiz Capitulino
> ---
> blockdev.c | 52
> blockdev.h |1 +
> qmp-commands.hx | 32
> 3 files changed, 85 insertions(+), 0 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 943905d..14c8312 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -721,6 +721,58 @@ static int tray_close(const char *device)
> return 0;
> }
>
> +static int media_insert(const char *device, const char *mediafile,
> +const char *format)
> +{
> +BlockDriver *drv = NULL;
> +BlockDriverState *bs;
> +int bdrv_flags;
> +
> +bs = bdrv_removable_find(device);
> +if (!bs) {
> +return -1;
> +}
> +
> +if (bdrv_is_locked(bs)) {
> +qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
> +return -1;
> +}
Why fail if locked?
If you managed to pry open the tray, I guess nothing should stop you
from changing the media in it.
> +
> +if (bdrv_is_inserted(bs)) {
> +/* FIXME: will report undefined error in QMP */
> +return -1;
> +}
> +
> +if (!bs->tray_open) {
> +/* FIXME: will report undefined error in QMP */
> +return 1;
> +}
> +
> +if (format) {
> +drv = bdrv_find_whitelisted_format(format);
> +if (!drv) {
> +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
> +return -1;
> +}
> +}
> +
> +bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> +bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> +if (bdrv_open(bs, mediafile, bdrv_flags, drv) < 0) {
> +qerror_report(QERR_OPEN_FILE_FAILED, mediafile);
> +return -1;
> +}
> +
> +return 0;
> +}
[...]
[Qemu-devel] [PATCH v2 2/3] softfloat: change default nan definitions to variables
Most definitions in softfloat.h are really target-independent, but the file is not because it includes definitions of the default NaN values. Change those to variables to allow including softfloat.h from files that are not compiled per-target. By making them const, the compiler is allowed to optimize them into softfloat functions that use them. Signed-off-by: Paolo Bonzini --- fpu/softfloat-specialize.h | 72 fpu/softfloat.h| 60 +--- 2 files changed, 81 insertions(+), 51 deletions(-) diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h index c7d35a1..c165205 100644 --- a/fpu/softfloat-specialize.h +++ b/fpu/softfloat-specialize.h @@ -35,6 +35,78 @@ these four paragraphs for those parts of this code that are retained. =*/ +#if defined(TARGET_MIPS) || defined(TARGET_SH4) || defined(TARGET_UNICORE32) +#define SNAN_BIT_IS_ONE1 +#else +#define SNAN_BIT_IS_ONE0 +#endif + +/* +| The pattern for a default generated half-precision NaN. +**/ +#if defined(TARGET_ARM) +const float16 float16_default_nan = const_float16(0x7E00); +#elif SNAN_BIT_IS_ONE +const float16 float16_default_nan = const_float16(0x7DFF); +#else +const float16 float16_default_nan = const_float16(0xFE00); +#endif + +/* +| The pattern for a default generated single-precision NaN. +**/ +#if defined(TARGET_SPARC) +const float32 float32_default_nan = const_float32(0x7FFF); +#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) +const float32 float32_default_nan = const_float32(0x7FC0); +#elif SNAN_BIT_IS_ONE +const float32 float32_default_nan = const_float32(0x7FBF); +#else +const float32 float32_default_nan = const_float32(0xFFC0); +#endif + +/* +| The pattern for a default generated double-precision NaN. +**/ +#if defined(TARGET_SPARC) +const float64 float64_default_nan = const_float64(LIT64( 0x7FFF )); +#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) +const float64 float64_default_nan = const_float64(LIT64( 0x7FF8 )); +#elif SNAN_BIT_IS_ONE +const float64 float64_default_nan = const_float64(LIT64( 0x7FF7 )); +#else +const float64 float64_default_nan = const_float64(LIT64( 0xFFF8 )); +#endif + +/* +| The pattern for a default generated extended double-precision NaN. +**/ +#if SNAN_BIT_IS_ONE +#define floatx80_default_nan_high 0x7FFF +#define floatx80_default_nan_low LIT64( 0xBFFF ) +#else +#define floatx80_default_nan_high 0x +#define floatx80_default_nan_low LIT64( 0xC000 ) +#endif + +const floatx80 floatx80_default_nan = make_floatx80(floatx80_default_nan_high, +floatx80_default_nan_low); + +/* +| The pattern for a default generated quadruple-precision NaN. The `high' and +| `low' values hold the most- and least-significant bits, respectively. +**/ +#if SNAN_BIT_IS_ONE +#define float128_default_nan_high LIT64( 0x7FFF7FFF ) +#define float128_default_nan_low LIT64( 0x ) +#else +#define float128_default_nan_high LIT64( 0x8000 ) +#define float128_default_nan_low LIT64( 0x ) +#endif + +const float128 float128_default_nan = make_float128(float128_default_nan_high, +float128_default_nan_low); + /* | Raises the exceptions specified by `flags'. Floating-point traps can be | defined here if desired. It is currently not possible for such a trap diff --git a/fpu/softfloat.h b/fpu/softfloat.h index bde2500..3bb7d8f 100644 --- a/fpu/softfloat.h +++ b/fpu/softfloat.h @@ -43,7 +43,7 @@ these four paragraphs for those parts of this code that are retained. #endif #include -#include "config.h" +#include "config-host.h" /* | Each of the following `typedef's defines the most convenient type that holds @@ -68,12 +68,6 @@ typedef int64_t int64;
Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions
Kevin Wolf writes: > Am 06.06.2011 13:57, schrieb Markus Armbruster: > Not sure what's the best way of fixing this. Maybe just ignoring > -snapshot for read-only block devices? Why not, the combination is pointless. >>> >>> It could start making a difference in some obscure combinations. Imagine >>> a read-only image with a backing file, -snapshot and the 'commit' >>> monitor command. >>> >>> Sounds pretty insane, but I wouldn't bet that people aren't using it... >> >> People try all kinds of insane things. The question is whether we can >> change it anyway. > > We have a backing file chain like base <- cow [<- tmp], and the drive is > read-only. > > Currently, 'commit' means that tmp is committed to cow (i.e. nothing > happens because it's read-only). After changing it, we would commit the > content of cow to base and possibly corrupt other images that are based > on base. > > We can hope that nobody would be hit by it in practice, but it's not a > change I'd feel very comfortable about. So the one effect -snapshot has on a read-only drive is to neuter the commit command. Hmm. Naive question: shouldn't commit require the drive to be read/write? It writes both the backing image and the COW... [...]
[Qemu-devel] [PATCH v2 3/3] move unaligned memory access functions to bswap.h
This is just code movement, and moving the fpu/ include path from
target-dependent to target-independent Make variables.
Signed-off-by: Paolo Bonzini
---
Makefile.hw |2 +-
bswap.h | 474 +++
configure |3 +-
cpu-all.h | 446 +---
4 files changed, 477 insertions(+), 448 deletions(-)
diff --git a/Makefile.hw b/Makefile.hw
index b9181ab..659e441 100644
--- a/Makefile.hw
+++ b/Makefile.hw
@@ -9,7 +9,7 @@ include $(SRC_PATH)/rules.mak
$(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
-QEMU_CFLAGS+=-I.. -I$(SRC_PATH)/fpu
+QEMU_CFLAGS+=-I..
include $(SRC_PATH)/Makefile.objs
diff --git a/bswap.h b/bswap.h
index 82a7951..f41bebe 100644
--- a/bswap.h
+++ b/bswap.h
@@ -11,6 +11,8 @@
#include
#else
+#include "softfloat.h"
+
#ifdef CONFIG_BYTESWAP_H
#include
#else
@@ -237,4 +239,476 @@ static inline uint32_t qemu_bswap_len(uint32_t value, int
len)
return bswap32(value) >> (32 - 8 * len);
}
+typedef union {
+float32 f;
+uint32_t l;
+} CPU_FloatU;
+
+typedef union {
+float64 d;
+#if defined(HOST_WORDS_BIGENDIAN)
+struct {
+uint32_t upper;
+uint32_t lower;
+} l;
+#else
+struct {
+uint32_t lower;
+uint32_t upper;
+} l;
+#endif
+uint64_t ll;
+} CPU_DoubleU;
+
+typedef union {
+ floatx80 d;
+ struct {
+ uint64_t lower;
+ uint16_t upper;
+ } l;
+} CPU_LDoubleU;
+
+typedef union {
+float128 q;
+#if defined(HOST_WORDS_BIGENDIAN)
+struct {
+uint32_t upmost;
+uint32_t upper;
+uint32_t lower;
+uint32_t lowest;
+} l;
+struct {
+uint64_t upper;
+uint64_t lower;
+} ll;
+#else
+struct {
+uint32_t lowest;
+uint32_t lower;
+uint32_t upper;
+uint32_t upmost;
+} l;
+struct {
+uint64_t lower;
+uint64_t upper;
+} ll;
+#endif
+} CPU_QuadU;
+
+/* unaligned/endian-independent pointer access */
+
+/*
+ * the generic syntax is:
+ *
+ * load: ld{type}{sign}{size}{endian}_p(ptr)
+ *
+ * store: st{type}{size}{endian}_p(ptr, val)
+ *
+ * Note there are small differences with the softmmu access API!
+ *
+ * type is:
+ * (empty): integer access
+ * f: float access
+ *
+ * sign is:
+ * (empty): for floats or 32 bit size
+ * u: unsigned
+ * s: signed
+ *
+ * size is:
+ * b: 8 bits
+ * w: 16 bits
+ * l: 32 bits
+ * q: 64 bits
+ *
+ * endian is:
+ * (empty): 8 bit access
+ * be : big endian
+ * le : little endian
+ */
+static inline int ldub_p(const void *ptr)
+{
+return *(uint8_t *)ptr;
+}
+
+static inline int ldsb_p(const void *ptr)
+{
+return *(int8_t *)ptr;
+}
+
+static inline void stb_p(void *ptr, int v)
+{
+*(uint8_t *)ptr = v;
+}
+
+/* NOTE: on arm, putting 2 in /proc/sys/debug/alignment so that the
+ kernel handles unaligned load/stores may give better results, but
+ it is a system wide setting : bad */
+#if defined(HOST_WORDS_BIGENDIAN) || defined(WORDS_ALIGNED)
+
+/* conservative code for little endian unaligned accesses */
+static inline int lduw_le_p(const void *ptr)
+{
+#ifdef _ARCH_PPC
+int val;
+__asm__ __volatile__ ("lhbrx %0,0,%1" : "=r" (val) : "r" (ptr));
+return val;
+#else
+const uint8_t *p = ptr;
+return p[0] | (p[1] << 8);
+#endif
+}
+
+static inline int ldsw_le_p(const void *ptr)
+{
+#ifdef _ARCH_PPC
+int val;
+__asm__ __volatile__ ("lhbrx %0,0,%1" : "=r" (val) : "r" (ptr));
+return (int16_t)val;
+#else
+const uint8_t *p = ptr;
+return (int16_t)(p[0] | (p[1] << 8));
+#endif
+}
+
+static inline int ldl_le_p(const void *ptr)
+{
+#ifdef _ARCH_PPC
+int val;
+__asm__ __volatile__ ("lwbrx %0,0,%1" : "=r" (val) : "r" (ptr));
+return val;
+#else
+const uint8_t *p = ptr;
+return p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24);
+#endif
+}
+
+static inline uint64_t ldq_le_p(const void *ptr)
+{
+const uint8_t *p = ptr;
+uint32_t v1, v2;
+v1 = ldl_le_p(p);
+v2 = ldl_le_p(p + 4);
+return v1 | ((uint64_t)v2 << 32);
+}
+
+static inline void stw_le_p(void *ptr, int v)
+{
+#ifdef _ARCH_PPC
+__asm__ __volatile__ ("sthbrx %1,0,%2" : "=m" (*(uint16_t *)ptr) : "r"
(v), "r" (ptr));
+#else
+uint8_t *p = ptr;
+p[0] = v;
+p[1] = v >> 8;
+#endif
+}
+
+static inline void stl_le_p(void *ptr, int v)
+{
+#ifdef _ARCH_PPC
+__asm__ __volatile__ ("stwbrx %1,0,%2" : "=m" (*(uint32_t *)ptr) : "r"
(v), "r" (ptr));
+#else
+uint8_t *p = ptr;
+p[0] = v;
+p[1] = v >> 8;
+p[2] = v >> 16;
+p[3] = v >> 24;
+#endif
+}
+
+static inline void stq_le_p(void *ptr, uint64_t v)
+{
+uint8_t *p = ptr;
+stl_le_p(p, (uint32_t)v);
+stl_le_p(p + 4, v >> 32);
+}
+
+/* float access */
+
+static inline float32 ldfl_le_p(const void *ptr)
+{
+union {
+float32 f;
+uint32_t i;
+} u;
+u.i = ldl_le_p(ptr);
+retur
Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
On 06/06/2011 06:39 AM, Gerd Hoffmann wrote:
> The state machine doesn't stop in EXECUTING state any more when async
> packets are in flight, so the checks are not needed any more and can
> be dropped.
>
> Also kick out the check for the frame timer. As we don't stop & sleep
> any more on async packets this is obsolete.
>
> Signed-off-by: Gerd Hoffmann
> ---
> hw/usb-ehci.c | 32 ++--
> 1 files changed, 2 insertions(+), 30 deletions(-)
>
> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
> index e345a1c..8b1ae3a 100644
> --- a/hw/usb-ehci.c
> +++ b/hw/usb-ehci.c
> @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int
> async)
> int again = 0;
> uint32_t entry = ehci_get_fetch_addr(ehci, async);
>
> -#if EHCI_DEBUG == 0
> -if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) {
> -if (async) {
> -DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n");
> -goto out;
> -} else {
> -DPRINTF("FETCHENTRY: WARNING "
> -"- frame timer elapsed during periodic\n");
> -}
> -}
> -#endif
> if (entry < 0x1000) {
> DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
> ehci_set_state(ehci, async, EST_ACTIVE);
> @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
> }
>
> ehci_set_state(ehci, async, EST_WAITLISTHEAD);
> -/* fall through */
> -
> -case EST_FETCHENTRY:
> -/* fall through */
Why drop this case too? As I recall that is needed for proper execution.
David
> -
> -case EST_EXECUTING:
> ehci_advance_state(ehci, async);
> break;
>
> @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState
> *ehci)
> ehci_advance_state(ehci, async);
> break;
>
> -case EST_EXECUTING:
> -DPRINTF("PERIODIC state adv for executing\n");
> -ehci_advance_state(ehci, async);
> -break;
> -
> default:
> /* this should only be due to a developer mistake */
> fprintf(stderr, "ehci: Bad periodic state %d. "
> @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque)
> if (frames - i > 10) {
> skipped_frames++;
> } else {
> -// TODO could this cause periodic frames to get skipped if async
> -// active?
> -if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
> -ehci_advance_periodic_state(ehci);
> -}
> +ehci_advance_periodic_state(ehci);
> }
>
> ehci->last_run_usec += FRAME_TIMER_USEC;
> @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque)
> /* Async is not inside loop since it executes everything it can once
> * called
> */
> -if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
> -ehci_advance_async_state(ehci);
> -}
> +ehci_advance_async_state(ehci);
>
> qemu_mod_timer(ehci->frame_timer, expire_time);
> }
Re: [Qemu-devel] [PATCH 0/4] introduce cpu_physical_memory_map_fast
On 06/06/2011 08:09 AM, Paolo Bonzini wrote: On 06/06/2011 02:56 PM, Anthony Liguori wrote: Oh, the patch series basically died for me when I saw: Avi> What performance benefit does this bring? Paolo> Zero :) Especially given Avi's efforts to introduce a new RAM API, I don't want yet another special case to handle. This is not a special case, the existing functions are all mapped onto the new cpu_physical_memory_map_internal. I don't think this is in any way related to Avi's RAM API which is (mostly) for MMIO. You're just trying to avoid having to handle map failures, right? Not just that. If you had a memory block at say 1 GB - 2 GB, and another at 2 GB - 3 GB, a DMA operation that crosses the two could be implemented with cpu_physical_memory_map_fast; you would simply build a two-element iovec in two steps, something the current API does not allow. You cannot assume RAM blocks are contiguous. This has nothing to do with PV or not PV but how the RAM API works today. The patch does not change virtio to do the split, but it is possible to do that. The reason I'm not doing the virtio change, is that I know mst has pending changes to virtio and I'd rather avoid the conflicts for now. However, for vmw_pvscsi I'm going to handle it using the new functions. Virtio can handle all of this today because it uses cpu_physical_memory_rw for ring access and then calls map for SG elements. SG elements are usually 4k so it's never really an issue to get a partial mapping. We could be more robust about it but in practice, it's not a problem. Regards, Anthony Liguori Paolo
[Qemu-devel] [PATCH v3 2/3] spapr: prepare for qdevification of irq
Restructure common properties for sPAPR devices so that IRQ definitions
can be added in one place.
Signed-off-by: Paolo Bonzini
---
hw/spapr_llan.c |4 +---
hw/spapr_vio.h |5 +
hw/spapr_vscsi.c |4 +---
hw/spapr_vty.c |2 +-
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index 2597748..abe1297 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -495,9 +495,7 @@ static VIOsPAPRDeviceInfo spapr_vlan = {
.qdev.name = "spapr-vlan",
.qdev.size = sizeof(VIOsPAPRVLANDevice),
.qdev.props = (Property[]) {
-DEFINE_PROP_UINT32("reg", VIOsPAPRDevice, reg, 0x1000),
-DEFINE_PROP_UINT32("dma-window", VIOsPAPRDevice, rtce_window_size,
- 0x1000),
+DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x1000),
DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
DEFINE_PROP_END_OF_LIST(),
},
diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
index faa5d94..7eb5367 100644
--- a/hw/spapr_vio.h
+++ b/hw/spapr_vio.h
@@ -60,6 +60,11 @@ typedef struct VIOsPAPRDevice {
VIOsPAPR_CRQ crq;
} VIOsPAPRDevice;
+#define DEFINE_SPAPR_PROPERTIES(type, field, default_reg, default_dma_window) \
+DEFINE_PROP_UINT32("reg", type, field.reg, default_reg), \
+DEFINE_PROP_UINT32("dma-window", type, field.rtce_window_size, \
+ default_dma_window)
+
typedef struct VIOsPAPRBus {
BusState bus;
int irq;
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 99a4b06..194db23 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -989,9 +989,7 @@ static VIOsPAPRDeviceInfo spapr_vscsi = {
.qdev.name = "spapr-vscsi",
.qdev.size = sizeof(VSCSIState),
.qdev.props = (Property[]) {
-DEFINE_PROP_UINT32("reg", VIOsPAPRDevice, reg, 0x2000),
-DEFINE_PROP_UINT32("dma-window", VIOsPAPRDevice,
- rtce_window_size, 0x1000),
+DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x2000, 0x1000),
DEFINE_PROP_END_OF_LIST(),
},
};
diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
index fa97cf7..abc41f8 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -140,7 +140,7 @@ static VIOsPAPRDeviceInfo spapr_vty = {
.qdev.name = "spapr-vty",
.qdev.size = sizeof(VIOsPAPRVTYDevice),
.qdev.props = (Property[]) {
-DEFINE_PROP_UINT32("reg", VIOsPAPRDevice, reg, 0),
+DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, 0, 0),
DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
DEFINE_PROP_END_OF_LIST(),
},
--
1.7.4.4
Re: [Qemu-devel] [RFC 05/10] QMP: Introduce the blockdev-tray-open command
On Mon, 6 Jun 2011 17:10:32 +0530
Amit Shah wrote:
> On (Fri) 03 Jun 2011 [16:03:57], Luiz Capitulino wrote:
>
> > +static int tray_open(const char *device, int remove, int force)
> > +{
> > +BlockDriverState *bs;
> > +
> > +bs = bdrv_removable_find(device);
> > +if (!bs) {
> > +return -1;
> > +}
> > +
> > +if (bdrv_eject(bs, 1, force) < 0) {
> > +/* FIXME: will report undefined error in QMP */
> > +return -1;
> > +}
> > +
> > +if (remove) {
> > +bdrv_close(bs);
> > +}
> > +
> > +return 0;
> > +}
>
> What's the reason to tie the 'remove' with tray open?
In my first try I had a command called 'blockdev-media-remove', but then
I had the impression that I was going too far as the only reason a client
would ever want to open the tray is to remove the media.
> Won't it be
> simpler to have it separated out, perhaps a 'change' event instead of
> 'insert' that can accept NULL which means just remove medium?
You meant 'command' instead of 'event', right?
I don't think a change command makes sense, because it's just a shortcut
to open/remove/insert/close.
Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
On 06/06/2011 07:34 AM, David Ahern wrote:
> On 06/06/2011 06:39 AM, Gerd Hoffmann wrote:
>> The state machine doesn't stop in EXECUTING state any more when async
>> packets are in flight, so the checks are not needed any more and can
>> be dropped.
>>
>> Also kick out the check for the frame timer. As we don't stop & sleep
>> any more on async packets this is obsolete.
>>
>> Signed-off-by: Gerd Hoffmann
>> ---
>> hw/usb-ehci.c | 32 ++--
>> 1 files changed, 2 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
>> index e345a1c..8b1ae3a 100644
>> --- a/hw/usb-ehci.c
>> +++ b/hw/usb-ehci.c
>> @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int
>> async)
>> int again = 0;
>> uint32_t entry = ehci_get_fetch_addr(ehci, async);
>>
>> -#if EHCI_DEBUG == 0
>> -if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) {
>> -if (async) {
>> -DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state
>> machine\n");
>> -goto out;
>> -} else {
>> -DPRINTF("FETCHENTRY: WARNING "
>> -"- frame timer elapsed during periodic\n");
>> -}
>> -}
>> -#endif
This check was added to per Section 4 of the EHCI spec -- the HC should
not start transactions that will not be completed before the end of the
micro-frame. If you remove this what causes the EHCI model to take a
breather?
David
>> if (entry < 0x1000) {
>> DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry);
>> ehci_set_state(ehci, async, EST_ACTIVE);
>> @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci)
>> }
>>
>> ehci_set_state(ehci, async, EST_WAITLISTHEAD);
>> -/* fall through */
>> -
>> -case EST_FETCHENTRY:
>> -/* fall through */
>
> Why drop this case too? As I recall that is needed for proper execution.
>
> David
>
>> -
>> -case EST_EXECUTING:
>> ehci_advance_state(ehci, async);
>> break;
>>
>> @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState
>> *ehci)
>> ehci_advance_state(ehci, async);
>> break;
>>
>> -case EST_EXECUTING:
>> -DPRINTF("PERIODIC state adv for executing\n");
>> -ehci_advance_state(ehci, async);
>> -break;
>> -
>> default:
>> /* this should only be due to a developer mistake */
>> fprintf(stderr, "ehci: Bad periodic state %d. "
>> @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque)
>> if (frames - i > 10) {
>> skipped_frames++;
>> } else {
>> -// TODO could this cause periodic frames to get skipped if async
>> -// active?
>> -if (ehci_get_state(ehci, 1) != EST_EXECUTING) {
>> -ehci_advance_periodic_state(ehci);
>> -}
>> +ehci_advance_periodic_state(ehci);
>> }
>>
>> ehci->last_run_usec += FRAME_TIMER_USEC;
>> @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque)
>> /* Async is not inside loop since it executes everything it can once
>> * called
>> */
>> -if (ehci_get_state(ehci, 0) != EST_EXECUTING) {
>> -ehci_advance_async_state(ehci);
>> -}
>> +ehci_advance_async_state(ehci);
>>
>> qemu_mod_timer(ehci->frame_timer, expire_time);
>> }
[Qemu-devel] [PATCH v3 3/3] spapr: make irq customizable via qdev
Signed-off-by: Paolo Bonzini
---
hw/spapr_vio.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 6f34159..a193caa 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -52,6 +52,10 @@
static struct BusInfo spapr_vio_bus_info = {
.name = "spapr-vio",
.size = sizeof(VIOsPAPRBus),
+.props = (Property[]) {
+DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
+DEFINE_PROP_END_OF_LIST(),
+},
};
VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
@@ -603,7 +607,9 @@ static int spapr_vio_busdev_init(DeviceState *qdev,
DeviceInfo *qinfo)
}
dev->qdev.id = id;
-dev->vio_irq_num = spapr_allocate_irq (spapr);
+if (!dev->vio_irq_num) {
+dev->vio_irq_num = spapr_allocate_irq (spapr);
+}
dev->qirq = xics_find_qirq(spapr->icp, dev->vio_irq_num);
rtce_init(dev);
--
1.7.4.4
Re: [Qemu-devel] [RFC 04/10] HMP: info block: Print the 'tray-open' key
On Mon, 06 Jun 2011 15:19:34 +0200 Markus Armbruster wrote: > Luiz Capitulino writes: > > > It's printed before the "[not inserted]" or the "file" fields, like this: > > > > (qemu) info block > > ide0-hd0: removable=0 file=disks/test.img ro=0 drv=qcow2 encrypted=0 > > ide1-cd0: removable=1 locked=0 tray-open=0 file=/Fedora-14-x86_64-DVD.iso > > ro=1 drv=raw encrypted=0 > > floppy0: removable=1 locked=0 tray-open=0 [not inserted] > > sd0: removable=1 locked=0 tray-open=0 [not inserted] > > (qemu) > > > > TODO: Confirm this won't break libvirt. > > As far as I can see, libvirt uses neither "info block" nor > "query-block". Thanks for checking it. > I'd squash this into the previous commit. >
[Qemu-devel] [PATCH] Make x86 mfence and lfence illegal without SSE2
Hi, While trying to use qemu -cpu pentium3 to test for incorrect uses of certain SSE2 instructions, I found that QEMU allowed the mfence and lfence instructions to be executed even though Pentium 3 doesn't support them. According to the processor specs (and experience on a real Pentium 3), these instructions are only available with SSE2, but QEMU is checking for SSE. The check for the related sfence instruction is correct (it works with SSE). This trival patch fixes the test: Signed-off-by: Martin Simmons --- a/target-i386/translate.c 2011-06-03 16:17:18.270208646 +0100 +++ b/target-i386/translate.c 2011-06-03 16:02:41.381556714 +0100 @@ -7560,7 +7560,7 @@ break; case 5: /* lfence */ case 6: /* mfence */ -if ((modrm & 0xc7) != 0xc0 || !(s->cpuid_features & CPUID_SSE)) +if ((modrm & 0xc7) != 0xc0 || !(s->cpuid_features & CPUID_SSE2)) goto illegal_op; break; case 7: /* sfence / clflush */ __Martin
Re: [Qemu-devel] [PATCH 2/5] QMP: add inject-nmi qmp command
On Sat, 4 Jun 2011 11:34:17 +0300
Blue Swirl wrote:
> On Wed, Jun 1, 2011 at 6:54 PM, Luiz Capitulino
> wrote:
> > From: Lai Jiangshan
> >
> > inject-nmi command injects an NMI on all CPUs of guest.
> > It is only supported for x86 guest currently, it will
> > returns "Unsupported" error for non-x86 guest.
>
> Please rename the command to 'x-inject-nmi' to point out that it will
> be replaced by a generic method later.
Won't the generic interface be called 'inject'? In that case calling this
command 'inject-nmi' makes sense as it's exactly what the command does and
in the future it can be written in terms of the new 'inject' command.
>
> > Signed-off-by: Luiz Capitulino
> > ---
> > monitor.c | 17 +
> > qmp-commands.hx | 27 +++
> > 2 files changed, 44 insertions(+), 0 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index f63cce0..81d3c9b 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2555,6 +2555,23 @@ static void do_inject_nmi(Monitor *mon, const QDict
> > *qdict)
> > break;
> > }
> > }
> > +
> > +static int do_inject_nmi_all(Monitor *mon, const QDict *qdict, QObject
> > **ret_data)
> > +{
> > + CPUState *env;
> > +
> > + for (env = first_cpu; env != NULL; env = env->next_cpu) {
> > + cpu_interrupt(env, CPU_INTERRUPT_NMI);
> > + }
> > +
> > + return 0;
> > +}
> > +#else
> > +static int do_inject_nmi_all(Monitor *mon, const QDict *qdict, QObject
> > **ret_data)
> > +{
> > + qerror_report(QERR_UNSUPPORTED);
> > + return -1;
> > +}
> > #endif
> >
> > static void do_info_status_print(Monitor *mon, const QObject *data)
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index a9f109a..ae08b7a 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -430,6 +430,33 @@ Example:
> > EQMP
> >
> > {
> > + .name = "inject-nmi",
> > + .args_type = "",
> > + .params = "",
> > + .help = "",
> > + .user_print = monitor_user_noop,
> > + .mhandler.cmd_new = do_inject_nmi_all,
> > + },
> > +
> > +SQMP
> > +inject-nmi
> > +--
> > +
> > +Inject an NMI on guest's CPUs.
> > +
> > +Arguments: None.
> > +
> > +Example:
> > +
> > +-> { "execute": "inject-nmi" }
> > +<- { "return": {} }
> > +
> > +Note: inject-nmi is only supported for x86 guest currently, it will
> > + returns "Unsupported" error for non-x86 guest.
> > +
> > +EQMP
> > +
> > + {
> > .name = "migrate",
> > .args_type = "detach:-d,blk:-b,inc:-i,uri:s",
> > .params = "[-d] [-b] [-i] uri",
> > --
> > 1.7.4.4
> >
> >
> >
>
Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks.
On 06/06/2011 08:25 AM, Gerd Hoffmann wrote:
>
> Hi,
>
-#if EHCI_DEBUG == 0
-if (qemu_get_clock_ns(vm_clock) / 1000>= ehci->frame_end_usec) {
-if (async) {
-DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state
machine\n");
-goto out;
-} else {
-DPRINTF("FETCHENTRY: WARNING "
-"- frame timer elapsed during periodic\n");
-}
-}
-#endif
>>
>> This check was added to per Section 4 of the EHCI spec -- the HC should
>> not start transactions that will not be completed before the end of the
>> micro-frame. If you remove this what causes the EHCI model to take a
>> breather?
>
> Look at patch #8. That brings a number of state machine changes. One
> of them is that the async schedule stops as soon as it notices it walks
> in circles (i.e. sees a QH the second time).
>
-case EST_FETCHENTRY:
-/* fall through */
>>>
>>> Why drop this case too? As I recall that is needed for proper execution.
>
> The async schedule doesn't pause in fetchentry state any more (also done
> by patch #8).
Ok. see that now.
David
>
> cheers,
> Gerd
>
[Qemu-devel] [PATCH v2 0/3] make endian-independent unaligned memory access functions available to libhw
Functions like ldl_be_p and ldl_le_p are currently used only as building
blocks for {ld,st}XX_p. As such, they are in cpu-all.h even though they
have absolutely no dependency on the target.
In order to make them globally available, this series moves them to
bswap.h instead.
An interesting part of this is that there are functions also for floating
point values. Leaving them in cpu-all.h would be possible but untidy.
In fact handling these is easy, but it requires to make softfloat.h
target-dependent as well. This is what patch 2 does.
v1->v2:
rebase, use softfloat-specialize.h instead of introducing
softfloat-target.h
Paolo Bonzini (3):
move WORDS_ALIGNED to qemu-common.h
softfloat: change default nan definitions to variables
move unaligned memory access functions to bswap.h
Makefile.hw|2 +-
bswap.h| 474
configure |3 +-
cpu-all.h | 446 +-
cpu-common.h |4 -
fpu/softfloat-specialize.h | 72 +++
fpu/softfloat.h| 60 +-
qemu-common.h |4 +
8 files changed, 562 insertions(+), 503 deletions(-)
--
1.7.4.4
[Qemu-devel] [RFC PATCH v2 02/13] scsi: support parsing of SAM logical unit numbers
SAM logical unit numbers are complicated beasts that can address
multiple levels of buses and targets before finally reaching
logical units. Begin supporting them by correctly parsing vSCSI
LUNs.
Note that the current (admittedly incorrect) code switched from
a "bus X, target 0, lun 0" representation while OpenFirmware is
running (because OF prefers access mode 0, which places bus numbers
in the top byte) to "bus 0, target Y, lun 0" while Linux is running
(because Linux uses access mode 2, which places target numbers in
the top byte). With this patch, everything consistently uses the
former notation.
Signed-off-by: Paolo Bonzini
---
hw/scsi-bus.c| 109 ++
hw/scsi-defs.h | 22 +++
hw/scsi.h|7 +++
hw/spapr_vscsi.c | 18 ++---
4 files changed, 142 insertions(+), 14 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 3918662..3b80541 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -741,3 +741,112 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev)
return strdup(path);
}
+
+/* Decode the bus and level parts of a LUN, as defined in the SCSI architecture
+ model. If false is returned, the LUN could not be parsed. If true
+ is return, "*bus" and "*target" identify the next two steps in the
+ hierarchical LUN.
+
+ "*lun" can be used with scsi_get_lun to continue the parsing. */
+static bool scsi_decode_level(uint64_t sam_lun, int *bus, int *target,
+ uint64_t *lun)
+{
+switch (sam_lun >> 62) {
+case ADDR_PERIPHERAL_DEVICE:
+*bus = (sam_lun >> 56) & 0x3f;
+if (*bus) {
+/* The TARGET OR LUN field selects a target; walk the next
+ 16-bits to find the LUN. */
+*target = (sam_lun >> 48) & 0xff;
+*lun = sam_lun << 16;
+} else {
+/* The TARGET OR LUN field selects a LUN on the current
+ node, identified by bus 0. */
+*target = 0;
+*lun = (sam_lun & 0xffLL) | (1LL << 62);
+}
+return true;
+case ADDR_LOGICAL_UNIT:
+*bus = (sam_lun >> 53) & 7;
+*target = (sam_lun >> 56) & 0x3f;
+*lun = (sam_lun & 0x1fLL) | (1LL << 62);
+return true;
+case ADDR_FLAT_SPACE:
+*bus = 0;
+*target = 0;
+*lun = sam_lun;
+return true;
+case ADDR_LOGICAL_UNIT_EXT:
+if ((sam_lun >> 56) == ADDR_WELL_KNOWN_LUN ||
+(sam_lun >> 56) == ADDR_FLAT_SPACE_EXT) {
+*bus = 0;
+*target = 0;
+*lun = sam_lun;
+return true;
+}
+return false;
+}
+abort();
+}
+
+/* Extract a single-level LUN number from a LUN, as specified in the
+ SCSI architecture model. Return -1 if this is not possible because
+ the LUN includes a bus or target component. */
+static int scsi_get_lun(uint64_t sam_lun)
+{
+int bus, target;
+
+retry:
+switch (sam_lun >> 62) {
+case ADDR_PERIPHERAL_DEVICE:
+case ADDR_LOGICAL_UNIT:
+scsi_decode_level(sam_lun, &bus, &target, &sam_lun);
+if (bus || target) {
+return LUN_INVALID;
+}
+goto retry;
+
+case ADDR_FLAT_SPACE:
+return (sam_lun >> 48) & 0x3fff;
+case ADDR_LOGICAL_UNIT_EXT:
+if ((sam_lun >> 56) == ADDR_WELL_KNOWN_LUN) {
+return LUN_WLUN_BASE | ((sam_lun >> 48) & 0xff);
+}
+if ((sam_lun >> 56) == ADDR_FLAT_SPACE_EXT) {
+return (sam_lun >> 32) & 0xff;
+}
+return LUN_INVALID;
+}
+abort();
+}
+
+/* Extract bus and target from the given LUN and use it to identify a
+ SCSIDevice from a SCSIBus. Right now, only 1 target per bus is
+ supported. In the future a SCSIDevice could host its own SCSIBus,
+ in an alternation of devices that select a bus (target ports) and
+ devices that select a target (initiator ports). */
+SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, int *lun)
+{
+int bus, target, decoded_lun;
+uint64_t next_lun;
+
+if (!scsi_decode_level(sam_lun, &bus, &target, &next_lun)) {
+ /* Unsupported LUN format. */
+ return NULL;
+}
+if (bus >= sbus->ndev || (bus == 0 && target > 0)) {
+/* Out of range. */
+return NULL;
+}
+if (target != 0) {
+/* Only one target for now. */
+return NULL;
+}
+
+decoded_lun = scsi_get_lun(next_lun);
+if (decoded_lun != LUN_INVALID) {
+*lun = decoded_lun;
+return sbus->devs[bus];
+}
+return NULL;
+}
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 413cce0..66dfd4a 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -164,3 +164,25 @@
#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
#define TYPE_NO_LUN 0x7f
+/*
+ * SCSI addressing methods (bits 62-63 of the LUN).
+ */
+#define ADDR_PERIPHERAL_DEVICE
Re: [Qemu-devel] [RFC 07/10] QMP: Introduce the blockdev-media-insert command
On Mon, 06 Jun 2011 15:40:28 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > This command inserts a new media in an already opened tray. It's only
> > available in QMP.
> >
> > Please, check the command's documentation (being introduced in this
> > commit) for a detailed description.
> >
> > Signed-off-by: Luiz Capitulino
> > ---
> > blockdev.c | 52
> > blockdev.h |1 +
> > qmp-commands.hx | 32
> > 3 files changed, 85 insertions(+), 0 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 943905d..14c8312 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -721,6 +721,58 @@ static int tray_close(const char *device)
> > return 0;
> > }
> >
> > +static int media_insert(const char *device, const char *mediafile,
> > +const char *format)
> > +{
> > +BlockDriver *drv = NULL;
> > +BlockDriverState *bs;
> > +int bdrv_flags;
> > +
> > +bs = bdrv_removable_find(device);
> > +if (!bs) {
> > +return -1;
> > +}
> > +
> > +if (bdrv_is_locked(bs)) {
> > +qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
> > +return -1;
> > +}
>
> Why fail if locked?
>
> If you managed to pry open the tray, I guess nothing should stop you
> from changing the media in it.
Makes sense.
>
> > +
> > +if (bdrv_is_inserted(bs)) {
> > +/* FIXME: will report undefined error in QMP */
> > +return -1;
> > +}
> > +
> > +if (!bs->tray_open) {
> > +/* FIXME: will report undefined error in QMP */
> > +return 1;
> > +}
> > +
> > +if (format) {
> > +drv = bdrv_find_whitelisted_format(format);
> > +if (!drv) {
> > +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
> > +return -1;
> > +}
> > +}
> > +
> > +bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
> > +bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
> > +if (bdrv_open(bs, mediafile, bdrv_flags, drv) < 0) {
> > +qerror_report(QERR_OPEN_FILE_FAILED, mediafile);
> > +return -1;
> > +}
> > +
> > +return 0;
> > +}
> [...]
>
