Re: [SeaBIOS] [PATCH 2/2] pci: load memory window setup from host

[Kevin O'Connor]

On Mon, Apr 29, 2013 at 06:21:40PM +0300, Michael S. Tsirkin wrote:
> Load memory window setup for pci from host.
> This makes it possible for host to make sure
> setup matches hardware exactly: especially important
> for when ACPI tables are loaded from host.
> This will also make it easier to add more chipsets
> down the road.
> Signed-off-by: Michael S. Tsirkin
>
> src/paravirt.c | 25 ++++++++++++++++++++++++-
>  src/pciinit.c  | 54 ++++++++++++++++++++++++++++++++++++++++--------------
>  src/util.h     |  8 +++++++-
>  3 files changed, 71 insertions(+), 16 deletions(-)
>
> diff --git a/src/paravirt.c b/src/paravirt.c
> index 85aa423..067a4dc 100644
> --- a/src/paravirt.c
> +++ b/src/paravirt.c
> @@ -91,14 +91,35 @@ qemu_preinit(void)
>      dprintf(1, "Ram Size=0x%08x (0x%016llx high)\n", RamSize, RamSizeOver4G);
>  }
>
> +struct pci_mem *qemu_get_pci_mem(void)
> +{
> int psize;
> struct pci_mem *mem = romfile_loadfile("etc/pci-info", &psize);
> if (!mem)
>    return NULL;
> if (psize < sizeof *mem) {

Please always add parenthesis to sizeof calls.
>    free(mem);
>    return NULL;
> }
> mem->start32 = le64_to_cpu(mem->start32);
> mem->end32 = le64_to_cpu(mem->end32);
> mem->start64 = le64_to_cpu(mem->start64);
> mem->end64 = le64_to_cpu(mem->end64);
> return mem;
>
> +}
>
> void
>  qemu_platform_setup(void)
>  {
>
> struct pci_mem *mem;
>
> if (!CONFIG_QEMU)
>        return;
>
> qemu_cfg_init();
>
> mem = qemu_get_pci_mem();
>
> if (runningOnXen()) {
>        pci_probe_devices();
>        xen_hypercall_setup();
>
> @@ -107,7 +128,7 @@ qemu_platform_setup(void)
>      }
>  // Initialize pci
> pci_setup();
> pci_setup(mem);

I don't understand why this code is being added to paravirt.c.  Just
update pciinit.c with the romfile_loadfile() calls it needs.  There is
no reason to change the order of initialization or to add parameters
to pci_setup.

Otherwise, I'm fine with having QEMU send the PCI windows explicitly
to SeaBIOS.

-Kevin

[PATCH 2/2] pci: load memory window setup from host

[Michael S. Tsirkin]

On Mon, Apr 29, 2013 at 07:27:29PM -0400, Kevin O'Connor wrote:
> On Mon, Apr 29, 2013 at 06:21:40PM +0300, Michael S. Tsirkin wrote:
> > Load memory window setup for pci from host.
> > This makes it possible for host to make sure
> > setup matches hardware exactly: especially important
> > for when ACPI tables are loaded from host.
> > This will also make it easier to add more chipsets
> > down the road.
> > Signed-off-by: Michael S. Tsirkin
> >
> > src/paravirt.c | 25 ++++++++++++++++++++++++-
> >  src/pciinit.c  | 54 ++++++++++++++++++++++++++++++++++++++++--------------
> >  src/util.h     |  8 +++++++-
> >  3 files changed, 71 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/paravirt.c b/src/paravirt.c
> > index 85aa423..067a4dc 100644
> > --- a/src/paravirt.c
> > +++ b/src/paravirt.c
> > @@ -91,14 +91,35 @@ qemu_preinit(void)
> >      dprintf(1, "Ram Size=0x%08x (0x%016llx high)\n", RamSize, RamSizeOver4G);
> >  }
> >
> > +struct pci_mem *qemu_get_pci_mem(void)
> > +{
> > int psize;
> > struct pci_mem *mem = romfile_loadfile("etc/pci-info", &psize);
> > if (!mem)
> >    return NULL;
> > if (psize < sizeof *mem) {
>
> Please always add parenthesis to sizeof calls.
> >    free(mem);
> >    return NULL;
> > }
> > mem->start32 = le64_to_cpu(mem->start32);
> > mem->end32 = le64_to_cpu(mem->end32);
> > mem->start64 = le64_to_cpu(mem->start64);
> > mem->end64 = le64_to_cpu(mem->end64);
> > return mem;
> >
> > +}
> >
> > void
> >  qemu_platform_setup(void)
> >  {
> >
> > struct pci_mem *mem;
> >
> > if (!CONFIG_QEMU)
> >        return;
> >
> > qemu_cfg_init();
> >
> > mem = qemu_get_pci_mem();
> >
> > if (runningOnXen()) {
> >        pci_probe_devices();
> >        xen_hypercall_setup();
> >
> > @@ -107,7 +128,7 @@ qemu_platform_setup(void)
> >      }
> >  // Initialize pci
> > pci_setup();
> > pci_setup(mem);
>
> I don't understand why this code is being added to paravirt.c.  Just
> update pciinit.c with the romfile_loadfile() calls it needs.  There is
> no reason to change the order of initialization or to add parameters
> to pci_setup.

Don't we need to do qemu_cfg_init to load the
list of files before we can call romfile_loadfile?
> Otherwise, I'm fine with having QEMU send the PCI windows explicitly
> to SeaBIOS.
>
> -Kevin