[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

答复: [PATCH] ide:do nothing for identify cmd if no any device attached

From: RockCui-oc
Subject: 答复: [PATCH] ide:do nothing for identify cmd if no any device attached
Date: Wed, 9 Sep 2020 03:13:41 +0000

Hi John & Max,

  1. Follow my Log,there are 10000 read 0x1x7 ops. On my Intel I5 platform, if down the frequency to 1.2G, you can see a obviously lag during WINDOWS LOGO animation playing.
  2. We must supply a CD-ROM to our VDI users.
  3. In ide_ioport_read() :
case 7:
       if ((!bus->ifs[0].blk && !bus->ifs[1].blk) || (s != bus->ifs && !s->blk)) {
            ret = 0;
       } else {
            ret = s->status;
            so I follow this.


发件人: Max Reitz <mreitz@redhat.com>
发送时间: 2020年9月3日 18:40
收件人: John Snow; RockCui-oc; qemu-devel@nongnu.org
抄送: Cobe Chen(BJ-RD); Peter Maydell
主题: Re: [PATCH] ide:do nothing for identify cmd if no any device attached
On 02.09.20 20:02, John Snow wrote:
> (CC Max for block backend model confusion, see below)
> On 8/16/20 11:38 PM, zhaoxin\RockCuioc wrote:
>> This patch is for avoiding win7 IDE driver polling 0x1f7 when
>> no any device attached. During Win7 VM boot procedure, if use virtio for
>> disk and there is no any device be attached on hda & hdb, the win7 IDE
>> driver
>> would poll 0x1f7 for a while. This action may be stop windows LOGO
>> atomic for
>> a while too on a poor performance CPU.
> A few questions:
> (1) How slow is the probing?
> (2) If there are no devices attached, why don't you remove the IDE
> controller so that Windows doesn't have to probe it?
>> Signed-off-by: zhaoxin\RockCuioc <RockCui-oc@zhaoxin.com>
>> ---
>>   hw/ide/core.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index d997a78e47..26d86f4b40 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2073,8 +2073,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>>       s = idebus_active_if(bus);
>>       trace_ide_exec_cmd(bus, s, val);
>>   -    /* ignore commands to non existent slave */
>> -    if (s != bus->ifs && !s->blk) {

(Was the first one basically meant to be “s != &bus->ifs[0]”, i.e. to
check that this doesn’t go to the ma^W primary?  Not too obvious.)

>> +    /* ignore commands if no any device exist or non existent slave */
>> +    if ((!bus->ifs[0].blk && !bus->ifs[1].blk) ||
>> +        (s != bus->ifs && !s->blk)) {

(Maybe this could be improved here)

>>           return;
>>       }
> I think it's the case that Empty CD-ROM drives will have an anonymous
> block backend representing the empty drive,

(As far as I remember,) yes.

(ide_dev_initfn() ensures all CD drives have one, even if it’s empty.)

> so I suppose this is maybe
> fine?
> I suppose the idea is that with no drives on the bus that it's fine to
> ignore the register writes, as there are no devices to record those writes.
> (But then, why did we ever only check device1? ...)
> Maybe before the block-backend split we used to have to check to see if
> we had attached media or not, but I think nowadays we should always have
> a blk pointer if we have a device model intended to be operating at this
> address.

The check in ide_dev_initfn() looks that way to me.

> So I guess it can be simplified ...?
> if (!s->blk) {
>     return;
> }

Probably.  Although there’s a difference, of course, namely if you have
only a secondary device and try to access the primary, this simplified
version will be a no-op, whereas the more complicated version in this
patch would still go on.  I don’t know how real hardware would handle
that case.  Is it even possible to have just a secondary with no primary?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]