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

[John Snow <jsnow@redhat.com>]

(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) {
> +    /* 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)) {
>           return;
>       }
>   

I think it's the case that Empty CD-ROM drives will have an anonymous 
block backend representing the empty drive, 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.

So I guess it can be simplified ...?

if (!s->blk) {
     return;
}

--js