From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH] slow_map: minor improvements to ROM BAR handling Date: Tue, 22 Dec 2009 14:43:11 +0200 Message-ID: <20091222124311.GD16165@redhat.com> References: <20091222111044.GA16000@redhat.com> <4B30B603.8080707@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@redhat.com, kvm@vger.kernel.org To: Alexander Graf Return-path: Received: from mx1.redhat.com ([209.132.183.28]:25810 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750917AbZLVMp7 (ORCPT ); Tue, 22 Dec 2009 07:45:59 -0500 Content-Disposition: inline In-Reply-To: <4B30B603.8080707@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: 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. > Better pad the ROM size to page boundary and use the shadow mapping we > have in place already. Changing BAR size might break some drivers. Our BIOS seems to shadow ROM instead of running it directly, so we should be fine I think? > > Alex