Re: [Qemu-devel] scp during migration with vhost fails
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
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
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?
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
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
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
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
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
- 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
- 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
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
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
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
- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
"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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[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.