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

Re: [PATCH v2] piix: fix regression during unplug in Xen HVM domUs


  • To: John Snow <jsnow@xxxxxxxxxx>
  • From: Olaf Hering <olaf@xxxxxxxxx>
  • Date: Thu, 25 Mar 2021 12:12:19 +0100
  • Arc-authentication-results: i=1; strato.com; dkim=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1616670753; s=strato-dkim-0002; d=strato.com; h=References:In-Reply-To:Message-ID:Subject:Cc:To:From:Date:Cc:Date: From:Subject:Sender; bh=BHLafyRjRvEA3puYQ80EOvJ1Vi/2uXDQaBXaQ3IFZJA=; b=pgA2u+tTUV4ad5xzkGDZ8z3CYeu8WwjTEJox4eYNtk0ZDYuoHBLq89hJ1oH4hc3tsa rsiyRnL4kPP8K/6WO2fAn57vzs6qDJj0I/TUKwCL96rRtw42Tn4gVK6p29S4prB1zo9i JIafA/+Rq2X9rGpkp6hI8f6mtE4prisvYBLmHUfFuy6x0B8w6KUGpYiWgHVDSe5QpHOr eLsipMPz/VBmNi2b36u1qMHt6qDgXnJYKhlTUt9F6TQ4gw86GvaBSwTUW7GiLq8RkJAu t8cF3xPdUL4wwB9s2Hj/LWkpJOzr6+sH0ucLiM/KW9fMlGCYTEeK3X8j0JxbzuFQMA6c E4pw==
  • Arc-seal: i=1; a=rsa-sha256; t=1616670753; cv=none; d=strato.com; s=strato-dkim-0002; b=JcWZ8Vl/Y739zpvIuEeh6F6rtvsYzFNPDhA1PZTV380+4Jc4Z8o+zQ1/NFhQGlZcxy WnH9WjJO9BERmhLzcuv28ybkXa2C4JCwRz4uQ6M618PPwpYqF0hh472337stu5sjNhwa YECnOl9IZ1n+jpCLFR0pD6Qv7dz9F85ghtgPLtaDHJPML2SuAMbIh5R5+c1p8wwGZcWD ozd7L4uHgSBLByW5hsWnqsAq2tgz5AnYeH9Y915OusZShBD9+l7jSEAF97JHYiY8/Mqy EBcKInxYF+199hzCGMehT8wR60Tv5NrMLD584l2rk0Sv0OTtOXni6fbSIZy20KkYsngA cCLA==
  • Authentication-results: strato.com; dkim=none
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, qemu-block@xxxxxxxxxx, qemu-devel@xxxxxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Philippe Mathieu-Daudé <f4bug@xxxxxxxxx>
  • Delivery-date: Thu, 25 Mar 2021 11:13:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Am Mon, 22 Mar 2021 18:09:17 -0400
schrieb John Snow <jsnow@xxxxxxxxxx>:

> My understanding is that XEN has some extra disks that it unplugs when 
> it later figures out it doesn't need them. How exactly this works is 
> something I've not looked into too closely.

It has no extra disks, why would it?

I assume each virtualization variant has some sort of unplug if it has to 
support guests that lack PV/virtio/enlightened/whatever drivers.

In case of HVM, the configured block or network devices can be either accessed 
via emulated PCI or via the PV drivers. Since the BIOS, the bootloader and 
potentially the operating system kernel typically lack PV drivers, they will 
find the devices only via the PCI bus. In case they happen to have PV drivers 
in addition to PCI drivers, both drivers will find and offer the same resource 
via different paths. In case of a block device, ata_piix.ko will show it via 
"/dev/sda" and xen-blkfront.ko will show it via "/dev/xvda". This is obviously 
bad, at least in the read-write case.

The pvops kernel triggers the unplug of the emulated PCI hardware early, prior 
any other PCI initialization. As a result the PCI drivers will not find their 
hardware anymore. In case of ata_piix, only the non-CDROM storage will be 
removed in qmeu, because there is no PV-CDROM driver.

The PV support in old xenlinux based kernels is only available as modules. As a 
result the unplug will happen after PCI was initialized, but it must happen 
before any PCI device drivers are loaded.


> So if these IDE devices have been "unplugged" already, we avoid 
> resetting them here. What about this reset causes the bug you describe 
> in the commit message?
> 
> Does this reset now happen earlier/later as compared to what it did 
> prior to ee358e91 ?

Prior this commit, piix_ide_reset was only called when the entire emulated 
machine was reset. Like: never.
With this commit, piix_ide_reset will be called from pci_piix3_xen_ide_unplug. 
For some reason it confuses the emulated USB hardware. Why it does confused it, 
no idea.

I wonder what the purpose of the qdev_reset_all() call really is. It is 10 
years old. It might be stale.


Olaf

Attachment: pgpiouPFGCjR9.pgp
Description: Digitale Signatur von OpenPGP


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.