From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it Date: Tue, 02 Aug 2011 14:26:13 -0600 Message-ID: <1312316773.2653.475.camel@bling.home> References: <4E37ED2E.6030700@siemens.com> <1312304525.2653.431.camel@bling.home> <4E3834B2.4040101@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , kvm To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:7337 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754944Ab1HBU0Q (ORCPT ); Tue, 2 Aug 2011 16:26:16 -0400 In-Reply-To: <4E3834B2.4040101@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 2011-08-02 at 19:32 +0200, Jan Kiszka wrote: > On 2011-08-02 19:02, Alex Williamson wrote: > > On Tue, 2011-08-02 at 14:27 +0200, Jan Kiszka wrote: > >> Add checks for KVM_CAP_ASSIGN_DEV_IRQ (MSI) and KVM_CAP_DEVICE_MSIX, do > >> not set up MSI/MSI-X if the required kernel features are missing. > >> > >> Signed-off-by: Jan Kiszka > >> --- > >> hw/device-assignment.c | 6 ++++-- > >> 1 files changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/device-assignment.c b/hw/device-assignment.c > >> index 4cc7b1a..7e965cb 100644 > >> --- a/hw/device-assignment.c > >> +++ b/hw/device-assignment.c > >> @@ -1201,7 +1201,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > >> > >> /* Expose MSI capability > >> * MSI capability is the 1st capability in capability config */ > >> - if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) { > >> + pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0); > >> + if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) { > > > > Is it even useful to have a device assigned w/o KVM_CAP_ASSIGN_DEV_IRQ? > > If that feature is lacking, we fall back to the old KVM_ASSIGN_IRQ > interface. I guess that used to work, but I bet no one tested it > recently. However, the code is there, also in the kvm core. Ah right. Not sure how much I trust that, but seems obviously correct to not expose capabilities we can't support. Acked-by: Alex Williamson