From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [PATCH 2/5] pci-assign: Fix dword read at PCI_COMMAND Date: Mon, 13 Dec 2010 16:53:21 -0700 Message-ID: <1292284401.2857.135.camel@x201> References: <5c4909dd1796ece440be44c6c07a70dbed3887bd.1292282738.git.jan.kiszka@web.de> 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" , Jan Kiszka To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:48890 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756584Ab0LMXx1 (ORCPT ); Mon, 13 Dec 2010 18:53:27 -0500 In-Reply-To: <5c4909dd1796ece440be44c6c07a70dbed3887bd.1292282738.git.jan.kiszka@web.de> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 2010-12-14 at 00:25 +0100, Jan Kiszka wrote: > From: Jan Kiszka > > If we emulate the command register, we must only read its content from > the shadow config space. For dword read of both PCI_COMMAND and > PCI_STATUS, at least the latter must be read from the device. > > For simplicity reasons and as the code path is not considered > performance critical for the affected SRIOV devices, the fix performes > device access to the command word unconditionally, even if emulation is > enabled and only that word is read. > > Signed-off-by: Jan Kiszka > --- > hw/device-assignment.c | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > index bc3a57b..6ff1456 100644 > --- a/hw/device-assignment.c > +++ b/hw/device-assignment.c > @@ -494,14 +494,11 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, > /* > * Catch access to > * - vendor & device ID > - * - command register (if emulation needed) > * - base address registers > * - ROM base address & capability pointer > * - interrupt line & pin > */ > if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) || > - (pci_dev->need_emulate_cmd && > - ranges_overlap(address, len, PCI_COMMAND, 2)) || > ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || > ranges_overlap(address, len, PCI_ROM_ADDRESS, 8) || > ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { > @@ -533,6 +530,17 @@ do_log: > DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", > (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); > > + if (pci_dev->need_emulate_cmd && > + ranges_overlap(address, len, PCI_COMMAND, 2)) { > + if (address == PCI_COMMAND) { > + val &= 0xffff0000; > + val |= pci_default_read_config(d, PCI_COMMAND, 2); > + } else { > + /* high-byte access */ > + val = pci_default_read_config(d, PCI_COMMAND+1, 1); > + } > + } > + > if (!pci_dev->cap.available) { > /* kill the special capabilities */ > if (address == PCI_COMMAND && len == 4) { We might be able to use the merge_bits function that I just added for capability support, perhaps something like: if (pci_dev->need_emulate_cmd) { val = merge_bits(val, pci_default_read_config(d, address, len), PCI_COMMAND, 0xffff) }