From patchwork Mon May 2 10:29:39 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 747012 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p42AToSX022954 for ; Mon, 2 May 2011 10:29:50 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751636Ab1EBK3r (ORCPT ); Mon, 2 May 2011 06:29:47 -0400 Received: from goliath.siemens.de ([192.35.17.28]:20659 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751222Ab1EBK3q (ORCPT ); Mon, 2 May 2011 06:29:46 -0400 Received: from mail1.siemens.de (localhost [127.0.0.1]) by goliath.siemens.de (8.13.6/8.13.6) with ESMTP id p42ATd4Z009107; Mon, 2 May 2011 12:29:40 +0200 Received: from mchn199C.mchp.siemens.de ([139.25.109.49]) by mail1.siemens.de (8.13.6/8.13.6) with ESMTP id p42ATdJd005175; Mon, 2 May 2011 12:29:39 +0200 Message-ID: <4DBE8793.3090505@siemens.com> Date: Mon, 02 May 2011 12:29:39 +0200 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: Alex Williamson CC: Avi Kivity , Marcelo Tosatti , "kvm@vger.kernel.org" Subject: [PATCH v3 4/6] pci-assign: Properly handle more overlapping accesses References: <1304109696.3266.19.camel@x201> In-Reply-To: <1304109696.3266.19.camel@x201> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Mon, 02 May 2011 10:29:50 +0000 (UTC) On 2011-04-29 22:41, Alex Williamson wrote: > On Fri, 2011-04-29 at 11:05 +0200, Jan Kiszka wrote: >> Ensure that accesses exceeding PCI_CAPABILITY_LIST and >> PCI_INTERRUPT_LINE+PIN hit the real device in areas we do not >> virtualize. Again, we do not optimize these checks and accesses a lot, >> they are considered to be slow paths. >> >> Signed-off-by: Jan Kiszka >> --- >> hw/device-assignment.c | 34 +++++++++++++++++++++++++++++----- >> 1 files changed, 29 insertions(+), 5 deletions(-) >> >> diff --git a/hw/device-assignment.c b/hw/device-assignment.c >> index cea072e..37c77e3 100644 >> --- a/hw/device-assignment.c >> +++ b/hw/device-assignment.c >> @@ -442,7 +442,29 @@ static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address, >> ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { >> /* used for update-mappings (BAR emulation) */ >> pci_default_write_config(d, address, val, len); >> - return; >> + >> + /* Ensure that writes to overlapping areas we don't virtualize still >> + * hit the device. */ >> + switch (address) { >> + case PCI_CAPABILITY_LIST: >> + if (len > 1) { >> + len -= 1; >> + address += 1; >> + val >>= 8; >> + break; /* continue writing to the device */ >> + } >> + return; >> + case PCI_INTERRUPT_LINE: >> + if (len > 2) { >> + len -= 2; >> + address += 2; >> + val >>= 16; >> + break; /* continue writing to the device */ >> + } >> + return; >> + default: >> + return; >> + } >> } > > It seems like we could be more symmetric with the below read. Maybe > something like: > > if (ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || > ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { > pci_default_write_config(d, address, val, len); > return; > } else if (ranges_overlap(address, len, PCI_CAPABILITY_LIST, 1) || > ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { > uint32_t real_val = assigned_dev_pci_read(d, address, len); > pci_default_write_config(d, address, val, len); > val = merge_bits(val, real_val, address, len, PCI_CAPABILITY_LIST, 0xff); > val = merge_bits(val, real_val, address, len, PCI_INTERRUPT_LINE, 0xffff); > } > > We might write out to real hardware when we could avoid it, but I don't > think it matters. Thanks, > Yep, makes sense. Find such a version below. Thanks, Jan ------8<------- Ensure that accesses exceeding PCI_CAPABILITY_LIST and PCI_INTERRUPT_LINE+PIN hit the real device in areas we do not virtualize. Again, we do not optimize these checks and accesses a lot, they are considered to be slow paths. Signed-off-by: Jan Kiszka Acked-by: Alex Williamson --- hw/device-assignment.c | 34 ++++++++++++++++++++++++++-------- 1 files changed, 26 insertions(+), 8 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index cea072e..8e95730 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -438,11 +438,22 @@ static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address, * - interrupt line & pin */ if (ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || - ranges_overlap(address, len, PCI_ROM_ADDRESS, 5) || - ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { - /* used for update-mappings (BAR emulation) */ + ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { pci_default_write_config(d, address, val, len); return; + } else if (ranges_overlap(address, len, PCI_CAPABILITY_LIST, 1) || + ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { + uint32_t real_val; + + pci_default_write_config(d, address, val, len); + + /* Ensure that writes to overlapping areas we don't virtualize still + * hit the device. */ + real_val = assigned_dev_pci_read(d, address, len); + val = merge_bits(val, real_val, address, len, + PCI_CAPABILITY_LIST, 0xff); + val = merge_bits(val, real_val, address, len, + PCI_INTERRUPT_LINE, 0xffff); } DEBUG("NON BAR (%x.%x): address=%04x val=0x%08x len=%d\n", @@ -467,7 +478,7 @@ again: static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, int len) { - uint32_t val = 0; + uint32_t val = 0, virt_val; int fd; ssize_t ret; AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); @@ -483,13 +494,11 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, * Catch access to * - vendor & device ID * - base address registers - * - ROM base address & capability pointer - * - interrupt line & pin + * - ROM base address */ if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) || ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || - ranges_overlap(address, len, PCI_ROM_ADDRESS, 5) || - ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { + ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { val = pci_default_read_config(d, address, len); DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); @@ -523,6 +532,15 @@ do_log: address, len, PCI_COMMAND, 0xffff); } + /* + * Merge bits from virtualized + * - capability pointer + * - interrupt line & pin + */ + virt_val = pci_default_read_config(d, address, len); + val = merge_bits(val, virt_val, address, len, PCI_CAPABILITY_LIST, 0xff); + val = merge_bits(val, virt_val, address, len, PCI_INTERRUPT_LINE, 0xffff); + if (!pci_dev->cap.available) { /* kill the special capabilities */ if (address == PCI_COMMAND && len == 4) {