Kevin O'Connor | 4 Nov 2010 14:19

seabios fails to send output to the QEMU BIOS debug port

On Thu, Nov 04, 2010 at 03:03:10PM +0800, Lin Ming wrote:
> Hi,
> 
> I just find that I can't send output to the QEMU BIOS debug port.
> It's caused by commit d4bded451.
> 
> In qemu-kvm/hw/acpi_piix4.c,
> #define ACPI_DBG_IO_ADDR  0xb044
> 
> Don't know why the DBG was changed from 0xb044 to 0x0402.
> But it still does not work if I changed the port back to 0xb044.
> -        OperationRegion (DBG, SystemIO, 0x0402, 0x01)
> +        OperationRegion (DBG, SystemIO, 0xb044, 0x01)
> 
> Any fix?

That was intentional.  The new system sends output using the regular
bios debug mechanism - you should be able to see that output by adding
the following to qemu command line:

        -chardev stdio,id=seabios \
        -device isa-debugcon,iobase=0x402,chardev=seabios

The above should be simpler then having to recompile qemu with acpi
debugging.

-Kevin

Isaku Yamahata | 6 Nov 2010 02:11
Picon
Favicon

FWD:[PATCH] [Seabios] Over 4GB address ranges for 64bit PCI BARs]

FWD

----- Forwarded message from Alexey Korolev <alexey.korolev at endace.com> -----

Subject: [PATCH] [Seabios] Over 4GB address ranges for 64bit PCI BARs
X-Mailer: Evolution 2.24.3 

Hi,

We have seen some issues with 64bit PCI address and large BAR region support in 
seabios. 

On attempting to register a 64bit BAR of 1GB size we had some strange failures
in qemu. After some debugging we find out that source of the issue was in 
seabios PCI enumeration code. 

The issue takes place because of u32 overflow in pciinit.c. 
?.........
             if (ALIGN(*paddr, size) + size >= BUILD_PCIPREFMEM_END) {
                 dprintf(1,
                         "prefmem region of (bdf 0x%x bar %d) can't be mapped. "
                         "decrease BUILD_PCIMEM_SIZE and recompile. size %x\n",
                         bdf, region_num, BUILD_PCIPREFMEM_SIZE);
                 size = 0;
             }
?...........
       if (size > 0) {
            *paddr = ALIGN(*paddr, size);
            pci_set_io_region_addr(bdf, region_num, *paddr);
            *paddr += size;
(Continue reading)

Isaku Yamahata | 6 Nov 2010 02:13
Picon
Favicon

FWD:[PATCH] [Seabios] Over 4GB address ranges for 64bit PCI BARs

FWD

----- Forwarded message from Isaku Yamahata <yamahata at valinux.co.jp> -----

Subject: Re: [PATCH] [Seabios] Over 4GB address ranges for 64bit PCI BARs
User-Agent: Mutt/1.5.19 (2009-01-05)

Hi.
The current BAR allocation doesn't check overflow and some patches
are floating around which aren't merged yet.
There are several issues.

- overflow check
  This should be fixed.
  Some patches are proposed. None hasn't been merged yet.
  Your patch also addresses this issue.
  http://www.seabios.org/pipermail/seabios/2010-July/000794.html
  http://www.seabios.org/pipermail/seabios/2010-October/001089.html

- abstraction of pci bar allocation 
  This is very coding detail. This should be addressed.
  The current PCI BAR allocation uses primitives. So overflow check becomes
  ugly. So it would desirable to abstract it.
  Kevin suggested to use the existing PMM framework. But I concluded PMM
  doesn't suit for it, so introduced new api. But I haven't got any feedback
  from him yet.

- >4GB 64bit bar allocation
  Your patche tries to address this issue. But it breaks PCI-to-PCI
  bridge filtering support.
(Continue reading)

Kevin O'Connor | 7 Nov 2010 18:14

[PATCH v3 0/2] pciinit: fix overflow when bar allocation

On Thu, Oct 28, 2010 at 03:54:34PM +0900, Isaku Yamahata wrote:
> Changes v2 -> v3:
> - use [first, last] instead of [start, end)
> 
> Changes v1 -> v2:
> - add comment.
> 
> Patch description:
> This patch set fixes PCI bar allocation when bar overflow occured.
> I checked if pmm_alloc facility can be used, but it doesn't suit for
> pci bar allocation. So I resulted in new API, pci_region which
> encapsulates region allocation and overflow checks.
> The first patch introduces pci_region, and the second patch fixes
> the overflow case with pci_region.

Looks okay to me.  If there are no further comments, I'll commit.

BTW, as a minor nit - I'd prefer to put function descriptions in the
.c file next to the code instead of in the .h file - no need to resend
the patch though.

-Kevin

Alexey Korolev | 8 Nov 2010 04:35
Picon
Favicon

[PATCH] [Seabios] Over 4GB address ranges for 64bit PCI BARs

Hi,
> Hi.
> The current BAR allocation doesn't check overflow and some patches
> are floating around which aren't merged yet.
> There are several issues.
> 
> - overflow check
>   This should be fixed.
>   Some patches are proposed. None hasn't been merged yet.
>   Your patch also addresses this issue.
>   http://www.seabios.org/pipermail/seabios/2010-July/000794.html
>   http://www.seabios.org/pipermail/seabios/2010-October/001089.html

Right. It would be great if the fix get included in seabios/qemu. 

Update: I've seen today's message from Kevin. He is going to merge your
patch. So it will be good news for us. 

> - >4GB 64bit bar allocation
>   Your patche tries to address this issue. But it breaks PCI-to-PCI
>   bridge filtering support.
Hmm, it is quite possible, as we don't know a lot about seabios PCI-to-PCI bridge filtering support.
Just out of curiosity: what is the issue?

>   If the BAR size is huge (or there are too many BARs), the bar can't
>   be allocated under 4G. So several persons want seabios to allocate
>   such BARs at >4GB area complaining that OS can't use BARs that seabios
>   didn't assigned.
> 
>   Others think such BAR can be left unallocated.
(Continue reading)

Isaku Yamahata | 8 Nov 2010 05:11
Picon
Favicon

[PATCH] [Seabios] Over 4GB address ranges for 64bit PCI BARs

On Mon, Nov 08, 2010 at 04:35:38PM +1300, Alexey Korolev wrote:
> > - >4GB 64bit bar allocation
> >   Your patche tries to address this issue. But it breaks PCI-to-PCI
> >   bridge filtering support.
> Hmm, it is quite possible, as we don't know a lot about seabios PCI-to-PCI bridge filtering support.
> Just out of curiosity: what is the issue?

It's pci_bios_init_device_bridge() in pciinit.c.
The function touches pci_bios_io_addr, pci_bios_mem_addr, and
pci_bios_prefmem_addr.
So we need to modify, not only pci_bios_allocate_region(),
but also pci_bios_init_device_bridge().

The function programs the P2P bridge to forward IO/memory access
on primary pci bus to secondary pci bus.
It needs to be aware of 64bit BAR allocation.

> >   If the BAR size is huge (or there are too many BARs), the bar can't
> >   be allocated under 4G. So several persons want seabios to allocate
> >   such BARs at >4GB area complaining that OS can't use BARs that seabios
> >   didn't assigned.
> > 
> >   Others think such BAR can be left unallocated.
> >   Seabios role is to setup minimal basic environment for bootloader
> >   to boot OS, 64bit bar allocation is beyond it's role.
> >   bootloader/rombios usually doesn't handle BARs that is allocated
> >   beyond 4GB, and Modern OSes can re-arrange PCI bar allocation itself.
> >   So 64bit bar allocation support wouldn't be needed.
> > 
> >   I'm not sure if there is enough demand to support 64bit BAR allocation
(Continue reading)

Kevin O'Connor | 9 Nov 2010 00:59

[PATCH] fix virtio-blk failure after reboot

On Wed, Sep 15, 2010 at 06:31:44PM +0200, Gleb Natapov wrote:
> vring_virtqueue should be zeroed otherwise old values will be reused
> after reboot.
> 
> Signed-off-by: Gleb Natapov <gleb at redhat.com>
> diff --git a/src/virtio-blk.c b/src/virtio-blk.c
> index 34d7863..7a25826 100644
> --- a/src/virtio-blk.c
> +++ b/src/virtio-blk.c
>  <at>  <at>  -109,6 +109,7  <at>  <at>  init_virtio_blk(u16 bdf)
>          goto fail;
>      }
>      memset(vdrive_g, 0, sizeof(*vdrive_g));
> +    memset(vq, 0, sizeof(*vq));
>      vdrive_g->drive.type = DTYPE_VIRTIO;
>      vdrive_g->drive.cntl_id = bdf;
>      vdrive_g->vq = vq;

This didn't make it into SeaBIOS v0.6.1.  Should we add this to the
stable branch as v0.6.1.2?  Any other bugfixes that need to go in to
the stable branch (maybe Isaku's pci overflow patches)?

-Kevin

Gleb Natapov | 9 Nov 2010 07:46
Picon
Favicon

[PATCH] fix virtio-blk failure after reboot

On Mon, Nov 08, 2010 at 06:59:37PM -0500, Kevin O'Connor wrote:
> On Wed, Sep 15, 2010 at 06:31:44PM +0200, Gleb Natapov wrote:
> > vring_virtqueue should be zeroed otherwise old values will be reused
> > after reboot.
> > 
> > Signed-off-by: Gleb Natapov <gleb at redhat.com>
> > diff --git a/src/virtio-blk.c b/src/virtio-blk.c
> > index 34d7863..7a25826 100644
> > --- a/src/virtio-blk.c
> > +++ b/src/virtio-blk.c
> >  <at>  <at>  -109,6 +109,7  <at>  <at>  init_virtio_blk(u16 bdf)
> >          goto fail;
> >      }
> >      memset(vdrive_g, 0, sizeof(*vdrive_g));
> > +    memset(vq, 0, sizeof(*vq));
> >      vdrive_g->drive.type = DTYPE_VIRTIO;
> >      vdrive_g->drive.cntl_id = bdf;
> >      vdrive_g->vq = vq;
> 
> This didn't make it into SeaBIOS v0.6.1.  Should we add this to the
> stable branch as v0.6.1.2?  Any other bugfixes that need to go in to
> the stable branch (maybe Isaku's pci overflow patches)?
> 
Yes. Please add it to stable branch.

--
			Gleb.

Kevin O'Connor | 13 Nov 2010 16:01

[PATCH] fix virtio-blk failure after reboot

On Tue, Nov 09, 2010 at 08:46:43AM +0200, Gleb Natapov wrote:
> On Mon, Nov 08, 2010 at 06:59:37PM -0500, Kevin O'Connor wrote:
> > On Wed, Sep 15, 2010 at 06:31:44PM +0200, Gleb Natapov wrote:
> > > vring_virtqueue should be zeroed otherwise old values will be reused
> > > after reboot.
> > > 
> > > Signed-off-by: Gleb Natapov <gleb at redhat.com>
> > > diff --git a/src/virtio-blk.c b/src/virtio-blk.c
> > > index 34d7863..7a25826 100644
> > > --- a/src/virtio-blk.c
> > > +++ b/src/virtio-blk.c
> > >  <at>  <at>  -109,6 +109,7  <at>  <at>  init_virtio_blk(u16 bdf)
> > >          goto fail;
> > >      }
> > >      memset(vdrive_g, 0, sizeof(*vdrive_g));
> > > +    memset(vq, 0, sizeof(*vq));
> > >      vdrive_g->drive.type = DTYPE_VIRTIO;
> > >      vdrive_g->drive.cntl_id = bdf;
> > >      vdrive_g->vq = vq;
> > 
> > This didn't make it into SeaBIOS v0.6.1.  Should we add this to the
> > stable branch as v0.6.1.2?  Any other bugfixes that need to go in to
> > the stable branch (maybe Isaku's pci overflow patches)?
> > 
> Yes. Please add it to stable branch.

I added it and tagged it as v0.6.1.2.

-Kevin

(Continue reading)

Kevin O'Connor | 13 Nov 2010 16:09

[PATCH v3 0/2] pciinit: fix overflow when bar allocation

On Thu, Oct 28, 2010 at 03:54:34PM +0900, Isaku Yamahata wrote:
> Changes v2 -> v3:
> - use [first, last] instead of [start, end)
> 
> Changes v1 -> v2:
> - add comment.
> 
> Patch description:
> This patch set fixes PCI bar allocation when bar overflow occured.
> I checked if pmm_alloc facility can be used, but it doesn't suit for
> pci bar allocation. So I resulted in new API, pci_region which
> encapsulates region allocation and overflow checks.
> The first patch introduces pci_region, and the second patch fixes
> the overflow case with pci_region.

Thanks.  I've committed these.  I put them on the master branch - I
can also place them on the stable branch if they're needed there.

-Kevin


Gmane