[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/6] pc: limit 64 bit hole to 2G by default
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 6/6] pc: limit 64 bit hole to 2G by default |
Date: |
Sun, 28 Jul 2013 19:34:06 +0200 |
On Sun, 28 Jul 2013 12:13:17 +0300
"Michael S. Tsirkin" <address@hidden> wrote:
> On Sun, Jul 28, 2013 at 10:21:56AM +0200, Igor Mammedov wrote:
> > On Sun, 28 Jul 2013 10:57:12 +0300
> > "Michael S. Tsirkin" <address@hidden> wrote:
> >
> > > On Sun, Jul 28, 2013 at 09:29:13AM +0200, Igor Mammedov wrote:
> > > > It turns out that some 32 bit windows guests crash
> > > > if 64 bit PCI hole size is >2G.
> > > > Limit it to 2G for piix and q35 by default.
> > > > User may override default boundaries by using
> > > > "pci_hole64_end " property.
> > > >
> > > > Examples:
> > > > -global i440FX-pcihost.pci_hole64_end=0xffffffffffffffff
> > > >
> > > > -global q35-pcihost.pci_hole64_end=0xffffffffffffffff
> > >
> > > IMO that's pretty bad as user interfaces go.
> > > In particular if you are not careful you can make
> > > end < start.
> > > Why not set the size, and include a patch that
> > > let people write "1G" or "2G" for convenience?
> > sure as convenience why not, on top of this patches.
> >
> > >
> > > > Reported-by: Igor Mammedov <address@hidden>,
> > > > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > > > Signed-off-by: Igor Mammedov <address@hidden>
> > > > ---
> > > > hw/i386/pc.c | 59
> > > > +++++++++++++++++++++++++++++------------------
> > > > hw/i386/pc_piix.c | 14 +----------
> > > > hw/pci-host/piix.c | 37 +++++++++++++++++++++++++----
> > > > hw/pci-host/q35.c | 32 +++++++++++++++----------
> > > > include/hw/i386/pc.h | 5 ++--
> > > > include/hw/pci-host/q35.h | 1 +
> > > > 6 files changed, 94 insertions(+), 54 deletions(-)
> > > >
> > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > > index b0b98a8..acaeb6c 100644
> > > > --- a/hw/i386/pc.c
> > > > +++ b/hw/i386/pc.c
> > > > @@ -1003,15 +1003,28 @@ typedef struct PcRomPciInfo {
> > > > static void pc_fw_cfg_guest_info(PcGuestInfo *guest_info)
> > > > {
> > > > PcRomPciInfo *info;
> > > > + Object *pci_info;
> > > > +
> > > > if (!guest_info->has_pci_info || !guest_info->fw_cfg) {
> > > > return;
> > > > }
> > > > + pci_info = object_resolve_path("/machine/i440fx", NULL);
> > > > + if (!pci_info) {
> > > > + pci_info = object_resolve_path("/machine/q35", NULL);
> > > > + if (!pci_info) {
> > > > + return;
> > > > + }
> > > > + }
> > >
> > >
> > > So is the path /machine/i440fx? /machine/i440FX?
> > > /machine/i440FX-pcihost?
> > > There's no way to check this code is right without
> > > actually running it.
> > that drawback of dynamic lookup,
> > QOM paths supposed to be stable.
> >
> > >
> > > How about i44fx_get_pci_info so we can have this
> > > knowledge of paths localized in specific chipset code?
> > I've seen objections from afaerber about this approach, so dropped
> > this idea.
>
> Could we lookup TYPE_PCI_HOST? This will make pc code
> independent of specific machine.
sure, thanks for idea.
>
> --
> MST
--
Regards,
Igor
- [Qemu-devel] [PATCH 5/6] pc: add Q35 to QOM composition tree under /machine, (continued)