From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40507) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abRHr-0003fo-4x for qemu-devel@nongnu.org; Thu, 03 Mar 2016 06:19:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abRHq-0005ut-65 for qemu-devel@nongnu.org; Thu, 03 Mar 2016 06:19:19 -0500 References: <1450176195-9066-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1450176195-9066-2-git-send-email-caoj.fnst@cn.fujitsu.com> <87wpplqm3i.fsf@blackfin.pond.sub.org> <56D80E0B.8070902@redhat.com> <20160303123221-mutt-send-email-mst@redhat.com> From: Marcel Apfelbaum Message-ID: <56D81DAD.8040101@redhat.com> Date: Thu, 3 Mar 2016 13:19:09 +0200 MIME-Version: 1.0 In-Reply-To: <20160303123221-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: kwolf@redhat.com, alex.williamson@redhat.com, jiri@resnulli.us, qemu-block@nongnu.org, jasowang@redhat.com, Markus Armbruster , qemu-devel@nongnu.org, keith.busch@intel.com, Cao jin , sfeldma@gmail.com, kraxel@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, jsnow@redhat.com, Jan Kiszka , hare@suse.de On 03/03/2016 12:45 PM, Michael S. Tsirkin wrote: > On Thu, Mar 03, 2016 at 12:12:27PM +0200, Marcel Apfelbaum wrote: >>>> +int msi_init(struct PCIDevice *dev, uint8_t offset, unsigned int nr_vectors, >>>> + bool msi64bit, bool msi_per_vector_mask, Error **errp) >>>> { >>>> unsigned int vectors_order; >>>> - uint16_t flags; >>>> + uint16_t flags; /* Message Control register value */ >>>> uint8_t cap_size; >>>> int config_offset; >>>> >>>> if (!msi_supported) { >>>> + error_setg(errp, "MSI is not supported by interrupt controller"); >>>> return -ENOTSUP; >>> >>> First failure mode: board does not support MSI (-ENOTSUP). >>> >>> Question to the PCI guys: why is this even an error? A device with >>> capability MSI should work just fine in such a board. >> >> Hi Markus, >> >> Adding Jan Kiszka, maybe he can help. >> >> That's a fair question. Is there any history for this decision? >> The board not supporting MSI has nothing to do with the capability being there. >> The HW should not change because the board doe not support it. >> >> The capability should be present but not active. > > Digging in git log will tell you why we have the msi_supported flag: > > commit 02eb84d0ec97f183ac23ee939403a139e8849b1d ("qemu/pci: MSI-X support functions") > > This is a safety measure to avoid breaking platforms which should support > MSI-X but currently lack this in the interrupt controller emulation. > > in other words, on some platforms we must hide msi support from devices > because otherwise guests will try to use it, and our emulation is > incomplete. OK, thanks. So the flag should be "msi_broken" or "msi_present_but_not_implemented" and not "msi_supported" that leads (at least me) to the assumption that some platform *does not support msi* rather than it supports it, but we don't emulate it. > > And the conclusion from that is that for msi_init to fail silently is > at the moment the right thing to do. But this is not the only thing we do, we are modifying the PCI devices. We could fail starting the VM if a device supporting MSI is added on a platform with broken msi, but this will prevent us to use assigned devices. Emulated devices should be created with a specific "msi=off" flag. Thanks, Marcel > > The only other reason for it to fail is pci config space corruption, > this probably never happens in practice. >