On Tue, Dec 22, 2009 at 02:34:42PM +0100, Alexander Graf wrote:
> 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 <m...@redhat.com>
> >>> ---
> >>>
> >>> 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
Correct: I think it's made readonly down the road with mprotect,
so attempt to do so will crash qemu :)
--
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
