From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [PATCH 1/5] pci-assign: Clean up assigned_dev_pci_read/write_config Date: Mon, 13 Dec 2010 16:53:30 -0700 Message-ID: <1292284410.2857.136.camel@x201> References: 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]:55667 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756937Ab0LMXxe (ORCPT ); Mon, 13 Dec 2010 18:53:34 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 2010-12-14 at 00:25 +0100, Jan Kiszka wrote: > From: Jan Kiszka > > Use rages_overlap and proper constants to match the access range against > regions that need special handling. This also fixes yet uncaught > high-byte write access to the command register. Moreover, use more > constants instead of magic numbers. > > Signed-off-by: Jan Kiszka > --- > hw/device-assignment.c | 39 +++++++++++++++++++++++++++++---------- > 1 files changed, 29 insertions(+), 10 deletions(-) A long overdue cleanup, looks good. Acked-by: Alex Williamson > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > index 50c6408..bc3a57b 100644 > --- a/hw/device-assignment.c > +++ b/hw/device-assignment.c > @@ -438,13 +438,20 @@ static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address, > return assigned_device_pci_cap_write_config(d, address, val, len); > } > > - if (address == 0x4) { > + if (ranges_overlap(address, len, PCI_COMMAND, 2)) { > pci_default_write_config(d, address, val, len); > /* Continue to program the card */ > } > > - if ((address >= 0x10 && address <= 0x24) || address == 0x30 || > - address == 0x34 || address == 0x3c || address == 0x3d) { > + /* > + * Catch access to > + * - base address registers > + * - ROM base address & capability pointer > + * - interrupt line & pin > + */ > + if (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)) { > /* used for update-mappings (BAR emulation) */ > pci_default_write_config(d, address, val, len); > return; > @@ -484,9 +491,20 @@ static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, > return val; > } > > - if (address < 0x4 || (pci_dev->need_emulate_cmd && address == 0x4) || > - (address >= 0x10 && address <= 0x24) || address == 0x30 || > - address == 0x34 || address == 0x3c || address == 0x3d) { > + /* > + * 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)) { > 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); > @@ -517,10 +535,11 @@ do_log: > > if (!pci_dev->cap.available) { > /* kill the special capabilities */ > - if (address == 4 && len == 4) > - val &= ~0x100000; > - else if (address == 6) > - val &= ~0x10; > + if (address == PCI_COMMAND && len == 4) { > + val &= ~(PCI_STATUS_CAP_LIST << 16); > + } else if (address == PCI_STATUS) { > + val &= ~PCI_STATUS_CAP_LIST; > + } > } > > return val;