From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v3 2/3] pci-assign: Use PCI-2.3-based shared legacy interrupts Date: Wed, 07 Mar 2012 19:48:03 +0100 Message-ID: <4F57AD63.9070303@siemens.com> References: <6ef91f83c1d72b986813350611c0df21de786760.1331140766.git.jan.kiszka@siemens.com> <1331142392.29701.329.camel@bling.home> <4F57A37F.3020502@siemens.com> <1331144049.29701.332.camel@bling.home> <4F57A661.5030009@siemens.com> <1331145159.29701.336.camel@bling.home> <4F57AC29.7060204@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , "kvm@vger.kernel.org" , "Michael S. Tsirkin" To: Alex Williamson Return-path: Received: from goliath.siemens.de ([192.35.17.28]:15924 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757328Ab2CGSsG (ORCPT ); Wed, 7 Mar 2012 13:48:06 -0500 In-Reply-To: <4F57AC29.7060204@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2012-03-07 19:42, Jan Kiszka wrote: > On 2012-03-07 19:32, Alex Williamson wrote: >> On Wed, 2012-03-07 at 19:18 +0100, Jan Kiszka wrote: >>> Enable the new KVM feature that allows legacy interrupt sharing for >>> PCI-2.3-compliant devices. This requires to synchronize any guest >>> change of the INTx mask bit to the kernel. >>> >>> The feature is controlled by the property 'share_intx' and is off by >>> default for now. >>> >>> Signed-off-by: Jan Kiszka >>> --- >>> >>> Changes in v3: >>> - fixed nit :) >>> >>> hw/device-assignment.c | 24 ++++++++++++++++++++++++ >>> hw/device-assignment.h | 10 ++++++---- >>> qemu-kvm.c | 9 +++++++++ >>> qemu-kvm.h | 2 ++ >>> 4 files changed, 41 insertions(+), 4 deletions(-) >>> >>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c >>> index a5f1abb..e2a8479 100644 >>> --- a/hw/device-assignment.c >>> +++ b/hw/device-assignment.c >>> @@ -782,6 +782,13 @@ static int assign_device(AssignedDevice *dev) >>> "cause host memory corruption if the device issues DMA write " >>> "requests!\n"); >>> } >>> + if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK) { >>> + assigned_dev_data.flags |= KVM_DEV_ASSIGN_PCI_2_3; >>> + >>> + /* hide host-side INTx masking from the guest */ >>> + dev->emulate_config_read[PCI_COMMAND + 1] |= >>> + PCI_COMMAND_INTX_DISABLE >> 8; >>> + } >>> >>> r = kvm_assign_pci_device(kvm_state, &assigned_dev_data); >>> if (r < 0) { >>> @@ -1121,10 +1128,25 @@ static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, >>> uint32_t val, int len) >>> { >>> AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); >>> + uint16_t old_cmd = pci_get_word(pci_dev->config + PCI_COMMAND); >>> uint32_t emulate_mask, full_emulation_mask; >>> + int ret; >>> >>> pci_default_write_config(pci_dev, address, val, len); >>> >>> + if (range_covers_byte(address, len, PCI_COMMAND + 1)) { >> >> Hmm, now that I've acked this... shouldn't we have a feature check here >> to avoid the ioctl when share_intx=false or when the host kernel doesn't >> support this? Thanks, > > Hmm, I thought I handled this, somehow. But it's not there. v4 follows... To be precise: We only need to worry about false positive error messages in the absence of KVM_CAP_PCI_2_3. Devices not supporting INTx sharing will either refuse to work due to IRQ conflicts, or we will map INTx masking on IRQ line masking. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux