From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] slow_map: minor improvements to ROM BAR handling Date: Tue, 22 Dec 2009 14:34:42 +0100 Message-ID: <4B30CAF2.4040409@suse.de> References: <20091222111044.GA16000@redhat.com> <4B30B603.8080707@suse.de> <20091222124311.GD16165@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: avi@redhat.com, kvm@vger.kernel.org To: "Michael S. Tsirkin" Return-path: Received: from cantor2.suse.de ([195.135.220.15]:34790 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752590AbZLVNeo (ORCPT ); Tue, 22 Dec 2009 08:34:44 -0500 In-Reply-To: <20091222124311.GD16165@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Michael S. Tsirkin wrote: > On Tue, Dec 22, 2009 at 01:05:23PM +0100, Alexander Graf wrote: > >> Michael S. Tsirkin wrote: >> >>> ROM BAR can be handled same as regular BAR: >>> load_option_roms utility will take care of >>> copying it to RAM as appropriate. >>> >>> Signed-off-by: Michael S. Tsirkin >>> --- >>> >>> This patch applies on top of agraf's one, >>> it takes care of non-page aligned ROM BARs as well: >>> they mostly are taken care of, we just do not >>> need to warn user about them. >>> >>> hw/device-assignment.c | 20 +++++++++----------- >>> 1 files changed, 9 insertions(+), 11 deletions(-) >>> >>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c >>> index 000fa61..066fdb6 100644 >>> --- a/hw/device-assignment.c >>> +++ b/hw/device-assignment.c >>> @@ -486,25 +486,23 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, >>> : PCI_BASE_ADDRESS_SPACE_MEMORY; >>> >>> if (cur_region->size & 0xFFF) { >>> - fprintf(stderr, "PCI region %d at address 0x%llx " >>> - "has size 0x%x, which is not a multiple of 4K. " >>> - "You might experience some performance hit due to that.\n", >>> - i, (unsigned long long)cur_region->base_addr, >>> - cur_region->size); >>> + if (i != PCI_ROM_SLOT) { >>> + fprintf(stderr, "PCI region %d at address 0x%llx " >>> + "has size 0x%x, which is not a multiple of 4K. " >>> + "You might experience some performance hit " >>> + "due to that.\n", >>> + i, (unsigned long long)cur_region->base_addr, >>> + cur_region->size); >>> + } >>> slow_map = 1; >>> >>> >> This is wrong. You're setting slow_map = 1 on code that is very likely >> to be executed inside the guest. That doesn't work. >> > > It is? Can you really run code directly from a PCI card? > I looked at BIOS boot specification and it always talks > about shadowing PCI ROMs. > I'm not sure the BIOS is the only one executing ROMs. If it is, then I'm good with the change. Maybe it'd make sense to also add a read only flag so we don't accidently try to write to the ROM region with slow_map. Alex