From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 1/5] pci-assign: Clean up assigned_dev_pci_read/write_config Date: Thu, 28 Apr 2011 16:46:55 +0200 Message-ID: <4DB97DDF.7020106@siemens.com> References: <1304000997.3125.2.camel@x201> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , "kvm@vger.kernel.org" To: Alex Williamson Return-path: Received: from goliath.siemens.de ([192.35.17.28]:32469 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754103Ab1D1OrD (ORCPT ); Thu, 28 Apr 2011 10:47:03 -0400 In-Reply-To: <1304000997.3125.2.camel@x201> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-04-28 16:29, Alex Williamson wrote: > On Thu, 2011-04-28 at 10:59 +0200, Jan Kiszka wrote: >> Use rages_overlap and proper constants to match the access range against > ^^^^^ typo - only if you resend > >> 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(-) >> >> diff --git a/hw/device-assignment.c b/hw/device-assignment.c >> index 606d725..3481c93 100644 >> --- a/hw/device-assignment.c >> +++ b/hw/device-assignment.c >> @@ -404,13 +404,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) || > > Should this be 5 bytes instead of 8? I'm not sure why we'd add catching > these reserved fields, but not those immediately after this range. Yes, that's asking for clarification: Should we allow direct access to the complete reserved space or virtualize it? Depending on this, the proper value should be 5 or 14 (the latter would also save one ranges_overlap). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux