From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48743) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cv8OM-0000XC-8P for qemu-devel@nongnu.org; Mon, 03 Apr 2017 16:15:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cv8OL-0004Zn-Au for qemu-devel@nongnu.org; Mon, 03 Apr 2017 16:15:58 -0400 References: <20170401004624.30886-1-ehabkost@redhat.com> <20170401004624.30886-4-ehabkost@redhat.com> <20170403201026.GJ1910@thinpad.lan.raisama.net> From: Alexander Graf Message-ID: Date: Mon, 3 Apr 2017 22:15:44 +0200 MIME-Version: 1.0 In-Reply-To: <20170403201026.GJ1910@thinpad.lan.raisama.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 03/19] sysbus: Set user_creatable=false by default on TYPE_SYS_BUS_DEVICE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , Peter Maydell Cc: QEMU Developers , Laszlo Ersek , Marcel Apfelbaum , Thomas Huth , Markus Armbruster , John Snow , Kevin Wolf , Max Reitz , Paolo Bonzini , Richard Henderson , "Michael S. Tsirkin" , Scott Wood , Jason Wang , David Gibson , Gerd Hoffmann , Alex Williamson , Qemu-block , "qemu-ppc@nongnu.org" , Alistair Francis , Beniamino Galvani , "Edgar E. Iglesias" , "Gabriel L . Somlo" , Igor Mammedov , Prasad J Pandit , qemu-arm , Shannon Zhao On 03.04.17 22:10, Eduardo Habkost wrote: > On Mon, Apr 03, 2017 at 08:49:16PM +0100, Peter Maydell wrote: >> On 1 April 2017 at 01:46, Eduardo Habkost wrote: >>> commit 33cd52b5d7b9adfd009e95f07e6c64dd88ae2a31 unset >>> cannot_instantiate_with_device_add_yet in TYPE_SYSBUS, making >>> all kinds of untested devices available to -device and >>> device_add. >>> >>> The problem with that is: setting has_dynamic_sysbus on a >>> machine-type lets it accept all the 288 sysbus device types we >>> have in QEMU, and most of them were never meant to be used with >>> -device. That's a lot of untested code. >>> >>> Fortunately today we have just a few has_dynamic_sysbus=1 >>> machines: virt, pc-q35-*, ppce500, and spapr. >>> >>> virt, ppce500, and spapr have extra checks to ensure just a few >>> device types can be instantiated: >>> >>> * virt supports only TYPE_VFIO_CALXEDA_XGMAC, TYPE_VFIO_AMD_XGBE. >>> * ppce500 supports only TYPE_ETSEC_COMMON. >>> * spapr supports only TYPE_SPAPR_PCI_HOST_BRIDGE. >>> >>> q35 has no code to block unsupported sysbus devices, however, and >>> accepts all device types. Fortunately, only the following 20 >>> device types are compiled into the qemu-system-x86_64 and >>> qemu-system-i386 binaries: >>> >>> * allwinner-ahci >>> * amd-iommu >>> * cfi.pflash01 >>> * esp >>> * fw_cfg_io >>> * fw_cfg_mem >>> * generic-sdhci >>> * hpet >>> * intel-iommu >>> * ioapic >>> * isabus-bridge >>> * kvmclock >>> * kvm-ioapic >>> * kvmvapic >>> * SUNW,fdtwo >>> * sysbus-ahci >>> * sysbus-fdc >>> * sysbus-ohci >>> * unimplemented-device >>> * virtio-mmio >>> >>> Instead of requiring each machine-type with has_dynamic_sysbus=1 >>> to implement its own mechanism to block unsupported devices, we >>> can use the user_creatable flag to ensure we won't let the user >>> plug anything that will never work. >> >> How does this work? Which devices can be dynamically >> plugged is machine dependent. You can't dynamically-plug >> an intel-iommu on the ARM virt board, and you can't >> dynamically-plug the vfio-calxeda-xgmac on the spapr >> board, and so on. So I don't see how we can just have >> a flag on the device itself that controls whether >> it can be dynamically plugged. >> >> So I'm definitely coming around to the opinion that >> it's just a bug in the q35 board that it doesn't have >> any device whitelisting, and we should fix that. > > OK, let's assume q35 must implement a whitelist: > > To build that whitelist, we need to be able to know what should > be in the whitelist, or not. And nobody knew for sure what was > user-creatable in q35 by accident, and what was really supposed > to be user-creatable in q35. See the "q35 and sysbus devices" > thread I started ~2 weeks ago. > > Building a q35 whitelist will be much easier if make > sys-bus-devices non-user-creatable by default. So why are they user creatable in the first place? We used to have boards that were dynamic sysbus aware (ppce500, virt) that allowed dynamic creation and every other board did not. I don't remember the exact mechanism behind it though. When did that behavior change? It sounds like a regression somewhere. Alex