More build breakage: signrom.sh vs kvmvapic.bin

Here is another source of build breakage that popped up over a month ago.

The patch below fixes the build but I'm wondering why the file in question
is being built on anything but Linux. It seems to be used for KVM support
and thus has no relevance on any other OS.

  AS    optionrom/kvmvapic.o
  Building optionrom/kvmvapic.img
  Building optionrom/kvmvapic.raw
  Signing optionrom/kvmvapic.bin
/buildbot-qemu/default_openbsd_current/build/scripts/signrom.sh[31]:           018                                                             * 512 - 1 : bad number `018'
gmake[1]: *** [kvmvapic.bin] Error 1

diff --git a/scripts/signrom.sh b/scripts/signrom.sh
index 9dc5c63..2d421e9 100755
--- a/scripts/signrom.sh
+++ b/scripts/signrom.sh
 <at>  <at>  -27,8 +27,8  <at>  <at>  sum=0

 # find out the file size
 x=`dd if="$1" bs=1 count=1 skip=2 2>/dev/null | od -t u1 -A n`
-#size=`expr $x \* 512 - 1`
-size=$(( $x * 512 - 1 ))
+size=`expr $x \* 512 - 1`
+#size=$(( $x * 512 - 1 ))

 # now get the checksum
 nums=`od -A n -t u1 -v -N $size "$1"`

--

-- 

Re: [PATCH 3/4] Switch from array based resource allocation to list

On Wed, Mar 28, 2012 at 05:54:10PM +1300, Alexey Korolev wrote:
> In this patch instead of array based resource allocation approach
> we calculate resource addresses linked lists of pci_region_entry structures.

Thanks.  I still think this migration can be done more seamlessly.  I
played with your patches a bit and came up with the attached patches
that do just code movement - no alogorithm changes.  See the attached.

Also, I think we should look to commit after the next SeaBIOS release.

-Kevin

From 2f6e81d884dfbb01a12ddfb10a64bf87e864a19c Mon Sep 17 00:00:00 2001
From: Alexey Korolev <alexey.korolev <at> endace.com>
Date: Wed, 28 Mar 2012 17:41:41 +1300
Subject: [PATCH 1/3] Added a pci_region_entry structure
To: seabios <at> seabios.org

In this patch the pci_region_entry structure is introduced.
The pci_device->bars are removed. The information from
pci_region_entry is used to program pci bars.

Signed-off-by: Alexey Korolev <alexey.korolev <at> endace.com>
Signed-off-by: Kevin O'Connor <kevin <at> koconnor.net>
---
 src/pci.h     |    5 --
 src/pciinit.c |  115 +++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 79 insertions(+), 41 deletions(-)

Re: [PATCHv2] piix: fix up/down races + document

On Sun, Apr 01, 2012 at 11:22:33AM +0300, Michael S. Tsirkin wrote:
> > And if pci-hotplug could be done as cpu-hotplug, then it would simplify pci-hotplug
> > (guest would only need to read PCI_BASE) and probably it would be possible to unify hotplug
> > code for pci/cpu. But that for sure will break compatibility.
> 
> Which isn't really acceptable.
> 
I do not think it should be taken for granted. We should be able to get
rid of a crud once in a while.

--
			Gleb.

Re: [PATCHv2] piix: fix up/down races + document

On Fri, Mar 30, 2012 at 06:05:26PM +0200, Igor Mammedov wrote:
> On 03/29/2012 02:51 PM, Michael S. Tsirkin wrote:
> >piix acpi interface suffers from the following 2 issues:
> >
> >1.
> >- delete device a
> >- quickly add device b in another slot
> >
> >if we do this before guest reads the down register,
> >the down event is discarded and device will never
> >be deleted.
> >
> >2.
> >- delete device a
> >- quickly reset before guest can respond
> >
> >interrupt is reset and guest will never eject the device.
> >
> >To fix this, we implement two changes:
> >
> >1. Add two new registers:
> >CLR_UP
> >CLR_DOWN
> >bios will now write to these the value it read from UP/DOWN
> >
> >2. on reset, remove all devices which have DOWN bit set
> >
> >For compatibility with old guests, we also clear
> >the DOWN bit on write to EJ0 for a device.
> >

Re: More build breakage: signrom.sh vs kvmvapic.bin

On 2012-04-01 04:48, Brad Smith wrote:
> Here is another source of build breakage that popped up over a month ago.
> 
> The patch below fixes the build but I'm wondering why the file in question
> is being built on anything but Linux. It seems to be used for KVM support
> and thus has no relevance on any other OS.

It is only build on x86 unix hosts minus Mac. We are shipping it as
binary for the rest. There is WIP on getting KVM working for Solaris, so
this may not be of Linux-only interest in the future.

> 
> 
>   AS    optionrom/kvmvapic.o
>   Building optionrom/kvmvapic.img
>   Building optionrom/kvmvapic.raw
>   Signing optionrom/kvmvapic.bin
> /buildbot-qemu/default_openbsd_current/build/scripts/signrom.sh[31]:           018                                                             * 512 - 1 : bad number `018'
> gmake[1]: *** [kvmvapic.bin] Error 1
> 
> 
> diff --git a/scripts/signrom.sh b/scripts/signrom.sh
> index 9dc5c63..2d421e9 100755
> --- a/scripts/signrom.sh
> +++ b/scripts/signrom.sh
>  <at>  <at>  -27,8 +27,8  <at>  <at>  sum=0
>  
>  # find out the file size
>  x=`dd if="$1" bs=1 count=1 skip=2 2>/dev/null | od -t u1 -A n`
> -#size=`expr $x \* 512 - 1`

Re: More build breakage: signrom.sh vs kvmvapic.bin

On 2012-04-01 10:36, Jan Kiszka wrote:
> On 2012-04-01 04:48, Brad Smith wrote:
>> Here is another source of build breakage that popped up over a month ago.
>>
>> The patch below fixes the build but I'm wondering why the file in question
>> is being built on anything but Linux. It seems to be used for KVM support
>> and thus has no relevance on any other OS.
> 
> It is only build on x86 unix hosts minus Mac. We are shipping it as
> binary for the rest. There is WIP on getting KVM working for Solaris, so
> this may not be of Linux-only interest in the future.
> 

Oh, and I totally forgot to mention that we are able to emulate the KVM
VAPIC in the absence of host-side KVM support.

Jan

Re: [PATCH 3/4] Switch from array based resource allocation to list

On Sun, Apr 01, 2012 at 03:28:34AM -0400, Kevin O'Connor wrote:
> On Wed, Mar 28, 2012 at 05:54:10PM +1300, Alexey Korolev wrote:
> > In this patch instead of array based resource allocation approach
> > we calculate resource addresses linked lists of pci_region_entry structures.
> 
> Thanks.  I still think this migration can be done more seamlessly.  I
> played with your patches a bit and came up with the attached patches
> that do just code movement - no alogorithm changes.  See the attached.
> 
> Also, I think we should look to commit after the next SeaBIOS release.

Looking closer at your new allocation algorithm, I see that it
effectively allocates largest sizes first.  It occurred to me that an
easy way to do this is to keep the pci_region_entry list sorted by
size.  See the patch below as an example (based on top of the previous
email I sent).

-Kevin

diff --git a/src/pciinit.c b/src/pciinit.c
index f2c839a..bfee178 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
 <at>  <at>  -41,16 +41,13  <at>  <at>  struct pci_region_entry {
     struct pci_bus *child_bus;
     struct pci_bus *parent_bus;
     struct pci_region_entry *next;
-    struct pci_region_entry **pprev;
 };

Re: [PATCH] vhost-net: Move asserts to after check for end < start

On Fri, Mar 30, 2012 at 07:21:22PM -0700, Josh Durgin wrote:
> On 12/16/2011 12:33 PM, Bruce Rogers wrote:
> >When migrating a vm using vhost-net we hit the following assertion:
> >
> >qemu-kvm: /usr/src/packages/BUILD/qemu-kvm-0.15.1/hw/vhost.c:30:
> >vhost_dev_sync_region: Assertion `start / (0x1000 * (8 *
> >sizeof(vhost_log_chunk_t)))<  dev->log_size' failed.
> 
> I consistently hit this assert while testing live migration with
> qemu 1.0.1 and the master branch. Applying this patch allowed live
> migration to complete successfully. Maybe it could be reviewed and
> merged?

Yes, thanks for the reminder. I've applied a patch by Alex Williamson that
addresses this and other crashes.

> >The cases which the end<  start check is intended to catch, such as
> >for vga video memory, will also likely trigger the assertion.
> >Reorder the code to handle this correctly.
> >
> >Signed-off-by: Bruce Rogers<brogers <at> suse.com>
> >---
> >  hw/vhost.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/vhost.c b/hw/vhost.c
> >index 0870cb7..7309f71 100644
> >--- a/hw/vhost.c
> >+++ b/hw/vhost.c
> > <at>  <at>  -26,11 +26,11  <at>  <at>  static void vhost_dev_sync_region(struct vhost_dev *dev,

Re: More build breakage: signrom.sh vs kvmvapic.bin

On 01/04/12 4:36 AM, Jan Kiszka wrote:
> It is only build on x86 unix hosts minus Mac. We are shipping it as
> binary for the rest. There is WIP on getting KVM working for Solaris, so
> this may not be of Linux-only interest in the future.

Ok so you then build it on Linux and Solaris. But I don't see the point of
building it on any OS that does not have KVM since you'll never use it 
there.

> signrom.sh is horribly slow and should be replaced soon, see
> http://thread.gmane.org/gmane.comp.emulators.qemu/139368

I don't care if it's slow. The build is broken. That is what matters.

--

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Re: [PATCHv2] piix: fix up/down races + document

On Sun, Apr 01, 2012 at 11:26:18AM +0300, Gleb Natapov wrote:
> On Sun, Apr 01, 2012 at 11:22:33AM +0300, Michael S. Tsirkin wrote:
> > > And if pci-hotplug could be done as cpu-hotplug, then it would simplify pci-hotplug
> > > (guest would only need to read PCI_BASE) and probably it would be possible to unify hotplug
> > > code for pci/cpu. But that for sure will break compatibility.
> > 
> > Which isn't really acceptable.
> > 
> I do not think it should be taken for granted. We should be able to get
> rid of a crud once in a while.

In general, yes. It needs to be well planned several years in
advance though. Something like
http://www.kernel.org/doc/Documentation/feature-removal-schedule.txt
might be a model.

Breaking compatibility would bring very little
gain in this case though, right?

> --
> 			Gleb.