From mboxrd@z Thu Jan  1 00:00:00 1970
From: Max Krasnyansky <maxk@kernel.org>
Subject: Re: [PATCH 4/5] uhci: rewrite UHCI emulator, fully async operation
 with multiple outstanding transactions
Date: Thu, 14 Aug 2008 12:49:56 -0700
Message-ID: <48A48C64.8050401@kernel.org>
References: <cover.1218685607.git.maxk@kernel.org> <1a8fc205ecb1a6dea2d6a2cc4019f359c90f39fb.1218685608.git.maxk@kernel.org> <48A47085.3060101@codemonkey.ws>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
To: Anthony Liguori <anthony@codemonkey.ws>
Return-path: <kvm-owner@vger.kernel.org>
Received: from hera.kernel.org ([140.211.167.34]:44761 "EHLO hera.kernel.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1760674AbYHNTuP (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 14 Aug 2008 15:50:15 -0400
In-Reply-To: <48A47085.3060101@codemonkey.ws>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Anthony Liguori wrote:
> Max Krasnyansky wrote:
>>  
>> +static UHCIAsync *uhci_async_alloc(UHCIState *s)
>> +{
>> +    UHCIAsync *async = qemu_malloc(sizeof(UHCIAsync));
>> +    if (async) {
>> +        memset(&async->packet, 0, sizeof(async->packet));
>>   
> 
> You can use qemu_mallocz() here.
The reasons I decided not too is because there is 2K buffer in each UHCIAsync
struct. memset()ing 2K buffer for each transaction did not seem like a good
idea. ie qemu_mallocz() simply does memset(0)

> 
>>  static void uhci_attach(USBPort *port1, USBDevice *dev);
>>  
>>  static void uhci_update_irq(UHCIState *s)
>> @@ -143,15 +320,18 @@ static void uhci_reset(UHCIState *s)
>>      s->intr = 0;
>>      s->fl_base_addr = 0;
>>      s->sof_timing = 64;
>> +
>>      for(i = 0; i < NB_PORTS; i++) {
>>          port = &s->ports[i];
>>          port->ctrl = 0x0080;
>>          if (port->port.dev)
>>              uhci_attach(&port->port, port->port.dev);
>>      }
>> +
>> +    uhci_async_cancel_all(s);
>>  }
>>  
>> -#if 0
>> +#if 1
>>   
> 
> Just drop the #if
This is probably mismerge. Let me check.

>>  static void uhci_save(QEMUFile *f, void *opaque)
>>  {
>>      UHCIState *s = opaque;
>> @@ -239,9 +419,8 @@ static void uhci_ioport_writew(void *opaque,
>> uint32_t addr, uint32_t val)
>>      UHCIState *s = opaque;
>>  
>>      addr &= 0x1f;
>> -#ifdef DEBUG
>> -    printf("uhci writew port=0x%04x val=0x%04x\n", addr, val);
>> -#endif
>> +    dprintf("uhci: writew port=0x%04x val=0x%04x\n", addr, val);
>> +
>>      switch(addr) {
>>      case 0x00:
>>          if ((val & UHCI_CMD_RS) && !(s->cmd & UHCI_CMD_RS)) {
>> @@ -350,9 +529,9 @@ static uint32_t uhci_ioport_readw(void *opaque,
>> uint32_t addr)
>>          val = 0xff7f; /* disabled port */
>>          break;
>>      }
>> -#ifdef DEBUG
>> -    printf("uhci readw port=0x%04x val=0x%04x\n", addr, val);
>> -#endif
>> +
>> +    dprintf("uhci: readw port=0x%04x val=0x%04x\n", addr, val);
>> +
>>      return val;
>>  }
> 
> How do you handle the outstanding asynchronous requests in save/restore?

I do not :).
I haven't really played with save/restore. I did not mean to intentionally
enable save/restore stuff, this probably came from KVM tree. Now that I'm
thinking about I did save/restore the VM once with KVM and USB worked fine.
That was before async patches. With async we can probably just cancel all
outstanding transactions. I'll try that out a bit later.

Max