Re: [Qemu-devel] scp during migration with vhost fails

2013-02-03 Thread Michael S. Tsirkin
On Fri, Feb 01, 2013 at 06:03:32PM +0800, Jason Wang wrote:
> Hello all:
> 
> During testing, I find doing scp during migration with vhost fails with 
> warnings in guest like:
> 
> Corrupted MAC on input.
> Disconnecting: Packet corrupt.
> lost connection
> 
> Here's the bisect result:
> 
> Commit a01672d3968cf91208666d371784110bfde9d4f8 kvm: convert to 
> MemoryListener 
> API is the last commit that works well.
> 
> With commit 04097f7c5957273c578f72b9bd603ba6b1d69e33 vhost: convert to 
> MemoryListener API, guest network is unusable with warning of "bad gso type"
> 
> With commit d743c382861eaa1e13f503b05aba5a382a7e7f7c vhost: fix incorrect 
> userspace address, guest network is available, but scp during migration may 
> fail.
> 
> Looks like the issue is related to memory api, any thoughts?
> 
> Thanks

Hmm this is worrying.
The problem doesn't seem to reproduce for me.  One explanation would be
that the memory table is setup incorrectly, if this happens it's very
worrying.
I think a good way to debug would be by dumping the vhost tables before
and after the change, verify that they are identical.

-- 
MST



Re: [Qemu-devel] [PATCH v3] bitops: unify bitops_ffsl with the one in host-utils.h, call it bitops_ctzl

2013-02-03 Thread Peter Maydell
On 3 February 2013 03:47, Paolo Bonzini  wrote:
> Actually I'm pretty sure that var == 0 is not possible in hbitmap.  But
> I still prefer having total functions, and also keeping the function
> monotonic.

Er, ctz() isn't monotonic:
  1 => 0
  2 => 1
  3 => 0
  4 => 2
etc...

-- PMM



Re: [Qemu-devel] [OpenBIOS] Fix double nvram entry on newworld

2013-02-03 Thread Alexander Graf


Am 02.02.2013 um 14:05 schrieb Blue Swirl :

> On Thu, Jan 31, 2013 at 3:10 PM, Alexander Graf  wrote:
>> 
>> On 30.01.2013, at 11:29, Amadeusz Sławiński wrote:
>> 
>>> Fix double nvram entry on newworld
>>> 
>>> There are two nvram entries on newworld (for example qemu -M mac99)
>>> 
>>> The first one (nvram@fff04000) has initialized .properties while the
>>> other one has words.
>>> 
>>> 0 > dev / ls
>>> ...
>>> fff75e24 pci@f200
>>> fff77848 nvram@fff04000
>>> fff778e0 nvram
>>> ok
>>> 0 > dev /nvram@fff04000  ok
>>> 0 > words
>>> ok
>>> 0 > .properties
>>> name  "nvram"
>>> #bytes2000
>>> reg   fff04000   4000
>>> device_type   "nvram"
>>> compatible"nvram,flash"
>>> ok
>>> 0 > dev /nvram@0  ok
>>> 0 > words close open seek write read size
>>> ok
>>> 0 > .properties
>>> name  "nvram"
>>> ok
>>> 
>>> This patch fixes initialization, so only one node is created
>>> containing both .properties and words.
>>> 
>>> 0 > dev / ls
>>> ...
>>> fff75e24 pci@f200
>>> fff77868 nvram@fff04000
>>> ok
>>> 0 > dev /nvram@fff04000  ok
>>> 0 > words close open seek write read size
>>> ok
>>> 0 > .properties
>>> name  "nvram"
>>> #bytes2000
>>> reg   fff04000   4000
>>> device_type   "nvram"
>>> compatible"nvram,flash"
>>> ok
>>> 
>>> Signed-off-by: Amadeusz Sławiński 
>> 
>> Blue, once this patch is in the tree, do you think you could rebuild 
>> OpenBIOS for QEMU, so that it will land in 1.4?
> 
> Sorry, it didn't happen before the freeze. I'm not sure if new
> OpenBIOS images qualifies as a bug fix.

I would say yes ;).

Anthony, any objections from your side?


Alex

> Most of the changes since
> r1063 (now in QEMU) are bug fixes, though:
> a5af2b3 macio.c: Fix double nvram entry on newworld
> ff86ced SPARC32: WIM register update delay
> 8e9793b SPARC32: Clear FP register
> c23f9f7 esp.c: fix SCSI command code displayed in do_command() debug 
> statement.
> 39988d6 esp.c: fix TEST_UNIT_READY SCSI command length.
> 75e29de Revert configuration change from previous commit.
> d5df782 video.c: Fix compilation when CONFIG_DEBUG_CONSOLE_VIDEO is
> set to false.
> 91119ec video.c: Fix incorrect sized type in fill_rect().
> 3caf41b mac-parts.c: Update bootpath to reflect the chosen partition
> if unspecified.
> ec237bb Switch partition argument parsing to use left-parse-string as
> per CHRP bindings.
> e4ada76 PPC: Mimic Apple's OpenFirmware behaviour if a divide by zero occurs.
> 7e64c09 mac-parts.c: Add Apple_Bootstrap to partition types considered
> for Mac boot.
> 5b48904 PPC: Fix filll word used by BootX
> 1711362 Ignore any attempts to emit a character to stdout when stdout
> is set to 0.
> 6294e00 adb_kbd.c: Implement dummy get-key-map word for the ADB
> keyboard package.
> 1484d2b PPC: Fix mapping of OpenBIOS ROM in RAM copy within OFMEM
> 69c27c4 PPC: Fix next slot eviction
> 1be3a15 video.c: Place framebuffer address in frame-buffer-adr
> e11cacd mac-parts.c: Fix detection of wrapped HFS+ volumes.
> 7694794 PPC: Implement filll (fill long) word for QEMU/PPC as required by 
> BootX.
> 1fbbbd2 Redefine "to" word in device.fs to allow the current package
> to be set like a standard value.
> 1da510f Rework mac-parts.c to use CHRP-compliant partition search,
> followed by Apple OF partition search.
> b70a1f7 PPC: Rework assignment of keyboard devalias.
> dd37f6b Add a default "decode-unit" word for devices that don't
> implement their own.
> 7a370cc PPC: Add keyboard device alias as a duplicate of stdin.
> 58106df Fix dir cd:,\ (no partition specified) when reading from Mac 
> partitions.
> 5f23f2c Improve dir word by reducing complexity and adding some more
> diagnostics.
> a78e3b4 The spin word is set by BootX when Mac OS X is booting.
> 37d2f65 PPC32: Enable local variables for the PPC32 build.
> 538d404 Implementation of Forth local variables for OpenBIOS.
> 5bb1484 amd64: Fix compilation from last commit to implement "dir"
> word for HFS+ filesystem.
> 0495b71 Add initial implementation of "dir" word for HFS+ filesystems.
> 21ed61f Fix HFS+ display for non-ASCII characters.
> 5b22479 Fix bug related to opening backup volumes in libhfsp's volume_open().
> f095c85 ppc qemu: Increase PCI hole for heathrow
> 
>> 
>> 
>> Thanks,
>> 
>> Alex
>> 
>>> 
>>> diff -uNr a/drivers/macio.c b/drivers/macio.c
>>> --- a/drivers/macio.c   2013-01-28 12:16:54.849868216 +0100
>>> +++ b/drivers/macio.c   2013-01-28 12:17:27.595867493 +0100
>>> @@ -57,12 +57,6 @@
>>>} else {
>>>nvram_offset = NW_IO_NVRAM_OFFSET;
>>>nvram_size = NW_IO_NVRAM_SIZE;
>>> -push_str("/");
>>> -fword("find-device");
>>> -fword("new-device");
>>> -push_str("nvram");
>>> -fword("device-name");
>>> -fword("finish-device"

Re: [Qemu-devel] How many msi-x vectors should be allocated for the virtio-serial device?

2013-02-03 Thread Michael S. Tsirkin
On Sat, Feb 02, 2013 at 12:01:52AM +0100, Paolo Bonzini wrote:
> Il 31/01/2013 12:25, Michael S. Tsirkin ha scritto:
> > On Thu, Jan 31, 2013 at 10:13:11AM +0200, Gal Hammer wrote:
> >> Hi,
> >>
> >> How many msi-x vectors should be allocated for the virtio-serial device?
> >>
> >> I'm asking this as it seems that a proposed patch
> >> (http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg02094.html)
> >> was not accepted and I re-encountered this issue while trying to
> >> upgrade the virtio-serial's Windows driver to use msi-x vectors.
> >>
> >> The virtio-serial's Linux module tries to use one vector per
> >> virtqueue (every serial port have two virtqueues) with a fall back
> >> to using only two vectors
> >> (http://lxr.linux.no/linux+v3.7.2/drivers/virtio/virtio_pci.c#L539).
> >> The problem is that qemu's virtio-pci device allocate less vectors
> >> than the modules expects. So, for example, if a serial device have
> >> 16 ports, 17 vectors are allocated. The module tries to use 34
> >> vectors, fails and choose to use only 2, leaving 15 unused vectors.
> >>
> >> Is it possible to increase the vectors number from
> >> "proxy->serial.max_virtserial_ports + 1" to
> >> "(proxy->serial.max_virtserial_ports + 1) * 2"?
> >>
> >> Thanks,
> >>
> >> Gal.
> > 
> > Allocated MSI-X vectors on x86 are a limited resource.  Let's not waste
> > them. From what you say virtio serial works fine with two
> > vectors and I do not see any reason to let us use more
> > until someone can show an important workload where this helps.
> > 
> > So I think we need something like the below, but we also
> > need to handle 1.3 and older compatibility so the
> > patch below shouldn't be applied. Want to try
> > writing up the complete patch?
> 
> I think the patch should instead be something like
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 9abbcdf..24e8232 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -1154,7 +1154,7 @@ static const TypeInfo virtio_net_info = {
>  
>  static Property virtio_serial_properties[] = {
>  DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, 
> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> -DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 
> DEV_NVECTORS_UNSPECIFIED),
> +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
>  DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
>  DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
>  DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, 
> serial.max_virtserial_ports, 31),
> 
> plus the backwards-compatibility stuff.
> 
> Paolo

Makes sense, but the logic in virtio_serial_init_pci is
then dead code and should go away.

> > -->
> > 
> > serial: use 2 vectors by default
> > 
> > Guests only utilize 2 vectors and this seems to be enough,
> > so let's only allocate as much.
> > serial is likely not so performance intensive to require more,
> > but if yes users can always override the nvectors property.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > 
> > ---
> > 
> > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> > index 9abbcdf..bb0f60e 100644
> > --- a/hw/virtio-pci.c
> > +++ b/hw/virtio-pci.c
> > @@ -975,8 +975,7 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
> >  if (!vdev) {
> >  return -1;
> >  }
> > -vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED
> > -? 
> > proxy->serial.max_virtserial_ports + 1
> > +vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED ? 2
> >  : proxy->nvectors;
> >  virtio_init_pci(proxy, vdev);
> >  proxy->nvectors = vdev->nvectors;
> > 



Re: [Qemu-devel] [OpenBIOS] Fix double nvram entry on newworld

2013-02-03 Thread Andreas Färber
Am 03.02.2013 12:25, schrieb Alexander Graf:
> 
> Am 02.02.2013 um 14:05 schrieb Blue Swirl :
> 
>> On Thu, Jan 31, 2013 at 3:10 PM, Alexander Graf  wrote:
>>>
>>> Blue, once this patch is in the tree, do you think you could rebuild 
>>> OpenBIOS for QEMU, so that it will land in 1.4?
>>
>> Sorry, it didn't happen before the freeze. I'm not sure if new
>> OpenBIOS images qualifies as a bug fix.
> 
> I would say yes ;).

+1

Quite a few bugfixes went into OpenBIOS recently, I'm truely amazed. :-)

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH WIP 0/4] vhost-scsi: new device supporting the tcm_vhost Linux kernel module

2013-02-03 Thread Michael S. Tsirkin
On Fri, Feb 01, 2013 at 11:46:28AM +0800, Asias He wrote:
> On 01/31/2013 07:12 PM, Michael S. Tsirkin wrote:
> > On Wed, Jan 30, 2013 at 05:41:22PM +0100, Paolo Bonzini wrote:
> >> Ok, so here is my attempt at a vhost-scsi device.  I'm creating an
> >> entirely separate device, with the common parts of virtio-scsi and
> >> vhost-scsi (actually little more than the initialization) grouped into
> >> a VirtIOSCSICommon type.  The device is used simply like "-device
> >> vhost-scsi-pci,wwpn=WWPN", with all configuration done in configfs
> >> beforehand.
> >>
> >> As expected, using a separate device finds a snag: vhost-scsi is passing
> >> force=false to vhost_dev_init, and the BIOS does not use MSI-X so it
> >> will actually use the non-vhost implementation which is wrong.  I fixed
> >> this by passing force=true; I'm not sure what that would break, but I
> >> figured "not much" since the BIOS polls and does not rely on interrupts.
> >>
> >> That makes vhost start, but it still doesn't work for me with a 3.7.2
> >> kernel on the host.  Even Nick's patches hang the guest as soon as vhost
> >> starts, and I get the same behavior with mine.  (Of course with my
> >> patches the BIOS hangs and you never reach Linux; but try a BIOS without
> >> virtio-scsi support, and you'll see Linux hanging in the same way).
> >>
> >> Here is my configuration:
> >>
> >>   cd /sys/kernel/config/target
> >>   mkdir -p core/fileio_0/fileio
> >>   echo 'fd_dev_name=/home/pbonzini/test.img,fd_dev_size=5905580032' > 
> >> core/fileio_0/fileio/control 
> >>   echo 1 > core/fileio_0/fileio/enable
> >>   mkdir -p vhost/naa.600140554cf3a18e/tpgt_0/lun/lun_0
> >>   cd vhost/naa.600140554cf3a18e/tpgt_0
> >>   ln -sf ../../../../../core/fileio_0/fileio/ lun/lun_0/virtual_scsi_port
> >>   echo naa.60014053226f0388 > nexus
> >>
> >> Nick's patches are run with "-vhost-scsi 
> >> id=vs,tpgt=0,wwpn=naa.600140554cf3a18e
> >> -device virtio-scsi-pci,vhost-scsi=vs".  Perhaps I'm doing something wrong.
> >>
> >> Another small bug I found is an ordering problem between
> >> VHOST_SET_VRING_KICK and VHOST_SCSI_SET_ENDPOINT.  Starting the vq
> >> causes a "vhost_scsi_handle_vq endpoint not set" error in dmesg.
> >> Because of this I added the first two patches, which let me do
> >> VHOST_SCSI_SET_ENDPOINT before VHOST_SET_VRING_KICK but after setting
> >> up the vring.
> >>
> >> Unfortunately, this is not enough to fix the hang.  And anyway, it's
> >> probably simpler to avoid the two patches and remove this test from the
> >> tcm_vhost.c vhost_scsi_set/clear_endpoint functions:
> >>
> >> mutex_lock(&vs->dev.mutex);
> >> /* Verify that ring has been setup correctly. */
> >> for (index = 0; index < vs->dev.nvqs; ++index) {
> >> /* Verify that ring has been setup correctly. */
> >> if (!vhost_vq_access_ok(&vs->vqs[index])) {
> >> mutex_unlock(&vs->dev.mutex);
> >> return -EFAULT;
> >> }
> >> }
> >> mutex_unlock(&vs->dev.mutex);
> > 
> > Well userspace should initialize the kick eventfd to 0,
> > it seems to init it to 1 which is why we get the error.
> > But I think the only issue is pr_err: vhost-net already
> > ignores such a kick with no backend. So let's just
> > remove it, preferably for 3.8.
> 
> With following patch, the kick before backend is set is gone.
> 
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -169,7 +169,7 @@ static int
> virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
>  int r = 0;
> 
>  if (assign) {
> -r = event_notifier_init(notifier, false);
> +r = event_notifier_init(notifier, 1);
> 

Hmm, not the reverse?
It's also int so should be 0 not false.


> > 
> > --->
> > tcm_vhost: fix pr_err on early kick
> > 
> > It's OK to get kick before backend is set or after
> > it is cleared, we can just ignore it.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > 
> > ---
> > 
> > diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> > index b20df5c..22321cf 100644
> > --- a/drivers/vhost/tcm_vhost.c
> > +++ b/drivers/vhost/tcm_vhost.c
> > @@ -575,10 +575,8 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs)
> >  
> > /* Must use ioctl VHOST_SCSI_SET_ENDPOINT */
> > tv_tpg = vs->vs_tpg;
> > -   if (unlikely(!tv_tpg)) {
> > -   pr_err("%s endpoint not set\n", __func__);
> > +   if (unlikely(!tv_tpg))
> > return;
> > -   }
> >  
> > mutex_lock(&vq->mutex);
> > vhost_disable_notify(&vs->dev, vq);
> > 
> 
> 
> -- 
> Asias



Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Yan Vugenfirer

On Feb 3, 2013, at 1:07 AM, Anthony Liguori wrote:

> Vadim Rozenfeld  writes:
> 
>> On Sat, 2013-02-02 at 20:42 +0800, Jason Wang wrote:
>>> 
>>> Have a look at this issue. It was caused by multiqueue patch who adds a
>>> new field to virtio_net_cfg. Not sure multiqueue is the root cause since
>>> I also find even w/o multiqueue, adding any new field to virtio_net_cfg
>>> will break windows guest. Haven't had a clue on this, will continue
>>> investigate.
>> 
>> cc'ing Yan, our NDIS guy.
> 
> If it helps, mq changes the config size from 8 to 16 bytes.  If the
> driver was making an assumption about an 8-byte config size, that's
> likely what the problem is.
> 
> Regards,
> 
> Anthony Liguori

That's exactly the problem.

Best regards,
Yan.


> 
>> Thank you,
>> Vadim.
 
>> Regards,
>> 
>> Anthony Liguori
>> 
>>> commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91
>>> Author: Jason Wang 
>>> Date:   Wed Jan 30 19:12:39 2013 +0800
>>> 
>>>  virtio-net: multiqueue support
>>> 
>>>  This patch implements both userspace and vhost support for 
>>> multiple queue
>>>  virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array 
>>> of
>>>  VirtIONetQueue to VirtIONet.
>>> 
>>>  Signed-off-by: Jason Wang 
>>>  Signed-off-by: Anthony Liguori 
>>> 
>>> After this commit, win guest (winXP and win7) shows yellow
>>> exclamation sign and is unable to start the device with
>>> code 10.
>>> 
>>> FWIW.  I'm not sure it is a good idea to make a release with
>>> such a breakage, even rc0.
>>> 
>>> Thanks,
>>> 
>>> /mjt
 
>>> 
> 




[Qemu-devel] [PATCH v3 0/3] AER-KVM: Error containment of VFIO devices assigned to KVM guests

2013-02-03 Thread Pandarathil, Vijaymohan R

Add support for error containment when a VFIO device assigned to a KVM
guest encounters an error. This is for PCIe devices/drivers that support AER
functionality. When the host OS is notified of an error in a device either
through the firmware first approach or through an interrupt handled by the AER
root port driver, the error handler registered by the vfio-pci driver gets
invoked. The qemu process is signaled through an eventfd registered per
VFIO device by the qemu process. In the eventfd handler, qemu decides on
what action to take. In this implementation, guest is brought down to
contain the error.


v3:
 - Removed PCI_AER* flags from device info ioctl.
 - Incorporated feedback
v2:
 - Rebased to latest upstream stable bits
 - Changed the new ioctl to be part of VFIO_SET_IRQs ioctl
 - Added a new patch to get/put reference to a vfio device from struct device
 - Incorporated all other feedback.

---

Vijay Mohan Pandarathil(3):

[PATCH 1/3] VFIO: Wrapper for getting reference to vfio_device from device 
[PATCH 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER
[PATCH 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices

Kernel files changed

 drivers/vfio/vfio.c  | 41 -
 include/linux/vfio.h |  3 +++
 2 files changed, 35 insertions(+), 9 deletions(-)

 drivers/vfio/pci/vfio_pci.c | 43 -
 drivers/vfio/pci/vfio_pci_intrs.c   | 30 ++
 drivers/vfio/pci/vfio_pci_private.h |  1 +
 include/uapi/linux/vfio.h   |  1 +
 4 files changed, 74 insertions(+), 1 deletion(-)

Qemu files changed

 hw/vfio_pci.c  | 105 +
 linux-headers/linux/vfio.h |   1 +
 2 files changed, 106 insertions(+)



[Qemu-devel] [PATCH v3 1/3] VFIO: Wrapper for getting reference to vfio_device from device

2013-02-03 Thread Pandarathil, Vijaymohan R
- Added vfio_device_get_from_dev() as wrapper to get
  reference to vfio_device from struct device.

- Added vfio_device_data() as a wrapper to get device_data from
  vfio_device.

Signed-off-by: Vijay Mohan Pandarathil 
---
 drivers/vfio/vfio.c  | 41 -
 include/linux/vfio.h |  3 +++
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 12c264d..f0a78a2 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -407,12 +407,13 @@ static void vfio_device_release(struct kref *kref)
 }
 
 /* Device reference always implies a group reference */
-static void vfio_device_put(struct vfio_device *device)
+void vfio_device_put(struct vfio_device *device)
 {
struct vfio_group *group = device->group;
kref_put_mutex(&device->kref, vfio_device_release, &group->device_lock);
vfio_group_put(group);
 }
+EXPORT_SYMBOL_GPL(vfio_device_put);
 
 static void vfio_device_get(struct vfio_device *device)
 {
@@ -642,8 +643,12 @@ int vfio_add_group_dev(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(vfio_add_group_dev);
 
-/* Test whether a struct device is present in our tracking */
-static bool vfio_dev_present(struct device *dev)
+/**
+ * This does a get on the vfio_device from device.
+ * Callers of this function will have to call vfio_put_device() to
+ * remove the reference.
+ */
+struct vfio_device *vfio_device_get_from_dev(struct device *dev)
 {
struct iommu_group *iommu_group;
struct vfio_group *group;
@@ -651,25 +656,43 @@ static bool vfio_dev_present(struct device *dev)
 
iommu_group = iommu_group_get(dev);
if (!iommu_group)
-   return false;
+   return NULL;
 
group = vfio_group_get_from_iommu(iommu_group);
if (!group) {
iommu_group_put(iommu_group);
-   return false;
+   return NULL;
}
 
device = vfio_group_get_device(group, dev);
if (!device) {
vfio_group_put(group);
iommu_group_put(iommu_group);
-   return false;
+   return NULL;
}
-
-   vfio_device_put(device);
vfio_group_put(group);
iommu_group_put(iommu_group);
-   return true;
+   return device;
+}
+EXPORT_SYMBOL_GPL(vfio_device_get_from_dev);
+
+void *vfio_device_data(struct vfio_device *device)
+{
+   return device->device_data;
+}
+EXPORT_SYMBOL_GPL(vfio_device_data);
+
+/* Test whether a struct device is present in our tracking */
+static bool vfio_dev_present(struct device *dev)
+{
+   struct vfio_device *device;
+
+   device = vfio_device_get_from_dev(dev);
+   if (device) {
+   vfio_device_put(device);
+   return true;
+   } else
+   return false;
 }
 
 /*
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index ab9e862..ac8d488 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -45,6 +45,9 @@ extern int vfio_add_group_dev(struct device *dev,
  void *device_data);
 
 extern void *vfio_del_group_dev(struct device *dev);
+extern struct vfio_device *vfio_device_get_from_dev(struct device *dev);
+extern void vfio_device_put(struct vfio_device *device);
+extern void *vfio_device_data(struct vfio_device *device);
 
 /**
  * struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks
-- 
1.7.11.3




[Qemu-devel] [PATCH v3 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER

2013-02-03 Thread Pandarathil, Vijaymohan R
- New VFIO_SET_IRQ ioctl option to pass the eventfd that is signaled 
when
  an error occurs in the vfio_pci_device

- Register pci_error_handler for the vfio_pci driver

- When the device encounters an error, the error handler registered by
  the vfio_pci driver gets invoked by the AER infrastructure

- In the error handler, signal the eventfd registered for the device.

- This results in the qemu eventfd handler getting invoked and
  appropriate action taken for the guest.

Signed-off-by: Vijay Mohan Pandarathil 
---
 drivers/vfio/pci/vfio_pci.c | 43 -
 drivers/vfio/pci/vfio_pci_intrs.c   | 30 ++
 drivers/vfio/pci/vfio_pci_private.h |  1 +
 include/uapi/linux/vfio.h   |  1 +
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index b28e66c..818b1ed 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -196,7 +196,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device 
*vdev, int irq_type)
 
return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
}
-   }
+   } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX)
+   if (pci_is_pcie(vdev->pdev))
+   return 1;
 
return 0;
 }
@@ -302,6 +304,16 @@ static long vfio_pci_ioctl(void *device_data,
if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS)
return -EINVAL;
 
+   switch (info.index) {
+   case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX:
+   break;
+   case VFIO_PCI_ERR_IRQ_INDEX:
+   if (pci_is_pcie(vdev->pdev))
+   break;
+   default:
+   return -EINVAL;
+   }
+
info.flags = VFIO_IRQ_INFO_EVENTFD;
 
info.count = vfio_pci_get_irq_count(vdev, info.index);
@@ -538,11 +550,40 @@ static void vfio_pci_remove(struct pci_dev *pdev)
kfree(vdev);
 }
 
+static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
+ pci_channel_state_t state)
+{
+   struct vfio_pci_device *vpdev;
+   void *vdev;
+
+   vdev = vfio_device_get_from_dev(&pdev->dev);
+   if (vdev == NULL)
+   return PCI_ERS_RESULT_DISCONNECT;
+
+   vpdev = vfio_device_data(vdev);
+   if (vpdev == NULL) {
+   vfio_device_put(vdev);
+   return PCI_ERS_RESULT_DISCONNECT;
+   }
+
+   if (vpdev->err_trigger)
+   eventfd_signal(vpdev->err_trigger, 1);
+
+   vfio_device_put(vdev);
+
+   return PCI_ERS_RESULT_CAN_RECOVER;
+}
+
+static struct pci_error_handlers vfio_err_handlers = {
+   .error_detected = vfio_pci_aer_err_detected,
+};
+
 static struct pci_driver vfio_pci_driver = {
.name   = "vfio-pci",
.id_table   = NULL, /* only dynamic ids */
.probe  = vfio_pci_probe,
.remove = vfio_pci_remove,
+   .err_handler= &vfio_err_handlers,
 };
 
 static void __exit vfio_pci_cleanup(void)
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c 
b/drivers/vfio/pci/vfio_pci_intrs.c
index 3639371..83035b1 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -745,6 +745,29 @@ static int vfio_pci_set_msi_trigger(struct vfio_pci_device 
*vdev,
return 0;
 }
 
+static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev,
+   unsigned index, unsigned start,
+   unsigned count, uint32_t flags, void *data)
+{
+   int32_t fd = *(int32_t *)data;
+
+   if ((index != VFIO_PCI_ERR_IRQ_INDEX) ||
+   !(flags & VFIO_IRQ_SET_DATA_EVENTFD))
+   return -EINVAL;
+
+   if (fd == -1) {
+   if (vdev->err_trigger)
+   eventfd_ctx_put(vdev->err_trigger);
+   vdev->err_trigger = NULL;
+   return 0;
+   } else if (fd >= 0) {
+   vdev->err_trigger = eventfd_ctx_fdget(fd);
+   if (IS_ERR(vdev->err_trigger))
+   return PTR_ERR(vdev->err_trigger);
+   return 0;
+   } else
+   return -EINVAL;
+}
 int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags,
unsigned index, unsigned start, unsigned count,
void *data)
@@ -779,6 +802,13 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, 
uint32_t flags,
break;
}
break;
+   case VFIO_PCI_ERR_IRQ_INDEX:
+   switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
+   case VFIO_IRQ_SET_ACTION_TRIGGER:
+   if (pci_is_pcie

Re: [Qemu-devel] [OpenBIOS] Fix double nvram entry on newworld

2013-02-03 Thread Anthony Liguori
Alexander Graf  writes:

> Am 02.02.2013 um 14:05 schrieb Blue Swirl :
>
>> On Thu, Jan 31, 2013 at 3:10 PM, Alexander Graf  wrote:
>>> 
>>> On 30.01.2013, at 11:29, Amadeusz Sławiński wrote:
>>> 
 Fix double nvram entry on newworld
 
 There are two nvram entries on newworld (for example qemu -M mac99)
 
 The first one (nvram@fff04000) has initialized .properties while the
 other one has words.
 
 0 > dev / ls
 ...
 fff75e24 pci@f200
 fff77848 nvram@fff04000
 fff778e0 nvram
 ok
 0 > dev /nvram@fff04000  ok
 0 > words
 ok
 0 > .properties
 name  "nvram"
 #bytes2000
 reg   fff04000   4000
 device_type   "nvram"
 compatible"nvram,flash"
 ok
 0 > dev /nvram@0  ok
 0 > words close open seek write read size
 ok
 0 > .properties
 name  "nvram"
 ok
 
 This patch fixes initialization, so only one node is created
 containing both .properties and words.
 
 0 > dev / ls
 ...
 fff75e24 pci@f200
 fff77868 nvram@fff04000
 ok
 0 > dev /nvram@fff04000  ok
 0 > words close open seek write read size
 ok
 0 > .properties
 name  "nvram"
 #bytes2000
 reg   fff04000   4000
 device_type   "nvram"
 compatible"nvram,flash"
 ok
 
 Signed-off-by: Amadeusz Sławiński 
>>> 
>>> Blue, once this patch is in the tree, do you think you could rebuild 
>>> OpenBIOS for QEMU, so that it will land in 1.4?
>> 
>> Sorry, it didn't happen before the freeze. I'm not sure if new
>> OpenBIOS images qualifies as a bug fix.
>
> I would say yes ;).
>
> Anthony, any objections from your side?

Nope.

Regards,

Anthony Liguori

>
>
> Alex
>
>> Most of the changes since
>> r1063 (now in QEMU) are bug fixes, though:
>> a5af2b3 macio.c: Fix double nvram entry on newworld
>> ff86ced SPARC32: WIM register update delay
>> 8e9793b SPARC32: Clear FP register
>> c23f9f7 esp.c: fix SCSI command code displayed in do_command() debug 
>> statement.
>> 39988d6 esp.c: fix TEST_UNIT_READY SCSI command length.
>> 75e29de Revert configuration change from previous commit.
>> d5df782 video.c: Fix compilation when CONFIG_DEBUG_CONSOLE_VIDEO is
>> set to false.
>> 91119ec video.c: Fix incorrect sized type in fill_rect().
>> 3caf41b mac-parts.c: Update bootpath to reflect the chosen partition
>> if unspecified.
>> ec237bb Switch partition argument parsing to use left-parse-string as
>> per CHRP bindings.
>> e4ada76 PPC: Mimic Apple's OpenFirmware behaviour if a divide by zero occurs.
>> 7e64c09 mac-parts.c: Add Apple_Bootstrap to partition types considered
>> for Mac boot.
>> 5b48904 PPC: Fix filll word used by BootX
>> 1711362 Ignore any attempts to emit a character to stdout when stdout
>> is set to 0.
>> 6294e00 adb_kbd.c: Implement dummy get-key-map word for the ADB
>> keyboard package.
>> 1484d2b PPC: Fix mapping of OpenBIOS ROM in RAM copy within OFMEM
>> 69c27c4 PPC: Fix next slot eviction
>> 1be3a15 video.c: Place framebuffer address in frame-buffer-adr
>> e11cacd mac-parts.c: Fix detection of wrapped HFS+ volumes.
>> 7694794 PPC: Implement filll (fill long) word for QEMU/PPC as required by 
>> BootX.
>> 1fbbbd2 Redefine "to" word in device.fs to allow the current package
>> to be set like a standard value.
>> 1da510f Rework mac-parts.c to use CHRP-compliant partition search,
>> followed by Apple OF partition search.
>> b70a1f7 PPC: Rework assignment of keyboard devalias.
>> dd37f6b Add a default "decode-unit" word for devices that don't
>> implement their own.
>> 7a370cc PPC: Add keyboard device alias as a duplicate of stdin.
>> 58106df Fix dir cd:,\ (no partition specified) when reading from Mac 
>> partitions.
>> 5f23f2c Improve dir word by reducing complexity and adding some more
>> diagnostics.
>> a78e3b4 The spin word is set by BootX when Mac OS X is booting.
>> 37d2f65 PPC32: Enable local variables for the PPC32 build.
>> 538d404 Implementation of Forth local variables for OpenBIOS.
>> 5bb1484 amd64: Fix compilation from last commit to implement "dir"
>> word for HFS+ filesystem.
>> 0495b71 Add initial implementation of "dir" word for HFS+ filesystems.
>> 21ed61f Fix HFS+ display for non-ASCII characters.
>> 5b22479 Fix bug related to opening backup volumes in libhfsp's volume_open().
>> f095c85 ppc qemu: Increase PCI hole for heathrow
>> 
>>> 
>>> 
>>> Thanks,
>>> 
>>> Alex
>>> 
 
 diff -uNr a/drivers/macio.c b/drivers/macio.c
 --- a/drivers/macio.c   2013-01-28 12:16:54.849868216 +0100
 +++ b/drivers/macio.c   2013-01-28 12:17:27.595867493 +0100
 @@ -57,12 +57,6 @@
} else {
nvram_offset = NW_IO_NVRAM_OFFSET;
nvram_size = NW_IO_NVRAM_SIZE;
 -push_str("/");
 - 

Re: [Qemu-devel] [PATCH v3 0/3] AER-KVM: Error containment of VFIO devices assigned to KVM guests

2013-02-03 Thread Gleb Natapov
Please use "git send-email --thread --no-chain-reply-to" when sending
patch series.

On Sun, Feb 03, 2013 at 02:10:08PM +, Pandarathil, Vijaymohan R wrote:
> 
> Add support for error containment when a VFIO device assigned to a KVM
> guest encounters an error. This is for PCIe devices/drivers that support AER
> functionality. When the host OS is notified of an error in a device either
> through the firmware first approach or through an interrupt handled by the AER
> root port driver, the error handler registered by the vfio-pci driver gets
> invoked. The qemu process is signaled through an eventfd registered per
> VFIO device by the qemu process. In the eventfd handler, qemu decides on
> what action to take. In this implementation, guest is brought down to
> contain the error.
> 
> 
> v3:
>  - Removed PCI_AER* flags from device info ioctl.
>  - Incorporated feedback
> v2:
>  - Rebased to latest upstream stable bits
>  - Changed the new ioctl to be part of VFIO_SET_IRQs ioctl
>  - Added a new patch to get/put reference to a vfio device from struct device
>  - Incorporated all other feedback.
> 
> ---
> 
> Vijay Mohan Pandarathil(3):
> 
> [PATCH 1/3] VFIO: Wrapper for getting reference to vfio_device from device 
> [PATCH 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER
> [PATCH 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices
> 
> Kernel files changed
> 
>  drivers/vfio/vfio.c  | 41 -
>  include/linux/vfio.h |  3 +++
>  2 files changed, 35 insertions(+), 9 deletions(-)
> 
>  drivers/vfio/pci/vfio_pci.c | 43 
> -
>  drivers/vfio/pci/vfio_pci_intrs.c   | 30 ++
>  drivers/vfio/pci/vfio_pci_private.h |  1 +
>  include/uapi/linux/vfio.h   |  1 +
>  4 files changed, 74 insertions(+), 1 deletion(-)
> 
> Qemu files changed
> 
>  hw/vfio_pci.c  | 105 
> +
>  linux-headers/linux/vfio.h |   1 +
>  2 files changed, 106 insertions(+)

--
Gleb.



Re: [Qemu-devel] [OpenBIOS] Fix double nvram entry on newworld

2013-02-03 Thread Alexander Graf


Am 03.02.2013 um 15:18 schrieb Anthony Liguori :

> Alexander Graf  writes:
> 
>> Am 02.02.2013 um 14:05 schrieb Blue Swirl :
>> 
>>> On Thu, Jan 31, 2013 at 3:10 PM, Alexander Graf  wrote:
 
 On 30.01.2013, at 11:29, Amadeusz Sławiński wrote:
 
> Fix double nvram entry on newworld
> 
> There are two nvram entries on newworld (for example qemu -M mac99)
> 
> The first one (nvram@fff04000) has initialized .properties while the
> other one has words.
> 
> 0 > dev / ls
> ...
> fff75e24 pci@f200
> fff77848 nvram@fff04000
> fff778e0 nvram
> ok
> 0 > dev /nvram@fff04000  ok
> 0 > words
> ok
> 0 > .properties
> name  "nvram"
> #bytes2000
> reg   fff04000   4000
> device_type   "nvram"
> compatible"nvram,flash"
> ok
> 0 > dev /nvram@0  ok
> 0 > words close open seek write read size
> ok
> 0 > .properties
> name  "nvram"
> ok
> 
> This patch fixes initialization, so only one node is created
> containing both .properties and words.
> 
> 0 > dev / ls
> ...
> fff75e24 pci@f200
> fff77868 nvram@fff04000
> ok
> 0 > dev /nvram@fff04000  ok
> 0 > words close open seek write read size
> ok
> 0 > .properties
> name  "nvram"
> #bytes2000
> reg   fff04000   4000
> device_type   "nvram"
> compatible"nvram,flash"
> ok
> 
> Signed-off-by: Amadeusz Sławiński 
 
 Blue, once this patch is in the tree, do you think you could rebuild 
 OpenBIOS for QEMU, so that it will land in 1.4?
>>> 
>>> Sorry, it didn't happen before the freeze. I'm not sure if new
>>> OpenBIOS images qualifies as a bug fix.
>> 
>> I would say yes ;).
>> 
>> Anthony, any objections from your side?
> 
> Nope.

Awesome!

Blue, mind to push new binaries? I'm currently at FOSDEM - not the greatest 
place to do this at ;).


Alex




[Qemu-devel] [PATCH v3 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices

2013-02-03 Thread Pandarathil, Vijaymohan R
- Create eventfd per vfio device assigned to a guest and register an
  event handler

- This fd is passed to the vfio_pci driver through the SET_IRQ ioctl

- When the device encounters an error, the eventfd is signalled
  and the qemu eventfd handler gets invoked.

- In the handler decide what action to take. Current action taken
  is to terminate the guest.

Signed-off-by: Vijay Mohan Pandarathil 
---
 hw/vfio_pci.c  | 105 +
 linux-headers/linux/vfio.h |   1 +
 2 files changed, 106 insertions(+)

diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
index c51ae67..4e2f768 100644
--- a/hw/vfio_pci.c
+++ b/hw/vfio_pci.c
@@ -130,6 +130,8 @@ typedef struct VFIODevice {
 QLIST_ENTRY(VFIODevice) next;
 struct VFIOGroup *group;
 bool reset_works;
+EventNotifier err_notifier;
+bool pci_aer;
 } VFIODevice;
 
 typedef struct VFIOGroup {
@@ -1922,6 +1924,106 @@ static void vfio_put_device(VFIODevice *vdev)
 }
 }
 
+static void vfio_err_notifier_handler(void *opaque)
+{
+VFIODevice *vdev = opaque;
+
+if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
+return;
+}
+
+/*
+ * TBD. Retrieve the error details and decide what action
+ * needs to be taken. One of the actions could be to pass
+ * the error to the guest and have the guest driver recover
+ * from the error. This requires that PCIe capabilities be
+ * exposed to the guest. At present, we just terminate the
+ * guest to contain the error.
+ */
+
+error_report("%s (%04x:%02x:%02x.%x)"
+"Unrecoverable error detected... Terminating guest\n",
+__func__, vdev->host.domain, vdev->host.bus,
+vdev->host.slot, vdev->host.function);
+
+hw_error("(%04x:%02x:%02x.%x) Unrecoverable device error\n",
+vdev->host.domain, vdev->host.bus,
+vdev->host.slot, vdev->host.function);
+
+return;
+}
+
+static void vfio_register_err_notifier(VFIODevice *vdev)
+{
+int ret;
+int argsz;
+struct vfio_irq_set *irq_set;
+int32_t *pfd;
+
+if (event_notifier_init(&vdev->err_notifier, 0)) {
+error_report("vfio: Warning: Unable to init event notifier for error 
detection\n");
+return;
+}
+
+argsz = sizeof(*irq_set) + sizeof(*pfd);
+
+irq_set = g_malloc0(argsz);
+irq_set->argsz = argsz;
+irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
+ VFIO_IRQ_SET_ACTION_TRIGGER;
+irq_set->index = VFIO_PCI_ERR_IRQ_INDEX;
+irq_set->start = 0;
+irq_set->count = 1;
+pfd = (int32_t *)&irq_set->data;
+
+*pfd = event_notifier_get_fd(&vdev->err_notifier);
+qemu_set_fd_handler(*pfd, vfio_err_notifier_handler, NULL, vdev);
+
+ret = ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
+if (ret) {
+DPRINTF("vfio: Error notification not supported for the device\n");
+qemu_set_fd_handler(*pfd, NULL, NULL, vdev);
+event_notifier_cleanup(&vdev->err_notifier);
+g_free(irq_set);
+return;
+}
+g_free(irq_set);
+vdev->pci_aer = 1;
+return;
+}
+static void vfio_unregister_err_notifier(VFIODevice *vdev)
+{
+int argsz;
+struct vfio_irq_set *irq_set;
+int32_t *pfd;
+int ret;
+
+if (!vdev->pci_aer) {
+return;
+}
+
+argsz = sizeof(*irq_set) + sizeof(*pfd);
+
+irq_set = g_malloc0(argsz);
+irq_set->argsz = argsz;
+irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
+ VFIO_IRQ_SET_ACTION_TRIGGER;
+irq_set->index = VFIO_PCI_ERR_IRQ_INDEX;
+irq_set->start = 0;
+irq_set->count = 1;
+pfd = (int32_t *)&irq_set->data;
+*pfd = -1;
+
+ret = ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
+if (ret) {
+DPRINTF("vfio: Failed to de-assign error fd: %d\n", ret);
+}
+g_free(irq_set);
+qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier),
+NULL, NULL, vdev);
+event_notifier_cleanup(&vdev->err_notifier);
+return;
+}
 static int vfio_initfn(PCIDevice *pdev)
 {
 VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
@@ -2032,6 +2134,8 @@ static int vfio_initfn(PCIDevice *pdev)
 }
 }
 
+vfio_register_err_notifier(vdev);
+
 return 0;
 
 out_teardown:
@@ -2049,6 +2153,7 @@ static void vfio_exitfn(PCIDevice *pdev)
 VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
 VFIOGroup *group = vdev->group;
 
+vfio_unregister_err_notifier(vdev);
 pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
 vfio_disable_interrupts(vdev);
 if (vdev->intx.mmap_timer) {
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index f787b72..6b20849 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -310,6 +310,7 @@ enum {
VFIO_PCI_INTX_IRQ_INDEX,
VFIO_PCI_MSI_IRQ_INDEX,
VFIO_PCI_MSIX_IRQ_INDEX,
+   VFIO_PCI_ERR_IRQ_INDEX,
 

[Qemu-devel] [Bug 826363] Re: qemu-img convert does not work with vdi files

2013-02-03 Thread Aaron Bouzek
Did this bug come back, or is "error while reading" unrelated but
similar?

Oh look, Debian is using an old version, what are the odds?

Distributor ID: Debian
Description:Debian GNU/Linux 6.0.6 (squeeze)
Release:6.0.6
Codename:   squeeze
root@nakvm:~# qemu-img --help
qemu-img version 0.12.5, Copyright (c) 2004-2008 Fabrice Bellard
usage: qemu-img command [command options]
QEMU disk image utility

So now I get to figure out where to get a new copy...

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/826363

Title:
  qemu-img convert does not work with vdi files

Status in QEMU:
  Fix Released

Bug description:
  qemu-img.exe convert -O raw  myfile.vdi zz.raw

  reports 'error while writing' both in 0.15.0 and Beta rc2
  v0.11.1 works fine on same file. myfile.vdi is 3.9GB made by VirtualBox.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/826363/+subscriptions



[Qemu-devel] [Bug 826363] Re: qemu-img convert does not work with vdi files

2013-02-03 Thread Aaron Bouzek
Or an old copy because Ubuntu proffers 1.0 which works fine on the same
image.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/826363

Title:
  qemu-img convert does not work with vdi files

Status in QEMU:
  Fix Released

Bug description:
  qemu-img.exe convert -O raw  myfile.vdi zz.raw

  reports 'error while writing' both in 0.15.0 and Beta rc2
  v0.11.1 works fine on same file. myfile.vdi is 3.9GB made by VirtualBox.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/826363/+subscriptions



Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Michael Tokarev

03.02.2013 17:23, Yan Vugenfirer wrote:


If it helps, mq changes the config size from 8 to 16 bytes.  If the
driver was making an assumption about an 8-byte config size, that's
likely what the problem is.


That's exactly the problem.


So what do we do?  It isn't nice to break existing setups.
Maybe mq can be disabled by default (together with Antony's
patch) until fixed drivers will be more widely available?
It's easy to turn off mq by default and turn it on when
needed...

The problem now is that it isn't obvious why your existing
setup breaks when you upgrade.  While when you especially
play with mq, you may look at options available around it...

How difficult it is to fix it in win driver?

Thanks,

/mjt



Re: [Qemu-devel] [PATCH v3 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER

2013-02-03 Thread Blue Swirl
On Sun, Feb 3, 2013 at 2:10 PM, Pandarathil, Vijaymohan R
 wrote:
> - New VFIO_SET_IRQ ioctl option to pass the eventfd that is signaled 
> when
>   an error occurs in the vfio_pci_device
>
> - Register pci_error_handler for the vfio_pci driver
>
> - When the device encounters an error, the error handler registered by
>   the vfio_pci driver gets invoked by the AER infrastructure
>
> - In the error handler, signal the eventfd registered for the device.
>
> - This results in the qemu eventfd handler getting invoked and
>   appropriate action taken for the guest.
>
> Signed-off-by: Vijay Mohan Pandarathil 
> ---
>  drivers/vfio/pci/vfio_pci.c | 43 
> -
>  drivers/vfio/pci/vfio_pci_intrs.c   | 30 ++
>  drivers/vfio/pci/vfio_pci_private.h |  1 +
>  include/uapi/linux/vfio.h   |  1 +
>  4 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index b28e66c..818b1ed 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -196,7 +196,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device 
> *vdev, int irq_type)
>
> return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
> }
> -   }
> +   } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX)
> +   if (pci_is_pcie(vdev->pdev))
> +   return 1;
>
> return 0;
>  }
> @@ -302,6 +304,16 @@ static long vfio_pci_ioctl(void *device_data,
> if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS)
> return -EINVAL;
>
> +   switch (info.index) {
> +   case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX:
> +   break;
> +   case VFIO_PCI_ERR_IRQ_INDEX:
> +   if (pci_is_pcie(vdev->pdev))
> +   break;

I don't know what is the policy in Linux kernel for this, but I'd add
a comment about fall through here.

> +   default:
> +   return -EINVAL;
> +   }
> +
> info.flags = VFIO_IRQ_INFO_EVENTFD;
>
> info.count = vfio_pci_get_irq_count(vdev, info.index);
> @@ -538,11 +550,40 @@ static void vfio_pci_remove(struct pci_dev *pdev)
> kfree(vdev);
>  }
>
> +static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
> + pci_channel_state_t state)
> +{
> +   struct vfio_pci_device *vpdev;
> +   void *vdev;
> +
> +   vdev = vfio_device_get_from_dev(&pdev->dev);
> +   if (vdev == NULL)
> +   return PCI_ERS_RESULT_DISCONNECT;
> +
> +   vpdev = vfio_device_data(vdev);
> +   if (vpdev == NULL) {
> +   vfio_device_put(vdev);
> +   return PCI_ERS_RESULT_DISCONNECT;
> +   }
> +
> +   if (vpdev->err_trigger)
> +   eventfd_signal(vpdev->err_trigger, 1);
> +
> +   vfio_device_put(vdev);
> +
> +   return PCI_ERS_RESULT_CAN_RECOVER;
> +}
> +
> +static struct pci_error_handlers vfio_err_handlers = {
> +   .error_detected = vfio_pci_aer_err_detected,
> +};
> +
>  static struct pci_driver vfio_pci_driver = {
> .name   = "vfio-pci",
> .id_table   = NULL, /* only dynamic ids */
> .probe  = vfio_pci_probe,
> .remove = vfio_pci_remove,
> +   .err_handler= &vfio_err_handlers,
>  };
>
>  static void __exit vfio_pci_cleanup(void)
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c 
> b/drivers/vfio/pci/vfio_pci_intrs.c
> index 3639371..83035b1 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -745,6 +745,29 @@ static int vfio_pci_set_msi_trigger(struct 
> vfio_pci_device *vdev,
> return 0;
>  }
>
> +static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev,
> +   unsigned index, unsigned start,
> +   unsigned count, uint32_t flags, void 
> *data)
> +{
> +   int32_t fd = *(int32_t *)data;
> +
> +   if ((index != VFIO_PCI_ERR_IRQ_INDEX) ||
> +   !(flags & VFIO_IRQ_SET_DATA_EVENTFD))
> +   return -EINVAL;
> +
> +   if (fd == -1) {
> +   if (vdev->err_trigger)
> +   eventfd_ctx_put(vdev->err_trigger);
> +   vdev->err_trigger = NULL;
> +   return 0;
> +   } else if (fd >= 0) {
> +   vdev->err_trigger = eventfd_ctx_fdget(fd);
> +   if (IS_ERR(vdev->err_trigger))
> +   return PTR_ERR(vdev->err_trigger);
> +   return 0;
> +   } else
> +   return -EINVAL;
> +}
>  int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags,
> unsigned index, unsigned s

Re: [Qemu-devel] [PATCH v3] bitops: unify bitops_ffsl with the one in host-utils.h, call it bitops_ctzl

2013-02-03 Thread Blue Swirl
Thanks, applied.

On Fri, Feb 1, 2013 at 10:03 PM, Paolo Bonzini  wrote:
> We had two copies of a ffs function for longs with subtly different
> semantics and, for the one in bitops.h, a confusing name: the result
> was off-by-one compared to the library function ffsl.
>
> Unify the functions into one, and solve the name problem by calling
> the 0-based functions "bitops_ctzl" and "bitops_ctol" respectively.
>
> This also fixes the build on platforms with ffsl, including Mac OS X
> and Windows.
>
> Signed-off-by: Paolo Bonzini 
> ---
>  include/qemu/bitops.h | 55 
> +++
>  include/qemu/hbitmap.h|  2 +-
>  include/qemu/host-utils.h | 26 --
>  memory.c  |  4 ++--
>  util/bitops.c |  4 ++--
>  util/hbitmap.c|  2 +-
>  6 files changed, 28 insertions(+), 65 deletions(-)
>
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 74e14e5..8b88791 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -13,6 +13,7 @@
>  #define BITOPS_H
>
>  #include "qemu-common.h"
> +#include "host-utils.h"
>
>  #define BITS_PER_BYTE   CHAR_BIT
>  #define BITS_PER_LONG   (sizeof (unsigned long) * BITS_PER_BYTE)
> @@ -23,41 +24,29 @@
>  #define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
>
>  /**
> - * bitops_ffs - find first bit in word.
> + * bitops_ctzl - count trailing zeroes in word.
>   * @word: The word to search
>   *
> - * Undefined if no bit exists, so code should check against 0 first.
> + * Returns -1 if no bit exists.  Note that compared to the C library
> + * routine ffsl, this one returns one less.
>   */
> -static unsigned long bitops_ffsl(unsigned long word)
> +static unsigned long bitops_ctzl(unsigned long word)
>  {
> -   int num = 0;
> +#if QEMU_GNUC_PREREQ(3, 4)
> +return __builtin_ffsl(word) - 1;
> +#else
> +if (!word) {
> +return -1;
> +}
>
> -#if LONG_MAX > 0x7FFF
> -   if ((word & 0x) == 0) {
> -   num += 32;
> -   word >>= 32;
> -   }
> +if (sizeof(long) == 4) {
> +return ctz32(word);
> +} else if (sizeof(long) == 8) {
> +return ctz64(word);
> +} else {
> +abort();
> +}
>  #endif
> -   if ((word & 0x) == 0) {
> -   num += 16;
> -   word >>= 16;
> -   }
> -   if ((word & 0xff) == 0) {
> -   num += 8;
> -   word >>= 8;
> -   }
> -   if ((word & 0xf) == 0) {
> -   num += 4;
> -   word >>= 4;
> -   }
> -   if ((word & 0x3) == 0) {
> -   num += 2;
> -   word >>= 2;
> -   }
> -   if ((word & 0x1) == 0) {
> -   num += 1;
> -}
> -   return num;
>  }
>
>  /**
> @@ -99,14 +88,14 @@ static inline unsigned long bitops_flsl(unsigned long 
> word)
>  }
>
>  /**
> - * ffz - find first zero in word.
> + * cto - count trailing ones in word.
>   * @word: The word to search
>   *
> - * Undefined if no zero exists, so code should check against ~0UL first.
> + * Returns -1 if all bit are set.
>   */
> -static inline unsigned long ffz(unsigned long word)
> +static inline unsigned long bitops_ctol(unsigned long word)
>  {
> -return bitops_ffsl(~word);
> +return bitops_ctzl(~word);
>  }
>
>  /**
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index 73f5d1d..250de03 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -170,7 +170,7 @@ static inline int64_t hbitmap_iter_next(HBitmapIter *hbi)
>
>  /* The next call will resume work from the next bit.  */
>  hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
> -item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ffsl(cur) - 1;
> +item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + bitops_ctzl(cur);
>
>  return item << hbi->granularity;
>  }
> diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
> index 2a32be4..81c9a75 100644
> --- a/include/qemu/host-utils.h
> +++ b/include/qemu/host-utils.h
> @@ -26,7 +26,6 @@
>  #define HOST_UTILS_H 1
>
>  #include "qemu/compiler.h"   /* QEMU_GNUC_PREREQ */
> -#include  /* ffsl */
>
>  #if defined(__x86_64__)
>  #define __HAVE_FAST_MULU64__
> @@ -238,29 +237,4 @@ static inline int ctpop64(uint64_t val)
>  #endif
>  }
>
> -/* glibc does not provide an inline version of ffsl, so always define
> - * ours.  We need to give it a different name, however.
> - */
> -#ifdef __GLIBC__
> -#define ffsl qemu_ffsl
> -#endif
> -static inline int ffsl(long val)
> -{
> -if (!val) {
> -return 0;
> -}
> -
> -#if QEMU_GNUC_PREREQ(3, 4)
> -return __builtin_ctzl(val) + 1;
> -#else
> -if (sizeof(long) == 4) {
> -return ctz32(val) + 1;
> -} else if (sizeof(long) == 8) {
> -return ctz64(val) + 1;
> -} else {
> -abort();
> -}
> -#endif
> -}
> -
>  #endif
> diff --git a/memory.c b/memory.c
> index 41

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

2013-02-03 Thread Blue Swirl
Thanks, applied.

On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil  wrote:
> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
> We must also fix the include statements in util/envlist.c to include
> that file.
>
> We currently don't need an implementation of strtok_r because the
> code is compiled but not linked for MinGW.
>
> Signed-off-by: Stefan Weil 
> ---
>  include/sysemu/os-win32.h |2 ++
>  util/envlist.c|7 +--
>  2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index d0e9234..bf9edeb 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -73,6 +73,8 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result);
>  #undef localtime_r
>  struct tm *localtime_r(const time_t *timep, struct tm *result);
>
> +char *strtok_r(char *str, const char *delim, char **saveptr);
> +
>  static inline void os_setup_signal_handling(void) {}
>  static inline void os_daemonize(void) {}
>  static inline void os_setup_post(void) {}
> diff --git a/util/envlist.c b/util/envlist.c
> index ff99fc4..ebc06cf 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -1,9 +1,4 @@
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> +#include "qemu-common.h"
>  #include "qemu/queue.h"
>  #include "qemu/envlist.h"
>
> --
> 1.7.10.4
>
>



Re: [Qemu-devel] [PATCH v3 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices

2013-02-03 Thread Blue Swirl
On Sun, Feb 3, 2013 at 2:10 PM, Pandarathil, Vijaymohan R
 wrote:
> - Create eventfd per vfio device assigned to a guest and register an
>   event handler
>
> - This fd is passed to the vfio_pci driver through the SET_IRQ ioctl
>
> - When the device encounters an error, the eventfd is signalled
>   and the qemu eventfd handler gets invoked.
>
> - In the handler decide what action to take. Current action taken
>   is to terminate the guest.

Usually this is not OK, but I guess this is not guest triggerable.

>
> Signed-off-by: Vijay Mohan Pandarathil 
> ---
>  hw/vfio_pci.c  | 105 
> +
>  linux-headers/linux/vfio.h |   1 +
>  2 files changed, 106 insertions(+)
>
> diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
> index c51ae67..4e2f768 100644
> --- a/hw/vfio_pci.c
> +++ b/hw/vfio_pci.c
> @@ -130,6 +130,8 @@ typedef struct VFIODevice {
>  QLIST_ENTRY(VFIODevice) next;
>  struct VFIOGroup *group;
>  bool reset_works;
> +EventNotifier err_notifier;
> +bool pci_aer;
>  } VFIODevice;
>
>  typedef struct VFIOGroup {
> @@ -1922,6 +1924,106 @@ static void vfio_put_device(VFIODevice *vdev)
>  }
>  }
>
> +static void vfio_err_notifier_handler(void *opaque)
> +{
> +VFIODevice *vdev = opaque;
> +
> +if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
> +return;
> +}
> +
> +/*
> + * TBD. Retrieve the error details and decide what action
> + * needs to be taken. One of the actions could be to pass
> + * the error to the guest and have the guest driver recover
> + * from the error. This requires that PCIe capabilities be
> + * exposed to the guest. At present, we just terminate the
> + * guest to contain the error.
> + */
> +
> +error_report("%s (%04x:%02x:%02x.%x)"
> +"Unrecoverable error detected... Terminating guest\n",
> +__func__, vdev->host.domain, vdev->host.bus,
> +vdev->host.slot, vdev->host.function);
> +
> +hw_error("(%04x:%02x:%02x.%x) Unrecoverable device error\n",
> +vdev->host.domain, vdev->host.bus,
> +vdev->host.slot, vdev->host.function);
> +
> +return;

Useless, please remove.

> +}
> +
> +static void vfio_register_err_notifier(VFIODevice *vdev)
> +{
> +int ret;
> +int argsz;
> +struct vfio_irq_set *irq_set;
> +int32_t *pfd;
> +
> +if (event_notifier_init(&vdev->err_notifier, 0)) {
> +error_report("vfio: Warning: Unable to init event notifier for error 
> detection\n");
> +return;
> +}
> +
> +argsz = sizeof(*irq_set) + sizeof(*pfd);
> +
> +irq_set = g_malloc0(argsz);
> +irq_set->argsz = argsz;
> +irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> +irq_set->index = VFIO_PCI_ERR_IRQ_INDEX;
> +irq_set->start = 0;
> +irq_set->count = 1;
> +pfd = (int32_t *)&irq_set->data;
> +
> +*pfd = event_notifier_get_fd(&vdev->err_notifier);
> +qemu_set_fd_handler(*pfd, vfio_err_notifier_handler, NULL, vdev);
> +
> +ret = ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +if (ret) {
> +DPRINTF("vfio: Error notification not supported for the device\n");
> +qemu_set_fd_handler(*pfd, NULL, NULL, vdev);
> +event_notifier_cleanup(&vdev->err_notifier);
> +g_free(irq_set);
> +return;
> +}
> +g_free(irq_set);
> +vdev->pci_aer = 1;
> +return;

Ditto.

> +}
> +static void vfio_unregister_err_notifier(VFIODevice *vdev)
> +{
> +int argsz;
> +struct vfio_irq_set *irq_set;
> +int32_t *pfd;
> +int ret;
> +
> +if (!vdev->pci_aer) {
> +return;
> +}
> +
> +argsz = sizeof(*irq_set) + sizeof(*pfd);
> +
> +irq_set = g_malloc0(argsz);
> +irq_set->argsz = argsz;
> +irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> +irq_set->index = VFIO_PCI_ERR_IRQ_INDEX;
> +irq_set->start = 0;
> +irq_set->count = 1;
> +pfd = (int32_t *)&irq_set->data;
> +*pfd = -1;
> +
> +ret = ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +if (ret) {
> +DPRINTF("vfio: Failed to de-assign error fd: %d\n", ret);
> +}
> +g_free(irq_set);
> +qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier),
> +NULL, NULL, vdev);
> +event_notifier_cleanup(&vdev->err_notifier);
> +return;

Ditto.

> +}
>  static int vfio_initfn(PCIDevice *pdev)
>  {
>  VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
> @@ -2032,6 +2134,8 @@ static int vfio_initfn(PCIDevice *pdev)
>  }
>  }
>
> +vfio_register_err_notifier(vdev);
> +
>  return 0;
>
>  out_teardown:
> @@ -2049,6 +2153,7 @@ static void vfio_exitfn(PCIDevice *pdev)
>  VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
>  VFIOGroup *group = vdev->group;
>
> +vfio_unregister_err_noti

Re: [Qemu-devel] [PATCH for-1.4] configure: Fix build with XFree

2013-02-03 Thread Richard Henderson

I should have put for-1.4 in the subject...

r~


On 2013-02-01 14:32, Richard Henderson wrote:

The build is broken on ppc64-linux, possibly only with new binutils:

ld: hw/lm32/../milkymist-tmu2.o: undefined reference to symbol 'XFree'
ld: note: 'XFree' is defined in DSO /lib64/libX11.so.6 so try \
   adding it to the linker command line

So let's follow the linker's advice.

Signed-off-by: Richard Henderson 
---
  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 0657b1a..8789324 100755
--- a/configure
+++ b/configure
@@ -2388,7 +2388,7 @@ fi
  ##
  # opengl probe, used by milkymist-tmu2
  if test "$opengl" != "no" ; then
-  opengl_libs="-lGL"
+  opengl_libs="-lGL -lX11"
cat > $TMPC << EOF
  #include 
  #include 






[Qemu-devel] [PATCH for 1.4] target-s390x: Fix wrong comparison in interrupt handling

2013-02-03 Thread Stefan Weil
gcc with -Wextra complains about an ordered pointer comparison:

target-s390x/helper.c:660:27: warning:
 ordered comparison of pointer with integer zero [-Wextra]

Obviously the index was missing in the code.

Signed-off-by: Stefan Weil 
---

I hope my analysis and the fix is correct - please review.

For local builds, I always compile with -Wextra. This bug shows that -Wextra
would be good as a default compiler option for QEMU. Of course some extra
warnings must be explicitly disabled then. I'll send a patch for this after 1.4.

Regards,

Stefan W.


 target-s390x/helper.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 3180b90..8bd84ef 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -657,7 +657,7 @@ static void do_io_interrupt(CPUS390XState *env)
 cpu_unmap_lowcore(lowcore);
 
 env->io_index[isc]--;
-if (env->io_index >= 0) {
+if (env->io_index[isc] >= 0) {
 disable = 0;
 }
 break;
-- 
1.7.10.4




[Qemu-devel] [PATCH] vnc: recognize Hungarian doubleacutes

2013-02-03 Thread Michael Tokarev
As reported in http://bugs.debian.org/697641 , some Hungarian keys
does not work with qemu when using vnc display.

This is because while the Hungarian keymap mentions these symbols,
qemu know nothing about them.  So add them.

This patch is applicable to -stable for all previous releases.

Signed-off-by: Michael Tokarev 
---
 ui/vnc_keysym.h |4 
 1 file changed, 4 insertions(+)

diff --git a/ui/vnc_keysym.h b/ui/vnc_keysym.h
index df33cfe..6250bec 100644
--- a/ui/vnc_keysym.h
+++ b/ui/vnc_keysym.h
@@ -215,10 +215,14 @@ static const name2keysym_t name2keysym[]={
 { "Zabovedot",0x1af},
 { "zacute",   0x1bc},
 { "Zacute",   0x1ac},
+{ "Odoubleacute", 0x1d5},
+{ "Udoubleacute", 0x1db},
 { "cacute",   0x1e6},
 { "Cacute",   0x1c6},
 { "nacute",   0x1f1},
 { "Nacute",   0x1d1},
+{ "odoubleacute", 0x1f5},
+{ "udoubleacute", 0x1fb},
 
 /* modifiers */
 {"ISO_Level3_Shift", 0xfe03}, /* XK_ISO_Level3_Shift */
-- 
1.7.10.4




Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Anthony Liguori
Michael Tokarev  writes:

> 03.02.2013 17:23, Yan Vugenfirer wrote:
>
>>> If it helps, mq changes the config size from 8 to 16 bytes.  If the
>>> driver was making an assumption about an 8-byte config size, that's
>>> likely what the problem is.
>>>
>> That's exactly the problem.
>
> So what do we do?  It isn't nice to break existing setups.
> Maybe mq can be disabled by default (together with Antony's
> patch) until fixed drivers will be more widely available?

I've got a patch that does exactly like this.  It's pretty ugly though
because of the relationship between virtio-pci and virtio-net.  I'm
going to try to see if I can find a cleaner way to do it on Monday.

I'm also contemplating just disabling mq by default.  Since a special
command line is needed to enable it anyway (on the backend), having to
specify mq=on for the device doesn't seem so bad.

But yeah, we don't want Windows guests to break with -M pc by default so
we should do something to work around it.

N.B. this is a pretty nasty bug in the guest driver.  Any new virtio-net
feature is going to trigger it (not just multiqueue).  So while pc-1.3
will work forever with this guest image, it's pretty much guaranteed to
break at some point in the future.

> It's easy to turn off mq by default and turn it on when
> needed...
>
> The problem now is that it isn't obvious why your existing
> setup breaks when you upgrade.  While when you especially
> play with mq, you may look at options available around it...

If we ever to get virtio2, a really handy feature to have would be a
driver identification string.  Even if was only informative (and not
authoritative, we've had a lot of issues like this and being able to
make work arounds conditional on the driver identification string would
be nice.

Regards,

Anthony Liguori

>
> How difficult it is to fix it in win driver?
>
> Thanks,
>
> /mjt



Re: [Qemu-devel] [PATCH V3 8/8] hw/mdio: Use bitbang core for smc91c111 network device

2013-02-03 Thread Grant Likely
On Sat, 2 Feb 2013 23:51:42 +, Peter Maydell  
wrote:
> On 2 February 2013 23:40, Grant Likely  wrote:
> >  static const VMStateDescription vmstate_smc91c111 = {
> > @@ -71,6 +76,8 @@ static const VMStateDescription vmstate_smc91c111 = {
> >  VMSTATE_BUFFER_UNSAFE(data, smc91c111_state, 0, NUM_PACKETS * 
> > 2048),
> >  VMSTATE_UINT8(int_level, smc91c111_state),
> >  VMSTATE_UINT8(int_mask, smc91c111_state),
> > +VMSTATE_MDIO(mdio_bus, smc91c111_state),
> > +VMSTATE_MDIO_PHY(phy, smc91c111_state),
> >  VMSTATE_END_OF_LIST()
> >  }
> 
> If you're adding vmstate fields to an existing structure
> you need to either:
>  (a) increment the .version_id field, and set .minimum_version_id
> field to the same value [and accept that old-to-new migration
> won't be possible, which is OK in this particular case as it's
> only used by ARM boards and I'm happy that we don't currently
> support cross version migration on ARM]
>  (b) increment .version_id only, mark the new vmstate fields
> as only-from-version-N, and cope with what you get if an
> incoming migration hasn't got the fields.
> 
> For a complicated thing like the phy I would suggest course
> (a) as the simplest approach.

Okay, I'll do that and submit a fixed up patch. Thanks.

g.




[Qemu-devel] [PATCH V4 8/8] hw/mdio: Use bitbang core for smc91c111 network device

2013-02-03 Thread Grant Likely
The smc91c111 device has bitbanged MDIO access, but the model doesn't
yet implement it. This patch uses the generalized bitbang MDIO support
pulled out of etraxfs Ethernet driver.

The MDIO state machine is driven by changes in state to the clock
control bit in the management register. The PHY model emulated is
currently trivial (being whatever was done for the etraxfs driver), but
it is enough to get an OS to recognize a PHY as being present.

Tested with the versatilepb model with U-Boot and the Linux Kernel as
client software.

v4: Updated .version_id and .minimum_version_id fields because this patch
add fields to the state structure.

Cc: Peter Maydell 
Cc: Paul Brook 
Cc: Edgar E. Iglesias 
Cc: Anthony Liguori 
Cc: Andreas Färber 
Signed-off-by: Grant Likely 
---
 hw/Makefile.objs |2 +-
 hw/smc91c111.c   |   27 ++-
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index af93dfc..4ec6fb8 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -117,7 +117,7 @@ common-obj-$(CONFIG_PCNET_COMMON) += pcnet.o
 common-obj-$(CONFIG_E1000_PCI) += e1000.o
 common-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o
 
-common-obj-$(CONFIG_SMC91C111) += smc91c111.o
+common-obj-$(CONFIG_SMC91C111) += smc91c111.o mdio.o
 common-obj-$(CONFIG_LAN9118) += lan9118.o
 common-obj-$(CONFIG_NE2000_ISA) += ne2000-isa.o
 common-obj-$(CONFIG_OPENCORES_ETH) += opencores_eth.o
diff --git a/hw/smc91c111.c b/hw/smc91c111.c
index a34698f..2dfad87 100644
--- a/hw/smc91c111.c
+++ b/hw/smc91c111.c
@@ -10,6 +10,7 @@
 #include "sysbus.h"
 #include "net/net.h"
 #include "devices.h"
+#include "mdio.h"
 /* For crc32 */
 #include 
 
@@ -44,12 +45,16 @@ typedef struct {
 uint8_t int_level;
 uint8_t int_mask;
 MemoryRegion mmio;
+
+/* MDIO bus and the attached phy */
+struct qemu_mdio mdio_bus;
+struct qemu_phy phy;
 } smc91c111_state;
 
 static const VMStateDescription vmstate_smc91c111 = {
 .name = "smc91c111",
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
 .fields  = (VMStateField []) {
 VMSTATE_UINT16(tcr, smc91c111_state),
 VMSTATE_UINT16(rcr, smc91c111_state),
@@ -71,6 +76,8 @@ static const VMStateDescription vmstate_smc91c111 = {
 VMSTATE_BUFFER_UNSAFE(data, smc91c111_state, 0, NUM_PACKETS * 2048),
 VMSTATE_UINT8(int_level, smc91c111_state),
 VMSTATE_UINT8(int_mask, smc91c111_state),
+VMSTATE_MDIO(mdio_bus, smc91c111_state),
+VMSTATE_MDIO_PHY(phy, smc91c111_state),
 VMSTATE_END_OF_LIST()
 }
 };
@@ -437,7 +444,15 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
 /* Multicast table.  */
 /* Not implemented.  */
 return;
-case 8: case 9: /* Management Interface.  */
+case 8: /* Management Interface.  */
+/* Update MDIO data line status; but only if output is enabled */
+if (value & 8) {
+mdio_bitbang_set_data(&s->mdio_bus, !!(value & 1));
+}
+/* Process the clock */
+mdio_bitbang_set_clk(&s->mdio_bus, value & 4);
+return;
+case 9: /* Management Interface.  */
 /* Not implemented.  */
 return;
 case 12: /* Early receive.  */
@@ -576,8 +591,7 @@ static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
 /* Not implemented.  */
 return 0;
 case 8: /* Management Interface.  */
-/* Not implemented.  */
-return 0x30;
+return 0x30 | (mdio_bitbang_get_data(&s->mdio_bus) ? 2 : 0);
 case 9:
 return 0x33;
 case 10: /* Revision.  */
@@ -754,6 +768,9 @@ static int smc91c111_init1(SysBusDevice *dev)
 s->nic = qemu_new_nic(&net_smc91c111_info, &s->conf,
   object_get_typename(OBJECT(dev)), dev->qdev.id, s);
 qemu_format_nic_info_str(&s->nic->nc, s->conf.macaddr.a);
+
+mdio_phy_init(&s->phy, 0x0016, 0xf84);
+mdio_attach(&s->mdio_bus, &s->phy, 0);
 /* ??? Save/restore.  */
 return 0;
 }
-- 
1.7.10.4




Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Michael S. Tsirkin
On Sun, Feb 03, 2013 at 03:09:39PM -0600, Anthony Liguori wrote:
> Michael Tokarev  writes:
> 
> > 03.02.2013 17:23, Yan Vugenfirer wrote:
> >
> >>> If it helps, mq changes the config size from 8 to 16 bytes.  If the
> >>> driver was making an assumption about an 8-byte config size, that's
> >>> likely what the problem is.
> >>>
> >> That's exactly the problem.
> >
> > So what do we do?  It isn't nice to break existing setups.
> > Maybe mq can be disabled by default (together with Antony's
> > patch) until fixed drivers will be more widely available?
> 
> I've got a patch that does exactly like this.  It's pretty ugly though
> because of the relationship between virtio-pci and virtio-net.  I'm
> going to try to see if I can find a cleaner way to do it on Monday.
> 
> I'm also contemplating just disabling mq by default.  Since a special
> command line is needed to enable it anyway (on the backend), having to
> specify mq=on for the device doesn't seem so bad.


It's after midnight here so didn't test yet, but wouldn't the following
achieve this in a way that is a bit cleaner?

Signed-off-by: Michael S. Tsirkin 


diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5699f5e..3342a76 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice 
*vdev, uint32_t features)
 
 features |= (1 << VIRTIO_NET_F_MAC);
 
+if (n->max_queues <= 1) {
+features &= ~(0x1 << VIRTIO_NET_F_MQ);
+}
+
 if (!peer_has_vnet_hdr(n)) {
 features &= ~(0x1 << VIRTIO_NET_F_CSUM);
 features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
@@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 int i;
 
 n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf->queues > 1 ?
+sizeof(struct virtio_net_config) :
+sizeof(struct 
virtio_net_config_compat),
 sizeof(VirtIONet));
 
 n->vdev.get_config = virtio_net_get_config;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1d5c721..95ad54f 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -69,6 +69,15 @@ typedef struct virtio_net_conf
 /* Maximum packet size we can receive from tap device: header + 64k */
 #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
 
+
+struct virtio_net_config_compat
+{
+/* The config defining mac address ($ETH_ALEN bytes) */
+uint8_t mac[ETH_ALEN];
+/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+uint16_t status;
+} QEMU_PACKED;
+
 struct virtio_net_config
 {
 /* The config defining mac address ($ETH_ALEN bytes) */



[Qemu-devel] [Bug 1090600] Re: Improve error reporting when -drive format= is incorrect

2013-02-03 Thread Stefan Weil
Fixed in QEMU 1.4

** Changed in: qemu
   Status: In Progress => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1090600

Title:
  Improve error reporting when -drive format= is incorrect

Status in QEMU:
  Fix Released

Bug description:
  With qemu.git as of  e376a788ae130454ad5e797f60cb70d0308babb6

  $ qemu-img create -f raw test.raw 1M
  Formatting 'test.raw', fmt=raw size=1048576 
  $ ./x86_64-softmmu/qemu-system-x86_64 -drive file=./test.raw,format=qcow2
  qemu-system-x86_64: -drive file=./test.raw,format=qcow2: could not open disk 
image ./test.raw: Invalid argument
  $ ./x86_64-softmmu/qemu-system-x86_64 -drive file=./test.raw,format=vdi
  qemu-system-x86_64: -drive file=./test.raw,format=vdi: could not open disk 
image ./test.raw: Operation not permitted

  Ideally we'd get something like
  could not open disk image ./test.raw: not a qcow2 format file

  (this was kicking around in Fedora bugzilla for a long time at
  https://bugzilla.redhat.com/show_bug.cgi?id=556482 )

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1090600/+subscriptions



Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Anthony Liguori
"Michael S. Tsirkin"  writes:

> On Sun, Feb 03, 2013 at 03:09:39PM -0600, Anthony Liguori wrote:
>> Michael Tokarev  writes:
>> 
>> > 03.02.2013 17:23, Yan Vugenfirer wrote:
>> >
>> >>> If it helps, mq changes the config size from 8 to 16 bytes.  If the
>> >>> driver was making an assumption about an 8-byte config size, that's
>> >>> likely what the problem is.
>> >>>
>> >> That's exactly the problem.
>> >
>> > So what do we do?  It isn't nice to break existing setups.
>> > Maybe mq can be disabled by default (together with Antony's
>> > patch) until fixed drivers will be more widely available?
>> 
>> I've got a patch that does exactly like this.  It's pretty ugly though
>> because of the relationship between virtio-pci and virtio-net.  I'm
>> going to try to see if I can find a cleaner way to do it on Monday.
>> 
>> I'm also contemplating just disabling mq by default.  Since a special
>> command line is needed to enable it anyway (on the backend), having to
>> specify mq=on for the device doesn't seem so bad.
>
>
> It's after midnight here so didn't test yet, but wouldn't the following
> achieve this in a way that is a bit cleaner?
>
> Signed-off-by: Michael S. Tsirkin 
>
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 5699f5e..3342a76 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice 
> *vdev, uint32_t features)
>  
>  features |= (1 << VIRTIO_NET_F_MAC);
>  
> +if (n->max_queues <= 1) {
> +features &= ~(0x1 << VIRTIO_NET_F_MQ);
> +}
> +

This is too late because the config size has already been set.

>  if (!peer_has_vnet_hdr(n)) {
>  features &= ~(0x1 << VIRTIO_NET_F_CSUM);
>  features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
> @@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
> *conf,
>  int i;
>  
>  n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
> -sizeof(struct virtio_net_config),
> +conf->queues > 1 ?
> +sizeof(struct virtio_net_config) 
> :
> +sizeof(struct 
> virtio_net_config_compat),
>  sizeof(VirtIONet));

This makes me a bit nervous.  It relies on setting the config size to
one thing before we've masked a feature.

What do you think about just defaulting mq=off?  The more I think about
it, the nicer of an option that is.

I'm not a huge fan of automagically changing the value of a feature bit
so I think defaulting it off is a bit more straight forward.

We can certainly revisit for 1.5+.

Regards,

Anthony Liguori

>  
>  n->vdev.get_config = virtio_net_get_config;
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 1d5c721..95ad54f 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -69,6 +69,15 @@ typedef struct virtio_net_conf
>  /* Maximum packet size we can receive from tap device: header + 64k */
>  #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
>  
> +
> +struct virtio_net_config_compat
> +{
> +/* The config defining mac address ($ETH_ALEN bytes) */
> +uint8_t mac[ETH_ALEN];
> +/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
> +uint16_t status;
> +} QEMU_PACKED;
> +
>  struct virtio_net_config
>  {
>  /* The config defining mac address ($ETH_ALEN bytes) */



Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Vadim Rozenfeld
On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote:
> Michael Tokarev  writes:
> 
> > 03.02.2013 17:23, Yan Vugenfirer wrote:
> >
> >>> If it helps, mq changes the config size from 8 to 16 bytes.  If the
> >>> driver was making an assumption about an 8-byte config size, that's
> >>> likely what the problem is.
> >>>
> >> That's exactly the problem.
> >
> > So what do we do?  It isn't nice to break existing setups.
> > Maybe mq can be disabled by default (together with Antony's
> > patch) until fixed drivers will be more widely available?
> 
> I've got a patch that does exactly like this.  It's pretty ugly though
> because of the relationship between virtio-pci and virtio-net.  I'm
> going to try to see if I can find a cleaner way to do it on Monday.
> 
> I'm also contemplating just disabling mq by default.  Since a special
> command line is needed to enable it anyway (on the backend), having to
> specify mq=on for the device doesn't seem so bad.
> 
> But yeah, we don't want Windows guests to break with -M pc by default so
> we should do something to work around it.
> 
> N.B. this is a pretty nasty bug in the guest driver.  Any new virtio-net
> feature is going to trigger it (not just multiqueue).  So while pc-1.3
> will work forever with this guest image, it's pretty much guaranteed to
> break at some point in the future.
> 
> > It's easy to turn off mq by default and turn it on when
> > needed...
> >
> > The problem now is that it isn't obvious why your existing
> > setup breaks when you upgrade.  While when you especially
> > play with mq, you may look at options available around it...
> 
> If we ever to get virtio2, a really handy feature to have would be a
> driver identification string.  Even if was only informative (and not

Why not just use revision id for that? It really can be very useful, at
least for virtio Windows drivers.
BTW, Yan already fixed this problem in the Windows driver code. So we
can make a new build and make it public. But it probably will not be
WHQL'ed in the nearest future.
 
> authoritative, we've had a lot of issues like this and being able to
> make work arounds conditional on the driver identification string would
> be nice.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > How difficult it is to fix it in win driver?
> >
> > Thanks,
> >
> > /mjt





[Qemu-devel] [PATCH 0/6] change acpi numa info format passed from qemu to seabios

2013-02-03 Thread liguang
orginally, numa info was packed into an array
of 64-bit data which was implicit and hard to
maintain, so we define a struct for these info,
hope to be as clear as enough.

these changes also involved seabios paches 
which was sent to seabios mail-list.

Li Guang(6)
[PATCH 1/6] pc/bios: move common BIOS_CFG_IOPORT into fw_cfg.h
[PATCH 2/6] pc/numa: refactor bios_init function
[PATCH 3/6] bitops: change BITS_TO_LONGS
[PATCH 4/6] pc: format load_linux()
[PATCH 5/6] load_linux: report open kernel file & its size error
[PATCH 6/6] load_linux: change kernel header size allocation

 hw/fw_cfg.h |4 
 hw/pc.c |  163 
+++---
 hw/sun4u.c  |3 +--
 3 files changed, 89 insertions(+), 81 deletions(-)




[Qemu-devel] [PATCH 4/6] pc: format load_linux()

2013-02-03 Thread liguang
seems this function was in wrong coding format
so try correct it boldly.

Signed-off-by: liguang 
---
 hw/pc.c |   90 +++---
 1 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 893c930..01d00f6 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -639,8 +639,8 @@ static long get_file_size(FILE *f)
 
 static void load_linux(void *fw_cfg,
const char *kernel_filename,
-  const char *initrd_filename,
-  const char *kernel_cmdline,
+   const char *initrd_filename,
+   const char *kernel_cmdline,
hwaddr max_ram_size)
 {
 uint16_t protocol;
@@ -657,11 +657,11 @@ static void load_linux(void *fw_cfg,
 /* load the kernel header */
 f = fopen(kernel_filename, "rb");
 if (!f || !(kernel_size = get_file_size(f)) ||
-   fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
-   MIN(ARRAY_SIZE(header), kernel_size)) {
-   fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
-   kernel_filename, strerror(errno));
-   exit(1);
+fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
+MIN(ARRAY_SIZE(header), kernel_size)) {
+fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
+kernel_filename, strerror(errno));
+exit(1);
 }
 
 /* kernel protocol version */
@@ -669,48 +669,48 @@ static void load_linux(void *fw_cfg,
 fprintf(stderr, "header magic: %#x\n", ldl_p(header+0x202));
 #endif
 if (ldl_p(header+0x202) == 0x53726448)
-   protocol = lduw_p(header+0x206);
+protocol = lduw_p(header+0x206);
 else {
-   /* This looks like a multiboot kernel. If it is, let's stop
-  treating it like a Linux kernel. */
+/* This looks like a multiboot kernel. If it is, let's stop
+   treating it like a Linux kernel. */
 if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
kernel_cmdline, kernel_size, header))
 return;
-   protocol = 0;
+protocol = 0;
 }
 
 if (protocol < 0x200 || !(header[0x211] & 0x01)) {
-   /* Low kernel */
-   real_addr= 0x9;
-   cmdline_addr = 0x9a000 - cmdline_size;
-   prot_addr= 0x1;
+/* Low kernel */
+real_addr= 0x9;
+cmdline_addr = 0x9a000 - cmdline_size;
+prot_addr= 0x1;
 } else if (protocol < 0x202) {
-   /* High but ancient kernel */
-   real_addr= 0x9;
-   cmdline_addr = 0x9a000 - cmdline_size;
-   prot_addr= 0x10;
+/* High but ancient kernel */
+real_addr= 0x9;
+cmdline_addr = 0x9a000 - cmdline_size;
+prot_addr= 0x10;
 } else {
-   /* High and recent kernel */
-   real_addr= 0x1;
-   cmdline_addr = 0x2;
-   prot_addr= 0x10;
+/* High and recent kernel */
+real_addr= 0x1;
+cmdline_addr = 0x2;
+prot_addr= 0x10;
 }
 
 #if 0
 fprintf(stderr,
-   "qemu: real_addr = 0x" TARGET_FMT_plx "\n"
-   "qemu: cmdline_addr  = 0x" TARGET_FMT_plx "\n"
-   "qemu: prot_addr = 0x" TARGET_FMT_plx "\n",
-   real_addr,
-   cmdline_addr,
-   prot_addr);
+"qemu: real_addr = 0x" TARGET_FMT_plx "\n"
+"qemu: cmdline_addr  = 0x" TARGET_FMT_plx "\n"
+"qemu: prot_addr = 0x" TARGET_FMT_plx "\n",
+real_addr,
+cmdline_addr,
+prot_addr);
 #endif
 
 /* highest address for loading the initrd */
 if (protocol >= 0x203)
-   initrd_max = ldl_p(header+0x22c);
+initrd_max = ldl_p(header+0x22c);
 else
-   initrd_max = 0x37ff;
+initrd_max = 0x37ff;
 
 if (initrd_max >= max_ram_size-ACPI_DATA_SIZE)
initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
@@ -720,10 +720,10 @@ static void load_linux(void *fw_cfg,
 fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
 
 if (protocol >= 0x202) {
-   stl_p(header+0x228, cmdline_addr);
+stl_p(header+0x228, cmdline_addr);
 } else {
-   stw_p(header+0x20, 0xA33F);
-   stw_p(header+0x22, cmdline_addr-real_addr);
+stw_p(header+0x20, 0xA33F);
+stw_p(header+0x22, cmdline_addr-real_addr);
 }
 
 /* handle vga= parameter */
@@ -749,22 +749,22 @@ static void load_linux(void *fw_cfg,
If this code is substantially changed, you may want to consider
incrementing the revision. */
 if (protocol >= 0x200)
-   header[0x210] = 0xB0;
+header[0x210] = 0xB0;
 
 /* heap */
 if (protocol >= 0x201) {
-   header[0x211] |= 0x80;  /* CAN_USE_HEAP */
-   stw_p(header+0x224, cmdline_addr-real_addr-0x200);
+header[0x211] |= 0x80; /* CAN_

[Qemu-devel] [PATCH 6/6] load_linux: change kernel header size allocation

2013-02-03 Thread liguang
it's not necessary to alloc 8K bytes for kernel
header, 0.5K is enough.

Signed-off-by: liguang 
---
 hw/pc.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 0ccd775..b6b236f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -637,6 +637,9 @@ static long get_file_size(FILE *f)
 return size;
 }
 
+#define HEADER_SIZE 0x200
+#define HEADER_SIGNATURE 0x53726448
+
 static void load_linux(void *fw_cfg,
const char *kernel_filename,
const char *initrd_filename,
@@ -646,7 +649,7 @@ static void load_linux(void *fw_cfg,
 uint16_t protocol;
 int setup_size, kernel_size, initrd_size = 0, cmdline_size;
 uint32_t initrd_max;
-uint8_t header[8192], *setup, *kernel, *initrd_data;
+uint8_t header[HEADER_SIZE], *setup, *kernel, *initrd_data;
 hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
 FILE *f;
 char *vmode;
@@ -665,8 +668,7 @@ static void load_linux(void *fw_cfg,
 fprintf(stderr, "can't get size of kernel image file\n");
 exit(1);
 }
-if (fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
-MIN(ARRAY_SIZE(header), kernel_size)) {
+if (fread(header, 1, HEADER_SIZE, f) <= 0) {
 fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
 kernel_filename, strerror(errno));
 exit(1);
@@ -676,7 +678,7 @@ static void load_linux(void *fw_cfg,
 #if 0
 fprintf(stderr, "header magic: %#x\n", ldl_p(header+0x202));
 #endif
-if (ldl_p(header+0x202) == 0x53726448)
+if (ldl_p(header+0x202) == HEADER_SIGNATURE)
 protocol = lduw_p(header+0x206);
 else {
 /* This looks like a multiboot kernel. If it is, let's stop
-- 
1.7.2.5




[Qemu-devel] [PATCH 5/6] load_linux: report open kernel file & its size error

2013-02-03 Thread liguang
Signed-off-by: liguang 
---
 hw/pc.c |   14 +++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 01d00f6..0ccd775 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -652,12 +652,20 @@ static void load_linux(void *fw_cfg,
 char *vmode;
 
 /* Align to 16 bytes as a paranoia measure */
-cmdline_size = (strlen(kernel_cmdline)+16) & ~15;
+cmdline_size = (strlen(kernel_cmdline)+16) & ~0xf;
 
 /* load the kernel header */
 f = fopen(kernel_filename, "rb");
-if (!f || !(kernel_size = get_file_size(f)) ||
-fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
+if (!f) {
+fprintf(stderr, "can't open kernel image file\n");
+exit(1);
+}
+kernel_size = get_file_size(f);
+if (kernel_size <= 0) {
+fprintf(stderr, "can't get size of kernel image file\n");
+exit(1);
+}
+if (fread(header, 1, MIN(ARRAY_SIZE(header), kernel_size), f) !=
 MIN(ARRAY_SIZE(header), kernel_size)) {
 fprintf(stderr, "qemu: could not load kernel '%s': %s\n",
 kernel_filename, strerror(errno));
-- 
1.7.2.5




[Qemu-devel] [seabios][PATCH 2/2] acpi: change numa data format from fw_cfg interface

2013-02-03 Thread liguang
the old numa format got form fw_cfg is:
number of nodes
node id of cpu (array)
node memory size (array)

now, format it like array of:
apci_map,
memory_size,

it has the advantage of simple and clear.

Signed-off-by: liguang 
---
 src/acpi.c |   57 ---
 src/acpi.h |5 
 src/paravirt.c |2 +-
 src/paravirt.h |1 +
 4 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index fde37e5..7004a60 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -473,44 +473,57 @@ acpi_build_srat_memory(struct srat_memory_affinity 
*numamem,
 static void *
 build_srat(void)
 {
-int nb_numa_nodes = qemu_cfg_get_numa_nodes();
+int nr_nodes = qemu_cfg_get_numa_nodes();
+int *node_cpu = malloc_tmp(sizeof(int));
 
-if (nb_numa_nodes == 0)
+if (nr_nodes == 0)
 return NULL;
 
-u64 *numadata = malloc_tmphigh(sizeof(u64) * (MaxCountCPUs + 
nb_numa_nodes));
-if (!numadata) {
+if (!node_cpu)
+return NULL;
+
+struct srat_data *sd = malloc_tmp(sizeof(struct srat_data)*nr_nodes);
+if (!sd) {
 warn_noalloc();
 return NULL;
 }
 
-qemu_cfg_get_numa_data(numadata, MaxCountCPUs + nb_numa_nodes);
+qemu_cfg_get_numa_data((u64 *)sd, nr_nodes);
 
 struct system_resource_affinity_table *srat;
 int srat_size = sizeof(*srat) +
 sizeof(struct srat_processor_affinity) * MaxCountCPUs +
-sizeof(struct srat_memory_affinity) * (nb_numa_nodes + 2);
+sizeof(struct srat_memory_affinity) * (nr_nodes + 2);
 
 srat = malloc_high(srat_size);
 if (!srat) {
 warn_noalloc();
-free(numadata);
+free(srat);
 return NULL;
 }
 
 memset(srat, 0, srat_size);
-srat->reserved1=1;
+srat->reserved1 = 1;
 struct srat_processor_affinity *core = (void*)(srat + 1);
-int i;
+int i, j;
 u64 curnode;
 
+for (i = 0; i < nr_nodes; i++) {
+if (sd[i].apic_map == 0)
+continue;
+for (j = 0; j < MaxCountCPUs; j++) {
+if (sd[i].apic_map & 1 << j)
+node_cpu[j] = i;
+}
+}
+
 for (i = 0; i < MaxCountCPUs; ++i) {
 core->type = SRAT_PROCESSOR;
 core->length = sizeof(*core);
 core->local_apic_id = i;
-curnode = *numadata++;
+curnode = i;
 core->proximity_lo = curnode;
-memset(core->proximity_hi, 0, 3);
+memset(core->proximity_hi, 0, sizeof(core->proximity_hi));
 core->local_sapic_eid = 0;
 if (apic_id_is_present(i))
 core->flags = cpu_to_le32(1);
@@ -527,15 +540,19 @@ build_srat(void)
 int slots = 0;
 u64 mem_len, mem_base, next_base = 0;
 
-acpi_build_srat_memory(numamem, 0, 640*1024, 0, 1);
-next_base = 1024 * 1024;
+#define MEM_1K (1UL << 10)
+#define MEM_1M (1UL << 20)
+#define MEM_1G (1ULL << 30)
+
+acpi_build_srat_memory(numamem, 0, 640*MEM_1K, 0, 1);
+next_base = MEM_1M;
 numamem++;
 slots++;
-for (i = 1; i < nb_numa_nodes + 1; ++i) {
+for (i = 1; i < nr_nodes + 1; ++i) {
 mem_base = next_base;
-mem_len = *numadata++;
+mem_len = sd[i].memory_size;
 if (i == 1)
-mem_len -= 1024 * 1024;
+mem_len -= MEM_1M;
 next_base = mem_base + mem_len;
 
 /* Cut out the PCI hole */
@@ -546,22 +563,22 @@ build_srat(void)
 numamem++;
 slots++;
 }
-mem_base = 1ULL << 32;
+mem_base = MEM_1G;
 mem_len = next_base - RamSize;
-next_base += (1ULL << 32) - RamSize;
+next_base += MEM_1G - RamSize;
 }
 acpi_build_srat_memory(numamem, mem_base, mem_len, i-1, 1);
 numamem++;
 slots++;
 }
-for (; slots < nb_numa_nodes + 2; slots++) {
+for (; slots < nr_nodes + 2; slots++) {
 acpi_build_srat_memory(numamem, 0, 0, 0, 0);
 numamem++;
 }
 
 build_header((void*)srat, SRAT_SIGNATURE, srat_size, 1);
 
-free(numadata);
+free(sd);
 return srat;
 }
 
diff --git a/src/acpi.h b/src/acpi.h
index 996edcb..bfa2c79 100644
--- a/src/acpi.h
+++ b/src/acpi.h
@@ -321,6 +321,11 @@ struct srat_memory_affinity
 u32reserved3[2];
 } PACKED;
 
+struct srat_data {
+u64 apic_map;
+u64 memory_size;
+};
+
 #include "acpi-dsdt.hex"
 
 
diff --git a/src/paravirt.c b/src/paravirt.c
index 4b5c441..e143ca7 100644
--- a/src/paravirt.c
+++ b/src/paravirt.c
@@ -291,7 +291,7 @@ void qemu_cfg_get_numa_data(u64 *data, int n)
 int i;
 
 for (i = 0; i < n; i++)
-qemu_cfg_read((u8*)(data + i), sizeof(u64));
+qemu_cfg_read((u8*)(data + i), sizeof(struct srat_data));
 }
 
 u16 qemu_cfg_get_max_cpus(void)
diff --git a/src/paravirt.h b/src/paravirt.h
index a284c41..2b25c4f 100644
--- a/src/paravirt.h
+++ b/src/paravirt.h
@@ -3,6 +3,7 @@
 
 #include "config.h" // CONFIG_COREBOOT
 #include "util

[Qemu-devel] [PATCH 2/6] pc/numa: refactor bios_init function

2013-02-03 Thread liguang
orginally, numa data was packed into an array,
which was implicit and hard to maintain, we
define a struct for this data, hope to be as
clear as enough.
also, we only pass cpumask of corresponding
nodes to seabios, and leave the paring work
for it.

Signed-off-by: liguang 
---
 hw/pc.c |   40 ++--
 1 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index d010c75..893c930 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -562,13 +562,21 @@ static unsigned int pc_apic_id_limit(unsigned int 
max_cpus)
 return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
 }
 
+struct srat_data {
+uint64_t apic_map; /* size is MAX_NODES */
+uint64_t memory_size;
+};
+
 static void *bios_init(void)
 {
 void *fw_cfg;
 uint8_t *smbios_table;
 size_t smbios_len;
-uint64_t *numa_fw_cfg;
-int i, j;
+struct fw_numa_cfg {
+uint32_t nr_node;
+struct srat_data *srat_data;
+} fw_cfg_numa;
+int i;
 unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
 fw_cfg = fw_cfg_init(FW_CFG_CTL_IOPORT, FW_CFG_DATA_IOPORT, 0, 0);
@@ -601,28 +609,16 @@ static void *bios_init(void)
  &e820_table, sizeof(e820_table));
 
 fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
-/* allocate memory for the NUMA channel: one (64bit) word for the number
- * of nodes, one word for each VCPU->node and one word for each node to
- * hold the amount of memory.
- */
-numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
-numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
-for (i = 0; i < max_cpus; i++) {
-unsigned int apic_id = x86_cpu_apic_id_from_index(i);
-assert(apic_id < apic_id_limit);
-for (j = 0; j < nb_numa_nodes; j++) {
-if (test_bit(i, node_cpumask[j])) {
-numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
-break;
-}
-}
-}
+
+fw_cfg_numa.srat_data = g_new0(struct srat_data, nb_numa_nodes);
+fw_cfg_numa.nr_node = cpu_to_le64(nb_numa_nodes);
+
 for (i = 0; i < nb_numa_nodes; i++) {
-numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
+fw_cfg_numa.srat_data[i].apic_map = *node_cpumask[i];
+fw_cfg_numa.srat_data[i].memory_size = cpu_to_le64(node_mem[i]);
 }
-fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
- (1 + apic_id_limit + nb_numa_nodes) *
- sizeof(*numa_fw_cfg));
+
+fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, &fw_cfg_numa, sizeof(fw_cfg_numa));
 
 return fw_cfg;
 }
-- 
1.7.2.5




[Qemu-devel] [PATCH 3/6] bitops: change BITS_TO_LONGS

2013-02-03 Thread liguang
Signed-off-by: liguang 
---
 include/qemu/bitops.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 74e14e5..7758792 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -20,7 +20,7 @@
 #define BIT(nr)(1UL << (nr))
 #define BIT_MASK(nr)   (1UL << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
-#define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+#define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_LONG)
 
 /**
  * bitops_ffs - find first bit in word.
-- 
1.7.2.5




[Qemu-devel] [seabios][PATCH 1/2] move all acpi-table related definitions to acpi.h

2013-02-03 Thread liguang
Signed-off-by: liguang 
---
 src/acpi.c |  209 ++--
 src/acpi.h |  201 +
 2 files changed, 206 insertions(+), 204 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index 6267d7b..fde37e5 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -15,204 +15,12 @@
 #include "paravirt.h" // qemu_cfg_irq0_override
 #include "dev-q35.h" // qemu_cfg_irq0_override
 
-//
-/* ACPI tables init */
-
-/* Table structure from Linux kernel (the ACPI tables are under the
-   BSD license) */
-
-struct acpi_table_header /* ACPI common table header */
-{
-ACPI_TABLE_HEADER_DEF
-} PACKED;
-
-/*
- * ACPI 1.0 Root System Description Table (RSDT)
- */
-#define RSDT_SIGNATURE 0x54445352 // RSDT
-struct rsdt_descriptor_rev1
-{
-ACPI_TABLE_HEADER_DEF   /* ACPI common table header */
-u32 table_offset_entry[0];  /* Array of pointers to other */
-/* ACPI tables */
-} PACKED;
-
-/*
- * ACPI 1.0 Firmware ACPI Control Structure (FACS)
- */
-#define FACS_SIGNATURE 0x53434146 // FACS
-struct facs_descriptor_rev1
-{
-u32 signature;   /* ACPI Signature */
-u32 length; /* Length of structure, in bytes */
-u32 hardware_signature; /* Hardware configuration signature */
-u32 firmware_waking_vector; /* ACPI OS waking vector */
-u32 global_lock;/* Global Lock */
-u32 S4bios_f: 1;/* Indicates if S4BIOS support is present */
-u32 reserved1   : 31;   /* Must be 0 */
-u8  resverved3 [40];/* Reserved - must be zero */
-} PACKED;
-
-
-/*
- * Differentiated System Description Table (DSDT)
- */
-#define DSDT_SIGNATURE 0x54445344 // DSDT
-
-/*
- * MADT values and structures
- */
-
-/* Values for MADT PCATCompat */
-
-#define DUAL_PIC0
-#define MULTIPLE_APIC   1
-
-
-/* Master MADT */
-
-#define APIC_SIGNATURE 0x43495041 // APIC
-struct multiple_apic_table
-{
-ACPI_TABLE_HEADER_DEF /* ACPI common table header */
-u32 local_apic_address; /* Physical address of local APIC */
-#if 0
-u32 PCATcompat  : 1;/* A one indicates system also has dual 8259s 
*/
-u32 reserved1   : 31;
-#else
-u32 flags;
-#endif
-} PACKED;
-
-
-/* Values for Type in APIC sub-headers */
-
-#define APIC_PROCESSOR  0
-#define APIC_IO 1
-#define APIC_XRUPT_OVERRIDE 2
-#define APIC_NMI3
-#define APIC_LOCAL_NMI  4
-#define APIC_ADDRESS_OVERRIDE   5
-#define APIC_IO_SAPIC   6
-#define APIC_LOCAL_SAPIC7
-#define APIC_XRUPT_SOURCE   8
-#define APIC_RESERVED   9   /* 9 and greater are reserved */
-
-/*
- * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE)
- */
-#define ACPI_SUB_HEADER_DEF   /* Common ACPI sub-structure header */\
-u8  type;   \
-u8  length;
-
-/* Sub-structures for MADT */
-
-struct madt_processor_apic
-{
-ACPI_SUB_HEADER_DEF
-u8  processor_id;   /* ACPI processor id */
-u8  local_apic_id;  /* Processor's local APIC id */
-#if 0
-u32 processor_enabled: 1;   /* Processor is usable if set */
-u32 reserved2   : 31;   /* Reserved, must be zero */
-#else
-u32 flags;
-#endif
-} PACKED;
-
-struct madt_io_apic
-{
-ACPI_SUB_HEADER_DEF
-u8  io_apic_id; /* I/O APIC ID */
-u8  reserved;   /* Reserved - must be zero */
-u32 address;/* APIC physical address */
-u32 interrupt;  /* Global system interrupt where INTI
- * lines start */
-} PACKED;
-
-/* IRQs 5,9,10,11 */
-#define PCI_ISA_IRQ_MASK0x0e20
-
-struct madt_intsrcovr {
-ACPI_SUB_HEADER_DEF
-u8  bus;
-u8  source;
-u32 gsi;
-u16 flags;
-} PACKED;
-
-struct madt_local_nmi {
-ACPI_SUB_HEADER_DEF
-u8  processor_id;   /* ACPI processor id */
-u16 flags;  /* MPS INTI flags */
-u8  lint;   /* Local APIC LINT# */
-} PACKED;
-
-
-/*
- * ACPI 2.0 Generic Address Space definition.
- */
-struct acpi_20_generic_address {
-u8  address_space_id;
-u8  register_bit_width;
-u8  register_bit_offset;
-u8  reserved;
-u64 address;
-} PACKED;
-
-/*
- * HPET Description Table
- */
-struct acpi_20_hpet {
-ACPI_TABLE_HEADER_DEF/* ACPI common table header */
-u32   timer_block_id;
-struct acpi_20_generic_address addr;
-u8hpet_number;
-u16   min_tick;
-u8page_protect;
-} PACKED;
-
-#define HPET_ID 0x000
-#define HPET_PERIOD 0x004
-
-/*
- * SRAT (NUMA topology description) table
- */
-
-#define SRAT_PROCESSOR  0
-#define SRAT_MEMORY 1
-
-struct system_resource_affinity_table
-{
-ACPI_TABLE_HEADER_DEF
-u32reserved1;
-u32reser

[Qemu-devel] [PATCH 1/6] pc/bios: move common BIOS_CFG_IOPORT into fw_cfg.h

2013-02-03 Thread liguang
BIOS_CFG_IOPORT are commonly used, so move it to fw_cfg.h
bochs_bios_init seems misleading, bios may be seabios,
seabios is not only for bochs, and also we are in qemu.

Signed-off-by: liguang 
---
 hw/fw_cfg.h |4 
 hw/pc.c |9 -
 hw/sun4u.c  |3 +--
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 05c8df1..6b3147d 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -38,6 +38,10 @@
 
 #define FW_CFG_INVALID  0x
 
+#define FW_CFG_CTL_IOPORT 0x510
+#define FW_CFG_DATA_IOPORT 0x511
+
+
 #ifndef NO_QEMU_PROTOS
 typedef struct FWCfgFile {
 uint32_t  size;/* file size */
diff --git a/hw/pc.c b/hw/pc.c
index 34b6dff..d010c75 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -64,7 +64,6 @@
 
 /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables.  */
 #define ACPI_DATA_SIZE   0x1
-#define BIOS_CFG_IOPORT 0x510
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
 #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
@@ -556,14 +555,14 @@ int e820_add_entry(uint64_t address, uint64_t length, 
uint32_t type)
  * This function returns the limit for the APIC ID value, so that all
  * CPU APIC IDs are < pc_apic_id_limit().
  *
- * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
+ * This is used for FW_CFG_MAX_CPUS. See comments on bios_init().
  */
 static unsigned int pc_apic_id_limit(unsigned int max_cpus)
 {
 return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
 }
 
-static void *bochs_bios_init(void)
+static void *bios_init(void)
 {
 void *fw_cfg;
 uint8_t *smbios_table;
@@ -572,7 +571,7 @@ static void *bochs_bios_init(void)
 int i, j;
 unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
-fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
+fw_cfg = fw_cfg_init(FW_CFG_CTL_IOPORT, FW_CFG_DATA_IOPORT, 0, 0);
 /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
  *
  * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
@@ -954,7 +953,7 @@ void *pc_memory_init(MemoryRegion *system_memory,
 option_rom_mr,
 1);
 
-fw_cfg = bochs_bios_init();
+fw_cfg = bios_init();
 rom_set_fw(fw_cfg);
 
 if (linux_boot) {
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 9fbda29..1bdc443 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -76,7 +76,6 @@
 #define PROM_FILENAME"openbios-sparc64"
 #define NVRAM_SIZE   0x2000
 #define MAX_IDE_BUS  2
-#define BIOS_CFG_IOPORT  0x510
 #define FW_CFG_SPARC64_WIDTH (FW_CFG_ARCH_LOCAL + 0x00)
 #define FW_CFG_SPARC64_HEIGHT (FW_CFG_ARCH_LOCAL + 0x01)
 #define FW_CFG_SPARC64_DEPTH (FW_CFG_ARCH_LOCAL + 0x02)
@@ -877,7 +876,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
graphic_width, graphic_height, graphic_depth,
(uint8_t *)&nd_table[0].macaddr);
 
-fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
+fw_cfg = fw_cfg_init(FW_CFG_CTL_IOPORT, FW_CFG_DATA_IOPORT, 0, 0);
 fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
 fw_cfg_add_i32(fw_cfg, FW_CFG_ID, 1);
 fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
-- 
1.7.2.5




Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Rusty Russell
Anthony Liguori  writes:
> Michael Tokarev  writes:
>
>> 03.02.2013 17:23, Yan Vugenfirer wrote:
>>
 If it helps, mq changes the config size from 8 to 16 bytes.  If the
 driver was making an assumption about an 8-byte config size, that's
 likely what the problem is.

>>> That's exactly the problem.
...
> N.B. this is a pretty nasty bug in the guest driver.  Any new virtio-net
> feature is going to trigger it (not just multiqueue).  So while pc-1.3
> will work forever with this guest image, it's pretty much guaranteed to
> break at some point in the future.

Wow, yes.  I never expected such a bug.  I probably don't even want to
know how this happened, right?

If I could find a way, I'd like to create some code as an appendix to
the virtio spec which would torture test each driver and/or device by
configuring it in strange ways.  But that's pure speculation at this
point.

>> It's easy to turn off mq by default and turn it on when
>> needed...
>>
>> The problem now is that it isn't obvious why your existing
>> setup breaks when you upgrade.  While when you especially
>> play with mq, you may look at options available around it...
>
> If we ever to get virtio2, a really handy feature to have would be a
> driver identification string.  Even if was only informative (and not
> authoritative, we've had a lot of issues like this and being able to
> make work arounds conditional on the driver identification string would
> be nice.

In practice, we'd want to formalize it into a string and a version, and
hope the version gets bumped appropriately.  Because it'll actually be
used for bug detection.

But AFAICT that would be useless in this case.  We really want to know about
the guest before we even start it.

Cheers,
Rusty.



Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Michael S. Tsirkin
On Mon, Feb 04, 2013 at 10:30:47AM +1100, Vadim Rozenfeld wrote:
> On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote:
> > Michael Tokarev  writes:
> > 
> > > 03.02.2013 17:23, Yan Vugenfirer wrote:
> > >
> > >>> If it helps, mq changes the config size from 8 to 16 bytes.  If the
> > >>> driver was making an assumption about an 8-byte config size, that's
> > >>> likely what the problem is.
> > >>>
> > >> That's exactly the problem.
> > >
> > > So what do we do?  It isn't nice to break existing setups.
> > > Maybe mq can be disabled by default (together with Antony's
> > > patch) until fixed drivers will be more widely available?
> > 
> > I've got a patch that does exactly like this.  It's pretty ugly though
> > because of the relationship between virtio-pci and virtio-net.  I'm
> > going to try to see if I can find a cleaner way to do it on Monday.
> > 
> > I'm also contemplating just disabling mq by default.  Since a special
> > command line is needed to enable it anyway (on the backend), having to
> > specify mq=on for the device doesn't seem so bad.
> > 
> > But yeah, we don't want Windows guests to break with -M pc by default so
> > we should do something to work around it.
> > 
> > N.B. this is a pretty nasty bug in the guest driver.  Any new virtio-net
> > feature is going to trigger it (not just multiqueue).  So while pc-1.3
> > will work forever with this guest image, it's pretty much guaranteed to
> > break at some point in the future.
> > 
> > > It's easy to turn off mq by default and turn it on when
> > > needed...
> > >
> > > The problem now is that it isn't obvious why your existing
> > > setup breaks when you upgrade.  While when you especially
> > > play with mq, you may look at options available around it...
> > 
> > If we ever to get virtio2, a really handy feature to have would be a
> > driver identification string.  Even if was only informative (and not
> 
> Why not just use revision id for that?

This will break well-behaved linux guests.

> It really can be very useful, at
> least for virtio Windows drivers.
> BTW, Yan already fixed this problem in the Windows driver code. So we
> can make a new build and make it public. But it probably will not be
> WHQL'ed in the nearest future.
>  
> > authoritative, we've had a lot of issues like this and being able to
> > make work arounds conditional on the driver identification string would
> > be nice.
> > 
> > Regards,
> > 
> > Anthony Liguori
> > 
> > >
> > > How difficult it is to fix it in win driver?
> > >
> > > Thanks,
> > >
> > > /mjt
> 



Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

2013-02-03 Thread Michael S. Tsirkin
On Sun, Feb 03, 2013 at 04:57:38PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
> > On Sun, Feb 03, 2013 at 03:09:39PM -0600, Anthony Liguori wrote:
> >> Michael Tokarev  writes:
> >> 
> >> > 03.02.2013 17:23, Yan Vugenfirer wrote:
> >> >
> >> >>> If it helps, mq changes the config size from 8 to 16 bytes.  If the
> >> >>> driver was making an assumption about an 8-byte config size, that's
> >> >>> likely what the problem is.
> >> >>>
> >> >> That's exactly the problem.
> >> >
> >> > So what do we do?  It isn't nice to break existing setups.
> >> > Maybe mq can be disabled by default (together with Antony's
> >> > patch) until fixed drivers will be more widely available?
> >> 
> >> I've got a patch that does exactly like this.  It's pretty ugly though
> >> because of the relationship between virtio-pci and virtio-net.  I'm
> >> going to try to see if I can find a cleaner way to do it on Monday.
> >> 
> >> I'm also contemplating just disabling mq by default.  Since a special
> >> command line is needed to enable it anyway (on the backend), having to
> >> specify mq=on for the device doesn't seem so bad.
> >
> >
> > It's after midnight here so didn't test yet, but wouldn't the following
> > achieve this in a way that is a bit cleaner?
> >
> > Signed-off-by: Michael S. Tsirkin 
> >
> >
> > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > index 5699f5e..3342a76 100644
> > --- a/hw/virtio-net.c
> > +++ b/hw/virtio-net.c
> > @@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice 
> > *vdev, uint32_t features)
> >  
> >  features |= (1 << VIRTIO_NET_F_MAC);
> >  
> > +if (n->max_queues <= 1) {
> > +features &= ~(0x1 << VIRTIO_NET_F_MQ);
> > +}
> > +
> 
> This is too late because the config size has already been set.
> 
> >  if (!peer_has_vnet_hdr(n)) {
> >  features &= ~(0x1 << VIRTIO_NET_F_CSUM);
> >  features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
> > @@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, 
> > NICConf *conf,
> >  int i;
> >  
> >  n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
> > -sizeof(struct virtio_net_config),
> > +conf->queues > 1 ?
> > +sizeof(struct 
> > virtio_net_config) :
> > +sizeof(struct 
> > virtio_net_config_compat),
> >  sizeof(VirtIONet));
> 
> This makes me a bit nervous.  It relies on setting the config size to
> one thing before we've masked a feature.
> 
> What do you think about just defaulting mq=off?  The more I think about
> it, the nicer of an option that is.
> 
> I'm not a huge fan of automagically changing the value of a feature bit
> so I think defaulting it off is a bit more straight forward.
> 
> We can certainly revisit for 1.5+.
> 
> Regards,
> 
> Anthony Liguori

Thinking about it some more, we have the queues option so
adding an mq option as well is not useful.
Let's clear it unless # of queues is set?
What do you think about the below?
This time tested with an sq guest but not an mq guest yet.

Signed-off-by: Michael S. Tsirkin 

---

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5699f5e..b8c8439 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice 
*vdev, uint32_t features)
 
 features |= (1 << VIRTIO_NET_F_MAC);
 
+if (n->max_queues > 1) {
+features |= (0x1 << VIRTIO_NET_F_MQ);
+}
+
 if (!peer_has_vnet_hdr(n)) {
 features &= ~(0x1 << VIRTIO_NET_F_CSUM);
 features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
@@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 int i;
 
 n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf->queues > 1 ?
+sizeof(struct virtio_net_config) :
+sizeof(struct 
virtio_net_config_compat),
 sizeof(VirtIONet));
 
 n->vdev.get_config = virtio_net_get_config;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1d5c721..19f63cf 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -69,6 +69,14 @@ typedef struct virtio_net_conf
 /* Maximum packet size we can receive from tap device: header + 64k */
 #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
 
+struct virtio_net_config_compat
+{
+/* The config defining mac address ($ETH_ALEN bytes) */
+uint8_t mac[ETH_ALEN];
+/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+uint16_t status;
+} QEMU_PACKED;
+
 struct virtio_net_config
 {
 /* The config defining mac address ($ETH_ALEN bytes) */
@@ -190,6 +199,5 @@ s

[Qemu-devel] QEMU does not communicate properly with GDB with a 64 bit guest

2013-02-03 Thread Furukawa, Eiji
When GDB is connected with qemu-1.1.0(target x86_64-softmmu), it is 
not possible to debug it. 
(Bug#640213)

The reason for the cause is that the correspondence of CPU register 
and the register value seen on the GDB side is incorrect. 

The evasion of the problem is possible by the replacement of the cpu_
gdb_read_register() of qeqemu-1.1.0/gdbstub.c with the cpu_gdb_
read_register() of qemu-0.10.6/gdbstub.c. 

What is the change intention of this source?


Qemu:qemu-1.1.0
GDB :6.8-27.el5

- Qemu start option
./qemu-system-x86_64 -m 2048 hda.img -machine pc-1.0 -L ./pc-bios/ -monitor pty 
-s -S

- CPU register dump from Qemu console
(qemu) info registers
EAX=7480 EBX=0350 ECX=0053 EDX=01f0
ESI=03f6 EDI=015a EBP=00074800 ESP=02ec
EIP=e8f9 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 
SMM=0 HLT=0 
ES =7480 00074800  9300 
CS =f000 000f  9e00 
SS =9f40 0009f400  9300 
DS =9f40 0009f400  9300 
FS =   9300 
GS =   9300
LDT=   8200
TR =   8b00
GDT= 9090 0027
IDT=  03ff
 
 

- CPU register dump from GDB
(gdb) info registers
eax0x76a0   30368
ecx0xe8f9   59641
edx0x46 70
ebx0xf000   61440
esp0x9f40   0x9f40
ebp0x9f40   0x9f40
esi0x76a0   30368
edi0x0  0
eip0x0  0
eflags 0x0  [ ]
cs 0x0  0
ss 0x0  0
ds 0x0  0
es 0x0  0
fs 0x0  0
gs 0x0  0

--
E.Furukawa



Re: [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init

2013-02-03 Thread Konrad Frederic

On 03/02/2013 00:06, Anthony Liguori wrote:

Signed-off-by: Anthony Liguori
---
  hw/s390x/s390-virtio-bus.c | 3 ++-
  hw/s390x/virtio-ccw.c  | 3 ++-
  hw/virtio-pci.c| 3 ++-
  hw/virtio.h| 3 ++-
  4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d467781..089ed92 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -153,7 +153,8 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
  {
  VirtIODevice *vdev;

-vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net);
+vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net,
+   dev->host_features);
  if (!vdev) {
  return -1;
  }
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 231f81e..d92e427 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -555,7 +555,8 @@ static int virtio_ccw_net_init(VirtioCcwDevice *dev)
  {
  VirtIODevice *vdev;

-vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net);
+vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net,
+   dev->host_features[0]);
  if (!vdev) {
  return -1;
  }
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9abbcdf..a869f53 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -997,7 +997,8 @@ static int virtio_net_init_pci(PCIDevice *pci_dev)
  VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
  VirtIODevice *vdev;

-vdev = virtio_net_init(&pci_dev->qdev,&proxy->nic,&proxy->net);
+vdev = virtio_net_init(&pci_dev->qdev,&proxy->nic,&proxy->net,
+   proxy->host_features);

Can't you pass the multiqueue enabled flags through proxy->net?

It would be better for me, as I only pass virtio net conf to device and
never host_features field.



  vdev->nvectors = proxy->nvectors;
  virtio_init_pci(proxy, vdev);
diff --git a/hw/virtio.h b/hw/virtio.h
index a29a54d..1e206b8 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -243,7 +243,8 @@ typedef struct VirtIOBlkConf VirtIOBlkConf;
  VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk);
  struct virtio_net_conf;
  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
-  struct virtio_net_conf *net);
+  struct virtio_net_conf *net,
+  uint32_t host_features);
  typedef struct virtio_serial_conf virtio_serial_conf;
  VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf 
*serial);
  VirtIODevice *virtio_balloon_init(DeviceState *dev);





[Qemu-devel] [PATCHv2] virtio-net: remove mq feature flag

2013-02-03 Thread Michael S. Tsirkin
mq flag is not needed: we can look at the number of queues and set
the flag accordingly.
Removing this feature removes ambiguity (what does it mean to have
queues=2 with mq=off?), and simplifies compatibility hacks.
work-around for buggy windows
guests.

Signed-off-by: Michael S. Tsirkin 
---
 hw/pc_piix.c|  4 
 hw/virtio-net.c |  8 +++-
 hw/virtio-net.h | 15 ---
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0af436c..ba09714 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -313,10 +313,6 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
 .driver   = "virtio-net-pci",\
 .property = "ctrl_mac_addr",\
 .value= "off",  \
-},{ \
-.driver   = "virtio-net-pci", \
-.property = "mq", \
-.value= "off", \
 }
 
 static QEMUMachine pc_machine_v1_3 = {
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index e37358a..8db1787 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice 
*vdev, uint32_t features)
 
 features |= (1 << VIRTIO_NET_F_MAC);
 
+if (n->max_queues > 1) {
+features |= (0x1 << VIRTIO_NET_F_MQ);
+}
+
 if (!peer_has_vnet_hdr(n)) {
 features &= ~(0x1 << VIRTIO_NET_F_CSUM);
 features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4);
@@ -1285,7 +1289,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 int i;
 
 n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf->queues > 1 ?
+sizeof(struct virtio_net_config) :
+sizeof(struct 
virtio_net_config_compat),
 sizeof(VirtIONet));
 
 n->vdev.get_config = virtio_net_get_config;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index f5fea6e..b1f7647 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -69,6 +69,17 @@ typedef struct virtio_net_conf
 /* Maximum packet size we can receive from tap device: header + 64k */
 #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10))
 
+/*
+ * Windows drivers from 22 Jan 2013 and older fail when config size != 32.
+ */
+struct virtio_net_config_compat
+{
+/* The config defining mac address ($ETH_ALEN bytes) */
+uint8_t mac[ETH_ALEN];
+/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+uint16_t status;
+} QEMU_PACKED;
+
 struct virtio_net_config
 {
 /* The config defining mac address ($ETH_ALEN bytes) */
@@ -190,7 +201,5 @@ struct virtio_net_ctrl_mq {
 DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, 
true), \
 DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, 
true), \
 DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, 
VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
-DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, 
VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
-DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, true)
-
+DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, 
VIRTIO_NET_F_CTRL_MAC_ADDR, true)
 #endif
-- 
MST



Re: [Qemu-devel] [PATCH for-1.4] tests/test-string-input-visitor: Handle errors provoked by fuzz test

2013-02-03 Thread Markus Armbruster
[Note cc: Luiz]

Peter Maydell  writes:

> On 2 February 2013 21:37, Andreas Färber  wrote:
>> Am 02.02.2013 22:19, schrieb Peter Maydell:
>>> It's OK and expected for visitors to return errors when presented with
>>> the fuzz test's random data. This means the test harness needs to
>>> handle them; check for and free any error after each visitor call,
>>> and only free the string returned by visit_type_str if visit_type_str
>>> succeeded.
>>>
>>> This fixes a problem where this test failed the MacOSX malloc()
>>> consistency checks and might segfault on other platforms [due
>>> to calling free() on an uninitialized pointer variable].
>>>
>>> Signed-off-by: Peter Maydell 
>>> ---
>>>  tests/test-string-input-visitor.c | 23 ++-
>>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/test-string-input-visitor.c 
>>> b/tests/test-string-input-visitor.c
>>> index f6b0093..793b334 100644
>>> --- a/tests/test-string-input-visitor.c
>>> +++ b/tests/test-string-input-visitor.c
>>> @@ -194,20 +194,41 @@ static void test_visitor_in_fuzz(TestInputVisitorData 
>>> *data,
>>>
>>>  v = visitor_input_test_init(data, buf);
>>>  visit_type_int(v, &ires, NULL, &errp);
>>> +if (error_is_set(&errp)) {
>>> +error_free(errp);
>>> +errp = NULL;
>>> +}
>>
>> It seems to me the naming is bad here: errp appears to be an Error*, not
>> an Error**. It would be nice to fix this within the function touched.
>
> "Error *errp" is blessed by docs/writing-qmp-commands.txt (and
> git grep 'Error \*errp' has 80 examples in the tree). I think
> if I were writing this code I'd probably agree with you about the
> naming, but I'm not and I don't particularly feel like changing
> names somebody else has been consistent about in this source file
> in the course of fixing a bug.
>
>> Since it is an Error*, I think it was said that we should not use
>> error_is_set() but err != NULL (or if you prefer, just err).
>> error_is_set() is intended for **errp arguments that may be NULL.
>
> Calling error_is_set(&some_local_err_ptr) is also in the
> examples in the docs. If not doing that is the recommendation
> there should be a doc comment in error.h about that.

I'd find this clearer:

v = visitor_input_test_init(data, buf);
visit_type_int(v, &ires, NULL, &errp);
error_free(errp);
errp = NULL;

Makes it blatantly obvious that the error is freed and the pointer reset
no matter what.