Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error
Hi Stefan, Thanks for looking into this. Please find the requested details below. If you need any further details (like the host storage details) just let me know. Host details: CPU: Intel(R) Xeon(R) CPU E3-1230 V2 Memory: 32GB (ECC) OS: Alpine 3.9 Kernel version: 4.19.18-0-vanilla Qemu version: 3.1 Libvirt version: 4.10 Networking: openvswitch-2.10.1 Windows guest: OS: Windows Server 2012 R2 64bit (up to date) Virtio drivers version: 141 (stable) Command line: /usr/bin/qemu-system-x86_64 -name guest=DCHOMENET02,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-17-DCHOMENET02/master-key.aes -machine pc-i440fx-3.1,accel=kvm,usb=off,dump-guest-core=off -cpu IvyBridge,ss=on,vmx=on,pcid=on,hypervisor=on,arat=on,tsc_adjust=on,umip=on,xsaveopt=on,hv_time,hv_relaxed,hv_vapic,hv_spinlocks=0x1fff -drive file=/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd,if=pflash,format=raw,unit=0,readonly=on -drive file=/var/lib/libvirt/qemu/nvram/DCHOMENET02_VARS.fd,if=pflash,format=raw,unit=1 -m 2048 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid 84afccf7-f164-4d0d-b9c9-c92ffab00848 -no-user-config -nodefaults -chardev socket,id=charmonitor,fd=42,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x5 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive file=/storage/storage-ssd-vms/virtual_machines_ssd/dchomenet02.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0,cache=none,aio=threads -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1,write-cache=on -drive file=/storage/storage-hdd-vms/virtual_machines_hdd/dchomenet02_storage.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-1,cache=none,aio=threads -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,id=scsi0-0-0-1,write-cache=on -netdev tap,fd=44,id=hostnet0,vhost=on,vhostfd=45 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:91:aa:a1,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -chardev socket,id=charchannel1,fd=47,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=org.qemu.guest_agent.0 -device usb-tablet,id=input0,bus=usb.0,port=1 -spice port=5905,addr=127.0.0.1,disable-ticketing,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny -msg timestamp=on Linux guest: OS: CentOS 7.6 64bit (up to date) Kernel version: 3.10.0-957.1.3.el7.x86_64 Command line: /usr/bin/qemu-system-x86_64 -name guest=DOCKER01,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-21-DOCKER01/master-key.aes -machine pc-i440fx-3.1,accel=kvm,usb=off,dump-guest-core=off -cpu IvyBridge,ss=on,vmx=on,pcid=on,hypervisor=on,arat=on,tsc_adjust=on,umip=on,xsaveopt=on -drive file=/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd,if=pflash,format=raw,unit=0,readonly=on -drive file=/var/lib/libvirt/qemu/nvram/DOCKER01_VARS.fd,if=pflash,format=raw,unit=1 -m 2048 -realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -uuid 4705b146-3b14-4c20-923c-42105d47e7fc -no-user-config -nodefaults -chardev socket,id=charmonitor,fd=44,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x6 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -
Re: [Qemu-devel] [PATCH] hw/s390x: Fix the function arguments in the pci stub file
On Fri, 1 Feb 2019 07:46:40 +0100
Thomas Huth wrote:
> On 2019-02-01 07:14, Thomas Huth wrote:
> > On 2019-01-31 19:08, Paolo Bonzini wrote:
> >> On 31/01/19 19:00, Thomas Huth wrote:
> > (and the prototypes in the header) anymore, so if you try to compile
> > s390x
> > without CONFIG_PCI, the build currently fails.
> >
> Fixes: 468a93898a97 ("s390x/pci: pass the retaddr to all PCI
> instructions")
>
> > Signed-off-by: Thomas Huth
> > ---
> > hw/s390x/s390-pci-stub.c | 16 +---
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> This file seems to be in danger of bitrot. Do you think it'll be easier
> to test rarely used configs like that after we switch to Kconfig?
> >>> I hope so, yes. There will be a new --without-default-devices options
> >>> for "configure" (which matches "make allnoconfig" from the kernel) - if
> >>> we do it right in the Kconfig file for s390x, it should be possible to
> >>> catch this problem with that option.
> >>
> >> Yes, it will be in .travis.yml too.
> >>
> >> Right now there is a "select PCI" in the hw/s390x/Kconfig file, but
> >> probably it's best to add a config S390_ZPCI with "default y if
> >> S390_CCW_VIRTIO" and "select PCI" in it. Not a blocker, but I can
> >> integrate it if you send me a fixup patch.
> >
> > Yes, that's what I had in mind, too. I'll send a fixup patch...
>
> Actually, disabling the s390-pci code works from a compiling+linking
> point of view, but when you then try to start the machine, it fails:
>
> $ s390x-softmmu/qemu-system-s390x -nographic
> qemu-system-s390x: Unknown device 's390-pcihost' for default sysbus
> Aborted (core dumped)
>
> IIRC we originally wanted to make the "s390-pcihost" device really
> optional, but then decided not to do it since it would break migration.
> So the status of the "PCI disablement" got stuck somewhere inbetween,
> where we've created the switch for the Makefile and the stub file, but
> never really got it to a point where it could really really be disabled.
Well you can disable it, but it's not usable ;)
But yes, that annoying pcihost device needed for backwards compat got
into the way.
>
> So I see two options now:
>
> 1) Finally really make the device optional, at least for new machine
> types, so we can really disable CONFIG_PCI and get a working executable.
>
> 2) Scratch the idea completely to make this optional, always link the
> s390-pci-bus.o and s390-pci-inst.o files unconditionally, and remove the
> s390-pci-stub.c file.
>
> I assume options 2 is preferred, since we likely rather want to move
> into the PCI direction in the long run, instead of ignoring it...
I think both options are viable, but option 1 is of course more work.
The win there is that we could disable an entire subsystem.
I guess that the basic questions are: How important is it that
subsystems can be compiled out, and do we see a use case for a pci-less
s390 machine in the future? We really don't want to spend much time on
something of dubious use...
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 0/6] s390x/pci: remaining hot/un)plug patches
On Thu, 31 Jan 2019 15:43:16 -0500
Collin Walling wrote:
> On 1/30/19 10:57 AM, David Hildenbrand wrote:
> > These are all the patches that are not yet upstream (@Conny you might
> > already picked some, including them for the full picture) and after
> > a good discussion yesterday, including a patch t get rid of the release
> > timer. I ran a couple of sanity tests on this series.
> >
> > #1 and #2 fix hotplugging of PCI bridges.
> > #3 warns when "zpci=off"
> > #4 refactors unplugging
> > #5 get's rid of the release timer
> > #6 processes all unplug requests on reboot
> >
> > @Colin, can you review/ack? Especially Patch #4 is needed for qdev
> > patches already on the list. ("[PATCH RFCv2 0/9] qdev: Hotplug handler
> > chaining + virtio-pmem")
>
> I've given my r-b for 4, 5, and 6. IIRC, the other patches in this
> series looked fine from previous versions, but I'd like to look at them
> more carefully before ack'ing. Please allow me some time to devote to
> them in the near future. Trying my best to balance things :)
Great, thanks for looking. If patches 4-6 can be applied without
patches 1-3 (I think they can?), I'll go ahead and queue them.
(I'll probably send a pull request next week, will be happy to include
the other patches as well.)
Re: [Qemu-devel] Configuring pflash devices for OVMF firmware
Paolo Bonzini writes: > On 31/01/19 13:12, Markus Armbruster wrote: >> Paolo Bonzini writes: >> >>> On 31/01/19 10:41, Markus Armbruster wrote: Paolo Bonzini writes: > On 31/01/19 09:40, Markus Armbruster wrote: >>> Maybe we should just add pflash block properties to the machine? And >>> then it can create the devices if the properties are set to a non-empty >>> value. >> What exactly do you have in mind? Something like >> >> -machine q35,ovmf-code=OVMF-CODE-NODE,ovmf-data=OVMF-DATA-NODE >> >> where OVMF-CODE-NODE and OVMF-DATA-NODE are block backend node names, >> i.e. >> >> -blockdev >> file,node-name=OVMF-CODE-NODE,read-only=on,filename=/usr/share/edk2/ovmf/OVMF_CODE.fd >> -blockdev file,node-name=OVMF-DATA-NODE,read-only=on,filename=... > > Yes, though I would call it pflash0 and pflash1. Digression... should we put traditional BIOS in flash as well? Only for new machine types, obviously. >>> >>> The blocker was that very old KVM didn't support ROMD memory regions. >>> Now on one hand we don't support those old kernel versions anymore; on >>> the other hand we have HAX and WHPX that do not support ROMD at all. >> >> This is all greek to me. I take it there's something wrong with these >> accelerators that makes (read-only?) flash memory not work, even though >> the read-only mapping we now create for traditional BIOS works. Weird, >> but I'm of course willing to take your word for it. > > Yes, as I wrote in the other message even read-only flash memory > supports commands, and these accelerators do not support direct reads + > MMIO writes on the same memory slot. Got it, thanks. > At least I checked HAX code and it doesn't; I don't know about WHPX. > >> Aside: accepting incomplete accelerators, then letting their >> incompleteness hold back things doesn't strike me as sound policy. > > Yes, there is a balance to be found between that and accepting features > from known out-of-tree forks, in order to help these out-of-tree forks > not remain forever on very old releases. I support inviting forks back into the fold, and I understand why "feature parity or go away" would be impractical there. But I'd like to see at least *commitment* to reaching feature parity. > However, another important question is---if you changed the default from > -bios to -pflash, would you also flip secure from off to on? Only TCG > and KVM supports SMM, and it's quite unlikely that the other > accelerators will support it (there are also "philosophical" debates > behind that choice...). I would not, simply because secure has always been off by default. >> Do we reject these accelerators when the user asks for firmware in >> flash? Or do we let the guest run into some more or less obscure >> failure? > > For HAX it just fails to boot I think. I'd call that a user interface bug.
Re: [Qemu-devel] [PATCH] block: Fix invalidate_cache error path for parent activation
Kevin Wolf writes:
> bdrv_co_invalidate_cache() clears the BDRV_O_INACTIVE flag before
> actually activating a node so that the correct permissions etc. are
> taken. In case of errors, the flag must be restored so that the next
> call to bdrv_co_invalidate_cache() retries activation.
>
> Restoring the flag was missing in the error path for a failed
> parent->role->activate() call. The consequence is that this attempt to
> activate all images correctly fails because we still set errp, however
> on the next attempt BDRV_O_INACTIVE is already clear, so we return
> success without actually retrying the failed action.
>
> An example where this is observable in practice is migration to a QEMU
> instance that has a raw format block node attached to a guest device
> with share-rw=off (the default) while another process holds
> BLK_PERM_WRITE for the same image. In this case, all activation steps
> before parent->role->activate() succeed because raw can tolerate other
> writers to the image. Only the parent callback (in particular
> blk_root_activate()) tries to implement the share-rw=on property and
> requests exclusive write permissions. This fails when the migration
> completes and correctly displays an error. However, a manual 'cont' will
> incorrectly resume the VM without calling blk_root_activate() again.
>
> This case is described in more detail in the following bug report:
> https://bugzilla.redhat.com/show_bug.cgi?id=1531888
>
> Fix this by correctly restoring the BDRV_O_INACTIVE flag in the error
> path.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Kevin Wolf
> ---
> block.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block.c b/block.c
> index 0eba8ebe5c..7f5c9bb02b 100644
> --- a/block.c
> +++ b/block.c
> @@ -4549,6 +4549,7 @@ static void coroutine_fn
> bdrv_co_invalidate_cache(BlockDriverState *bs,
> if (parent->role->activate) {
> parent->role->activate(parent, &local_err);
> if (local_err) {
> +bs->open_flags |= BDRV_O_INACTIVE;
> error_propagate(errp, local_err);
> return;
> }
Tested-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 2/5] virtio-blk: add "discard-wzeroes" boolean property
On Fri, Feb 01, 2019 at 12:29:28PM +0800, Stefan Hajnoczi wrote: > On Thu, Jan 31, 2019 at 04:19:11PM +0100, Stefano Garzarella wrote: > > In order to avoid migration issues, we enable DISCARD and > > WRITE ZEROES features only for machine type >= 4.0 > > Please use two separate properties that correspond to the > VIRTIO_BLK_F_DISCARD and VIRTIO_BLK_F_WRITE_ZEROES virtio-blk feature > bits. Okay. As Michael suggested, what do you think if I use a similar approach of virtio-net, adding an host_features variable and setting a corresponding feature bit? Thanks, Stefano
Re: [Qemu-devel] [PATCH] monitor: Add whitelist support for QMP commands
Julia Suvorova via Qemu-devel writes: > The whitelist option allows to run a reduced monitor with a subset of > QMP commands. This allows the monitor to run in secure mode, which is For a value of "secure". I'm not saying this can't be useful, just tempering expecations. I guess you intend to use this to restrict the monitor to sufficiently harmless commands, such as commands that merely return information without changing anything. However, even such commands can be abused for denial of service. Whether that's an issue depends on your use case. > convenient for sending commands via the WebSocket monitor using the > web UI. This is planned to be done on micro:bit board. > > The list of allowed commands should be written to a file, one per line. > The command line will look like this: > -mon chardev_name,mode=control,whitelist=path_to_file > > Signed-off-by: Julia Suvorova Please describe your intended use case in more detail, and provide at least a rough security analysis that includes the denial of service aspect.
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 0/6] s390x/pci: remaining hot/un)plug patches
On 01.02.19 09:35, Cornelia Huck wrote:
> On Thu, 31 Jan 2019 15:43:16 -0500
> Collin Walling wrote:
>
>> On 1/30/19 10:57 AM, David Hildenbrand wrote:
>>> These are all the patches that are not yet upstream (@Conny you might
>>> already picked some, including them for the full picture) and after
>>> a good discussion yesterday, including a patch t get rid of the release
>>> timer. I ran a couple of sanity tests on this series.
>>>
>>> #1 and #2 fix hotplugging of PCI bridges.
>>> #3 warns when "zpci=off"
>>> #4 refactors unplugging
>>> #5 get's rid of the release timer
>>> #6 processes all unplug requests on reboot
>>>
>>> @Colin, can you review/ack? Especially Patch #4 is needed for qdev
>>> patches already on the list. ("[PATCH RFCv2 0/9] qdev: Hotplug handler
>>> chaining + virtio-pmem")
>>
>> I've given my r-b for 4, 5, and 6. IIRC, the other patches in this
>> series looked fine from previous versions, but I'd like to look at them
>> more carefully before ack'ing. Please allow me some time to devote to
>> them in the near future. Trying my best to balance things :)
>
> Great, thanks for looking. If patches 4-6 can be applied without
> patches 1-3 (I think they can?), I'll go ahead and queue them.
Yes, I think they can, patch 1-3 mainly deals with plugging, these ones
with unplugging. Thanks Conny!
>
> (I'll probably send a pull request next week, will be happy to include
> the other patches as well.)
>
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH 0/5] misc fixes to deal with icon location changes
On Fri, Feb 01, 2019 at 08:52:25AM +0100, Stefan Weil wrote: > On 29.01.19 12:39, Daniel P. Berrangé wrote: > > This is a few misc fixes identified after the icon location changes > > were merged. Most importantly it deals with the nsis installer file > > manifest. > > > > Daniel P. Berrangé (5): > > nsis: don't install files into /tmp > > make: don't insert a '/' after $(DESTDIR) > > nsis: ensure we always pass -W64 flag to nsis installer script > > nsis: fix location of icons to be bundled in the installer > > configure: stop trying to link non-existant bmp file from bios dir > > > > Makefile | 23 +-- > > configure | 1 - > > qemu.nsi | 4 ++-- > > 3 files changed, 15 insertions(+), 13 deletions(-) > > > > There was also a file rename which needs to be handled, > otherwise NSIS still fails: > > File: "/tmp/qemu-nsis\qemu_logo_no_text.svg" -> no files found. I don't see that prolem - NSIS builds successfully for me, and I can't find any reference to .svg files in the qemu.nsi > The new qemu.svg still includes the old file name. That could be fixed, too. Oh, in its metadata I see. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v5 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
On Thu, 31 Jan 2019 at 20:02, Peter Maydell wrote:
>
> On Wed, 30 Jan 2019 at 08:59, Hongbo Zhang wrote:
> >
> > On Tue, 22 Jan 2019 at 19:42, Peter Maydell
> > wrote:
> > >
> > > On Fri, 7 Dec 2018 at 09:08, Hongbo Zhang wrote:
> > > > +[VIRT_SECURE_MEM] = { 0x2000, 0x2000 },
> > > > +[VIRT_CPUPERIPHS] = { 0x4000, 0x0008 },
> > > > +/* GIC distributor and CPU interface expansion spaces reserved */
> > > > +[VIRT_GIC_DIST] = { 0x4000, 0x0001 },
> > > > +[VIRT_GIC_CPU] ={ 0x4004, 0x0001 },
> > >
> > > If they're just reserved you don't really need to list them here,
> > > as they're covered by the VIRT_CPUPERIPHS space anyway. (You
> > > don't list the VIRT_GIC_HYP registers or VIRT_GIC_VCPU.)
> > >
> > We need more consideration here.
>
> Yeah, this is a little more complex than I thought.
>
> > Why CPUPERIPHS is used to cover DIST and CPU interface? is this from
> > some old platform? I don't see the term in GICv3 memory map in the
> > user manual, so do we still need it for this Arm64 GICv3?
> > If we only list CPUPERIPHS but without DIST, I am afraid firmware or
> > driver developer still looking for the term of DIST for base address.
>
> OK, so what CPUPERIPHS does (in the virt board) is set the
> CPU property which defines the reset value of the CBAR_EL1 register.
> (The size specified isn't used.)
>
> The set of memory mapped things in this area should (in theory)
> be the same as what the hardware CPU puts there, because guest
> code can reasonably look at CBAR and expect to find there the
> devices that the real CPU puts there. Unfortunately on the virt
> board we don't get this right (and in any case we support multiple
> CPU types which don't necessarily have the same set of devices).
> Things work in practice because Linux and OVMF both look at
> the device tree rather than assuming they can find things
> via CBAR_EL1. This is awkward to fix in the virt board without
> breaking compatibility, but we can get it right for this new board.
>
> > For GICv3, what we can have are DIST, CPU, REDIST, VCPU and HYP.
> > CPU, VCPU and HYP are not simulated for GICv3 (curious it still works
> > without CPU interface emulated), so we only have DIST and REDIST.
> > Can we only list the DIST and REDIST without CPUPERIPHS?
>
> QEMU's GICv3 implementation only supports the system register
> interface, not the memory-mapped interface. This is why we don't
> have a memory mapped thing to put into VIRT_GIC_HYP/VIRT_GIC_CPU/
> VIRT_GIC_VCPU.
>
> For the Cortex-A53/A57/A72, the only things in the CBAR area are
> these memory mapped interfaces that we don't implement:
> CBAR+0x0 : cpu interface
> CBAR+0x1 : virt interface control
> CBAR+0x2 : vcpu interface
> CBAR+0x2F000 : alias of vcpu interface
> (other parts of the CBAR+0x0 to CBAR+0x3 reserved)
>
> So for the SBSA reference board, I think the right thing is to
> leave a gap in the memory map of 0x4 in size for CPUPERIPHS,
> with a comment that this is reserved CPU peripheral space
> because we don't implement the GICv3 memory-mapped CPU interface.
> Put GIC_DIST somewhere else (it is a GICv3 device register
> bank, not part of the CPU's peripherals), and don't define
> GIC_CPU at all (since you don't use it).
>
Yes, should like this, thanks.
> thanks
> -- PMM
Re: [Qemu-devel] [PATCH v11 for-4.0 11/11] qemu_thread: supplement error handling for touch_all_pages
Fei Li writes: > It seems that this poor patch is left alone. :( > I sent all, but this patch failed to join them, so sorry for that.. > Could we just let it be? It'll do, thanks.
Re: [Qemu-devel] [PATCH v2 3/5] virtio-blk: add DISCARD and WRITE ZEROES features
On Fri, Feb 01, 2019 at 12:58:31PM +0800, Stefan Hajnoczi wrote:
> On Thu, Jan 31, 2019 at 04:19:12PM +0100, Stefano Garzarella wrote:
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 542ec52536..34ee676895 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -147,6 +147,30 @@ out:
> > aio_context_release(blk_get_aio_context(s->conf.conf.blk));
> > }
> >
> > +static void virtio_blk_discard_wzeroes_complete(void *opaque, int ret)
> > +{
> > +VirtIOBlockReq *req = opaque;
> > +VirtIOBlock *s = req->dev;
> > +bool is_wzeroes = (virtio_ldl_p(VIRTIO_DEVICE(req->dev),
> > &req->out.type) &
>
> s/req->dev/s/
>
> > + ~VIRTIO_BLK_T_BARRIER) == VIRTIO_BLK_T_WRITE_ZEROES;
> > +
> > +aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
> > +if (ret) {
> > +if (virtio_blk_handle_rw_error(req, -ret, 0, is_wzeroes)) {
>
> The third argument is bool, please use false instead of 0.
>
> > +goto out;
> > +}
> > +}
> > +
> > +virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
> > +if (is_wzeroes) {
> > +block_acct_done(blk_get_stats(req->dev->blk), &req->acct);
>
> s/req->dev->blk/s->blk/
>
> > +static uint8_t virtio_blk_handle_dwz(VirtIOBlockReq *req, bool is_wzeroes,
> > +struct virtio_blk_discard_write_zeroes *dwz_hdr)
> > +{
> > +VirtIOBlock *s = req->dev;
> > +uint64_t sector;
> > +uint32_t num_sectors, flags;
> > +uint8_t err_status;
> > +int bytes;
> > +
> > +sector = virtio_ldq_p(VIRTIO_DEVICE(req->dev), &dwz_hdr->sector);
>
> Here and throughout the rest of the function:
>
> VirtIODevice *vdev = VIRTIO_DEVICE(s);
>
> s/VIRTIO_DEVICE(req->dev)/vdev/
>
> and then to clean up the remaining instances:
>
> s/req->dev/s/
>
Thanks! I'll follow all of these suggestions.
> > +if (is_wzeroes) { /* VIRTIO_BLK_T_WRITE_ZEROES */
> > +int blk_aio_flags = 0;
> > +
> > +if (s->conf.wz_may_unmap &&
>
> The inconsistent naming is a bit messy and could confuse readers:
> write_zeroes vs wzeroes vs wz
>
> The VIRTIO spec and QEMU code uses write_zeroes, please stick to that
> even though it is longer.
>
> s/is_wzeroes/is_write_zeroes/
> s/wz_map_unmap/write_zeroes_may_unmap/
> s/virtio_blk_discard_wzeroes_complete/virtio_blk_discard_write_zeroes_complete/
>
> This is a question of style and a local dwz_hdr variable does make the
> code easier to read, so I'm not totally against shortening the name, but
> please consistently use the long form in user-visible options, struct
> field names, and function names because these things have a large scope.
>
Thanks! I'll change all the name.
> > @@ -765,6 +904,22 @@ static void virtio_blk_update_config(VirtIODevice
> > *vdev, uint8_t *config)
> > blkcfg.alignment_offset = 0;
> > blkcfg.wce = blk_enable_write_cache(s->blk);
> > virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);
> > +if (s->conf.discard_wzeroes) {
> > +virtio_stl_p(vdev, &blkcfg.max_discard_sectors,
> > + s->conf.dwz_max_sectors);
> > +virtio_stl_p(vdev, &blkcfg.discard_sector_alignment,
> > + blk_size >> BDRV_SECTOR_BITS);
> > +virtio_stl_p(vdev, &blkcfg.max_write_zeroes_sectors,
> > + s->conf.dwz_max_sectors);
> > +blkcfg.write_zeroes_may_unmap = s->conf.wz_may_unmap;
>
> Does this need to be an option since MAY_UNMAP is advisory anyway?
>
> Another way of asking: what happens in the worst case if the guest sends
> MAY_UNMAP but the QEMU block device doesn't support unmap?
>
> Dropping this option would make the user interface simpler (no need to
> worry about the flag) and the implementation too.
Make sense, I'll drop this option.
Only a question about options: I used a single option "dwz_max_sectors"
for both "max_discard_sectors" and "max_write_zeroes_sectors".
Since I'll include two options to enable/disable discard and
write_zeroes features, do you think make sense to split this
configurable option in two?
Thanks,
Stefano
Re: [Qemu-devel] [PATCH v11 for-4.0 01/11] qemu_thread: make qemu_thread_create() take Error ** argument
Fei Li writes: > From: Fei Li > > qemu_thread_create() abort()s on error. Not nice. Give it a return > value and an Error ** argument, so it can return success/failure. > > Considering qemu_thread_create() is quite widely used in qemu, split > this into two steps: this patch passes the &error_abort to > qemu_thread_create() everywhere, and the next 10 patches will improve > on &error_abort for callers who need. To differentiate callers who > need the improvement, temporarily add the "TODO:" comment for them. > > Cc: Markus Armbruster > Cc: Paolo Bonzini > Signed-off-by: Fei Li Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 2/5] virtio-blk: add "discard-wzeroes" boolean property
On Fri, Feb 01, 2019 at 10:09:08AM +0100, Stefano Garzarella wrote: > On Fri, Feb 01, 2019 at 12:29:28PM +0800, Stefan Hajnoczi wrote: > > On Thu, Jan 31, 2019 at 04:19:11PM +0100, Stefano Garzarella wrote: > > > In order to avoid migration issues, we enable DISCARD and > > > WRITE ZEROES features only for machine type >= 4.0 > > > > Please use two separate properties that correspond to the > > VIRTIO_BLK_F_DISCARD and VIRTIO_BLK_F_WRITE_ZEROES virtio-blk feature > > bits. > > Okay. > As Michael suggested, what do you think if I use a similar approach of > virtio-net, adding an host_features variable and setting a corresponding > feature bit? Yes, that's a clean way of supporting feature bit options. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v2 3/5] virtio-blk: add DISCARD and WRITE ZEROES features
On Fri, Feb 01, 2019 at 10:54:30AM +0100, Stefano Garzarella wrote: > On Fri, Feb 01, 2019 at 12:58:31PM +0800, Stefan Hajnoczi wrote: > > On Thu, Jan 31, 2019 at 04:19:12PM +0100, Stefano Garzarella wrote: > Only a question about options: I used a single option "dwz_max_sectors" > for both "max_discard_sectors" and "max_write_zeroes_sectors". > Since I'll include two options to enable/disable discard and > write_zeroes features, do you think make sense to split this > configurable option in two? Yes, please. Eventually a user will want full control so it makes sense to expose what virtio offers instead of coming up with higher-level options. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v2 5/6] s390x/pci: Drop release timer and replace it with a flag
On Thu, 31 Jan 2019 22:21:01 +0100 David Hildenbrand wrote: > Thinking out loud: > > We should migrate the flag in the future. This is already a problem > right now, as the timer is also not migrated. > > If the unplug request is sent and we migrate before the guest can react, > the unplug would not happen. > > However, looks like migration of zpci devices is not implemented _at all_. Oh, joy. > > This does not matter for pci passthrough (main use case). But when > wanting to use e.g. virtio-pci-net, things are shaky after migration. > > So this is future work. > Hopefully folks running s390x guests are rather using virtio-ccw devices anyway... I agree that this needs to be taken care of on top of these changes. Should we mark zpci devices unmigratable, or does it work for some values of "work"?
Re: [Qemu-devel] [PATCH v2 6/6] s390x/pci: Unplug remaining requested devices on pcihost reset
On Wed, 30 Jan 2019 16:57:33 +0100
David Hildenbrand wrote:
> When resetting the guest we should unplug and remove all devices that
> are still pending.
>
> With this patch, the requested device will be unpluged on reboot
> (S390_RESET_EXTERNAL and S390_RESET_REIPL, which reset the pcihost bridge
> via qemu_devices_reset()).
>
> This approach is similar to what's done for acpi PCI hotplug in
> acpi_pcihp_reset() -> acpi_pcihp_update() ->
> acpi_pcihp_update_hotplug_bus() -> acpi_pcihp_eject_slot().
>
> s390_pci_generate_plug_event()'s will still be generated, I guess this
> is not an issue. The same thing would happen right now when unplugging
> a device just before starting the guest.
>
> Signed-off-by: David Hildenbrand
> ---
> hw/s390x/s390-pci-bus.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 867801ccf9..b9b0f44087 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1092,6 +1092,21 @@ static void s390_pcihost_reset(DeviceState *dev)
> {
> S390pciState *s = S390_PCI_HOST_BRIDGE(dev);
> PCIBus *bus = s->parent_obj.bus;
> +S390PCIBusDevice *pbdev, *next;
> +
> +/* Process all pending unplug requests */
> +QTAILQ_FOREACH_SAFE(pbdev, &s->zpci_devs, link, next) {
> +if (pbdev->unplug_requested) {
> +if (pbdev->summary_ind) {
> +pci_dereg_irqs(pbdev);
> +}
> +if (pbdev->iommu->enabled) {
> +pci_dereg_ioat(pbdev->iommu);
> +}
> +pbdev->state = ZPCI_FS_STANDBY;
> +s390_pci_perform_unplug(pbdev);
> +}
> +}
>
> /*
> * When resetting a PCI bridge, the assigned numbers are set to 0. So
I'm afraid this one doesn't quite apply without the first patches in
the series, and I'm not sure how to fix it up. I'll either take a
rebase on a codebase without patches 1-3, or this patch together with
patches 1-3 after they get acks (whatever comes first :)
Re: [Qemu-devel] [RFC PATCH 01/11] qcow2: Extend spec for external data files
Am 31.01.2019 um 19:43 hat Eric Blake geschrieben: > On 1/31/19 11:55 AM, Kevin Wolf wrote: > > This adds external data file to the qcow2 spec as a new incompatible > > feature. > > > > Signed-off-by: Kevin Wolf > > --- > > docs/interop/qcow2.txt | 19 --- > > 1 file changed, 16 insertions(+), 3 deletions(-) > > > > > @@ -148,6 +158,7 @@ be stored. Each extension has a structure like the > > following: > > 0x6803f857 - Feature name table > > 0x23852875 - Bitmaps extension > > 0x0537be77 - Full disk encryption header pointer > > +0x44415441 - External data file name > > other - Unknown header extension, can be > > safely > > ignored > > Missing a section describing the new header. The header extension that have a separate section are those that contain structured data. The backing file format name doesn't have one because it's just a string, and so is the external data file name. I guess I could add a section to describe the general concept of external data files, if you think we have information to put there. All that is really required should be contained in the feature bit description already. > > @@ -450,8 +461,10 @@ Standard Cluster Descriptor: > > 1 - 8:Reserved (set to 0) > > > > 9 - 55:Bits 9-55 of host cluster offset. Must be aligned to a > > -cluster boundary. If the offset is 0, the cluster is > > -unallocated. > > +cluster boundary. If the offset is 0 and bit 63 is > > clear, > > +the cluster is unallocated. The offset may only be 0 > > with > > +bit 63 set (indicating a host cluster offset of 0) > > when an > > +external data file is used. > > Does that mean that the value 0x is invalid for external data > files Not sure whether you mean the L2 entry as 0x or the offset field as 0x (neither of them are 32 bit like your value). In any case, 0 is a valid value for an L2 entry, it means an unallocated cluster. The same offset, but with bit 63 set (0x8000) is valid only for external data files and means an allocated cluster at offset 0. > and that 0x0001 is special-cased to mean read the contents of > the external file (and NOT that the cluster reads as all zeroes)? No, where do you read that? This is the description of the offset field, and bit 0 isn't mentioned anywhere. The zero flag works the same way as it always does. > Is bit 0 allowed to be set for any other clusters when there is an > external data file? Wait, what are "other clusters"? Are you assuming that we have an image that stores guest data both internally and externally, in some kind of a mixed setup? The idea is that the feature bit signals that _all_ guest data is stored in the external data file. The offset in the L2 table always refers to the external file then. Only metadata remains in the qcow2 file. > And if so, are we requiring that it only be set > when the external file is known to read as zero, or can we run into > the situation where qcow2 says the cluster reads as 0 but the host > file contains garbage? Hm... This is a good point. Currently it behaves just like normal qcow2 files, but this means that the external file can contain stale data and the zero flag determines the content. This makes it impossible to use the external data file as a raw image. So I think we need to add a requirement to write actual zeroes to the external file there. > Should the external file header contain a flag > that states whether writes to the image should wipe vs. leave > unchanged a cluster in the external file when the qcow2 metadata > prefers to grab that cluster's contents as all-0s or by reading from > the backing file? There are security vs. speed implications - > security insists on wiping the host file to NOT leave stale data, but > that slows things down compared to just leaving garbage if the qcow2 > metadata can effectively ignore those parts of the external file - > hence a knob may be worth exposing? If your argument is security, wouldn't the same tradeoff apply to internally stored data as well? zero_in_l2_slice() only adds the zero flag, without overwriting the data in bs->file. But then, for actual security, wouldn't we need to do an explicit write rather that write_zeroes so that the same problem doesn't reoccur just one layer down? Or potentially even more specialised operations? Security is a different goal than just keeping the data file consistent enough to be readable as a raw image. > Should we preserve the meaning of bit 0 SOLELY for its 'reads-as-zeroes' > meaning, and instead make bit 1 (currently reserved) as the special > marker that MUST be set for clusters read from the external file, > keeping the two ideas or
Re: [Qemu-devel] [PULL 00/13] Block patches
On Thu, 31 Jan 2019 at 00:59, Max Reitz wrote: > > The following changes since commit b4fbe1f65a4769c09e6bf2d79fc84360f840f40e: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20190129' into staging (2019-01-29 > 12:00:19 +) > > are available in the Git repository at: > > https://git.xanclic.moe/XanClic/qemu.git tags/pull-block-2019-01-31 > > for you to fetch changes up to 908b30164bbffad7430d551b2a03a8fbcaa631ef: > > iotests: Allow 147 to be run concurrently (2019-01-31 00:44:55 +0100) > > > Block patches: > - New debugging QMP command to explore block graphs > - Converted DPRINTF()s to trace events > - Fixed qemu-io's use of getopt() for systems with optreset > - Minor NVMe emulation fixes > - An iotest fix > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM
Re: [Qemu-devel] [Qemu-block] [RFC PATCH 01/11] qcow2: Extend spec for external data files
Am 31.01.2019 um 22:44 hat Nir Soffer geschrieben: > On Thu, Jan 31, 2019 at 8:43 PM Eric Blake wrote: > ... > > > > @@ -450,8 +461,10 @@ Standard Cluster Descriptor: > > > > 1 - 8:Reserved (set to 0) > > > > > > 9 - 55:Bits 9-55 of host cluster offset. Must be aligned > > to a > > > -cluster boundary. If the offset is 0, the cluster is > > > -unallocated. > > > +cluster boundary. If the offset is 0 and bit 63 is > > clear, > > > +the cluster is unallocated. The offset may only be > > 0 with > > > +bit 63 set (indicating a host cluster offset of 0) > > when an > > > +external data file is used. > > > > Does that mean that the value 0x is invalid for external data > > files, and that 0x0001 is special-cased to mean read the contents of > > the external file (and NOT that the cluster reads as all zeroes)? Is > > bit 0 allowed to be set for any other clusters when there is an external > > data file? And if so, are we requiring that it only be set when the > > external file is known to read as zero, or can we run into the situation > > where qcow2 says the cluster reads as 0 but the host file contains > > garbage? Should the external file header contain a flag that states > > whether writes to the image should wipe vs. leave unchanged a cluster in > > the external file when the qcow2 metadata prefers to grab that cluster's > > contents as all-0s or by reading from the backing file? > > I hope we are not going to mix qcow2 layer with data file and backing > file in the same chain. > > For oVirt we would never want to have a backing chain when using qcow2 > metadata layer. I think this is the same use case for KubeVirt. There is no reason not to allow this setup. In fact, it would be more work to forbid it than to just let it happen. Of course, if you use an external data file and have a backing file, too, you can't use the external data file as a raw image any more (but you're not supposed to do that anyway, except maybe for copying it away or if you want to drop all associated metadata). > > There are security vs. speed implications - security insists on > > wiping the host file to NOT leave stale data, but that slows things > > down compared to just leaving garbage if the qcow2 metadata can > > effectively ignore those parts of the external file - hence a knob > > may be worth exposing? > > Since qcow2 is just used to managed metadata about a raw file, I don't > think it should do any optimizations like this. > > What if we implement this differently - a qcow2 layer that keeps only > metadata for a backing file? > > All reads will go directly to the backing file, since there is no data > in the qcow2 file. All writes will go directly to the backing file, > since no data should be in the qcow2 file. But before writing, update > the qcow2 metadata to reference the cluster that will be written to > the backing file. Writable backing files are a design that we considered, but it wouldn't be as flexible (you couldn't have a real backing file at the same time, and you probably also would know the cluster allocation status), but it would be harder to implement. Kevin
Re: [Qemu-devel] [RFC PATCH 08/11] qcow2: Add basic data-file infrastructure
Am 31.01.2019 um 19:44 hat Eric Blake geschrieben: > On 1/31/19 11:55 AM, Kevin Wolf wrote: > > This adds a .bdrv_open option to specify the external data file node. > > > > Signed-off-by: Kevin Wolf > > --- > > qapi/block-core.json | 3 ++- > > block/qcow2.h| 4 +++- > > block/qcow2.c| 25 +++-- > > 3 files changed, 28 insertions(+), 4 deletions(-) > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index 7f6b4b3ddd..fc46396079 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -2928,7 +2928,8 @@ > > '*l2-cache-entry-size': 'int', > > '*refcount-cache-size': 'int', > > '*cache-clean-interval': 'int', > > -'*encrypt': 'BlockdevQcow2Encryption' } } > > +'*encrypt': 'BlockdevQcow2Encryption', > > +'*data-file': 'BlockdevRef' } } > > Missing docs and a '(since 4.0)' tag. Then again, it's still RFC, so > that's understandable for this draft. Yes, as mentioned in the cover letter. Kevin signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
On Fri, 18 Jan 2019 at 16:17, Kevin Wolf wrote: > > Am 10.12.2018 um 12:26 hat Peter Maydell geschrieben: > > This patchset fixes the remaining clang warnings in the block/ code > > about taking the address of a packed struct member, which are all > > in block/vpc and block/vdi code handling UUIDs. Mostly I fix > > these by copying the unaligned field to/from a local variable. > > In the case of qemu_uuid_bswap() I opted to change the API to > > take and return the QemuUUID rather than taking a pointer to it, > > which makes almost all the callsites simpler. This does mean > > a struct copy but the struct is only 16 bytes and I didn't > > judge any of the callsites performance-sensitive enough to care > > about a struct copy of that size. > > > > As usual, tested with "make check" only. > > Thanks, applied to the block branch. Ping? These don't seem to have made it into master yet. thanks -- PMM
Re: [Qemu-devel] [PATCH v5 00/35] target/riscv: Convert to decodetree
Patchew URL: https://patchew.org/QEMU/20190122092909.5341-1-kbast...@mail.uni-paderborn.de/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20190122092909.5341-1-kbast...@mail.uni-paderborn.de Subject: [Qemu-devel] [PATCH v5 00/35] target/riscv: Convert to decodetree === TEST SCRIPT BEGIN === #!/bin/bash git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20190122092909.5341-1-kbast...@mail.uni-paderborn.de -> patchew/20190122092909.5341-1-kbast...@mail.uni-paderborn.de Switched to a new branch 'test' 707ba25 target/riscv: Remaining rvc insn reuse 32 bit translators e8e3fdd target/riscv: Splice remaining compressed insn pairs for riscv32 vs riscv64 1271fb3 target/riscv: Splice fsw_sd and flw_ld for riscv32 vs riscv64 e577171 target/riscv: Convert @cl_d, @cl_w, @cs_d, @cs_w insns 9a48b38 target/riscv: Convert @cs_2 insns to share translation functions ae50456 target/riscv: Remove decode_RV32_64G() 689db53 target/riscv: Remove gen_system() ba19bbe target/riscv: Rename trans_arith to gen_arith 4821480 target/riscv: Remove manual decoding of RV32/64M insn bc78e8b target/riscv: Remove shift and slt insn manual decoding b24202e target/riscv: make ADD/SUB/OR/XOR/AND insn use arg lists c4655ea target/riscv: Move gen_arith_imm() decoding into trans_* functions f4b51a8 target/riscv: Remove manual decoding from gen_store() 2717536 target/riscv: Remove manual decoding from gen_load() ce1206d target/riscv: Remove manual decoding from gen_branch() 35a81db target/riscv: Remove gen_jalr() 8d3033d target/riscv: Convert quadrant 2 of RVXC insns to decodetree 7f04cc7 target/riscv: Convert quadrant 1 of RVXC insns to decodetree 6a1f939 target/riscv: Convert quadrant 0 of RVXC insns to decodetree 84311c6 target/riscv: Convert RV priv insns to decodetree 801dbbd target/riscv: Convert RV64D insns to decodetree 5669654 target/riscv: Convert RV32D insns to decodetree e2646f4 target/riscv: Convert RV64F insns to decodetree 188a6e6 target/riscv: Convert RV32F insns to decodetree c39a5a5 target/riscv: Convert RV64A insns to decodetree 74169b8 target/riscv: Convert RV32A insns to decodetree f0db8bc target/riscv: Convert RVXM insns to decodetree 2db8b9a target/riscv: Convert RVXI csr insns to decodetree abf36db target/riscv: Convert RVXI fence insns to decodetree 437709e target/riscv: Convert RVXI arithmetic insns to decodetree 201f108 target/riscv: Convert RV64I load/store insns to decodetree 7c6f475 target/riscv: Convert RV32I load/store insns to decodetree 9e5332d target/riscv: Convert RVXI branch insns to decodetree dc69d0a target/riscv: Activate decodetree and implemnt LUI & AUIPC b4e4978 target/riscv: Move CPURISCVState pointer to DisasContext === OUTPUT BEGIN === 1/35 Checking commit b4e49789dd4a (target/riscv: Move CPURISCVState pointer to DisasContext) 2/35 Checking commit dc69d0a0852a (target/riscv: Activate decodetree and implemnt LUI & AUIPC) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #34: new file mode 100644 ERROR: externs should be avoided in .c files #125: FILE: target/riscv/translate.c:1687: +bool decode_insn32(DisasContext *ctx, uint32_t insn); total: 1 errors, 1 warnings, 125 lines checked Patch 2/35 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/35 Checking commit 9e5332de20d5 (target/riscv: Convert RVXI branch insns to decodetree) 4/35 Checking commit 7c6f47530cd9 (target/riscv: Convert RV32I load/store insns to decodetree) 5/35 Checking commit 201f108c71a8 (target/riscv: Convert RV64I load/store insns to decodetree) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #39: new file mode 100644 total: 0 errors, 1 warnings, 76 lines checked Patch 5/35 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 6/35 Checking commit 437709e78b8c (target/riscv: Convert RVXI arithmetic insns to decodetree) 7/35 Checking commit abf36dbe8296 (target/riscv: Convert RVXI fence insns to decodetree) 8/35 Checking commit 2db8b9a36725 (target/riscv: Convert RVXI csr insns to decodetree) 9/35 Checking commit f0db8bcb9cb8 (target/riscv: Convert RVXM insns to decodetree) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #48: new file mode 100644 total: 0 errors, 1 warnings, 145 lines checked Patch 9/35 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 10/35 Checking commit 74169b8e3825 (target/riscv: Convert RV32A insns to decodetree) W
Re: [Qemu-devel] [PATCH v2 5/6] s390x/pci: Drop release timer and replace it with a flag
On 01.02.19 11:08, Cornelia Huck wrote: > On Thu, 31 Jan 2019 22:21:01 +0100 > David Hildenbrand wrote: > >> Thinking out loud: >> >> We should migrate the flag in the future. This is already a problem >> right now, as the timer is also not migrated. >> >> If the unplug request is sent and we migrate before the guest can react, >> the unplug would not happen. >> >> However, looks like migration of zpci devices is not implemented _at all_. > > Oh, joy. > >> >> This does not matter for pci passthrough (main use case). But when >> wanting to use e.g. virtio-pci-net, things are shaky after migration. >> >> So this is future work. >> > > Hopefully folks running s390x guests are rather using virtio-ccw > devices anyway... > > I agree that this needs to be taken care of on top of these changes. > Should we mark zpci devices unmigratable, or does it work for some > values of "work"? > It works if the guest never configures a zpci device I guess ... so I think this is pretty much broken. E.g. the state of the zpci device is not even migrated unless I am missing something. -- Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH v2 4/6] s390x/pci: Introduce unplug requests and split unplug handler
On Wed, 30 Jan 2019 16:57:31 +0100 David Hildenbrand wrote: > PCI on s390x is really weird and how it was modeled in QEMU might not have > been the right choice. Anyhow, right now it is the case that: > - Hotplugging a PCI device will silently create a zPCI device > (if none is provided) > - Hotunplugging a zPCI device will unplug the PCI device (if any) > - Hotunplugging a PCI device will unplug also the zPCI device > As far as I can see, we can no longer change this behavior. But we > should fix it. > > Both device types are handled via a single hotplug handler call. This > is problematic for various reasons: > 1. Unplugging via the zPCI device allows to unplug devices that are not >hot removable. (check performed in qdev_unplug()) - bad. > 2. Hotplug handler chains are not possible for the unplug case. In the >future, the machine might want to override hotplug handlers, to >process device specific stuff and to then branch off to the actual >hotplug handler. We need separate hotplug handler calls for both the >PCI and zPCI device to make this work reliably. All other PCI >implementations are already prepared to handle this correctly, only >s390x is missing. > > Therefore, introduce the unplug_request handler and properly perform > unplug checks by redirecting to the separate unplug_request handlers. > When finally unplugging, perform two separate hotplug_handler_unplug() > calls, first for the PCI device, followed by the zPCI device. This now > nicely splits unplugging paths for both devices. > > The redirect part is a little hairy, as the user is allowed to trigger > unplug either via the PCI or the zPCI device. So redirect always to the > PCI unplug request handler first and remember if that check has been > performed in the zPCI device. Redirect then to the zPCI device unplug > request handler to perform the magic. Remembering that we already > checked the PCI device breaks the redirect loop. > > Signed-off-by: David Hildenbrand > --- > hw/s390x/s390-pci-bus.c | 166 +++- > hw/s390x/s390-pci-bus.h | 1 + > 2 files changed, 113 insertions(+), 54 deletions(-) Thanks, applied.
Re: [Qemu-devel] [PATCH v5 07/18] kvm: add kvm_arm_get_max_vm_phys_shift
Hi Hia,
On 1/29/19 3:25 PM, Jia He wrote:
> Hi Eric
>
> On 2019/1/23 18:14, Eric Auger wrote:
>> Add the kvm_arm_get_max_vm_phys_shift() helper that returns the
>> log of the maximum IPA size supported by KVM. This capability
>> needs to be known to create the VM with a specific IPA max size
>> (kvm_type passed along KVM_CREATE_VM ioctl.
>>
>> Signed-off-by: Eric Auger
>>
>> ---
>> v3 -> v4:
>> - s/s/ms in kvm_arm_get_max_vm_phys_shift function comment
>> - check KVM_CAP_ARM_VM_IPA_SIZE extension
>>
>> v1 -> v2:
>> - put this in ARM specific code
>> ---
>> target/arm/kvm.c | 8
>> target/arm/kvm_arm.h | 16
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index e00ccf9c98..877c588ef5 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -18,6 +18,7 @@
>> #include "qemu/error-report.h"
>> #include "sysemu/sysemu.h"
>> #include "sysemu/kvm.h"
>> +#include "sysemu/kvm_int.h"
>> #include "kvm_arm.h"
>> #include "cpu.h"
>> #include "trace.h"
>> @@ -162,6 +163,13 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>> env->features = arm_host_cpu_features.features;
>> }
>> +int kvm_arm_get_max_vm_phys_shift(MachineState *ms)
>> +{
>> + KVMState *s = KVM_STATE(ms->accelerator);
>> +
>> + return kvm_check_extension(s, KVM_CAP_ARM_VM_IPA_SIZE);
>
> If KVM_CAP_ARM_VM_IPA_SIZE is not supported in host kernel, would it be
>
> better if kvm_arm_get_max_vm_phys_shift returns a default value
>
> (e.g. 40),instead of an errno?
Yes I can change that. At the moment kvm_check_extension() should return
0 if the KVM_CAP_ARM_VM_IPA_SIZE entry is not supported by the kernel.
Thanks
Eric
>
> ---
> Cheers,
> Jia He
>
>> +}
>> +
>> int kvm_arch_init(MachineState *ms, KVMState *s)
>> {
>> /* For ARM interrupt delivery is always asynchronous,
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index 6393455b1d..5969c41f83 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -207,6 +207,17 @@ bool
>> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf);
>> */
>> void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu);
>> +/**
>> + * kvm_arm_get_max_vm_phys_shift - Returns log2 of the max IPA size
>> + * supported by KVM
>> + *
>> + * @ms: Machine state handle
>> + *
>> + * Return the max number of IPA bits or a negative value if
>> + * the host kernel does not expose this value.
>> + */
>> +int kvm_arm_get_max_vm_phys_shift(MachineState *ms);
>> +
>> /**
>> * kvm_arm_sync_mpstate_to_kvm
>> * @cpu: ARMCPU
>> @@ -239,6 +250,11 @@ static inline void
>> kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>> cpu->host_cpu_probe_failed = true;
>> }
>> +static inline int kvm_arm_get_max_vm_phys_shift(MachineState *ms)
>> +{
>> + return -ENOENT;
>> +}
>> +
>> static inline int kvm_arm_vgic_probe(void)
>> {
>> return 0;
>
Re: [Qemu-devel] [RFC PATCH v5 00/52] Support Kconfig in QEMU
Hi, On 1/25/19 11:06 AM, Paolo Bonzini wrote: > (I'm only momentarily at the helm and will give control back to Yang after > this iteration.) > > This is still RFC mostly because of the lack of documentation, and because > only x86 is fully converted, but it's converging. Other targets still > enable embedded devices in default-configs/ instead of using "select" > directives. For many targets, the conversion will be trivial because > they only support one board. The complex ones are ARM, MIPS and PPC > of course. s390 as usual is just different in some respects, but all > of its issues are sorted out already in this series and so it's just > yet another single-board target. > > It supports defconfig (default-configs file chooses boards only) > and allnoconfig (default-configs file chooses devices too) and builds > all targets. I haven't yet checked that the configuration is the same > before and after the conversion, but at least device-introspection-test > and other qtests all pass, which did catch some errors. > > As mentioned in the previous versions, this is only a replacement > for default-configs, in order to simplify configuration and remove > the need to track dependencies between configuration symbols. In > fact, even with the current incomplete conversion the diffstat > for default-configs is already > > 31 files changed, 108 insertions(+), 241 deletions(-) > > Devices can be disabled by adding for example > >CONFIG_HPET=n > > to default-configs/i386-softmmu.mak. If you prefer they can be > listed manually and "make allnoconfig" can be executed before > building. This probably should become a configure option > "--without-default-devices" instead. > > For the previous discussions on the Kconfig design, see > http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02827.html > > Patches 1-27 should probably be committed now, so please review! Yang, > once they are reviewed you can extract them and post them to the > mailing list as non-RFC! > > As to the rest, you're welcome to try them, post conversions for the > simple targets, suggest usability improvements, and whatever. Thanks to > everyone for the work on the previous iterations. It's great to see > the work on this GSoC project come back to life after five years! How to express "depends of (TARGET_LONG_BITS > 32)"?
Re: [Qemu-devel] [PATCH v2] gdbstub: allow killing QEMU via vKill command
Le 1/31/19 à 5:15 PM, Luc Michel a écrit :
On 1/30/19 8:24 PM, Max Filippov wrote:
With multiprocess extensions gdb uses 'vKill' packet instead of 'k' to
kill the inferior. Handle 'vKill' the same way 'k' was handled in the
presence of single process.
Fixes: 7cf48f6752e5 ("gdbstub: add multiprocess support to
(f|s)ThreadInfo and ThreadExtraInfo")
Cc: Luc Michel
Signed-off-by: Max Filippov
Reviewed-by: Luc Michel
Reviewed-by: KONRAD Frederic
Tested-By: KONRAD Frederic
Thanks!
Fred
---
Changes v1->v2:
- terminate QEMU in the vKill packet handler regardless of whatever the
PID is or how many processes are attached [Luc Michel]
gdbstub.c | 4
1 file changed, 4 insertions(+)
diff --git a/gdbstub.c b/gdbstub.c
index bfc7afb50968..96ffcd9d9d1d 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1383,6 +1383,10 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
put_packet(s, buf);
break;
+} else if (strncmp(p, "Kill;", 5) == 0) {
+/* Kill the target */
+error_report("QEMU: Terminated via GDBstub");
+exit(0);
} else {
goto unknown_command;
}
Re: [Qemu-devel] [PATCH v2 5/6] s390x/pci: Drop release timer and replace it with a flag
On Fri, 1 Feb 2019 11:37:56 +0100 David Hildenbrand wrote: > On 01.02.19 11:08, Cornelia Huck wrote: > > On Thu, 31 Jan 2019 22:21:01 +0100 > > David Hildenbrand wrote: > > > >> Thinking out loud: > >> > >> We should migrate the flag in the future. This is already a problem > >> right now, as the timer is also not migrated. > >> > >> If the unplug request is sent and we migrate before the guest can react, > >> the unplug would not happen. > >> > >> However, looks like migration of zpci devices is not implemented _at all_. > >> > > > > Oh, joy. > > > >> > >> This does not matter for pci passthrough (main use case). But when > >> wanting to use e.g. virtio-pci-net, things are shaky after migration. > >> > >> So this is future work. > >> > > > > Hopefully folks running s390x guests are rather using virtio-ccw > > devices anyway... > > > > I agree that this needs to be taken care of on top of these changes. > > Should we mark zpci devices unmigratable, or does it work for some > > values of "work"? > > > > It works if the guest never configures a zpci device I guess ... so I > think this is pretty much broken. E.g. the state of the zpci device is > not even migrated unless I am missing something. Ok, that seems to call for marking zpci devices unmigratable...
Re: [Qemu-devel] [PATCH v2 5/6] s390x/pci: Drop release timer and replace it with a flag
On Wed, 30 Jan 2019 16:57:32 +0100 David Hildenbrand wrote: > Let's handle it similar to x86 ACPI PCI code and don't use a timer. > Instead, remember if an unplug request is pending and keep it pending > for eternity. (a follow up patch will process the request on > reboot). > > We expect that a guest that is up and running, will process the unplug > request and trigger the unplug. This is normal operation, no timer needed. > > If the guest does not react, this usually means something in the guest > is going wrong. Simply removing the device after 30 seconds does not > really sound like a good idea. It might sometimes be wanted, but I > consider this rather an "opt-in" decision as it might harm a guest not > prepared for it. > > If we ever actually want a "forced/surprise removal", we will have to > implement something on top of the existing "device_del" framework. E.g. > also x86 might want to do a forced/surprise removal of PCI devices under > some conditions. "device_del X, forced=true" could be an option and will > require changes to the hotplug handler infrastructure. > > This will then move the responsibility on when to do a forced removal > to a higher level. Doing a forced removal right now overcomplicates > things and doesn't really. > > Let's allow to send multiple requests. > > Signed-off-by: David Hildenbrand > --- > hw/s390x/s390-pci-bus.c | 38 +++--- > hw/s390x/s390-pci-bus.h | 3 +-- > 2 files changed, 8 insertions(+), 33 deletions(-) Thanks, applied.
Re: [Qemu-devel] [PATCH RFCv2 7/9] hmp: Handle virtio-pmem when printing memory device infos
* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert" writes:
>
> > * David Hildenbrand (da...@redhat.com) wrote:
> >> Print the memory device info just like for PCDIMM/NVDIMM.
> >>
> >> Signed-off-by: David Hildenbrand
> >> ---
> >> hmp.c | 27 +++
> >> 1 file changed, 15 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/hmp.c b/hmp.c
> >> index 8da5fd8760..25c32e0810 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -2553,6 +2553,7 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> QDict *qdict)
> >> Error *err = NULL;
> >> MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err);
> >> MemoryDeviceInfoList *info;
> >> +VirtioPMEMDeviceInfo *vpi;
> >> MemoryDeviceInfo *value;
> >> PCDIMMDeviceInfo *di;
> >>
> >> @@ -2562,19 +2563,9 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> QDict *qdict)
> >> if (value) {
> >> switch (value->type) {
> >> case MEMORY_DEVICE_INFO_KIND_DIMM:
> >> -di = value->u.dimm.data;
> >> -break;
> >> -
> >> case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> >> -di = value->u.nvdimm.data;
> >> -break;
> >> -
> >> -default:
> >> -di = NULL;
> >> -break;
> >> -}
> >> -
> >> -if (di) {
> >> +di = value->type == MEMORY_DEVICE_INFO_KIND_DIMM ?
> >> + value->u.dimm.data : value->u.nvdimm.data;
> >> monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
> >> MemoryDeviceInfoKind_str(value->type),
> >> di->id ? di->id : "");
> >> @@ -2587,6 +2578,18 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> QDict *qdict)
> >> di->hotplugged ? "true" : "false");
> >> monitor_printf(mon, " hotpluggable: %s\n",
> >> di->hotpluggable ? "true" : "false");
> >> +break;
> >> +case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM:
> >> +vpi = value->u.virtio_pmem.data;
> >> +monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
> >> + MemoryDeviceInfoKind_str(value->type),
> >> + vpi->id ? vpi->id : "");
> >> +monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n",
> >> vpi->memaddr);
> >> +monitor_printf(mon, " size: %" PRIu64 "\n", vpi->size);
> >> +monitor_printf(mon, " memdev: %s\n", vpi->memdev);
> >> +break;
> >> +default:
> >> +g_assert_not_reached();
> >
> >
> > Although I'd prefer if that assert was replaced by a print
> > saying it was an unknown type.
>
> I would not. If we reach this, something must have scribbled over
> value->type and who knows what else. Continuing is unsafe. Looks like
> a textbook use of assertions to me.
Or it could be that someone added a new type of memory device and forgot
to update the hmp code.
Dave
> > Reviewed-by: Dr. David Alan Gilbert
> >
> >> }
> >> }
> >> }
> >> --
> >> 2.17.2
> >>
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] block: fix last address-of-packed-member warnings
Am 01.02.2019 um 11:30 hat Peter Maydell geschrieben: > On Fri, 18 Jan 2019 at 16:17, Kevin Wolf wrote: > > > > Am 10.12.2018 um 12:26 hat Peter Maydell geschrieben: > > > This patchset fixes the remaining clang warnings in the block/ code > > > about taking the address of a packed struct member, which are all > > > in block/vpc and block/vdi code handling UUIDs. Mostly I fix > > > these by copying the unaligned field to/from a local variable. > > > In the case of qemu_uuid_bswap() I opted to change the API to > > > take and return the QemuUUID rather than taking a pointer to it, > > > which makes almost all the callsites simpler. This does mean > > > a struct copy but the struct is only 16 bytes and I didn't > > > judge any of the callsites performance-sensitive enough to care > > > about a struct copy of that size. > > > > > > As usual, tested with "make check" only. > > > > Thanks, applied to the block branch. > > Ping? These don't seem to have made it into master yet. It's in my queue. Kevin
Re: [Qemu-devel] [PATCH 07/10] monitor: Expose pvrdma device statistics counters
* Markus Armbruster (arm...@redhat.com) wrote: > Eric Blake writes: > > > On 1/31/19 2:08 PM, Yuval Shaia wrote: > >> On Thu, Jan 31, 2019 at 07:17:16AM -0600, Eric Blake wrote: > >>> On 1/31/19 7:08 AM, Yuval Shaia wrote: > Signed-off-by: Yuval Shaia > --- > hmp-commands-info.hx | 14 ++ > monitor.c| 6 ++ > 2 files changed, 20 insertions(+) > >> > >> Hi Eric, > >> > >>> > >>> Commit message should state WHY this is being added as an HMP-only > >>> command, and does not have a QMP counterpart. It may be okay if the > >>> interface is only designed to be useful to developers, but having that > >>> justification in the git log is important. > >> > >> Thanks for your review. > >> > >> See, i need this interface mainly for development/debug purposes, to help > >> troubleshot problems and to give insights to what device "is doing". > >> > >> Trace points are great but not effective in high load. > >> QMP as i see it, and correct me if i'm wrong, is used to report management > >> events etc and also here, is not effective in high load. > > If QMP is not effective, HMP won't be effective, either. But I guess > you mean something else, namely QMP *events* aren't effective, but > *polling* is. > > That's an argument for polling, not an argument for not supporting QMP. > > >> I choose this interface as it is interactive, i.e. whenever i need the info > >> i trigger 'info pvrdmastats' command from the monitor console. > >> > >> During my research i notice that some devices (or families) have nice user > >> interface via virsh (blkstat, ifstat, memstat etc). Is it the preferred way > >> for non-devel/debug purposes? > > Libvirt interfaces like these are built on top of *QMP* interfaces. If > a libvirt interface would be useful, that's another argument for > supporting QMP. > > > Using existing HMP-only debug interfaces as the design you copied is > > indeed acceptable justification for making yours HMP-only as well. So > > now you just need to copy the rationale from this email into your commit > > message, so it doesn't get lost. > > Yes. If we conclude HMP-only is okay, then the rationale for it goes > into your commit message. > > If we conclude we want HMP and QMP, I'll be happy to assist you with > adapting your patch. > > HMP commands without a QMP equivalent are okay if their functionality > makes no sense in QMP, or is of use only for human users. > > Example for "makes no sense in QMP": setting the current CPU, because a > QMP monitor doesn't have a current CPU. > > Examples for "is of use only for human users": HMP command "help", the > integrated pocket calculator. > > Debugging commands are kind of borderline. Debugging is commonly a > human activity, where HMP is just fine. However, humans create tools to > assist with their activities, and then QMP is useful. While I wouldn't > encourage HMP-only for the debugging use case, I wouldn't veto it. > > "Device statistics" sounds like it should have debugging uses. But > statistics often have non-debugging uses as well. What use cases can > you imagine for this command? I think the question here partially comes down to how 'stable' the set of statistics is and how useful they are to non-developers. The 'stable' part is a question because when you expose something via QMP it's part of the ABI so people don't like it changing. If they're things that are generic and likely to stay the same then they should probably go via QMP (e.g. bandwidth, requests/second). If they're things that are about the internal state of your implementation and just for debug, then they're fine to be HMP only. You can add them to the qmp as well, even if they're not stable by prefixing the name with an x-. Dave > >> If this is the correct method for this purpose then let me know and i'll > >> update the git log message accordingly. -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v5 00/18] ARM virt: Initial RAM expansion and PCDIMM/NVDIMM support
Patchew URL: https://patchew.org/QEMU/20190123101458.12478-1-eric.au...@redhat.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=14 === TEST SCRIPT END === CC aarch64-softmmu/hw/arm/nseries.o CC aarch64-softmmu/hw/arm/omap_sx1.o /tmp/qemu-test/src/hw/arm/virt-acpi-build.c: In function 'build_srat': /tmp/qemu-test/src/hw/arm/virt-acpi-build.c:548:48: error: incompatible type for argument 3 of 'build_srat_memory' ms->device_memory->mr.size, nb_numa_nodes - 1, ~^ In file included from /tmp/qemu-test/src/hw/arm/virt-acpi-build.c:42: The full log is available at http://patchew.org/logs/20190123101458.12478-1-eric.au...@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
Alexey Kardashevskiy writes:
> On 01/02/2019 08:57, Fabiano Rosas wrote:
>> Alexey Kardashevskiy writes:
>>
>>> On 31/01/2019 03:30, Fabiano Rosas wrote:
Alexey Kardashevskiy writes:
>
> but this is a register which does not have endianness, the endianness
> appears here because the interface between gdb and qemu is
> uint8_t*==bytestream but this interface should have fixed endianness
> imho (now it is bigendian afaict).
>
> Something is not right here...
Having a fixed endianness would not work because GDB have no way of
knowing how to represent what comes from the remote end.
>>>
>>> It definitely would. A register is stored as "unsigned long" in QEMU and
>>> all gdb has to do is printf("%lx") and that is it.
>>
>> OK, but something is not clear to me. Even if GDB just printf("%lx") the
>> value, we would still have to bswap when the host is LE, right?
>
> Not for %lx, this should just print a correct value.
>
>> QEMU BE:
>> (gdb) x/8xb &env->spr[287]
>> 0x11391760: 0x000x000x000x000x000x4e0x120x00
>>
>> QEMU LE:
>> (gdb) x/8xb &env->spr[287]
>> 0x75bd98c0: 0x010x020x4b0x000x000x000x000x00
>>
>>> The problem is that
>>> we want to pass it as a byte stream from the gdb_read_register() hook
>>> all the way to gdb and for some reason gdb does not define endianness of
>>> that stream but rather tries guessing the endianness which is broken.
>>
>> GDB does define the endianness of the stream (in a way):
>>
>> "Each byte of register data is described by two hex digits. The bytes
>> with the register are transmitted in target byte order."
>>
>> https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets
>
>
> Target byte order changes all the time so we need a endian-agnostic way
> of knowing the current endianness.
>
>
>>
>>> Today I was debugging rtas/clientinterface calls which a little endian
>>> kernel makes and these calls need to switch to the big endian first. And
>>> gdb goes nuts when this switch happens (note that I did not give an ELF
>>> to gdb this time so it picked LE itself). Even if it could fetch the
>>> endianness from QEMU, it would fail as it is an LE bit in MSR which is a
>>> register which is parsed according to the gdb's idea of endianness :)
>>
>> I think it would be possible to define another packet for the remote
>> protocol that informs the endianness explicitly at each time the guest
>> stops. If you provide more info on how to reproduce that issue I could
>> put in my list or go bug GDB folks about it.
>>
It will
always check the target endianness before printing a value, even if it
refers to a register:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49
So in our case the contents of mem_buf need to match both the guest
endianness *and* what GDB has set for 'show endian' because it will
detect it automatically from the ELF. If it guesses incorrectly because
there is no ELF, we need to use the 'set endian' command.
By the way, this is already the behavior for the registers that are
already implemented (e.g. $msr). Here's the commit that introduced
that:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7
Now, what might be a source of confusion here is the fact that we
*always* do a bswap when the host is LE because QEMU thinks that the ppc
guest is always BE. That requires the maybe_bswap function to make
things right in the end.
What I could do is try to improve this by only swapping when the
guest's actual endianness (msr_le) is different from the host's.
>>>
>>> The bytestream for registers should have fixed endianness. But looking
>>> at the gdb code makes me think it is not going to happen :(
>>
>> Yes, I can't think of a way to fix that without changing the way GDB
>> exhibits the values or the remote protocol.
>>
>> May I proceed with this series as it is with the bswaps?
>>
That
is not entirely within the scope of this patch, though.
>>>
>>> True. But since you are touching this, may be you could fix gdb too :)
>>>
>>> Does gdb tell QEMU about what endianness it thinks that QEMU is using?
>>> Or can it read it from QEMU? I cannot easily spot this in QEMU...
>>
>> GDB currently does not tell and does not ask about endianness. So I
>> believe there is room for improvement here. I could not find it in the
>> documentation but I think that GDB supports file transfers and it asks
>> for the ELF in some scenarios. This approach could be one way of
>> informing it about the endianness, although it has its own shortcomings.
>
>
> There is no ELF in my scenario really. What I did was:
>
> 1. hack qemu to not load slof.bin but load vmlinux instead and changed
> starting pc to where I loaded the kernel (I did not have to but it is a
> lot eas
[Qemu-devel] [PULL 3/5] hw/display/milkymist-tmu2: Explicit the dependency to both X11 / OpenGL
From: Philippe Mathieu-Daudé
The TMU device requires both X11 and OpenGL.
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20190130120005.23123-4-phi...@redhat.com
Signed-off-by: Gerd Hoffmann
---
hw/lm32/milkymist-hw.h | 4 ++--
default-configs/lm32-softmmu.mak | 2 +-
hw/display/Makefile.objs | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h
index d3be0cfb3a..32c344ef9f 100644
--- a/hw/lm32/milkymist-hw.h
+++ b/hw/lm32/milkymist-hw.h
@@ -88,7 +88,7 @@ static inline DeviceState *milkymist_pfpu_create(hwaddr base,
return dev;
}
-#ifdef CONFIG_OPENGL
+#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
#include
#include
#include
@@ -103,7 +103,7 @@ static const int glx_fbconfig_attr[] = {
static inline DeviceState *milkymist_tmu2_create(hwaddr base,
qemu_irq irq)
{
-#ifdef CONFIG_OPENGL
+#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
DeviceState *dev;
Display *d;
GLXFBConfig *configs;
diff --git a/default-configs/lm32-softmmu.mak b/default-configs/lm32-softmmu.mak
index 4889348a10..4049b23562 100644
--- a/default-configs/lm32-softmmu.mak
+++ b/default-configs/lm32-softmmu.mak
@@ -2,7 +2,7 @@
CONFIG_LM32=y
CONFIG_MILKYMIST=y
-CONFIG_MILKYMIST_TMU2=$(CONFIG_OPENGL)
+CONFIG_MILKYMIST_TMU2=$(call land,$(CONFIG_X11),$(CONFIG_OPENGL))
CONFIG_FRAMEBUFFER=y
CONFIG_PTIMER=y
CONFIG_PFLASH_CFI01=y
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 5b770817c7..7c4ae9a0fd 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -29,8 +29,8 @@ obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o
common-obj-$(CONFIG_ZAURUS) += tc6393xb.o
obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o
-milkymist-tmu2.o-cflags := $(X11_CFLAGS)
-milkymist-tmu2.o-libs := $(X11_LIBS)
+milkymist-tmu2.o-cflags := $(X11_CFLAGS) $(OPENGL_CFLAGS)
+milkymist-tmu2.o-libs := $(X11_LIBS) $(OPENGL_LIBS)
obj-$(CONFIG_OMAP) += omap_dss.o
obj-$(CONFIG_OMAP) += omap_lcdc.o
--
2.9.3
[Qemu-devel] [PULL 1/5] hw/display: Move Milkymist specific hardware out of common-obj list
From: Philippe Mathieu-Daudé The Milkymist specific hardware is only used by the LM32 target, it is pointless to compile those objects in other targets. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20190130120005.23123-2-phi...@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/Makefile.objs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs index 97acd5b6cb..5b770817c7 100644 --- a/hw/display/Makefile.objs +++ b/hw/display/Makefile.objs @@ -25,10 +25,10 @@ common-obj-$(CONFIG_BOCHS_DISPLAY) += edid-region.o common-obj-$(CONFIG_BLIZZARD) += blizzard.o common-obj-$(CONFIG_EXYNOS4) += exynos4210_fimd.o common-obj-$(CONFIG_FRAMEBUFFER) += framebuffer.o -common-obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o +obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o common-obj-$(CONFIG_ZAURUS) += tc6393xb.o -common-obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o +obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o milkymist-tmu2.o-cflags := $(X11_CFLAGS) milkymist-tmu2.o-libs := $(X11_LIBS) -- 2.9.3
[Qemu-devel] [PULL 4/5] hw/display/milkymist-tmu2: Move inlined code from header to source
From: Philippe Mathieu-Daudé
Move the complexity of milkymist_tmu2_create() into the
source file. Doing so we avoid to include the X11/OpenGL
headers in all LM32 devices, and we also avoid the duplicate
declaration of glx_fbconfig_attr[] (it is already declared
in hw/display/milkymist-tmu2.c).
Since TYPE_MILKYMIST_TMU2 is now accessible, use it.
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20190130120005.23123-5-phi...@redhat.com
Signed-off-by: Gerd Hoffmann
---
hw/lm32/milkymist-hw.h | 63 -
include/hw/display/milkymist_tmu2.h | 41
hw/display/milkymist-tmu2.c | 49 +
hw/lm32/milkymist.c | 1 +
MAINTAINERS | 1 +
5 files changed, 92 insertions(+), 63 deletions(-)
create mode 100644 include/hw/display/milkymist_tmu2.h
diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h
index 32c344ef9f..976cf9254d 100644
--- a/hw/lm32/milkymist-hw.h
+++ b/hw/lm32/milkymist-hw.h
@@ -88,69 +88,6 @@ static inline DeviceState *milkymist_pfpu_create(hwaddr base,
return dev;
}
-#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
-#include
-#include
-#include
-static const int glx_fbconfig_attr[] = {
-GLX_GREEN_SIZE, 5,
-GLX_GREEN_SIZE, 6,
-GLX_BLUE_SIZE, 5,
-None
-};
-#endif
-
-static inline DeviceState *milkymist_tmu2_create(hwaddr base,
-qemu_irq irq)
-{
-#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
-DeviceState *dev;
-Display *d;
-GLXFBConfig *configs;
-int nelements;
-int ver_major, ver_minor;
-
-/* check that GLX will work */
-d = XOpenDisplay(NULL);
-if (d == NULL) {
-return NULL;
-}
-
-if (!glXQueryVersion(d, &ver_major, &ver_minor)) {
-/* Yeah, sometimes getting the GLX version can fail.
- * Isn't X beautiful? */
-XCloseDisplay(d);
-return NULL;
-}
-
-if ((ver_major < 1) || ((ver_major == 1) && (ver_minor < 3))) {
-printf("Your GLX version is %d.%d,"
- "but TMU emulation needs at least 1.3. TMU disabled.\n",
- ver_major, ver_minor);
-XCloseDisplay(d);
-return NULL;
-}
-
-configs = glXChooseFBConfig(d, 0, glx_fbconfig_attr, &nelements);
-if (configs == NULL) {
-XCloseDisplay(d);
-return NULL;
-}
-
-XFree(configs);
-XCloseDisplay(d);
-
-dev = qdev_create(NULL, "milkymist-tmu2");
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
-
-return dev;
-#else
-return NULL;
-#endif
-}
-
static inline DeviceState *milkymist_ac97_create(hwaddr base,
qemu_irq crrequest_irq, qemu_irq crreply_irq, qemu_irq dmar_irq,
qemu_irq dmaw_irq)
diff --git a/include/hw/display/milkymist_tmu2.h
b/include/hw/display/milkymist_tmu2.h
new file mode 100644
index 00..148a119a1d
--- /dev/null
+++ b/include/hw/display/milkymist_tmu2.h
@@ -0,0 +1,41 @@
+/*
+ * QEMU model of the Milkymist texture mapping unit.
+ *
+ * Copyright (c) 2010 Michael Walle
+ * Copyright (c) 2010 Sebastien Bourdeauducq
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
[Qemu-devel] [PULL 0/5] Ui 20190201 patches
The following changes since commit e8977901b79fb678f288dd372261b640bbeccd0d: Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-pull-request' into staging (2019-01-31 15:40:39 +) are available in the git repository at: git://git.kraxel.org/qemu tags/ui-20190201-pull-request for you to fetch changes up to 0015ca5cbabe0b31d31610ddfaafd90a9e5911a4: ui: remove support for SDL1.2 in favour of SDL2 (2019-02-01 11:59:12 +0100) ui: fix build with SDL disabled, drop SDL1 support. Daniel P. Berrangé (1): ui: remove support for SDL1.2 in favour of SDL2 Philippe Mathieu-Daudé (4): hw/display: Move Milkymist specific hardware out of common-obj list configure: LM32 Milkymist Texture Mapping Unit (tmu2) also depends of X11 hw/display/milkymist-tmu2: Explicit the dependency to both X11 / OpenGL hw/display/milkymist-tmu2: Move inlined code from header to source configure | 70 +-- hw/lm32/milkymist-hw.h | 63 --- include/hw/display/milkymist_tmu2.h | 41 ++ ui/sdl_zoom.h | 25 - ui/sdl_zoom_template.h | 219 hw/display/milkymist-tmu2.c | 49 ++ hw/lm32/milkymist.c |1 + ui/sdl.c| 1027 --- ui/sdl_zoom.c | 93 MAINTAINERS |1 + default-configs/lm32-softmmu.mak|2 +- hw/display/Makefile.objs|8 +- qemu-deprecated.texi|9 - ui/Makefile.objs|5 - 14 files changed, 114 insertions(+), 1499 deletions(-) create mode 100644 include/hw/display/milkymist_tmu2.h delete mode 100644 ui/sdl_zoom.h delete mode 100644 ui/sdl_zoom_template.h delete mode 100644 ui/sdl.c delete mode 100644 ui/sdl_zoom.c -- 2.9.3
[Qemu-devel] [PULL 5/5] ui: remove support for SDL1.2 in favour of SDL2
From: Daniel P. Berrangé
SDL1.2 was deprecated in the 2.12.0 release with:
commit e52c6ba34149b4f39c3fd60e59ee32b809db2bfa
Author: Daniel P. Berrange
Date: Mon Jan 15 14:25:33 2018 +
ui: deprecate use of SDL 1.2 in favour of 2.0 series
The SDL 2.0 release was made in Aug, 2013:
https://www.libsdl.org/release/
That will soon be 4 + 1/2 years ago, which is enough time to consider
the 2.0 series widely supported.
Thus we deprecate the SDL 1.2 support, which will allow us to delete it
in the last release of 2018. By this time, SDL 2.0 will be more than 5
years old.
Signed-off-by: Daniel P. Berrange
Reviewed-by: Marc-André Lureau
Message-id: 20180115142533.24585-1-berra...@redhat.com
Signed-off-by: Gerd Hoffmann
It is thus able to be removed in the 3.1.0 release.
Signed-off-by: Daniel P. Berrangé
Message-Id: <20180822131554.3398-4-berra...@redhat.com>
[ kraxel: rebase ]
Signed-off-by: Gerd Hoffmann
---
configure | 60 +--
ui/sdl_zoom.h | 25 --
ui/sdl_zoom_template.h | 219 ---
ui/sdl.c | 1027
ui/sdl_zoom.c | 93 -
qemu-deprecated.texi |9 -
ui/Makefile.objs |5 -
7 files changed, 7 insertions(+), 1431 deletions(-)
delete mode 100644 ui/sdl_zoom.h
delete mode 100644 ui/sdl_zoom_template.h
delete mode 100644 ui/sdl.c
delete mode 100644 ui/sdl_zoom.c
diff --git a/configure b/configure
index c7024d6662..b229f43334 100755
--- a/configure
+++ b/configure
@@ -348,7 +348,6 @@ docs=""
fdt=""
netmap="no"
sdl=""
-sdlabi=""
sdl_image=""
virtfs=""
mpath=""
@@ -577,7 +576,6 @@ query_pkg_config() {
"${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
}
pkg_config=query_pkg_config
-sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
# If the user hasn't specified ARFLAGS, default to 'rv', just as make does.
@@ -1044,8 +1042,6 @@ for opt do
;;
--enable-sdl) sdl="yes"
;;
- --with-sdlabi=*) sdlabi="$optarg"
- ;;
--disable-sdl-image) sdl_image="no"
;;
--enable-sdl-image) sdl_image="yes"
@@ -1711,7 +1707,6 @@ disabled with --disable-FEATURE, default is enabled if
available:
nettle nettle cryptography support
gcrypt libgcrypt cryptography support
sdl SDL UI
- --with-sdlabi select preferred SDL ABI 1.2 or 2.0
sdl_image SDL Image support for icons
gtk gtk UI
vte vte support for the gtk UI
@@ -2927,37 +2922,11 @@ fi
sdl_probe ()
{
- sdl_too_old=no
- if test "$sdlabi" = ""; then
- if $pkg_config --exists "sdl2"; then
- sdlabi=2.0
- elif $pkg_config --exists "sdl"; then
- sdlabi=1.2
- else
- sdlabi=2.0
- fi
- fi
-
- if test $sdlabi = "2.0"; then
- sdl_config=$sdl2_config
- sdlname=sdl2
- sdlconfigname=sdl2_config
- elif test $sdlabi = "1.2"; then
- sdlname=sdl
- sdlconfigname=sdl_config
- else
- error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
- fi
-
- if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config};
then
-sdl_config=$sdlconfigname
- fi
-
- if $pkg_config $sdlname --exists; then
-sdlconfig="$pkg_config $sdlname"
+ if $pkg_config sdl2 --exists; then
+sdlconfig="$pkg_config sdl2"
sdlversion=$($sdlconfig --modversion 2>/dev/null)
elif has ${sdl_config}; then
-sdlconfig="$sdl_config"
+sdlconfig="$sdl2_config"
sdlversion=$($sdlconfig --version)
else
if test "$sdl" = "yes" ; then
@@ -2979,8 +2948,8 @@ EOF
sdl_cflags=$($sdlconfig --cflags 2>/dev/null)
sdl_cflags="$sdl_cflags -Wno-undef" # workaround 2.0.8 bug
if test "$static" = "yes" ; then
-if $pkg_config $sdlname --exists; then
- sdl_libs=$($pkg_config $sdlname --static --libs 2>/dev/null)
+if $pkg_config sdl2 --exists; then
+ sdl_libs=$($pkg_config sdl2 --static --libs 2>/dev/null)
else
sdl_libs=$($sdlconfig --static-libs 2>/dev/null)
fi
@@ -2988,11 +2957,7 @@ EOF
sdl_libs=$($sdlconfig --libs 2>/dev/null)
fi
if compile_prog "$sdl_cflags" "$sdl_libs" ; then
-if test $(echo $sdlversion | sed 's/[^0-9]//g') -lt 121 ; then
- sdl_too_old=yes
-else
- sdl=yes
-fi
+sdl=yes
# static link with sdl ? (note: sdl.pc's --static --libs is broken)
if test "$sdl" = "yes" -a "$static" = "yes" ; then
@@ -3008,7 +2973,7 @@ EOF
fi # static link
else # sdl not found
if test "$sdl" = "yes" ; then
- feature_not_found "sdl" "Install SDL devel"
+ feature_not_found "sdl" "Install SDL2 devel"
fi
sdl=no
fi # sdl compile test
@@ -6220,16 +6185,6 @@ echo "docker$docker"
echo "libpmem support $libpmem"
echo "libudev $libudev"
-if test "$sdl_too_old" = "yes"; then
-echo "-> Your SDL version is too old - please upgrade
Re: [Qemu-devel] [PATCH v11 for-4.0 03/11] qemu_thread: supplement error handling for qmp_dump_guest_memory
Fei Li writes: > From: Fei Li > > Utilize the existed errp to propagate the error instead of the > temporary &error_abort. > > Cc: Markus Armbruster > Cc: Marc-André Lureau > Signed-off-by: Fei Li Reviewed-by: Markus Armbruster
[Qemu-devel] [PULL 2/5] configure: LM32 Milkymist Texture Mapping Unit (tmu2) also depends of X11
From: Philippe Mathieu-Daudé Commit 5f9b1e35060b8 remove the dependency between OpenGL and X11. However the milkymist-tmu2 device do require X11. When using SDL, the configure script sets need_x11=yes, so the X11 flags are populated to the makefiles. When building without SDL, X11 is not pulled and populated, leading to a link failure: LINKlm32-softmmu/qemu-system-lm32 hw/lm32/milkymist.o: In function `milkymist_tmu2_create': hw/lm32/milkymist-hw.h:114: undefined reference to `XOpenDisplay' hw/lm32/milkymist-hw.h:140: undefined reference to `XFree' hw/lm32/milkymist-hw.h:141: undefined reference to `XCloseDisplay' hw/lm32/milkymist-hw.h:130: undefined reference to `XCloseDisplay' ../hw/display/milkymist-tmu2.o: In function `tmu2_glx_init': hw/display/milkymist-tmu2.c:112: undefined reference to `XOpenDisplay' hw/display/milkymist-tmu2.c:123: undefined reference to `XFree' collect2: error: ld returned 1 exit status gmake[1]: *** [Makefile:199: qemu-system-lm32] Error 1 Enforce the X11 dependency when the LM32 target is built. This will allow us to build QEMU without SDL. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20190130120005.23123-3-phi...@redhat.com Signed-off-by: Gerd Hoffmann --- configure | 10 ++ 1 file changed, 10 insertions(+) diff --git a/configure b/configure index b18281c61f..c7024d6662 100755 --- a/configure +++ b/configure @@ -4047,6 +4047,16 @@ EOF fi fi +if test "$opengl" = "yes" -a "$have_x11" = "yes"; then + for target in $target_list; do +case $target in + lm32-softmmu) # milkymist-tmu2 requires X11 and OpenGL +need_x11=yes + ;; +esac + done +fi + ## # libxml2 probe if test "$libxml2" != "no" ; then -- 2.9.3
Re: [Qemu-devel] [PATCH 01/10] hw/rdma: Switch to generic error reporting way
* Yuval Shaia (yuval.sh...@oracle.com) wrote:
> Utilize error_report for all pr_err calls and some pr_dbg that are
> considered as errors.
> For the remaining pr_dbg calls, the important ones were replaced by
> trace points while other deleted.
>
> Signed-off-by: Yuval Shaia
> ---
> hw/rdma/rdma_backend.c| 336 ++
> hw/rdma/rdma_rm.c | 121 +---
> hw/rdma/rdma_utils.c | 11 +-
> hw/rdma/rdma_utils.h | 45 +
> hw/rdma/trace-events | 32 +++-
> hw/rdma/vmw/pvrdma.h | 2 +-
> hw/rdma/vmw/pvrdma_cmd.c | 113 +++-
> hw/rdma/vmw/pvrdma_dev_ring.c | 26 +--
> hw/rdma/vmw/pvrdma_main.c | 132 +
> hw/rdma/vmw/pvrdma_qp_ops.c | 49 ++---
> hw/rdma/vmw/trace-events | 16 +-
> 11 files changed, 337 insertions(+), 546 deletions(-)
>
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index fd571f21e5..5f60856d19 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -14,7 +14,6 @@
> */
>
> #include "qemu/osdep.h"
> -#include "qemu/error-report.h"
> #include "sysemu/sysemu.h"
> #include "qapi/error.h"
> #include "qapi/qmp/qlist.h"
> @@ -39,7 +38,6 @@
>
> typedef struct BackendCtx {
> void *up_ctx;
> -bool is_tx_req;
> struct ibv_sge sge; /* Used to save MAD recv buffer */
> } BackendCtx;
>
> @@ -52,7 +50,7 @@ static void (*comp_handler)(void *ctx, struct ibv_wc *wc);
>
> static void dummy_comp_handler(void *ctx, struct ibv_wc *wc)
> {
> -pr_err("No completion handler is registered\n");
> +rdma_error_report("No completion handler is registered");
> }
>
> static inline void complete_work(enum ibv_wc_status status, uint32_t
> vendor_err,
> @@ -66,29 +64,24 @@ static inline void complete_work(enum ibv_wc_status
> status, uint32_t vendor_err,
> comp_handler(ctx, &wc);
> }
>
> -static void poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
> +static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq
> *ibcq)
> {
> int i, ne;
> BackendCtx *bctx;
> struct ibv_wc wc[2];
>
> -pr_dbg("Entering poll_cq loop on cq %p\n", ibcq);
> do {
> ne = ibv_poll_cq(ibcq, ARRAY_SIZE(wc), wc);
>
> -pr_dbg("Got %d completion(s) from cq %p\n", ne, ibcq);
> +trace_rdma_poll_cq(ne, ibcq);
>
> for (i = 0; i < ne; i++) {
> -pr_dbg("wr_id=0x%" PRIx64 "\n", wc[i].wr_id);
> -pr_dbg("status=%d\n", wc[i].status);
> -
> bctx = rdma_rm_get_cqe_ctx(rdma_dev_res, wc[i].wr_id);
> if (unlikely(!bctx)) {
> -pr_dbg("Error: Failed to find ctx for req %" PRId64 "\n",
> - wc[i].wr_id);
> +rdma_error_report("No matching ctx for req %"PRId64,
> + wc[i].wr_id);
> -#define init_pr_dbg(void)
> -#define pr_dbg(fmt, ...)
> -#define pr_dbg_buf(title, buf, len)
> -#endif
> +#define rdma_error_report(fmt, ...) \
> +error_report("%s: " fmt, "rdma", ## __VA_ARGS__)
> +#define rdma_warn_report(fmt, ...) \
> +warn_report("%s: " fmt, "rdma", ## __VA_ARGS__)
> +#define rdma_info_report(fmt, ...) \
> +info_report("%s: " fmt, "rdma", ## __VA_ARGS__)
Is it worth defining these? My temptation would be just to use the
__func__ string, i.e. the case above would become:
error_report("%s: No matching ctx for req %"PRId64,
__func__, wc[i].wr_id);
That's used in lots of places, and gives more useful information.
Dave
>
> void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen);
> void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len);
> diff --git a/hw/rdma/trace-events b/hw/rdma/trace-events
> index c4c202e647..09cec43fd8 100644
> --- a/hw/rdma/trace-events
> +++ b/hw/rdma/trace-events
> @@ -1,5 +1,31 @@
> # See docs/tracing.txt for syntax documentation.
>
> -#hw/rdma/rdma_backend.c
> -create_ah_cache_hit(uint64_t subnet, uint64_t net_id) "subnet = 0x%"PRIx64"
> net_id = 0x%"PRIx64
> -create_ah_cache_miss(uint64_t subnet, uint64_t net_id) "subnet = 0x%"PRIx64"
> net_id = 0x%"PRIx64
> +# hw/rdma/rdma_backend.c
> +rdma_check_dev_attr(const char *name, int max_bk, int max_fe) "%s: be=%d,
> fe=%d"
> +rdma_create_ah_cache_hit(uint64_t subnet, uint64_t if_id)
> "subnet=0x%"PRIx64",if_id=0x%"PRIx64
> +rdma_create_ah_cache_miss(uint64_t subnet, uint64_t if_id)
> "subnet=0x%"PRIx64",if_id=0x%"PRIx64
> +rdma_poll_cq(int ne, void *ibcq) "Got %d completion(s) from cq %p"
> +rdmacm_mux(const char *title, int msg_type, int op_code) "%s: msg_type=%d,
> op_code=%d"
> +rdmacm_mux_check_op_status(int msg_type, int op_code, int err_code) "resp:
> msg_type=%d, op_code=%d, err_code=%d"
> +rdma_mad_message(const char *title, size_t len, char *data) "mad %s (%ld):
> %s"
> +rdma_backend_rc_qp_state_init(uint32_t qpn) "RC QP 0x%x switch to INIT
Re: [Qemu-devel] [PATCH v11 for-4.0 02/11] qemu_thread: supplement error handling for qemu_X_start_vcpu
Fei Li writes:
> From: Fei Li
>
> The callers of qemu_init_vcpu() already passed the **errp to handle
> errors. In view of this, add a new Error parameter to qemu_init_vcpu()
> and all qemu_X_start_vcpu() functions called by qemu_init_vcpu() to
> propagate the error and let the further callers check it.
>
> Besides, make qemu_init_vcpu() return a Boolean value to let its
> callers know whether it succeeds.
>
> Cc: Paolo Bonzini
> Signed-off-by: Fei Li
> Reviewed-by: Fam Zheng
> Reviewed-by: Juan Quintela
> ---
> accel/tcg/user-exec-stub.c | 3 +-
> cpus.c | 74 +++--
> include/qom/cpu.h | 2 +-
> target/alpha/cpu.c | 4 +-
> target/arm/cpu.c| 4 +-
> target/cris/cpu.c | 4 +-
> target/hppa/cpu.c | 4 +-
> target/i386/cpu.c | 4 +-
> target/lm32/cpu.c | 4 +-
> target/m68k/cpu.c | 4 +-
> target/microblaze/cpu.c | 4 +-
> target/mips/cpu.c | 4 +-
> target/moxie/cpu.c | 4 +-
> target/nios2/cpu.c | 4 +-
> target/openrisc/cpu.c | 4 +-
> target/ppc/translate_init.inc.c | 4 +-
> target/riscv/cpu.c | 4 +-
> target/s390x/cpu.c | 4 +-
> target/sh4/cpu.c| 4 +-
> target/sparc/cpu.c | 4 +-
> target/tilegx/cpu.c | 4 +-
> target/tricore/cpu.c| 4 +-
> target/unicore32/cpu.c | 4 +-
> target/xtensa/cpu.c | 4 +-
> 24 files changed, 108 insertions(+), 55 deletions(-)
>
> diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
> index a32b4496af..f8c38a375c 100644
> --- a/accel/tcg/user-exec-stub.c
> +++ b/accel/tcg/user-exec-stub.c
> @@ -10,8 +10,9 @@ void cpu_resume(CPUState *cpu)
> {
> }
>
> -void qemu_init_vcpu(CPUState *cpu)
> +bool qemu_init_vcpu(CPUState *cpu, Error **errp)
> {
> +return true;
> }
>
> /* User mode emulation does not support record/replay yet. */
> diff --git a/cpus.c b/cpus.c
> index 843a0f06a2..4ed7d62e58 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1931,7 +1931,7 @@ void cpu_remove_sync(CPUState *cpu)
> /* For temporary buffers for forming a name */
> #define VCPU_THREAD_NAME_SIZE 16
>
> -static void qemu_tcg_init_vcpu(CPUState *cpu)
> +static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp)
> {
> char thread_name[VCPU_THREAD_NAME_SIZE];
> static QemuCond *single_tcg_halt_cond;
> @@ -1961,17 +1961,20 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
> cpu->cpu_index);
>
> -/* TODO: let the callers handle the error instead of abort()
> here */
> -qemu_thread_create(cpu->thread, thread_name,
> qemu_tcg_cpu_thread_fn,
> - cpu, QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(cpu->thread, thread_name,
> + qemu_tcg_cpu_thread_fn, cpu,
> + QEMU_THREAD_JOINABLE, errp) < 0) {
> +return;
> +}
>
> } else {
> /* share a single thread for all cpus with TCG */
> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
> -/* TODO: let the callers handle the error instead of abort()
> here */
> -qemu_thread_create(cpu->thread, thread_name,
> - qemu_tcg_rr_cpu_thread_fn,
> - cpu, QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(cpu->thread, thread_name,
> + qemu_tcg_rr_cpu_thread_fn, cpu,
> + QEMU_THREAD_JOINABLE, errp) < 0) {
> +return;
> +}
>
> single_tcg_halt_cond = cpu->halt_cond;
> single_tcg_cpu_thread = cpu->thread;
> @@ -1989,7 +1992,7 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
> }
> }
>
> -static void qemu_hax_start_vcpu(CPUState *cpu)
> +static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp)
> {
> char thread_name[VCPU_THREAD_NAME_SIZE];
>
> @@ -1999,15 +2002,16 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
>
> snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
> cpu->cpu_index);
> -/* TODO: let the further caller handle the error instead of abort() here
> */
> -qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
> - cpu, QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
> + cpu, QEMU_THREAD_JOINABLE, errp) < 0) {
> +return;
> +}
> #ifdef _WIN32
> cpu->hThread = qemu_thread_get_handle(cpu->thread);
> #endif
> }
>
> -static void qemu_kvm_sta
Re: [Qemu-devel] [Qemu-block] [PATCH] qtest.py: Wait for the result of qtest commands
Am 31.01.2019 um 13:38 hat Alberto Garcia geschrieben:
> The cmd() method of the QEMUQtestProtocol class sends a qtest command
> to QEMU but doesn't wait for the return message ("OK", "FAIL", "ERR").
> Because of this, it can return control to the caller before the
> command has actually finished.
>
> In cases like clock_step or clock_set this means that cmd() can return
> before all the timers triggered by the clock change have been fired.
> This can be fixed by making cmd() wait for the output of the qtest
> command.
>
> This fixes iotests 093 and 136, which are flaky since commit
> 8258292e18c39480b64eba9f3551 when the machine is under heavy workload.
>
> Signed-off-by: Alberto Garcia
Thanks, applied to the block branch.
Kevin
[Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable
We currently don't migrate any state for zpci devices, which are
coupled with standard pci devices. This means funny things happen
when we e.g. try to migrate with a virtio-pci device but the s390x-
specific zpci state is not migrated (vfio-pci is not affected, as
it is not migratable anyway.)
Until this is fixed, mark zpci devices as unmigratable.
Reported-by: David Hildenbrand
Signed-off-by: Cornelia Huck
---
This is just a stop-gap measure to give us time to implement the
needed migration code properly.
---
hw/s390x/s390-pci-bus.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index c96a7cba34..96c7c18f3f 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const VMStateDescription s390_pci_device_vmstate = {
+.name = TYPE_S390_PCI_DEVICE,
+/*
+ * TODO: add state handling here, so migration works at least with
+ * emulated pci devices on s390x
+ */
+.unmigratable = 1,
+};
+
static void s390_pci_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass
*klass, void *data)
dc->bus_type = TYPE_S390_PCI_BUS;
dc->realize = s390_pci_device_realize;
dc->props = s390_pci_device_properties;
+dc->vmsd = &s390_pci_device_vmstate;
}
static const TypeInfo s390_pci_device_info = {
--
2.17.2
Re: [Qemu-devel] [PATCH 0/3] scsi-disk: Device Identification fixes
Am 25.01.2019 um 18:46 hat Kevin Wolf geschrieben: > The vendor specific designator in the Device Identification VPD page has > two problems: > > 1. It defaults to the BlockBackend name (-drive id=...), which everyone >expected to be a host detail that the guest never sees > > 2. With -blockdev based setups it defaults to an empty string; if this >default is used with more than one disk, the guest OS will interpret >this as a single multipath disk. > > We can address problem 2 immediately, and start running the deprecation > clock for problem 1. > > Related bug reports: > https://bugzilla.redhat.com/show_bug.cgi?id=1669446 > https://bugzilla.redhat.com/show_bug.cgi?id=1668248 Applied patches 1 and 2 to the block branch so that the bug is fixed. I'll try to rewrite patch 3 along the lines Dan suggested. Kevin
Re: [Qemu-devel] [PATCH v11 for-4.0 04/11] qemu_thread: supplement error handling for pci_edu_realize
Fei Li writes:
> From: Fei Li
>
> Utilize the existed errp to propagate the error and do the
> corresponding cleanup to replace the temporary &error_abort.
>
> Cc: Markus Armbruster
> Cc: Jiri Slaby
> Signed-off-by: Fei Li
> ---
> hw/misc/edu.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 21adbfddce..8fe232b6d6 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -356,9 +356,14 @@ static void pci_edu_realize(PCIDevice *pdev, Error
> **errp)
>
> qemu_mutex_init(&edu->thr_mutex);
> qemu_cond_init(&edu->thr_cond);
> -/* TODO: let the further caller handle the error instead of abort() here
> */
> -qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
> - edu, QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
> + edu, QEMU_THREAD_JOINABLE, errp) < 0) {
> +qemu_cond_destroy(&edu->thr_cond);
> +qemu_mutex_destroy(&edu->thr_mutex);
> +timer_del(&edu->dma_timer);
> +msi_uninit(pdev);
> +return;
> +}
>
> memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
> "edu-mmio", 1 * MiB);
In review of v9, I pointed out that pci_edu_uninit() neglects to call
msix_uninit(), and you offered to fix it. Not in this series. Do you
still intend to fix it?
That's a separate patch, though, so
Reviewed-by: Markus Armbruster
[Qemu-devel] [PATCH v5 0/6] NBD reconnect: prep. refactoring
Hi all. Here is most of preparatory refactoring for NBD reconnect, rebased on master. Let's push it. Sorry for a long delay in answering on v4 review and for v4 which I pinged a lot can't be applied on master directly :( v5: 02: rebased on master, object_unref() moved to fail: block 05: tiny fixes in commit message [Eric] Vladimir Sementsov-Ogievskiy (6): block/nbd-client: split channel errors from export errors block/nbd: move connection code from block/nbd to block/nbd-client block/nbd-client: split connection from initialization block/nbd-client: fix nbd_reply_chunk_iter_receive block/nbd-client: don't check ioc block/nbd-client: rename read_reply_co to connection_co block/nbd-client.h | 6 +- block/nbd-client.c | 196 + block/nbd.c| 40 + 3 files changed, 131 insertions(+), 111 deletions(-) -- 2.18.0
[Qemu-devel] [PATCH v5 3/6] block/nbd-client: split connection from initialization
Split connection code to reuse it for reconnect.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/nbd-client.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 641666d3bc..a8ce0824c2 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -1014,13 +1014,13 @@ static QIOChannelSocket
*nbd_establish_connection(SocketAddress *saddr,
return sioc;
}
-int nbd_client_init(BlockDriverState *bs,
-SocketAddress *saddr,
-const char *export,
-QCryptoTLSCreds *tlscreds,
-const char *hostname,
-const char *x_dirty_bitmap,
-Error **errp)
+static int nbd_client_connect(BlockDriverState *bs,
+ SocketAddress *saddr,
+ const char *export,
+ QCryptoTLSCreds *tlscreds,
+ const char *hostname,
+ const char *x_dirty_bitmap,
+ Error **errp)
{
NBDClientSession *client = nbd_get_client_session(bs);
int ret;
@@ -1073,8 +1073,6 @@ int nbd_client_init(BlockDriverState *bs,
bs->supported_zero_flags |= BDRV_REQ_MAY_UNMAP;
}
-qemu_co_mutex_init(&client->send_mutex);
-qemu_co_queue_init(&client->free_sema);
client->sioc = sioc;
if (!client->ioc) {
@@ -1107,3 +1105,20 @@ int nbd_client_init(BlockDriverState *bs,
return ret;
}
}
+
+int nbd_client_init(BlockDriverState *bs,
+SocketAddress *saddr,
+const char *export,
+QCryptoTLSCreds *tlscreds,
+const char *hostname,
+const char *x_dirty_bitmap,
+Error **errp)
+{
+NBDClientSession *client = nbd_get_client_session(bs);
+
+qemu_co_mutex_init(&client->send_mutex);
+qemu_co_queue_init(&client->free_sema);
+
+return nbd_client_connect(bs, saddr, export, tlscreds, hostname,
+ x_dirty_bitmap, errp);
+}
--
2.18.0
Re: [Qemu-devel] [PATCH v11 for-4.0 05/11] qemu_thread: supplement error handling for h_resize_hpt_prepare
Fei Li writes: > From: Fei Li > > Add a local_err to hold the error, and return the corresponding > error code to replace the temporary &error_abort. > > Cc: Markus Armbruster > Cc: David Gibson > Signed-off-by: Fei Li > Acked-by: David Gibson Reviewed-by: Markus Armbruster
[Qemu-devel] [PATCH v5 2/6] block/nbd: move connection code from block/nbd to block/nbd-client
Keep all connection code in one file, to be able to implement reconnect
in further patches.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
Notes:
v5:
rebase on master, object_unref under fail:.
fix comment to "/*\n" style.
block/nbd-client.h | 2 +-
block/nbd-client.c | 40 ++--
block/nbd.c| 40 ++--
3 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index cfc90550b9..2f047ba614 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -41,7 +41,7 @@ typedef struct NBDClientSession {
NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
int nbd_client_init(BlockDriverState *bs,
-QIOChannelSocket *sock,
+SocketAddress *saddr,
const char *export_name,
QCryptoTLSCreds *tlscreds,
const char *hostname,
diff --git a/block/nbd-client.c b/block/nbd-client.c
index b023a35d1d..641666d3bc 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -991,8 +991,31 @@ void nbd_client_close(BlockDriverState *bs)
nbd_teardown_connection(bs);
}
+static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
+ Error **errp)
+{
+QIOChannelSocket *sioc;
+Error *local_err = NULL;
+
+sioc = qio_channel_socket_new();
+qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
+
+qio_channel_socket_connect_sync(sioc,
+saddr,
+&local_err);
+if (local_err) {
+object_unref(OBJECT(sioc));
+error_propagate(errp, local_err);
+return NULL;
+}
+
+qio_channel_set_delay(QIO_CHANNEL(sioc), false);
+
+return sioc;
+}
+
int nbd_client_init(BlockDriverState *bs,
-QIOChannelSocket *sioc,
+SocketAddress *saddr,
const char *export,
QCryptoTLSCreds *tlscreds,
const char *hostname,
@@ -1002,6 +1025,16 @@ int nbd_client_init(BlockDriverState *bs,
NBDClientSession *client = nbd_get_client_session(bs);
int ret;
+/*
+ * establish TCP connection, return error if it fails
+ * TODO: Configurable retry-until-timeout behaviour.
+ */
+QIOChannelSocket *sioc = nbd_establish_connection(saddr, errp);
+
+if (!sioc) {
+return -ECONNREFUSED;
+}
+
/* NBD handshake */
logout("session init %s\n", export);
qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL);
@@ -1017,6 +1050,7 @@ int nbd_client_init(BlockDriverState *bs,
g_free(client->info.name);
if (ret < 0) {
logout("Failed to negotiate with the NBD server\n");
+object_unref(OBJECT(sioc));
return ret;
}
if (x_dirty_bitmap && !client->info.base_allocation) {
@@ -1042,7 +1076,6 @@ int nbd_client_init(BlockDriverState *bs,
qemu_co_mutex_init(&client->send_mutex);
qemu_co_queue_init(&client->free_sema);
client->sioc = sioc;
-object_ref(OBJECT(client->sioc));
if (!client->ioc) {
client->ioc = QIO_CHANNEL(sioc);
@@ -1068,6 +1101,9 @@ int nbd_client_init(BlockDriverState *bs,
NBDRequest request = { .type = NBD_CMD_DISC };
nbd_send_request(client->ioc ?: QIO_CHANNEL(sioc), &request);
+
+object_unref(OBJECT(sioc));
+
return ret;
}
}
diff --git a/block/nbd.c b/block/nbd.c
index e87699fb73..9db5eded89 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -295,30 +295,6 @@ NBDClientSession *nbd_get_client_session(BlockDriverState
*bs)
return &s->client;
}
-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
- Error **errp)
-{
-QIOChannelSocket *sioc;
-Error *local_err = NULL;
-
-sioc = qio_channel_socket_new();
-qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
-
-qio_channel_socket_connect_sync(sioc,
-saddr,
-&local_err);
-if (local_err) {
-object_unref(OBJECT(sioc));
-error_propagate(errp, local_err);
-return NULL;
-}
-
-qio_channel_set_delay(QIO_CHANNEL(sioc), false);
-
-return sioc;
-}
-
-
static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
{
Object *obj;
@@ -394,7 +370,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options,
int flags,
BDRVNBDState *s = bs->opaque;
QemuOpts *opts = NULL;
Error *local_err = NULL;
-QIOChannelSocket *sioc = NULL;
QCryptoTLSCreds *tlscreds = NULL;
const char *hostname = NULL;
int ret = -EINVAL;
@@ -434,22 +409,11 @@ static int nbd_open(BlockDriverState *bs, QDict *options,
int flags,
hostname = s->saddr->u.inet.host;
}
-/* establish TCP connection, return
[Qemu-devel] [PATCH v5 1/6] block/nbd-client: split channel errors from export errors
To implement nbd reconnect in further patches, we need to distinguish
error codes, returned by nbd server, from channel errors, to reconnect
only in the latter case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/nbd-client.c | 83 ++
1 file changed, 47 insertions(+), 36 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 813539676d..b023a35d1d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -504,11 +504,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
*/
static coroutine_fn int nbd_co_receive_one_chunk(
NBDClientSession *s, uint64_t handle, bool only_structured,
-QEMUIOVector *qiov, NBDReply *reply, void **payload, Error **errp)
+int *request_ret, QEMUIOVector *qiov, NBDReply *reply, void **payload,
+Error **errp)
{
-int request_ret;
int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured,
- &request_ret, qiov, payload, errp);
+ request_ret, qiov, payload, errp);
if (ret < 0) {
s->quit = true;
@@ -518,7 +518,6 @@ static coroutine_fn int nbd_co_receive_one_chunk(
*reply = s->reply;
}
s->reply.handle = 0;
-ret = request_ret;
}
if (s->read_reply_co) {
@@ -530,22 +529,17 @@ static coroutine_fn int nbd_co_receive_one_chunk(
typedef struct NBDReplyChunkIter {
int ret;
-bool fatal;
+int request_ret;
Error *err;
bool done, only_structured;
} NBDReplyChunkIter;
-static void nbd_iter_error(NBDReplyChunkIter *iter, bool fatal,
- int ret, Error **local_err)
+static void nbd_iter_channel_error(NBDReplyChunkIter *iter,
+ int ret, Error **local_err)
{
assert(ret < 0);
-if ((fatal && !iter->fatal) || iter->ret == 0) {
-if (iter->ret != 0) {
-error_free(iter->err);
-iter->err = NULL;
-}
-iter->fatal = fatal;
+if (!iter->ret) {
iter->ret = ret;
error_propagate(&iter->err, *local_err);
} else {
@@ -555,6 +549,15 @@ static void nbd_iter_error(NBDReplyChunkIter *iter, bool
fatal,
*local_err = NULL;
}
+static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
+{
+assert(ret < 0);
+
+if (!iter->request_ret) {
+iter->request_ret = ret;
+}
+}
+
/* NBD_FOREACH_REPLY_CHUNK
*/
#define NBD_FOREACH_REPLY_CHUNK(s, iter, handle, structured, \
@@ -570,13 +573,13 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession
*s,
QEMUIOVector *qiov, NBDReply *reply,
void **payload)
{
-int ret;
+int ret, request_ret;
NBDReply local_reply;
NBDStructuredReplyChunk *chunk;
Error *local_err = NULL;
if (s->quit) {
error_setg(&local_err, "Connection closed");
-nbd_iter_error(iter, true, -EIO, &local_err);
+nbd_iter_channel_error(iter, -EIO, &local_err);
goto break_loop;
}
@@ -590,10 +593,12 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession
*s,
}
ret = nbd_co_receive_one_chunk(s, handle, iter->only_structured,
- qiov, reply, payload, &local_err);
+ &request_ret, qiov, reply, payload,
+ &local_err);
if (ret < 0) {
-/* If it is a fatal error s->quit is set by nbd_co_receive_one_chunk */
-nbd_iter_error(iter, s->quit, ret, &local_err);
+nbd_iter_channel_error(iter, ret, &local_err);
+} else if (request_ret < 0) {
+nbd_iter_request_error(iter, request_ret);
}
/* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
@@ -630,7 +635,7 @@ break_loop:
}
static int nbd_co_receive_return_code(NBDClientSession *s, uint64_t handle,
- Error **errp)
+ int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
@@ -639,12 +644,13 @@ static int nbd_co_receive_return_code(NBDClientSession
*s, uint64_t handle,
}
error_propagate(errp, iter.err);
+*request_ret = iter.request_ret;
return iter.ret;
}
static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle,
uint64_t offset, QEMUIOVector *qiov,
-Error **errp)
+int *request_ret, Error **errp)
{
NBDReplyChunkIter iter;
NBDReply reply;
@@ -669,7 +675,7 @@ static int nbd_co_receive_cmdread_reply(NBDClientSession
*s, uint64_t handle,
offset, qiov, &local_err);
if (ret < 0) {
s->quit = true;
-nbd_
[Qemu-devel] [PATCH v5 5/6] block/nbd-client: don't check ioc
We have several paranoid checks for ioc != NULL. But ioc may become
NULL only on close, which should not happen during requests handling.
Also, we check ioc only sometimes, not after each yield, which is
inconsistent. Let's drop these checks. However, for safety, let's leave
asserts instead.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
Notes:
v5: fix commit message a bit [Eric]
block/nbd-client.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 8e9b3dbedd..d8bf1bbb8b 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -53,9 +53,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
{
NBDClientSession *client = nbd_get_client_session(bs);
-if (!client->ioc) { /* Already closed */
-return;
-}
+assert(client->ioc);
/* finish any pending coroutines */
qio_channel_shutdown(client->ioc,
@@ -153,10 +151,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
rc = -EIO;
goto err;
}
-if (!s->ioc) {
-rc = -EPIPE;
-goto err;
-}
+assert(s->ioc);
if (qiov) {
qio_channel_set_cork(s->ioc, true);
@@ -429,10 +424,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
s->requests[i].receiving = true;
qemu_coroutine_yield();
s->requests[i].receiving = false;
-if (!s->ioc || s->quit) {
+if (s->quit) {
error_setg(errp, "Connection closed");
return -EIO;
}
+assert(s->ioc);
assert(s->reply.handle == handle);
@@ -982,9 +978,7 @@ void nbd_client_close(BlockDriverState *bs)
NBDClientSession *client = nbd_get_client_session(bs);
NBDRequest request = { .type = NBD_CMD_DISC };
-if (client->ioc == NULL) {
-return;
-}
+assert(client->ioc);
nbd_send_request(client->ioc, &request);
--
2.18.0
[Qemu-devel] [PATCH v5 4/6] block/nbd-client: fix nbd_reply_chunk_iter_receive
Use exported report, not the variable to be reused (should not really
matter).
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/nbd-client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index a8ce0824c2..8e9b3dbedd 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -602,7 +602,7 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession
*s,
}
/* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
-if (nbd_reply_is_simple(&s->reply) || s->quit) {
+if (nbd_reply_is_simple(reply) || s->quit) {
goto break_loop;
}
--
2.18.0
Re: [Qemu-devel] [PATCH v11 for-4.0 06/11] qemu_thread: supplement error handling for emulated_realize
Fei Li writes:
> From: Fei Li
>
> Utilize the existed errp to propagate the error and do the
> corresponding cleanup to replace the temporary &error_abort.
>
> Cc: Markus Armbruster
> Cc: Gerd Hoffmann
> Signed-off-by: Fei Li
> ---
> hw/usb/ccid-card-emulated.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index 0b170f6328..19b4b9a8fa 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -544,11 +544,16 @@ static void emulated_realize(CCIDCardState *base, Error
> **errp)
> error_setg(errp, "%s: failed to initialize vcard",
> TYPE_EMULATED_CCID);
> goto out2;
> }
> -/* TODO: let the further caller handle the error instead of abort() here
> */
> -qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
> - card, QEMU_THREAD_JOINABLE, &error_abort);
> -qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
> handle_apdu_thread,
> - card, QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(&card->event_thread_id, "ccid/event",
> event_thread,
> + card, QEMU_THREAD_JOINABLE, errp) < 0) {
> +goto out2;
> +}
> +if (qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
> + handle_apdu_thread, card,
> + QEMU_THREAD_JOINABLE, errp) < 0) {
> +qemu_thread_join(&card->event_thread_id);
What makes event_thread terminate here?
I'm asking because if it doesn't, the join will hang.
> +goto out2;
> +}
>
> return;
[Qemu-devel] [PATCH v5 6/6] block/nbd-client: rename read_reply_co to connection_co
This coroutine will serve nbd reconnects, so, rename it to be something
more generic.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/nbd-client.h | 4 ++--
block/nbd-client.c | 24
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 2f047ba614..d990207a5c 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -20,7 +20,7 @@
typedef struct {
Coroutine *coroutine;
uint64_t offset;/* original offset of the request */
-bool receiving; /* waiting for read_reply_co? */
+bool receiving; /* waiting for connection_co? */
} NBDClientRequest;
typedef struct NBDClientSession {
@@ -30,7 +30,7 @@ typedef struct NBDClientSession {
CoMutex send_mutex;
CoQueue free_sema;
-Coroutine *read_reply_co;
+Coroutine *connection_co;
int in_flight;
NBDClientRequest requests[MAX_NBD_REQUESTS];
diff --git a/block/nbd-client.c b/block/nbd-client.c
index d8bf1bbb8b..d9e33751ad 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -59,7 +59,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
qio_channel_shutdown(client->ioc,
QIO_CHANNEL_SHUTDOWN_BOTH,
NULL);
-BDRV_POLL_WHILE(bs, client->read_reply_co);
+BDRV_POLL_WHILE(bs, client->connection_co);
nbd_client_detach_aio_context(bs);
object_unref(OBJECT(client->sioc));
@@ -68,7 +68,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
client->ioc = NULL;
}
-static coroutine_fn void nbd_read_reply_entry(void *opaque)
+static coroutine_fn void nbd_connection_entry(void *opaque)
{
NBDClientSession *s = opaque;
uint64_t i;
@@ -100,14 +100,14 @@ static coroutine_fn void nbd_read_reply_entry(void
*opaque)
}
/* We're woken up again by the request itself. Note that there
- * is no race between yielding and reentering read_reply_co. This
+ * is no race between yielding and reentering connection_co. This
* is because:
*
* - if the request runs on the same AioContext, it is only
* entered after we yield
*
* - if the request runs on a different AioContext, reentering
- * read_reply_co happens through a bottom half, which can only
+ * connection_co happens through a bottom half, which can only
* run after we yield.
*/
aio_co_wake(s->requests[i].coroutine);
@@ -116,7 +116,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
s->quit = true;
nbd_recv_coroutines_wake_all(s);
-s->read_reply_co = NULL;
+s->connection_co = NULL;
}
static int nbd_co_send_request(BlockDriverState *bs,
@@ -420,7 +420,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
}
*request_ret = 0;
-/* Wait until we're woken up by nbd_read_reply_entry. */
+/* Wait until we're woken up by nbd_connection_entry. */
s->requests[i].receiving = true;
qemu_coroutine_yield();
s->requests[i].receiving = false;
@@ -495,7 +495,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
}
/* nbd_co_receive_one_chunk
- * Read reply, wake up read_reply_co and set s->quit if needed.
+ * Read reply, wake up connection_co and set s->quit if needed.
* Return value is a fatal error code or normal nbd reply error code
*/
static coroutine_fn int nbd_co_receive_one_chunk(
@@ -509,15 +509,15 @@ static coroutine_fn int nbd_co_receive_one_chunk(
if (ret < 0) {
s->quit = true;
} else {
-/* For assert at loop start in nbd_read_reply_entry */
+/* For assert at loop start in nbd_connection_entry */
if (reply) {
*reply = s->reply;
}
s->reply.handle = 0;
}
-if (s->read_reply_co) {
-aio_co_wake(s->read_reply_co);
+if (s->connection_co) {
+aio_co_wake(s->connection_co);
}
return ret;
@@ -970,7 +970,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
{
NBDClientSession *client = nbd_get_client_session(bs);
qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), new_context);
-aio_co_schedule(new_context, client->read_reply_co);
+aio_co_schedule(new_context, client->connection_co);
}
void nbd_client_close(BlockDriverState *bs)
@@ -1077,7 +1077,7 @@ static int nbd_client_connect(BlockDriverState *bs,
/* Now that we're connected, set the socket to be non-blocking and
* kick the reply mechanism. */
qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry,
client);
+client->connection_co = qemu_coroutine_create(nbd_connection_entry,
client);
nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs));
logout("Established connection with NBD server\n");
--
2.18.0
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Fix msync to do what hardware does
On Sat, 26 Jan 2019, BALATON Zoltan wrote:
According to BookE docs, invalid bits (while undefined behaviour) should
not raise exception but be ignored. This seems to be implementation
dependent though and QEMU currently does what e500 CPUs do and raise
exception for invalid bits. Unfortunately some versions of libstdc++
(and so all programs compiled with it) have lwsync on PPC440 which is
invalid but on real hardware it's just executed as msync ignoring the
invalid bits (maybe that's why it got undetected) but they fail on QEMU.
This patch changes invalid mask of msync to allow these programs to run
but keep generating exception on e500 cores to follow what hardware does.
Signed-off-by: BALATON Zoltan
Ping? This seems to have been ignored so far.
---
target/ppc/translate.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index e169c43643..5429ceb1ab 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6476,7 +6476,12 @@ static void gen_mbar(DisasContext *ctx)
/* msync replaces sync on 440 */
static void gen_msync_4xx(DisasContext *ctx)
{
-/* interpreted as no-op */
+/* Only e500 seems to treat reserved bits as invalid */
+if ((ctx->insns_flags2 & PPC2_BOOKE206) &&
+(ctx->opcode & 0x03FFF801)) {
+gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+}
+/* otherwise interpreted as no-op */
}
/* icbt */
@@ -7054,11 +7059,11 @@ GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01,
PPC_WRTEE),
GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x, PPC_440_SPEC),
GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801,
PPC_BOOKE, PPC2_BOOKE206),
-GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE),
+GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x039FF801, PPC_BOOKE),
GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E1,
PPC_BOOKE, PPC2_BOOKE206),
GEN_HANDLER2(icbt_440, "icbt", 0x1F, 0x06, 0x08, 0x03E1,
- PPC_440_SPEC),
+ PPC_440_SPEC),
GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x0001, PPC_ALTIVEC),
GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x0001, PPC_ALTIVEC),
GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC),
Re: [Qemu-devel] [PULL 00/26] target-arm queue
Patchew URL: https://patchew.org/QEMU/20190128181047.20781-1-peter.mayd...@linaro.org/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=14 === TEST SCRIPT END === Configure options: --enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install --python=/usr/bin/python3 --cross-prefix=x86_64-w64-mingw32- --enable-trace-backends=simple --enable-gnutls --enable-nettle --enable-curl --enable-vnc --enable-bzip2 --enable-guest-agent --with-sdlabi=2.0 ERROR: "x86_64-w64-mingw32-gcc" either does not exist or does not work # QEMU configure log Fri Feb 1 12:51:18 UTC 2019 # Configured with: '/tmp/qemu-test/src/configure' '--enable-werror' '--target-list=x86_64-softmmu,aarch64-softmmu' '--prefix=/tmp/qemu-test/install' '--python=/usr/bin/python3' '--cross-prefix=x86_64-w64-mingw32-' '--enable-trace-backends=simple' '--enable-gnutls' '--enable-nettle' '--enable-curl' '--enable-vnc' '--enable-bzip2' '--enable-guest-agent' '--with-sdlabi=2.0' --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 636 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 638 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 640 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 642 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 644 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 646 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 648 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc compile_object check_define main lines: 92 122 619 650 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c ccache: error: Failed to create temporary file for /var/tmp/ccache/tmp/qemu-conf.stdout: Permission denied funcs: do_compiler do_cc co
Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable
On 01.02.19 13:45, Cornelia Huck wrote:
> We currently don't migrate any state for zpci devices, which are
> coupled with standard pci devices. This means funny things happen
> when we e.g. try to migrate with a virtio-pci device but the s390x-
> specific zpci state is not migrated (vfio-pci is not affected, as
> it is not migratable anyway.)
>
> Until this is fixed, mark zpci devices as unmigratable.
>
> Reported-by: David Hildenbrand
> Signed-off-by: Cornelia Huck
> ---
>
> This is just a stop-gap measure to give us time to implement the
> needed migration code properly.
>
> ---
> hw/s390x/s390-pci-bus.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index c96a7cba34..96c7c18f3f 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static const VMStateDescription s390_pci_device_vmstate = {
> +.name = TYPE_S390_PCI_DEVICE,
> +/*
> + * TODO: add state handling here, so migration works at least with
> + * emulated pci devices on s390x
> + */
> +.unmigratable = 1,
> +};
> +
> static void s390_pci_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass
> *klass, void *data)
> dc->bus_type = TYPE_S390_PCI_BUS;
> dc->realize = s390_pci_device_realize;
> dc->props = s390_pci_device_properties;
> +dc->vmsd = &s390_pci_device_vmstate;
> }
>
> static const TypeInfo s390_pci_device_info = {
>
I guess this should be good enough (e.g. pci-bridge without a zpci
device should migrate "itself" I assume).
Reviewed-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH] Fix for RSP vCont packet
Hi Lucien,
On 1/31/19 5:48 AM, Lucien Murray-Pitts wrote:
> The result is that vCont now does not recognise the case where no
> process/thread is provided after the action.
>
> This may not show up with GDB, but using Lauterbach Trace32, and Hexrays IDA
> Pro this issue is immediately seen.
> The response is a "$#00" empty packet, showing it is unsupported packet.
>
> This is defined in the RSP document as "An action with no thread-id matches
> all threads."
> (https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#vCont-packet )
>
> Thus the valid vCont packets now are as below, however parsing is still not
> very strict.
> vCont;c/s - Step/Continue all threads
> vCont;c/s:[pX.]Y - Step/Continue optional process X, thread Y
> vCont;C##/S##:[pX.]Y - Step/Continue with signal ## on optional
> process X, thread Y
> * If X or Y are -1 then it applies the action to all processes/threads.
>
> Signed-off-by: Lucien Murray-Pitts
> ---
> gdbstub.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index bfc7afb509..ce0dde2e24 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1169,6 +1169,7 @@ static int is_query_packet(const char *p, const char
> *query, char separator)
> */
> static int gdb_handle_vcont(GDBState *s, const char *p)
> {
> +GDBThreadIdKind vcontThreadType ;
The coding style for variable names is lower_case_with_underscores (see
CODING_STYLE). I think you can go with a simpler name like
GDBThreadIdKind kind;
> int res, signal = 0;
> char cur_action;
> char *newstates;
> @@ -1218,12 +1219,23 @@ static int gdb_handle_vcont(GDBState *s, const char
> *p)
> goto out;
> }
>
> -if (*p++ != ':') {
> +/*
> + * In the case we have vCont;c or vCont;s - action is on all threads
> + * Alternatively vCont;c;s:p1.1 is a possible, but meaningless
> format,
> + * And in the else the "vCont;c:p1.1;... format is supported.
> + */> +if (*p == '\0' || *p == ';') {
> +vcontThreadType = GDB_ALL_THREAD ;> +pid = 1 ;
The spec is not clear but I would opt for GDB_ALL_PROCESSES instead of
GDB_ALL_THREAD here. pid = 1; is clearly wrong since you don't know if
this PID exists or is currently attached.
> +tid = 1 ;
This one is not useful either (not used in the switch..case bellow).
Thanks
Luc
> +} else if (*p++ == ':') {
> +vcontThreadType = read_thread_id(p, &p, &pid, &tid) ;
> +} else {
> res = -ENOTSUP;
> goto out;
> }
>
> -switch (read_thread_id(p, &p, &pid, &tid)) {
> +switch (vcontThreadType) {
> case GDB_READ_THREAD_ERR:
> res = -EINVAL;
> goto out;
>
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] Crash when booting KDE Neon using qxl-vga
* Leonardo Soares Müller (leozinho29...@hotmail.com) wrote:
> libspice-server1 on host: 0.14.0-1ubuntu2.2
> spice-vdagent (the only package) on guest: 0.17.0-1ubuntu2
> Guest kernel version: 4.15.0-44-generic
Hmm, I'm also getting a crash, but I think it's very different from
yours:
./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -smp 3 -m 8G -cdrom
/home/vmimages/neon-useredition-current.iso -drive
if=virtio,file=/home/vmimages/kde-neon.qcow2 -vga qxl
kvm_mem_ioeventfd_add: error adding ioeventfd: No space left on device
Aborted (core dumped)
#2 0x55893ee6 in kvm_mem_ioeventfd_add (listener=,
section=0x7fffdbffb9d0, match_data=, data=0, e=)
at /home/dgilbert/git/qemu/accel/kvm/kvm-all.c:866
#3 0x5588123d in address_space_add_del_ioeventfds
(fds_old_nb=0, fds_old=0x0, fds_new_nb=1, fds_new=0x7fffd0944570,
as=0x563bbea0 ) at
/home/dgilbert/git/qemu/memory.c:793
#4 0x5588123d in address_space_update_ioeventfds
(as=as@entry=0x563bbea0 ) at
/home/dgilbert/git/qemu/memory.c:843
#5 0x558816c8 in memory_region_transaction_commit () at
/home/dgilbert/git/qemu/memory.c:1087
#6 0x558816c8 in memory_region_transaction_commit () at
/home/dgilbert/git/qemu/memory.c:1071
#7 0x55aab425 in pci_update_mappings (d=d@entry=0x57617ce0) at
/home/dgilbert/git/qemu/hw/pci/pci.c:1357
#8 0x55aaba89 in pci_default_write_config (d=d@entry=0x57617ce0,
addr=addr@entry=4, val_in=263, l=l@entry=2)
at /home/dgilbert/git/qemu/hw/pci/pci.c:1413
#9 0x55b216c1 in virtio_write_config (pci_dev=0x57617ce0,
address=4, val=, len=2)
at /home/dgilbert/git/qemu/hw/virtio/virtio-pci.c:598
#10 0x55ab26bf in pci_host_config_write_common (pci_dev=0x57617ce0,
addr=4, limit=, val=263, len=2)
at /home/dgilbert/git/qemu/hw/pci/pci_host.c:87
#11 0x5587f721 in memory_region_write_accessor
(mr=0x56a0bf80, addr=0, value=, size=2, shift=, mask=, attrs=...)
at /home/dgilbert/git/qemu/memory.c:502
#12 0x5587d276 in access_with_adjusted_size
(addr=addr@entry=0, value=value@entry=0x7fffdbffbce8, size=size@entry=2,
access_size_min=, access_size_max=,
access_fn=access_fn@entry=0x5587f6a0 ,
mr=0x56a0bf80, attrs=...) at /home/dgilbert/git/qemu/memory.c:568
#13 0x55881bfc in memory_region_dispatch_write
(mr=mr@entry=0x56a0bf80, addr=0, data=, size=2,
attrs=attrs@entry=...)
at /home/dgilbert/git/qemu/memory.c:1499
#14 0x55828923 in flatview_write_continue
(fv=fv@entry=0x7fffd0847c00, addr=addr@entry=3324, attrs=...,
buf=buf@entry=0x77fc5000 "\a\001", len=len@entry=2, addr1=,
l=, mr=0x56a0bf80) at /home/dgilbert/git/qemu/exec.c:3247
#15 0x55828b49 in flatview_write (fv=0x7fffd0847c00, addr=3324,
attrs=..., buf=0x77fc5000 "\a\001", len=2)
at /home/dgilbert/git/qemu/exec.c:3286
#16 0x5582cc7f in address_space_write (as=,
addr=addr@entry=3324, attrs=..., buf=, len=len@entry=2)
at /home/dgilbert/git/qemu/exec.c:3376
#17 0x5582cd0a in address_space_rw (as=,
addr=addr@entry=3324, attrs=...,
attrs@entry=..., buf=, len=len@entry=2,
is_write=is_write@entry=true) at /home/dgilbert/git/qemu/exec.c:3387
#18 0x55894e45 in kvm_handle_io (count=1, size=2, direction=, data=, attrs=..., port=3324)
at /home/dgilbert/git/qemu/accel/kvm/kvm-all.c:1775
#19 0x55894e45 in kvm_cpu_exec (cpu=cpu@entry=0x5670f6f0) at
/home/dgilbert/git/qemu/accel/kvm/kvm-all.c:2021
#20 0x5586a6be in qemu_kvm_cpu_thread_fn (arg=0x5670f6f0) at
/home/dgilbert/git/qemu/cpus.c:1281
#21 0x5586a6be in qemu_kvm_cpu_thread_fn (arg=arg@entry=0x5670f6f0)
at /home/dgilbert/git/qemu/cpus.c:1254
#22 0x55ca0e7a in qemu_thread_start (args=) at
/home/dgilbert/git/qemu/util/qemu-thread-posix.c:502
#23 0x7536258e in start_thread () at /lib64/libpthread.so.0
(gdb) p section->offset_within_address_space
$3 = 4273991680
(gdb) p/x section->offset_within_address_space
$4 = 0xfebff000
(gdb) p section
$5 = (MemoryRegionSection *) 0x7fffdbffb9d0
(gdb) p *section
$6 = {mr = 0x0, fv = 0x7fffd08fe680, offset_within_region = 0, size = 0,
offset_within_address_space = 4273991680, readonly = false, nonvolatile = false}
Dave
> >
> > OK, great; can can you confirm the version of the spice packages
> > on both the guest and host, and the kernel on the guest.
> >
> > Dave
> >
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> >
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] s390x/pci: mark zpci devices as unmigratable
On Fri, 1 Feb 2019 14:29:47 +0100
David Hildenbrand wrote:
> On 01.02.19 13:45, Cornelia Huck wrote:
> > We currently don't migrate any state for zpci devices, which are
> > coupled with standard pci devices. This means funny things happen
> > when we e.g. try to migrate with a virtio-pci device but the s390x-
> > specific zpci state is not migrated (vfio-pci is not affected, as
> > it is not migratable anyway.)
> >
> > Until this is fixed, mark zpci devices as unmigratable.
> >
> > Reported-by: David Hildenbrand
> > Signed-off-by: Cornelia Huck
> > ---
> >
> > This is just a stop-gap measure to give us time to implement the
> > needed migration code properly.
> >
> > ---
> > hw/s390x/s390-pci-bus.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > index c96a7cba34..96c7c18f3f 100644
> > --- a/hw/s390x/s390-pci-bus.c
> > +++ b/hw/s390x/s390-pci-bus.c
> > @@ -1253,6 +1253,15 @@ static Property s390_pci_device_properties[] = {
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > +static const VMStateDescription s390_pci_device_vmstate = {
> > +.name = TYPE_S390_PCI_DEVICE,
> > +/*
> > + * TODO: add state handling here, so migration works at least with
> > + * emulated pci devices on s390x
> > + */
> > +.unmigratable = 1,
> > +};
> > +
> > static void s390_pci_device_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -1263,6 +1272,7 @@ static void s390_pci_device_class_init(ObjectClass
> > *klass, void *data)
> > dc->bus_type = TYPE_S390_PCI_BUS;
> > dc->realize = s390_pci_device_realize;
> > dc->props = s390_pci_device_properties;
> > +dc->vmsd = &s390_pci_device_vmstate;
> > }
> >
> > static const TypeInfo s390_pci_device_info = {
> >
>
> I guess this should be good enough (e.g. pci-bridge without a zpci
> device should migrate "itself" I assume).
I don't think we need to do anything special for those.
We probably need to go through the whole zpci code and check where we
might miss some state handling. The original use case for that support
was vfio-pci, which probably explains why migration had been
neglected...
However, unmigratable zpci devices should be an effective stopper until
this has been fixed.
>
> Reviewed-by: David Hildenbrand
>
Thanks!
Re: [Qemu-devel] [PATCH 2/2] mmap-alloc: fix hugetlbfs misaligned length in ppc64
On Wed, Jan 30, 2019 at 09:36:05PM -0200, Murilo Opsfelder Araujo wrote:
> The commit 7197fb4058bcb68986bae2bb2c04d6370f3e7218 ("util/mmap-alloc:
> fix hugetlb support on ppc64") fixed Huge TLB mappings on ppc64.
>
> However, we still need to consider the underlying huge page size
> during munmap() because it requires that both address and length be a
> multiple of the underlying huge page size for Huge TLB mappings.
> Quote from "Huge page (Huge TLB) mappings" paragraph under NOTES
> section of the munmap(2) manual:
>
> "For munmap(), addr and length must both be a multiple of the
> underlying huge page size."
>
> On ppc64, the munmap() in qemu_ram_munmap() does not work for Huge TLB
> mappings because the mapped segment can be aligned with the underlying
> huge page size, not aligned with the native system page size, as
> returned by getpagesize().
>
> This has the side effect of not releasing huge pages back to the pool
> after a hugetlbfs file-backed memory device is hot-unplugged.
>
> This patch fixes the situation in qemu_ram_mmap() and
> qemu_ram_munmap() by considering the underlying page size on ppc64.
>
> After this patch, memory hot-unplug releases huge pages back to the
> pool.
>
> Fixes: 7197fb4058bcb68986bae2bb2c04d6370f3e7218
> Signed-off-by: Murilo Opsfelder Araujo
Reported-by: Balamuruhan S
Tested-by: Balamuruhan S
I tried to test the patch in POWER8,
Distro: Fedora
Host Kernel: 5.0.0-rc3-gdbaaa7d30 (upstream)
Guest Kernel: 5.0.0-rc3-gdbaaa7d30 (upstream)
Qemu: 3.1.50 (v3.1.0-1313-gbd08d7ec4d-dirty) (upstream)
+ mmap-alloc: unfold qemu_ram_mmap()
+ mmap-alloc: fix hugetlbfs misaligned length in ppc64
1. object_add and object_del works,
Allocated 256M from host with 16M Hugepage
# echo 16 > /proc/sys/vm/nr_hugepages
# cat /proc/meminfo | grep Huge
AnonHugePages:737280 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free: 16
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
(qemu) object_add memory-backend-file,id=mem1,size=256M,mem-path=/dev/hugepages
# cat /proc/meminfo | grep Huge
AnonHugePages:737280 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free: 16
HugePages_Rsvd: 16
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
(qemu) object_del mem1
# cat /proc/meminfo | grep Huge
AnonHugePages:671744 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free: 16
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
2. Perform complete hotplug-hotunplug works,
Hotplug memory:
(qemu) object_add memory-backend-file,id=mem1,size=256M,mem-path=/dev/hugepages
On Host:
# cat /proc/meminfo | grep Huge
AnonHugePages:737280 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free: 16
HugePages_Rsvd: 16
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
(qemu) device_add pc-dimm,id=dimm1,memdev=mem1
# cat /proc/meminfo | grep Huge
AnonHugePages:737280 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free:0
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
Inside guest:
[ 27.380848] pseries-hotplug-mem: Attempting to hot-add 1 LMB(s) at index
8010
[ 27.420225] lpar: Attempting to resize HPT to shift 23
[ 27.619202] lpar: Hash collision while resizing HPT
[ 27.620113] Unable to resize hash page table to target order 23: -28
[ 27.748767] pseries-hotplug-mem: Memory at 1 (drc index 8010)
was hot-added
Hotunplug memory:
(qemu) device_del dimm1
# cat /proc/meminfo | grep Huge
AnonHugePages:737280 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free:0
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
(qemu) object_del mem1
# cat /proc/meminfo | grep Huge
AnonHugePages:737280 kB
ShmemHugePages:0 kB
HugePages_Total: 16
HugePages_Free: 16
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize: 16384 kB
Hugetlb: 262144 kB
Inside guest:
[ 29.640670] pseries-hotplug-mem: Attempting to hot-remove 1 LMB(s) at
8010
[ 29.710276] Offlined Pages 4096
[ 29.733824] lpar: Attempting to resize HPT to shift 22
[ 29.880381] lpar: Hash collision while resizing HPT
[ 29.881521] Unable to resize hash page table to target order 22: -28
[ 29.900542] pseries-hotplug-mem: Memory at 1 (drc index 8010)
was hot-removed
Thanks Murilo,
-- Bala
> ---
> exec.c| 4 ++--
> include/qemu/mmap-alloc.h | 2 +-
> util/mmap-alloc.c | 22 --
> util/oslib-posix.c| 2 +-
> 4 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index da3e635f91..0db6d8bf34 100644
> --- a/ex
Re: [Qemu-devel] [PATCH 1/2] mmap-alloc: unfold qemu_ram_mmap()
On Wed, Jan 30, 2019 at 09:36:04PM -0200, Murilo Opsfelder Araujo wrote:
> Unfold parts of qemu_ram_mmap() for the sake of understanding, moving
> declarations to the top, and keeping architecture-specifics in the
> ifdef-else blocks. No changes in the function behaviour.
>
> Give ptr and ptr1 meaningful names:
> ptr -> guardptr : pointer to the PROT_NONE guard region
> ptr1 -> ptr : pointer to the mapped memory returned to caller
>
> Signed-off-by: Murilo Opsfelder Araujo
Reported-by: Balamuruhan S
Tested-by: Balamuruhan S
> ---
> util/mmap-alloc.c | 53 ++-
> 1 file changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index fd329eccd8..f71ea038c8 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -77,11 +77,19 @@ size_t qemu_mempath_getpagesize(const char *mem_path)
>
> void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared)
> {
> +int flags;
> +int guardfd;
> +size_t offset;
> +size_t total;
> +void *guardptr;
> +void *ptr;
> +
> /*
> * Note: this always allocates at least one extra page of virtual address
> * space, even if size is already aligned.
> */
> -size_t total = size + align;
> +total = size + align;
> +
> #if defined(__powerpc64__) && defined(__linux__)
> /* On ppc64 mappings in the same segment (aka slice) must share the same
> * page size. Since we will be re-allocating part of this segment
> @@ -91,16 +99,22 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
> bool shared)
> * We do this unless we are using the system page size, in which case
> * anonymous memory is OK.
> */
> -int anonfd = fd == -1 || qemu_fd_getpagesize(fd) == getpagesize() ? -1 :
> fd;
> -int flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;
> -void *ptr = mmap(0, total, PROT_NONE, flags | MAP_PRIVATE, anonfd, 0);
> +flags = MAP_PRIVATE;
> +if (fd == -1 || qemu_fd_getpagesize(fd) == getpagesize()) {
> +guardfd = -1;
> +flags |= MAP_ANONYMOUS;
> +} else {
> +guardfd = fd;
> +flags |= MAP_NORESERVE;
> +}
> #else
> -void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1,
> 0);
> +guardfd = -1;
> +flags = MAP_PRIVATE | MAP_ANONYMOUS;
> #endif
> -size_t offset;
> -void *ptr1;
>
> -if (ptr == MAP_FAILED) {
> +guardptr = mmap(0, total, PROT_NONE, flags, guardfd, 0);
> +
> +if (guardptr == MAP_FAILED) {
> return MAP_FAILED;
> }
>
> @@ -108,19 +122,20 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
> bool shared)
> /* Always align to host page size */
> assert(align >= getpagesize());
>
> -offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
> -ptr1 = mmap(ptr + offset, size, PROT_READ | PROT_WRITE,
> -MAP_FIXED |
> -(fd == -1 ? MAP_ANONYMOUS : 0) |
> -(shared ? MAP_SHARED : MAP_PRIVATE),
> -fd, 0);
> -if (ptr1 == MAP_FAILED) {
> -munmap(ptr, total);
> +flags = MAP_FIXED;
> +flags |= fd == -1 ? MAP_ANONYMOUS : 0;
> +flags |= shared ? MAP_SHARED : MAP_PRIVATE;
> +offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - (uintptr_t)guardptr;
> +
> +ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE, flags, fd,
> 0);
> +
> +if (ptr == MAP_FAILED) {
> +munmap(guardptr, total);
> return MAP_FAILED;
> }
>
> if (offset > 0) {
> -munmap(ptr, offset);
> +munmap(guardptr, offset);
> }
>
> /*
> @@ -129,10 +144,10 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
> bool shared)
> */
> total -= offset;
> if (total > size + getpagesize()) {
> -munmap(ptr1 + size + getpagesize(), total - size - getpagesize());
> +munmap(ptr + size + getpagesize(), total - size - getpagesize());
> }
>
> -return ptr1;
> +return ptr;
> }
>
> void qemu_ram_munmap(void *ptr, size_t size)
> --
> 2.20.1
>
>
Re: [Qemu-devel] [PATCH v11 for-4.0 07/11] qemu_thread: supplement error handling for iothread_complete
Fei Li writes: > From: Fei Li > > For iothread_complete: utilize the existed errp to propagate the "For iothread_complete" is redundant, isn't it? > error and do the corresponding cleanup to replace the temporary > &error_abort. > > Cc: Markus Armbruster > Cc: Stefan Hajnoczi > Cc: Eric Blake > Signed-off-by: Fei Li Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH RFCv2 7/9] hmp: Handle virtio-pmem when printing memory device infos
"Dr. David Alan Gilbert" writes:
> * Markus Armbruster (arm...@redhat.com) wrote:
>> "Dr. David Alan Gilbert" writes:
>>
>> > * David Hildenbrand (da...@redhat.com) wrote:
>> >> Print the memory device info just like for PCDIMM/NVDIMM.
>> >>
>> >> Signed-off-by: David Hildenbrand
>> >> ---
>> >> hmp.c | 27 +++
>> >> 1 file changed, 15 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/hmp.c b/hmp.c
>> >> index 8da5fd8760..25c32e0810 100644
>> >> --- a/hmp.c
>> >> +++ b/hmp.c
>> >> @@ -2553,6 +2553,7 @@ void hmp_info_memory_devices(Monitor *mon, const
>> >> QDict *qdict)
>> >> Error *err = NULL;
>> >> MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err);
>> >> MemoryDeviceInfoList *info;
>> >> +VirtioPMEMDeviceInfo *vpi;
>> >> MemoryDeviceInfo *value;
>> >> PCDIMMDeviceInfo *di;
>> >>
>> >> @@ -2562,19 +2563,9 @@ void hmp_info_memory_devices(Monitor *mon, const
>> >> QDict *qdict)
>> >> if (value) {
>> >> switch (value->type) {
>> >> case MEMORY_DEVICE_INFO_KIND_DIMM:
>> >> -di = value->u.dimm.data;
>> >> -break;
>> >> -
>> >> case MEMORY_DEVICE_INFO_KIND_NVDIMM:
>> >> -di = value->u.nvdimm.data;
>> >> -break;
>> >> -
>> >> -default:
>> >> -di = NULL;
>> >> -break;
>> >> -}
>> >> -
>> >> -if (di) {
>> >> +di = value->type == MEMORY_DEVICE_INFO_KIND_DIMM ?
>> >> + value->u.dimm.data : value->u.nvdimm.data;
>> >> monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
>> >> MemoryDeviceInfoKind_str(value->type),
>> >> di->id ? di->id : "");
>> >> @@ -2587,6 +2578,18 @@ void hmp_info_memory_devices(Monitor *mon, const
>> >> QDict *qdict)
>> >> di->hotplugged ? "true" : "false");
>> >> monitor_printf(mon, " hotpluggable: %s\n",
>> >> di->hotpluggable ? "true" : "false");
>> >> +break;
>> >> +case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM:
>> >> +vpi = value->u.virtio_pmem.data;
>> >> +monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
>> >> + MemoryDeviceInfoKind_str(value->type),
>> >> + vpi->id ? vpi->id : "");
>> >> +monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n",
>> >> vpi->memaddr);
>> >> +monitor_printf(mon, " size: %" PRIu64 "\n", vpi->size);
>> >> +monitor_printf(mon, " memdev: %s\n", vpi->memdev);
>> >> +break;
>> >> +default:
>> >> +g_assert_not_reached();
>> >
>> >
>> > Although I'd prefer if that assert was replaced by a print
>> > saying it was an unknown type.
>>
>> I would not. If we reach this, something must have scribbled over
>> value->type and who knows what else. Continuing is unsafe. Looks like
>> a textbook use of assertions to me.
>
> Or it could be that someone added a new type of memory device and forgot
> to update the hmp code.
Programming error -> assert. Nothing catches attention in testing like
an assertion failure.
Re: [Qemu-devel] [PATCH v11 for-4.0 08/11] qemu_thread: supplement error handling for qemu_signalfd_compat
Fei Li writes: > From: Fei Li > > For qemu_signalfd_compat: set errno, do some cleanup, and return "For iothread_complete" is redundant, isn't it? > -1 to replace the temporary &error_abort when failing to create > sigwait_compat. > > Cc: Markus Armbruster > Cc: Eric Blake > Signed-off-by: Fei Li Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH] qtest.py: Wait for the result of qtest commands
On 31.01.19 13:38, Alberto Garcia wrote:
> The cmd() method of the QEMUQtestProtocol class sends a qtest command
> to QEMU but doesn't wait for the return message ("OK", "FAIL", "ERR").
> Because of this, it can return control to the caller before the
> command has actually finished.
>
> In cases like clock_step or clock_set this means that cmd() can return
> before all the timers triggered by the clock change have been fired.
> This can be fixed by making cmd() wait for the output of the qtest
> command.
>
> This fixes iotests 093 and 136, which are flaky since commit
> 8258292e18c39480b64eba9f3551 when the machine is under heavy workload.
>
> Signed-off-by: Alberto Garcia
> ---
> scripts/qtest.py | 6 ++
> 1 file changed, 6 insertions(+)
Thanks a lot!
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v11 for-4.0 10/11] qemu_thread: supplement error handling for vnc_start_worker_thread
Fei Li writes: > From: Fei Li > > Supplement the error handling for vnc_thread_worker_thread: add > an Error parameter for it to propagate the error to its caller to > handle in case it fails, and make it return a Boolean to indicate > whether it succeeds. > > Cc: Markus Armbruster > Cc: Gerd Hoffmann > Signed-off-by: Fei Li Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [Qemu-block] [PATCH] iotests: Filter second BLOCK_JOB_ERROR from 229
On 31.01.19 03:21, John Snow wrote:
>
>
> On 1/30/19 6:52 PM, Max Reitz wrote:
>> Without this filter, this test sometimes fails.
>>
>> Signed-off-by: Max Reitz
>> ---
>> I intended to send this as part of my iotest fixes series, but it ended
>> up on the wrong branch... Doesn't really matter, though, as there is no
>> functional dependency.
>> ---
>> tests/qemu-iotests/229 | 6 +-
>> tests/qemu-iotests/229.out | 1 -
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229
>> index 893d098ad2..b0d4885fa6 100755
>> --- a/tests/qemu-iotests/229
>> +++ b/tests/qemu-iotests/229
>> @@ -81,11 +81,15 @@ echo
>> echo '=== Force cancel job paused in error state ==='
>> echo
>>
>> +# Filter out BLOCK_JOB_ERROR events because they may or may not occur.
>> +# Cancelling the job means resuming it for a bit before it is actually
>> +# aborted, and in that time it may or may not re-encounter the error.
>
> Oh, because the job is "paused" and cancelling it involves job_enter,
> which we then allow the job to gracefully fail through it's own pathways
> -- but depending on where it failed originally, it may-or-may-not wind
> up trying something else that fails before it finds the "exit
> gracefully" signal, is that right?
That's at least how I explained it to me, yes.
> I guess there's no real way to adjust that behavior.
>
>> success_or_failure="y" _send_qemu_cmd $QEMU_HANDLE \
>> "{'execute': 'block-job-cancel',
>> 'arguments': { 'device': 'testdisk',
>> 'force': true}}" \
>> - "BLOCK_JOB_CANCELLED" "Assertion"
>> + "BLOCK_JOB_CANCELLED" "Assertion" \
>> +| grep -v '"BLOCK_JOB_ERROR"'
>>
>> # success, all done
>> echo "*** done"
>> diff --git a/tests/qemu-iotests/229.out b/tests/qemu-iotests/229.out
>> index 4c4112805f..a3eb33788a 100644
>> --- a/tests/qemu-iotests/229.out
>> +++ b/tests/qemu-iotests/229.out
>> @@ -17,7 +17,6 @@ wrote 2097152/2097152 bytes at offset 0
>>
>> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
>> {"return": {}}
>> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write",
>> "action": "stop"}}
>> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}
>> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152,
>> "offset": 1048576, "speed": 0, "type": "mirror"}}
>> *** done
>>
>
> I think this is fine, if we cannot help to make this any more
> deterministic, so I'm fine with:
>
> Reviewed-by: John Snow
>
> but I am curious to know if this poses any theoretical problems for
> libvirt having to deal with possibly an extra hiccup before the cancel
> registers.
Hm... I would say the bigger issue is the error event appearing than
that not being the case. Because it appears most of the time, I think
libvirt can deal with it.
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] Fix for RSP vCont packet
Hi Luc, Great feedback from both you and Eric - im blown away, I dont thinktheres a single 1byte ascii character left unturned there :)My coding style snuck in erywhere, as did my Scottish English! On Friday, February 1, 2019, 10:33:18 PM GMT+9, Luc Michel wrote: > > + GDBThreadIdKind vcontThreadType ; > The coding style ... CODING_STYLE ... Ah I didnt pick up on this whilst reading the Wiki for coding style, thank you! > > + vcontThreadType = GDB_ALL_THREAD ;> + pid = 1 ; > The spec is not clear but I would opt for GDB_ALL_PROCESSES instead of > GDB_ALL_THREAD here. pid = 1; is clearly wrong since you don't know if > this PID exists or is currently attached. I was in a quandary here, and I did see your previous email too.However, I looked at how the call to read_thread_id() behaved and copied over the same behavior as if the command was vCont;c:-1which I understand to mean continue all threads? So maybe we should look at altering read_thread_id() which sets pid=1 when thereis no "ppid." part to the thread-id description, and if you set "tid=-1" then it performs a GDB_ALL_THREAD >From the vCont spec I also got;"p-1 ... means all processes""An action with no >thread-id matches all threads." "tid=-1 ... matches all threads" But it is not very clear what happens if you omit ppid and supply tid as -1. What do you think? Cheers, Luc
Re: [Qemu-devel] sparc: crash when using initrd > 5M
On 18/01/2019 13:33, Mark Cave-Ayland wrote:
> On 03/01/2019 15:48, Corentin Labbe wrote:
>
>> Hello
>>
>> When using an initrd > 5M, I hit the following kernel crash:
>> qemu-system-sparc -kernel vmlinux -initrd rootfs.cpio.gz -nographic
>> Configuration device id QEMU version 1 machine id 32
>> Probing SBus slot 0 offset 0
>> Probing SBus slot 1 offset 0
>> Probing SBus slot 2 offset 0
>> Probing SBus slot 3 offset 0
>> Probing SBus slot 4 offset 0
>> Probing SBus slot 5 offset 0
>> Invalid FCode start byte
>> CPUs: 1 x FMI,MB86904
>> UUID: ----
>> Welcome to OpenBIOS v1.1 built on Oct 5 2018 08:20
>> Type 'help' for detailed information
>> [sparc] Kernel already loaded
>> switching to new context:
>> PROMLIB: obio_ranges 1
>> [0.00] PROMLIB: Sun Boot Prom Version 3 Revision 2
>> [0.00] Linux version 4.20.0-next-20190102+ (compile@Red) (gcc
>> version 7.3.0 (Gentoo 7.3.0-r3 p1.4)) #148 Thu Jan 3 16:17:08 CET 2019
>> [0.00] printk: bootconsole [earlyprom0] enabled
>> [0.00] ARCH: SUN4M
>> [0.00] TYPE: SPARCstation 5
>> [0.00] Ethernet address: 52:54:00:12:34:56
>> [0.00] Unable to handle kernel NULL pointer dereference
>> [0.00] tsk->{mm,active_mm}->context =
>> [0.00] tsk->{mm,active_mm}->pgd =
>> [0.00] \|/ \|/
>> [0.00] "@'/ ,. \`@"
>> [0.00] /_| \__/ |_\
>> [0.00] \__U_/
>> [0.00] swapper(0): Oops [#1]
>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.20.0-next-20190102+
>> #148
>> [0.00] PSR: 04001fc0 PC: f0010ef0 NPC: f0010ef4 Y: Not
>> tainted
>> [0.00] PC:
>> [0.00] %G: 000a 03c4 f05ece08 f05ecc00 00e0
>> f05d4000 0001
>> [0.00] %O: 00e0 0080 00e0 0002
>> f05d5bb8 f00bba58
>> [0.00] RPC:
>> [0.00] %L: 0040 f05dfaf8 f05d5c68 0001 0003 006951e0
>> f05ed014 f0674ab4
>> [0.00] %I: f05d5c80 0002 f100
>> f05d5c20 f0007fd8
>> [0.00] Disabling lock debugging due to kernel taint
>> [0.00] Caller[f0007fd8]: srmmu_fault+0x58/0x68
>> [0.00] Caller[f0618598]: memblock_alloc_try_nid+0xb8/0xc8
>> [0.00] Caller[f0611094]: srmmu_paging_init+0x174/0xaf8
>> [0.00] Caller[f06106a8]: paging_init+0x4/0x24
>> [0.00] Caller[f060e4f0]: setup_arch+0x3e8/0x480
>> [0.00] Caller[f060ab50]: start_kernel+0x48/0x460
>> [0.00] Caller[f060a43c]: continue_boot+0x324/0x334
>> [0.00] Caller[]: (null)
>> [0.00] Instruction DUMP:
>> [0.00] c800a024
>> [0.00] 83286002
>> [0.00] 073c17b3
>> [0.00]
>> [0.00] c600e22c
>> [0.00] 8a08a003
>> [0.00] 80a16001
>> [0.00] 0280003b
>> [0.00] c600c001
>> [0.00]
>> [0.00] Kernel panic - not syncing: Attempted to kill the idle task!
>> [0.00] Press Stop-A (L1-A) from sun keyboard or send break
>> [0.00] twice on console to return to the boot prom
>> [0.00] ---[ end Kernel panic - not syncing: Attempted to kill the
>> idle task! ]---
>> qemu-system-sparc: terminating on signal 15 from pid 13043 (killall)
>>
>> The NULL ptr dereference is done by memset() in srmmu_nocache_init() and
>> memblock_alloc_try_nid().
>> If I comment both memset, the boot pass
>>
>> But since nothing explain the NULL ptr deref in memset(), I suspect
>> something is overriden by the initrd
>
> Sorry about the delay in replying to this, I haven't been too well recently.
>
> Looking at the code I suspect the problem is that when loading a kernel
> directly,
> OpenBIOS isn't adding the kernel/initrd memory ranges to the DT properties,
> and so
> the kernel doesn't recreate its own mapping on boot.
>
> It shouldn't be too hard to make this happen, let me take and look and see how
> difficult this would be.
I think I now have a fix for this, with changes needed in both QEMU and
OpenBIOS.
Firstly you'll need to apply the QEMU patch from
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06635.html and then
you'll
need an updated OpenBIOS.
I've uploaded a pre-compiled openbios-sparc32 with the patches from
https://mail.coreboot.org/hyperkitty/list/openb...@openbios.org/thread/E6IMJNUFRF7W6ALWSYBOOCEYLBFXXQEN/
to https://www.ilande.co.uk/tmp/qemu/openbios-sparc32-initrdfix for testing.
Please can you test and let me know if this solves the issue? If so, I'll see
if I
can get them merged in time for the upcoming QEMU 4.0 release.
ATB,
Mark.
Re: [Qemu-devel] [PATCH v11 1/3] bdrv_query_image_info Error parameter added
On Fri, Feb 01, 2019 at 02:42:10PM +, Vladimir Sementsov-Ogievskiy wrote:
> 31.01.2019 16:46, Andrey Shinkevich wrote:
> > Inform a user in case qcow2_get_specific_info fails to obtain
> > QCOW2 image specific information. This patch is preliminary to
> > the print of bitmap information in the 'qemu-img info' output.
> >
> > Signed-off-by: Andrey Shinkevich
> > Reviewed-by: Eric Blake
> > ---
>
>
> [...]
>
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -594,13 +594,13 @@ static int
> > block_crypto_get_info_luks(BlockDriverState *bs,
> > }
> >
> > static ImageInfoSpecific *
> > -block_crypto_get_specific_info_luks(BlockDriverState *bs)
> > +block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
> > {
> > BlockCrypto *crypto = bs->opaque;
> > ImageInfoSpecific *spec_info;
> > QCryptoBlockInfo *info;
> >
> > -info = qcrypto_block_get_info(crypto->block, NULL);
> > +info = qcrypto_block_get_info(crypto->block, errp);
> > if (!info) {
> > return NULL;
> > }
>
> more context:
>
>if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
>qapi_free_QCryptoBlockInfo(info);
>return NULL;
>}
>
> for a fast look, I think it should be assertion, not if, Daniel, am I right?
Sure, it could be an assertion. In practice it should never trigger
eitherway.
> Also, I think we don't have block/crypto.c in Cryptography section of
> MAINTAINERS
> by mistake, so you were not CC'ed.
I tend to let the block maintainers merge patches under block/,
since that's largely block layer integration. The guts of the
block crypto stuff is under crypto/
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v11 1/3] bdrv_query_image_info Error parameter added
31.01.2019 16:46, Andrey Shinkevich wrote:
> Inform a user in case qcow2_get_specific_info fails to obtain
> QCOW2 image specific information. This patch is preliminary to
> the print of bitmap information in the 'qemu-img info' output.
>
> Signed-off-by: Andrey Shinkevich
> Reviewed-by: Eric Blake
> ---
[...]
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -594,13 +594,13 @@ static int block_crypto_get_info_luks(BlockDriverState
> *bs,
> }
>
> static ImageInfoSpecific *
> -block_crypto_get_specific_info_luks(BlockDriverState *bs)
> +block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
> {
> BlockCrypto *crypto = bs->opaque;
> ImageInfoSpecific *spec_info;
> QCryptoBlockInfo *info;
>
> -info = qcrypto_block_get_info(crypto->block, NULL);
> +info = qcrypto_block_get_info(crypto->block, errp);
> if (!info) {
> return NULL;
> }
more context:
if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
qapi_free_QCryptoBlockInfo(info);
return NULL;
}
for a fast look, I think it should be assertion, not if, Daniel, am I right?
Also, I think we don't have block/crypto.c in Cryptography section of
MAINTAINERS
by mistake, so you were not CC'ed.
> diff --git a/block/qapi.c b/block/qapi.c
> index c66f949..f53f100 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -282,7 +282,12 @@ void bdrv_query_image_info(BlockDriverState *bs,
> info->dirty_flag = bdi.is_dirty;
> info->has_dirty_flag = true;
> }
> -info->format_specific = bdrv_get_specific_info(bs);
> +info->format_specific = bdrv_get_specific_info(bs, &err);
while being here, let's drop these extra whitespaces
> +if (err) {
> +error_propagate(errp, err);
> +qapi_free_ImageInfo(info);
> +goto out;
> +}
> info->has_format_specific = info->format_specific != NULL;
>
> backing_filename = bs->backing_file;
So, I'm unsure about crypto, but it's safe to keep it as is, so for this patch:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
[Qemu-devel] [PATCH 01/11] docs/cpu-hotplug.rst: Fix rST markup issues
sphinx-build complains:
docs/cpu-hotplug.rst:67: ERROR: Unexpected indentation.
docs/cpu-hotplug.rst:69: ERROR: Unexpected indentation.
docs/cpu-hotplug.rst:74: WARNING: Block quote ends without a blank line;
unexpected unindent.
docs/cpu-hotplug.rst:75: WARNING: Block quote ends without a blank line;
unexpected unindent.
docs/cpu-hotplug.rst:76: SEVERE: Unexpected section title.
}
{
docs/cpu-hotplug.rst:78: WARNING: Block quote ends without a blank line;
unexpected unindent.
These are the result of not indicating one of the literal
blocks by finishing the preceding paragraph with the "::" marker.
Signed-off-by: Peter Maydell
---
docs/cpu-hotplug.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/cpu-hotplug.rst b/docs/cpu-hotplug.rst
index 1c268e00b41..e2d4e893b01 100644
--- a/docs/cpu-hotplug.rst
+++ b/docs/cpu-hotplug.rst
@@ -60,7 +60,7 @@ vCPU hotplug
hot-plugged (no "qom-path" member). From its output in step (3), we
can see that ``IvyBridge-IBRS-x86_64-cpu`` is present in socket 0,
while hot-plugging a CPU into socket 1 requires passing the listed
-properties to QMP ``device_add``:
+properties to QMP ``device_add``::
(QEMU) device_add id=cpu-2 driver=IvyBridge-IBRS-x86_64-cpu socket-id=1
core-id=0 thread-id=0
{
--
2.20.1
[Qemu-devel] [PATCH 02/11] docs: Convert memory.txt to rst format
Convert the memory API documentation from plain text
to restructured text format.
This is a very minimal conversion: all I had to change
was to mark up the ASCII art parts as Sphinx expects
for 'literal blocks', and fix up the bulleted lists
(Sphinx expects no leading space before the bullet, and
wants a blank line before after any list).
Signed-off-by: Peter Maydell
---
docs/devel/{memory.txt => memory.rst} | 128 ++
1 file changed, 70 insertions(+), 58 deletions(-)
rename docs/devel/{memory.txt => memory.rst} (85%)
diff --git a/docs/devel/memory.txt b/docs/devel/memory.rst
similarity index 85%
rename from docs/devel/memory.txt
rename to docs/devel/memory.rst
index 42577e1d860..b6a4c37ea5e 100644
--- a/docs/devel/memory.txt
+++ b/docs/devel/memory.rst
@@ -1,19 +1,20 @@
+==
The memory API
==
The memory API models the memory and I/O buses and controllers of a QEMU
machine. It attempts to allow modelling of:
- - ordinary RAM
- - memory-mapped I/O (MMIO)
- - memory controllers that can dynamically reroute physical memory regions
- to different destinations
+- ordinary RAM
+- memory-mapped I/O (MMIO)
+- memory controllers that can dynamically reroute physical memory regions
+ to different destinations
The memory model provides support for
- - tracking RAM changes by the guest
- - setting up coalesced memory for kvm
- - setting up ioeventfd regions for kvm
+- tracking RAM changes by the guest
+- setting up coalesced memory for kvm
+- setting up ioeventfd regions for kvm
Memory is modelled as an acyclic graph of MemoryRegion objects. Sinks
(leaves) are RAM and MMIO regions, while other nodes represent
@@ -98,25 +99,30 @@ ROM device memory region types), this host memory needs to
be
copied to the destination on migration. These APIs which allocate
the host memory for you will also register the memory so it is
migrated:
- - memory_region_init_ram()
- - memory_region_init_rom()
- - memory_region_init_rom_device()
+
+- memory_region_init_ram()
+- memory_region_init_rom()
+- memory_region_init_rom_device()
For most devices and boards this is the correct thing. If you
have a special case where you need to manage the migration of
the backing memory yourself, you can call the functions:
- - memory_region_init_ram_nomigrate()
- - memory_region_init_rom_nomigrate()
- - memory_region_init_rom_device_nomigrate()
+
+- memory_region_init_ram_nomigrate()
+- memory_region_init_rom_nomigrate()
+- memory_region_init_rom_device_nomigrate()
+
which only initialize the MemoryRegion and leave handling
migration to the caller.
The functions:
- - memory_region_init_resizeable_ram()
- - memory_region_init_ram_from_file()
- - memory_region_init_ram_from_fd()
- - memory_region_init_ram_ptr()
- - memory_region_init_ram_device_ptr()
+
+- memory_region_init_resizeable_ram()
+- memory_region_init_ram_from_file()
+- memory_region_init_ram_from_fd()
+- memory_region_init_ram_ptr()
+- memory_region_init_ram_device_ptr()
+
are for special cases only, and so they do not automatically
register the backing memory for migration; the caller must
manage migration if necessary.
@@ -218,7 +224,7 @@ For example, suppose we have a container A of size 0x8000
with two subregions
B and C. B is a container mapped at 0x2000, size 0x4000, priority 2; C is
an MMIO region mapped at 0x0, size 0x6000, priority 1. B currently has two
of its own subregions: D of size 0x1000 at offset 0 and E of size 0x1000 at
-offset 0x2000. As a diagram:
+offset 0x2000. As a diagram::
0 1000 2000 3000 4000 5000 6000 7000 8000
|--|--|--|--|--|--|--|--|
@@ -228,8 +234,9 @@ offset 0x2000. As a diagram:
D: [D]
E:[E]
-The regions that will be seen within this address range then are:
-[][D][C][E][C]
+The regions that will be seen within this address range then are::
+
+ [][D][C][E][C]
Since B has higher priority than C, its subregions appear in the flat map
even where they overlap with C. In ranges where B has not mapped anything
@@ -237,8 +244,9 @@ C's region appears.
If B had provided its own MMIO operations (ie it was not a pure container)
then these would be used for any addresses in its range not handled by
-D or E, and the result would be:
-[][D][B][E][B]
+D or E, and the result would be::
+
+ [][D][B][E][B]
Priority values are local to a container, because the priorities of two
regions are only compared when they are both children of the same container.
@@ -257,6 +265,7 @@ guest accesses an address:
- all direct subregions of the root region are matched against the address, in
descending priority order
+
- if the address lies outside the region offset/size, the subregion is
discarded
- if the subre
[Qemu-devel] [PATCH 00/11] Enable build and install of our rST docs
This patchset enables building and installing the various rST
docs we have started to accumulate in our docs/ directory.
It does this using Sphinx (which is the docs tooling that the
Linux kernel uses). The series is not trying to take us in one
giant leap to a brave new Sphinx-powered world -- it is simply
setting up a framework so that we are at least building and
shipping these documents and can gradually migrate other parts
of our documentation to it.
The approach I've used here is that we will have multiple "manuals",
as proposed by Paolo here:
https://wiki.qemu.org/Features/Documentation
For the moment I've only created 'interop' and 'devel' as we don't
yet have any rST files for 'user', 'system' or 'specs'.
One slightly awkward mismatch between how Sphinx naturally wants
to work and our requirements is that when Sphinx generates a
documentation set all in one go it creates hyperlinks between
all the docs, they all appear in a single top level table of
contents, and so on. But for QEMU's docs we don't want to
ship the "devel" manual to end-users. I've taken an approach
suggested to me on sphinx-users
(https://www.mail-archive.com/sphinx-users@googlegroups.com/msg03224.html)
where we run Sphinx once per manual, and treat them as
entirely separate documents. The config/tooling in this patchset
also supports building everything in a single run, for compatibility
with third-party docs sites like readthedocs.org.
To see the results:
What you get in the docs/ subdir of your build directory
when you do a local build:
http://people.linaro.org/~peter.maydell/build-dir-docs/
(follow the links to 'devel' and 'interop' for the two manuals)
What we'll ship in 'make install' in /usr/local/share/doc/qemu/
http://people.linaro.org/~peter.maydell/installed-docs/
(should be same as the build dir except we don't ship 'devel')
What you get with a standalone single-run docs build:
http://people.linaro.org/~peter.maydell/standalone-docs/index.html
These use the default 'alabaster' theme from Sphinx. I
also experimented with the 'read_the_docs' theme, which I
do think looks nicer. Unfortunately it also requires the
docs we install to include about 3MB of TrueType font files
per manual, which is awkward licensing-wise as the TTFs are
under the Open Font License and it's not completely clear to
me that it's OK to ship those to use with a doc file that is
GPLed. Alabaster doesn't ship fonts, which sidesteps both
those problems.
Other notes:
* this does not build the two .rst files that are directly
in docs/ (cpu-hotplug.rst and pr-manager.rst) -- we should
move these to whichever of the five manuals is the best place
* I do have some prototype patches which integrate the kernel's
kerneldoc Sphinx extension to parse doc comments in source
code. I haven't included them here because I think the 'devel'
manual is the lowest priority of the five. They might be
useful if we want to try things like building documentation
of supported machine models from in-code comments/etc, though.
* as noted in a previous email thread, the configure changes
now mean that building docs depends on build-sphinx being
available, so this is a new build-dep for --enable-docs.
thanks
-- PMM
Peter Maydell (11):
docs/cpu-hotplug.rst: Fix rST markup issues
docs: Convert memory.txt to rst format
docs: Commit initial files from sphinx-quickstart
docs/conf.py: Disable unused _static directory
docs/conf.py: Configure the 'alabaster' theme
docs/conf.py: Don't include rST sources in HTML build
docs/conf.py: Disable option warnings
Separate conf.py for each manual we want
Makefile, configure: Support building rST documentation
Makefile: Abstract out "identify the pkgversion" code
docs/conf.py: Don't hard-code QEMU version
configure | 4 +-
Makefile | 78 +++---
docs/conf.py | 215 ++
docs/cpu-hotplug.rst | 2 +-
docs/devel/conf.py| 15 ++
docs/devel/index.rst | 21 +++
docs/devel/{memory.txt => memory.rst} | 128 ---
docs/index.rst| 15 ++
docs/interop/conf.py | 15 ++
docs/interop/index.rst| 18 +++
10 files changed, 430 insertions(+), 81 deletions(-)
create mode 100644 docs/conf.py
create mode 100644 docs/devel/conf.py
create mode 100644 docs/devel/index.rst
rename docs/devel/{memory.txt => memory.rst} (85%)
create mode 100644 docs/index.rst
create mode 100644 docs/interop/conf.py
create mode 100644 docs/interop/index.rst
--
2.20.1
[Qemu-devel] [PATCH 10/11] Makefile: Abstract out "identify the pkgversion" code
Abstract out the "identify the pkgversion" code from the
rule for creating qemu-version.h, so it sets makefile
variables for QEMU_PKGVERSION and QEMU_FULL_VERSION.
(We will want to use these when building the Sphinx docs.)
Signed-off-by: Peter Maydell
---
Makefile | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index d519fadee39..2d19d28a271 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,20 @@ endif
include $(SRC_PATH)/rules.mak
+# Create QEMU_PKGVERSION and FULL_VERSION strings
+# If PKGVERSION is set, use that; otherwise get version and -dirty status from
git
+QEMU_PKGVERSION := $(if $(PKGVERSION),$(PKGVERSION),$(shell \
+ cd $(SRC_PATH); \
+ if test -d .git; then \
+git describe --match 'v*' 2>/dev/null | tr -d '\n'; \
+if ! git diff-index --quiet HEAD &>/dev/null; then \
+ echo "-dirty"; \
+fi; \
+ fi))
+
+# Either "version (pkgversion)", or just "version" if pkgversion not set
+FULL_VERSION := $(if $(QEMU_PKGVERSION),$(VERSION)
($(QEMU_PKGVERSION)),$(VERSION))
+
GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
#see Makefile.objs for the definition of QAPI_MODULES
@@ -391,23 +405,8 @@ all: $(DOCS) sphinxdocs $(TOOLS) $(HELPERS-y) recurse-all
modules
qemu-version.h: FORCE
$(call quiet-command, \
- (cd $(SRC_PATH); \
- if test -n "$(PKGVERSION)"; then \
- pkgvers="$(PKGVERSION)"; \
- else \
- if test -d .git; then \
- pkgvers=$$(git describe --match 'v*'
2>/dev/null | tr -d '\n');\
- if ! git diff-index --quiet HEAD &>/dev/null;
then \
- pkgvers="$${pkgvers}-dirty"; \
- fi; \
- fi; \
- fi; \
- printf "#define QEMU_PKGVERSION \"$${pkgvers}\"\n"; \
- if test -n "$${pkgvers}"; then \
- printf '#define QEMU_FULL_VERSION QEMU_VERSION " ("
QEMU_PKGVERSION ")"\n'; \
- else \
- printf '#define QEMU_FULL_VERSION QEMU_VERSION\n'; \
- fi; \
+(printf '#define QEMU_PKGVERSION "$(QEMU_PKGVERSION)"\n'; \
+ printf '#define QEMU_FULL_VERSION "$(FULL_VERSION)"\n'; \
) > $@.tmp)
$(call quiet-command, if ! cmp -s $@ $@.tmp; then \
mv $@.tmp $@; \
--
2.20.1
[Qemu-devel] [PATCH 04/11] docs/conf.py: Disable unused _static directory
We don't yet have any custom static files, so disable this config file setting to avoid a warning from sphinx about not being able to find the directory. Signed-off-by: Peter Maydell --- docs/conf.py | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 53a17506615..e1d08a34a65 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -93,7 +93,11 @@ html_theme = 'alabaster' # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". -html_static_path = ['_static'] +# QEMU doesn't yet have any static files, so comment this out so we don't +# get a warning about a missing directory. +# If we do ever add this then it would probably be better to call the +# subdirectory sphinx_static, as the Linux kernel does. +# html_static_path = ['_static'] # Custom sidebar templates, must be a dictionary that maps document names # to template names. -- 2.20.1
[Qemu-devel] [PATCH 07/11] docs/conf.py: Disable option warnings
sphinx-build complains about using :option: to mark up option flags that it doesn't know about (because they were not defined using the "option::" directive): docs/pr-manager.rst:68: WARNING: unknown option: -d Suppress these warnings. This way we get the semantic markup of the option flag but no cross-referencing hyperlink. Signed-off-by: Peter Maydell --- docs/conf.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/conf.py b/docs/conf.py index 6ddaa549f28..c04000e78e4 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -76,6 +76,9 @@ pygments_style = 'sphinx' # If true, `todo` and `todoList` produce output, else they produce nothing. todo_include_todos = False +# Sphinx defaults to warning about use of :option: for options not defined +# with "option::" in the document being processed. Turn that off. +suppress_warnings = ["ref.option"] # -- Options for HTML output -- -- 2.20.1
[Qemu-devel] [PATCH 09/11] Makefile, configure: Support building rST documentation
Add support to our configure and makefile machinery for building our rST docs into HTML files. Building the documentation now requires that sphinx-build is available; this seems better than allowing half the docs to be built if it is not present but having half of them missing. (In particular it means that assuming that distros configured with --enable-docs they'll get a helpful error from configure telling them the new build dependency.) Signed-off-by: Peter Maydell --- configure | 4 ++-- Makefile | 45 ++--- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/configure b/configure index b18281c61f3..9cd5c0cd0bd 100755 --- a/configure +++ b/configure @@ -4561,11 +4561,11 @@ fi # Check if tools are available to build documentation. if test "$docs" != "no" ; then - if has makeinfo && has pod2man; then + if has makeinfo && has pod2man && has sphinx-build; then docs=yes else if test "$docs" = "yes" ; then - feature_not_found "docs" "Install texinfo and Perl/perl-podlators" + feature_not_found "docs" "Install texinfo, Perl/perl-podlators and python-sphinx" fi docs=no fi diff --git a/Makefile b/Makefile index 1278a3eb529..d519fadee39 100644 --- a/Makefile +++ b/Makefile @@ -387,7 +387,7 @@ dummy := $(call unnest-vars,, \ include $(SRC_PATH)/tests/Makefile.include -all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all modules +all: $(DOCS) sphinxdocs $(TOOLS) $(HELPERS-y) recurse-all modules qemu-version.h: FORCE $(call quiet-command, \ @@ -631,6 +631,14 @@ dist: qemu-$(VERSION).tar.bz2 qemu-%.tar.bz2: $(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst qemu-%.tar.bz2,%,$@)" +# Note that these commands assume that there are no HTML files in +# the docs subdir in the source tree! If there are then this will +# blow them away for an in-source-tree 'make clean'. +define clean-manual = +rm -rf docs/$1/_static +rm docs/$1/objects.inv docs/$1/searchindex.js docs/$1/*.html +endef + distclean: clean rm -f config-host.mak config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi qemu-monitor-info.texi rm -f config-all-devices.mak config-all-disas.mak config.status @@ -651,6 +659,9 @@ distclean: clean rm -f docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html rm -f docs/qemu-block-drivers.7 rm -f docs/qemu-cpu-models.7 + rm -f .doctrees + $(call clean-manual,devel) + $(call clean-manual,interop) for d in $(TARGET_DIRS); do \ rm -rf $$d || exit 1 ; \ done @@ -684,7 +695,18 @@ else BLOBS= endif -install-doc: $(DOCS) +define install-manual = +for d in $$(cd docs && find $1 -type d); do $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)/$$d"; done +for f in $$(cd docs && find $1 -type f); do $(INSTALL_DATA) "docs/$$f" "$(DESTDIR)$(qemu_docdir)/$$f"; done +endef + +# Note that we deliberately do not install the "devel" manual: it is +# for QEMU developers, and not interesting to our users. +.PHONY: install-sphinxdocs +install-sphinxdocs: sphinxdocs + $(call install-manual,interop) + +install-doc: $(DOCS) install-sphinxdocs $(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)" $(INSTALL_DATA) qemu-doc.html "$(DESTDIR)$(qemu_docdir)" $(INSTALL_DATA) qemu-doc.txt "$(DESTDIR)$(qemu_docdir)" @@ -835,6 +857,23 @@ docs/version.texi: $(SRC_PATH)/VERSION %.pdf: %.texi docs/version.texi $(call quiet-command,texi2pdf $(TEXI2PDFFLAGS) $< -o $@,"GEN","$@") +# Sphinx builds all its documentation at once in one invocation +# and handles "don't rebuild things unless necessary" itself. +# The '.doctrees' files are cached information to speed this up. +.PHONY: sphinxdocs +sphinxdocs: docs/devel/index.html docs/interop/index.html + +# Canned command to build a single manual +build-manual = $(call quiet-command,sphinx-build $(if $(V),,-q) -b html -d .doctrees/$1 $(SRC_PATH)/docs/$1 docs/$1 ,"SPHINX","docs/$1") +# We assume all RST files in the manual's directory are used in it +manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst) $(SRC_PATH)/docs/$1/conf.py $(SRC_PATH)/docs/conf.py + +docs/devel/index.html: $(call manual-deps,devel) + $(call build-manual,devel) + +docs/interop/index.html: $(call manual-deps,interop) + $(call build-manual,interop) + qemu-options.texi: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"GEN","$@") @@ -863,7 +902,7 @@ docs/qemu-block-drivers.7: docs/qemu-block-drivers.texi docs/qemu-cpu-models.7: docs/qemu-cpu-models.texi scripts/qemu-trace-stap.1: scripts/qemu-trace-stap.texi -html: qemu-doc.html docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html +html: qemu-doc.html docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html sphinxdocs info: qemu-doc.info docs/interop/qemu-qmp-ref.info docs/interop/qemu-ga-ref.info pdf: qemu-
[Qemu-devel] [PATCH 05/11] docs/conf.py: Configure the 'alabaster' theme
Add the 'navigation' bar to the sidebar, which for some
reason is not enabled by default. Remove 'relations', which
is effectively disabled anyway and isn't useful for us.
Signed-off-by: Peter Maydell
---
docs/conf.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/docs/conf.py b/docs/conf.py
index e1d08a34a65..348e6358740 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -106,7 +106,8 @@ html_theme = 'alabaster'
# refs: http://alabaster.readthedocs.io/en/latest/installation.html#sidebars
html_sidebars = {
'**': [
-'relations.html', # needs 'show_related': True theme option to display
+'about.html',
+'navigation.html',
'searchbox.html',
]
}
--
2.20.1
Re: [Qemu-devel] [PATCH 33/52] build: switch to Kconfig
On 1/31/19 11:15 PM, Paolo Bonzini wrote: > On 31/01/19 22:48, Philippe Mathieu-Daudé wrote: >> There is something I don't understand here: Does CONFIG_XEN in >> Kconfig.host take precedence over the target configs? I'm looking at >> these configs: >> >> if supported_xen_target $target; then >> echo "CONFIG_XEN=n" >> $config_target_mak >> if test "$xen_pci_passthrough" = yes; then >> echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak" >> fi >> fi > > You're right, Kconfig.host should contain CONFIG_XEN_BACKEND and not > CONFIG_XEN. Now when I disable Xen, exec.o isn't rebuilt, so I get link errors: /usr/bin/ld: exec.o: in function `reclaim_ramblock': qemu/exec.c:2392: undefined reference to `xen_invalidate_map_cache_entry' /usr/bin/ld: exec.o: in function `qemu_ram_ptr_length': qemu/exec.c:2531: undefined reference to `xen_map_cache' /usr/bin/ld: exec.o: in function `qemu_map_ram_ptr': qemu/exec.c:2498: undefined reference to `xen_map_cache' /usr/bin/ld: exec.o: in function `qemu_ram_block_from_host': qemu/exec.c:2573: undefined reference to `xen_ram_addr_from_mapcache' /usr/bin/ld: exec.o: in function `address_space_unmap': qemu/exec.c:3699: undefined reference to `xen_invalidate_map_cache_entry' /usr/bin/ld: exec.o: in function `address_space_cache_destroy': qemu/exec.c:3791: undefined reference to `xen_invalidate_map_cache_entry' /usr/bin/ld: exec.o: in function `qemu_ram_ptr_length': qemu/exec.c:2528: undefined reference to `xen_map_cache' /usr/bin/ld: exec.o: in function `qemu_map_ram_ptr': qemu/exec.c:2495: undefined reference to `xen_map_cache' collect2: error: ld returned 1 exit status make[1]: *** [Makefile:204: qemu-system-i386] Error 1 Moving those function stubs into a real xen-stub.c file would be simpler from the buildsys PoV IMHO, but I also remember we prefer to avoid stubs, so not sure what's better here, neither what's wrong with make rules.
[Qemu-devel] [PATCH 03/11] docs: Commit initial files from sphinx-quickstart
Commit the initial Sphinx conf.py and skeleton index.rst as
generated with sphinx-quickstart
---
docs/conf.py | 168 +
docs/index.rst | 20 ++
2 files changed, 188 insertions(+)
create mode 100644 docs/conf.py
create mode 100644 docs/index.rst
diff --git a/docs/conf.py b/docs/conf.py
new file mode 100644
index 000..53a17506615
--- /dev/null
+++ b/docs/conf.py
@@ -0,0 +1,168 @@
+# -*- coding: utf-8 -*-
+#
+# QEMU documentation build configuration file, created by
+# sphinx-quickstart on Thu Jan 31 16:40:14 2019.
+#
+# This file is execfile()d with the current directory set to its
+# containing dir.
+#
+# Note that not all possible configuration values are present in this
+# autogenerated file.
+#
+# All configuration values have a default; values that are commented out
+# serve to show the default.
+
+# If extensions (or modules to document with autodoc) are in another directory,
+# add these directories to sys.path here. If the directory is relative to the
+# documentation root, use os.path.abspath to make it absolute, like shown here.
+#
+# import os
+# import sys
+# sys.path.insert(0, os.path.abspath('.'))
+
+
+# -- General configuration
+
+# If your documentation needs a minimal Sphinx version, state it here.
+#
+# needs_sphinx = '1.0'
+
+# Add any Sphinx extension module names here, as strings. They can be
+# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
+# ones.
+extensions = []
+
+# Add any paths that contain templates here, relative to this directory.
+templates_path = ['_templates']
+
+# The suffix(es) of source filenames.
+# You can specify multiple suffix as a list of string:
+#
+# source_suffix = ['.rst', '.md']
+source_suffix = '.rst'
+
+# The master toctree document.
+master_doc = 'index'
+
+# General information about the project.
+project = u'QEMU'
+copyright = u'2019, The QEMU Project Developers'
+author = u'The QEMU Project Developers'
+
+# The version info for the project you're documenting, acts as replacement for
+# |version| and |release|, also used in various other places throughout the
+# built documents.
+#
+# The short X.Y version.
+version = u'4.0'
+# The full version, including alpha/beta/rc tags.
+release = u'4.0'
+
+# The language for content autogenerated by Sphinx. Refer to documentation
+# for a list of supported languages.
+#
+# This is also used if you do content translation via gettext catalogs.
+# Usually you set "language" from the command line for these cases.
+language = None
+
+# List of patterns, relative to source directory, that match files and
+# directories to ignore when looking for source files.
+# This patterns also effect to html_static_path and html_extra_path
+exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
+
+# The name of the Pygments (syntax highlighting) style to use.
+pygments_style = 'sphinx'
+
+# If true, `todo` and `todoList` produce output, else they produce nothing.
+todo_include_todos = False
+
+
+# -- Options for HTML output --
+
+# The theme to use for HTML and HTML Help pages. See the documentation for
+# a list of builtin themes.
+#
+html_theme = 'alabaster'
+
+# Theme options are theme-specific and customize the look and feel of a theme
+# further. For a list of options available for each theme, see the
+# documentation.
+#
+# html_theme_options = {}
+
+# Add any paths that contain custom static files (such as style sheets) here,
+# relative to this directory. They are copied after the builtin static files,
+# so a file named "default.css" will overwrite the builtin "default.css".
+html_static_path = ['_static']
+
+# Custom sidebar templates, must be a dictionary that maps document names
+# to template names.
+#
+# This is required for the alabaster theme
+# refs: http://alabaster.readthedocs.io/en/latest/installation.html#sidebars
+html_sidebars = {
+'**': [
+'relations.html', # needs 'show_related': True theme option to display
+'searchbox.html',
+]
+}
+
+
+# -- Options for HTMLHelp output --
+
+# Output file base name for HTML help builder.
+htmlhelp_basename = 'QEMUdoc'
+
+
+# -- Options for LaTeX output -
+
+latex_elements = {
+# The paper size ('letterpaper' or 'a4paper').
+#
+# 'papersize': 'letterpaper',
+
+# The font size ('10pt', '11pt' or '12pt').
+#
+# 'pointsize': '10pt',
+
+# Additional stuff for the LaTeX preamble.
+#
+# 'preamble': '',
+
+# Latex figure (float) alignment
+#
+# 'figure_align': 'htbp',
+}
+
+# Grouping the document tree into LaTeX files. List of tuples
+# (source start file, target name, title,
+# author, documentclass [howto, manual, or own class]).
+latex_documents = [
+(master_doc, 'QEMU.tex', u'QEMU Documentation',
+ u'The QEMU Project Developers', 'manua
[Qemu-devel] [PATCH 11/11] docs/conf.py: Don't hard-code QEMU version
Don't hard-code the QEMU version number into conf.py. Instead we either pass it to sphinx-build on the command line, or (if doing a standalone Sphinx run in a readthedocs.org setup) extract it from the VERSION file. Signed-off-by: Peter Maydell --- Makefile | 2 +- docs/conf.py | 21 - 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 2d19d28a271..e342242d268 100644 --- a/Makefile +++ b/Makefile @@ -863,7 +863,7 @@ docs/version.texi: $(SRC_PATH)/VERSION sphinxdocs: docs/devel/index.html docs/interop/index.html # Canned command to build a single manual -build-manual = $(call quiet-command,sphinx-build $(if $(V),,-q) -b html -d .doctrees/$1 $(SRC_PATH)/docs/$1 docs/$1 ,"SPHINX","docs/$1") +build-manual = $(call quiet-command,sphinx-build $(if $(V),,-q) -b html -D version=$(VERSION) -D release="$(FULL_VERSION)" -d .doctrees/$1 $(SRC_PATH)/docs/$1 docs/$1 ,"SPHINX","docs/$1") # We assume all RST files in the manual's directory are used in it manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst) $(SRC_PATH)/docs/$1/conf.py $(SRC_PATH)/docs/conf.py diff --git a/docs/conf.py b/docs/conf.py index 6a334f545ec..0842d44e930 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -75,11 +75,22 @@ author = u'The QEMU Project Developers' # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the # built documents. -# -# The short X.Y version. -version = u'4.0' -# The full version, including alpha/beta/rc tags. -release = u'4.0' + +# Extract this information from the VERSION file, for the benefit of +# standalone Sphinx runs as used by readthedocs.org. Builds run from +# the Makefile will pass version and release on the sphinx-build +# command line, which override this. +try: +extracted_version = None +with open(os.path.join(qemu_docdir, '../VERSION')) as f: +extracted_version = f.readline().strip() +except: +pass +finally: +if extracted_version: +version = release = extracted_version +else: +version = release = "unknown version" # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. -- 2.20.1
[Qemu-devel] [PATCH 08/11] Separate conf.py for each manual we want
---
docs/conf.py | 37 +++--
docs/devel/conf.py | 15 +++
docs/devel/index.rst | 21 +
docs/index.rst | 9 ++---
docs/interop/conf.py | 15 +++
docs/interop/index.rst | 18 ++
6 files changed, 102 insertions(+), 13 deletions(-)
create mode 100644 docs/devel/conf.py
create mode 100644 docs/devel/index.rst
create mode 100644 docs/interop/conf.py
create mode 100644 docs/interop/index.rst
diff --git a/docs/conf.py b/docs/conf.py
index c04000e78e4..6a334f545ec 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -3,6 +3,20 @@
# QEMU documentation build configuration file, created by
# sphinx-quickstart on Thu Jan 31 16:40:14 2019.
#
+# This config file can be used in one of two ways:
+# (1) as a common config file which is included by the conf.py
+# for each of QEMU's manuals: in this case sphinx-build is run multiple
+# times, once per subdirectory.
+# (2) as a top level conf file which will result in building all
+# the manuals into a single document: in this case sphinx-build is
+# run once, on the top-level docs directory.
+#
+# QEMU's makefiles take option (1), which allows us to install
+# only the ones the user cares about (in particular we don't want
+# to ship the 'devel' manual to end-users).
+# Third-party sites such as readthedocs.org will take option (2).
+#
+#
# This file is execfile()d with the current directory set to its
# containing dir.
#
@@ -12,13 +26,22 @@
# All configuration values have a default; values that are commented out
# serve to show the default.
+import os
+import sys
+
+# The per-manual conf.py will set qemu_docdir for a single-manual build;
+# otherwise set it here if this is an entire-manual-set build.
+# This is always the absolute path of the docs/ directory in the source tree.
+try:
+qemu_docdir
+except NameError:
+qemu_docdir = os.path.abspath(".")
+
# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
-# documentation root, use os.path.abspath to make it absolute, like shown here.
+# documentation root, use an absolute path starting from qemu_docdir.
#
-# import os
-# import sys
-# sys.path.insert(0, os.path.abspath('.'))
+# sys.path.insert(0, os.path.join(qemu_docdir, "my_subdir"))
# -- General configuration
@@ -90,8 +113,10 @@ html_theme = 'alabaster'
# Theme options are theme-specific and customize the look and feel of a theme
# further. For a list of options available for each theme, see the
# documentation.
-#
-# html_theme_options = {}
+# We initialize this to empty here, so the per-manual conf.py can just
+# add individual key/value entries.
+html_theme_options = {
+}
# Add any paths that contain custom static files (such as style sheets) here,
# relative to this directory. They are copied after the builtin static files,
diff --git a/docs/devel/conf.py b/docs/devel/conf.py
new file mode 100644
index 000..7441f87e7f5
--- /dev/null
+++ b/docs/devel/conf.py
@@ -0,0 +1,15 @@
+# -*- coding: utf-8 -*-
+#
+# QEMU documentation build configuration file for the 'devel' manual.
+#
+# This includes the top level conf file and then makes any necessary tweaks.
+import sys
+import os
+
+qemu_docdir = os.path.abspath("..")
+parent_config = os.path.join(qemu_docdir, "conf.py")
+exec(compile(open(parent_config, "rb").read(), parent_config, 'exec'))
+
+# This slightly misuses the 'description', but is the best way to get
+# the manual title to appear in the sidebar.
+html_theme_options['description'] = u'Developer''s Guide'
diff --git a/docs/devel/index.rst b/docs/devel/index.rst
new file mode 100644
index 000..cd0fa6c9ba2
--- /dev/null
+++ b/docs/devel/index.rst
@@ -0,0 +1,21 @@
+.. This is the top level page for the 'devel' manual.
+
+
+QEMU Developer's Guide
+==
+
+This manual documents various parts of the internals of QEMU.
+You only need to read it if you are interested in reading or
+modifying QEMU's source code.
+
+Contents:
+
+.. toctree::
+ :maxdepth: 2
+
+ loads-stores
+ memory
+ migration
+ stable-process
+ testing
+
diff --git a/docs/index.rst b/docs/index.rst
index 93f82228310..3690955dd1f 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -10,11 +10,6 @@ Welcome to QEMU's documentation!
:maxdepth: 2
:caption: Contents:
+ interop/index
+ devel/index
-
-Indices and tables
-==
-
-* :ref:`genindex`
-* :ref:`modindex`
-* :ref:`search`
diff --git a/docs/interop/conf.py b/docs/interop/conf.py
new file mode 100644
index 000..cf3c69d4a7e
--- /dev/null
+++ b/docs/interop/conf.py
@@ -0,0 +1,15 @@
+# -*- coding: utf-8 -*-
+#
+# QEMU documentation build configuration file for the 'interop' manual.
+#
+# This includes the top level conf file and then makes any necessary tweaks.
+import sy
Re: [Qemu-devel] [PATCH v11 for-4.0 11/11] qemu_thread: supplement error handling for touch_all_pages
Cc: Paolo for preexisting issues due to commit 1e356fc14be.
Fei Li writes:
> From: Fei Li
>
> Supplement the error handling for touch_all_pages: add an Error
> parameter for it to propagate the error to its caller to do the
> handling in case it fails.
>
> Cc: Markus Armbruster
> Signed-off-by: Fei Li
> ---
> util/oslib-posix.c | 26 --
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index b6c2ee270d..b4dd3d8970 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
Some context we'll need below:
static void sigbus_handler(int signal)
{
int i;
if (memset_thread) {
for (i = 0; i < memset_num_threads; i++) {
if (qemu_thread_is_self(&memset_thread[i].pgthread)) {
siglongjmp(memset_thread[i].env, 1);
}
}
}
}
Preexisting, not this patch's business: touch_all_pages() sets
@memset_num_threads to the number of desired threads, then leisurely
populates memset_thread[], starting threads as it goes. If one of these
threads catches SIGBUS before memset_thread[] is completely populated,
memset_thread[i].pgthread can be null here. qemu_thread_is_self()
dereferences it. Oops.
Note there's a time window between qemu_thread_create() starting the
thread and storing it in memset_thread[i].pgthread. Any fix will have
to work even when the thread catches SIGBUS within that time window.
static void *do_touch_pages(void *arg)
{
MemsetThread *memset_args = (MemsetThread *)arg;
sigset_t set, oldset;
/* unblock SIGBUS */
sigemptyset(&set);
sigaddset(&set, SIGBUS);
pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
if (sigsetjmp(memset_args->env, 1)) {
memset_thread_failed = true;
} else {
[...]
}
pthread_sigmask(SIG_SETMASK, &oldset, NULL);
return NULL;
}
We set @memset_thread_failed on SIGBUS in a memset thread.
> @@ -435,7 +435,7 @@ static inline int get_memset_num_threads(int smp_cpus)
> }
>
> static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
> -int smp_cpus)
> +int smp_cpus, Error **errp)
> {
> size_t numpages_per_thread;
> size_t size_per_thread;
char *addr = area;
int i = 0;
memset_thread_failed = false;
We clear @memset_thread_failed before we create the memset threads.
Preexisting, not this patch's business: use of global state without
synchronization leaves touch_all_pages() and thus os_mem_prealloc() not
thread-safe. Even when that's just fine, it warrants at least a big fat
comment, and ideally assertions to catch misuse.
memset_num_threads = get_memset_num_threads(smp_cpus);
memset_thread = g_new0(MemsetThread, memset_num_threads);
numpages_per_thread = (numpages / memset_num_threads);
size_per_thread = (hpagesize * numpages_per_thread);
for (i = 0; i < memset_num_threads; i++) {
memset_thread[i].addr = addr;
> @@ -452,20 +452,29 @@ static bool touch_all_pages(char *area, size_t
> hpagesize, size_t numpages,
> memset_thread[i].numpages = (i == (memset_num_threads - 1)) ?
> numpages : numpages_per_thread;
> memset_thread[i].hpagesize = hpagesize;
> -/* TODO: let the callers handle the error instead of abort() here */
> -qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
> - do_touch_pages, &memset_thread[i],
> - QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
> + do_touch_pages, &memset_thread[i],
> + QEMU_THREAD_JOINABLE, errp) < 0) {
> +memset_thread_failed = true;
We set @memset_thread_failed when creating a memset thread fails. @errp
contains an error then.
> +break;
> +}
> addr += size_per_thread;
> numpages -= numpages_per_thread;
> }
> +
> +memset_num_threads = i;
> for (i = 0; i < memset_num_threads; i++) {
> qemu_thread_join(&memset_thread[i].pgthread);
> }
> g_free(memset_thread);
> memset_thread = NULL;
>
> -return memset_thread_failed;
> +if (memset_thread_failed) {
> +error_append_hint(errp, "os_mem_prealloc: Insufficient free host "
> + "memory pages available to allocate guest RAM");
> +return false;
@memset_thread_failed is true when creating a memset thread failed, or a
memset thread caught SIGBUS.
@errp contains an error only if the former is the case.
If we succeeded in creating all threads, but got SIGBUS, @errp is null,
and error_append_hint() does nothing. touch_all_pages() then fails
without setting an error.
Suggested fix: drop the m
Re: [Qemu-devel] [PATCH 42/52] i386: express dependencies with Kconfig
Hi Paolo,
On 1/25/19 11:07 AM, Paolo Bonzini wrote:
> This way, the default-configs file only need to specify the boards
> and any optional devices.
>
> Signed-off-by: Paolo Bonzini
> Signed-off-by: Yang Zhong
> Message-Id: <20190123065618.3520-37-yang.zh...@intel.com>
> Signed-off-by: Paolo Bonzini
> ---
> default-configs/i386-softmmu.mak | 44 ++--
> hw/acpi/Kconfig | 3 +++
> hw/i2c/Makefile.objs | 2 +-
> hw/i386/Kconfig | 54
>
> hw/isa/Kconfig | 1 +
> hw/pci-host/Kconfig | 4 +++
> hw/tpm/Kconfig | 2 ++
> 7 files changed, 73 insertions(+), 37 deletions(-)
>
> diff --git a/default-configs/i386-softmmu.mak
> b/default-configs/i386-softmmu.mak
> index 8e6a810..9eb9a5e 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -1,11 +1,6 @@
> # Default configuration for i386-softmmu
>
> -CONFIG_PCI=y
> -CONFIG_PCI_DEVICES=y
> -CONFIG_ISA_BUS=y
> include hyperv.mak
> -CONFIG_VGA_ISA=y
> -CONFIG_VMWARE_VGA=y
> CONFIG_VMXNET3_PCI=y
> CONFIG_VIRTIO_VGA=y
> CONFIG_IPMI=y
> @@ -13,49 +8,26 @@ CONFIG_IPMI_LOCAL=y
> CONFIG_IPMI_EXTERN=y
> CONFIG_ISA_IPMI_KCS=y
> CONFIG_ISA_IPMI_BT=y
> -CONFIG_I8254=y
> -CONFIG_ACPI=y
> -CONFIG_ACPI_X86=y
> -CONFIG_ACPI_X86_ICH=y
> -CONFIG_ACPI_MEMORY_HOTPLUG=y
> -CONFIG_ACPI_CPU_HOTPLUG=y
> -CONFIG_APM=y
> -CONFIG_I8257=y
> -CONFIG_IDE_ISA=y
> -CONFIG_IDE_PIIX=y
> +
> +# Optional devices:
> +#
> CONFIG_HPET=y
> CONFIG_APPLESMC=y
> -CONFIG_I8259=y
> CONFIG_PFLASH_CFI01=y
> -CONFIG_MC146818RTC=y
> -CONFIG_PCI_PIIX=y
> -CONFIG_ISA_DEBUG=y
> CONFIG_ISA_TESTDEV=y
> -CONFIG_VMPORT=y
> CONFIG_SGA=y
> -CONFIG_LPC_ICH9=y
> -CONFIG_PCI_EXPRESS=y
> -CONFIG_PCI_EXPRESS_Q35=y
> -CONFIG_APIC=y
> -CONFIG_IOAPIC=y
> CONFIG_PVPANIC=y
> CONFIG_MEM_DEVICE=y
> -CONFIG_DIMM=y
> CONFIG_NVDIMM=y
> CONFIG_ACPI_NVDIMM=y
> -CONFIG_XIO3130=y
> -CONFIG_IOH3420=y
> -CONFIG_I82801B11=y
> -CONFIG_SMBIOS=y
> CONFIG_PXB=y
> CONFIG_ACPI_VMGENID=y
> -CONFIG_ACPI_SMBUS=y
> CONFIG_SMBUS_EEPROM=y
> -CONFIG_FW_CFG_DMA=y
> CONFIG_I2C=y
> -CONFIG_VTD=y
> -CONFIG_AMD_IOMMU=y
> -CONFIG_PAM=y
> -CONFIG_PC=y
> +CONFIG_PCI_DEVICES=y
> +
> +# Boards:
> +#
> +CONFIG_ISAPC=y
> CONFIG_I440FX=y
> CONFIG_Q35=y
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index c485a34..035a28f 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -3,15 +3,18 @@ config ACPI
>
> config ACPI_X86
> bool
> +select ACPI
>
> config ACPI_X86_ICH
> bool
> +select ACPI_X86
>
> config ACPI_CPU_HOTPLUG
> bool
>
> config ACPI_MEMORY_HOTPLUG
> bool
> +select MEM_DEVICE
>
> config ACPI_NVDIMM
> bool
> diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
> index 61ac50a..ff22aa6 100644
> --- a/hw/i2c/Makefile.objs
> +++ b/hw/i2c/Makefile.objs
> @@ -2,7 +2,7 @@ common-obj-$(CONFIG_I2C) += core.o smbus.o
> common-obj-$(CONFIG_SMBUS_EEPROM) += smbus_eeprom.o
> common-obj-$(CONFIG_DDC) += i2c-ddc.o
> common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o
> -common-obj-$(CONFIG_ACPI_X86) += smbus_ich9.o
> +common-obj-$(CONFIG_ACPI_X86_ICH) += smbus_ich9.o
> common-obj-$(CONFIG_ACPI_SMBUS) += pm_smbus.o
> common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
> common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index ff41be3..8814b7c 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -5,20 +5,73 @@ config SEV
> config PC
> bool
>
> +config PC
> +bool
> +select ISA_DEBUG
> +select I8259
> +select I8254
> +select PCSPK
> +select I82374
> +select I8257
> +select MC146818RTC
> +
> +config PC_PCI
> +bool
> +select APIC
> +select IOAPIC
> +select APM
> +select PC
> +
> +config PC_ACPI
> +bool
> +select ACPI_X86
> +select ACPI_CPU_HOTPLUG
> +select ACPI_MEMORY_HOTPLUG
> +depends on ACPI_SMBUS
> +
> config I440FX
> bool
> +select PC_PCI
> +select PC_ACPI
> +select ACPI_SMBUS
> +select PCI_PIIX
> +select FDC
> +select IDE_PIIX
> +select DIMM
> +select SMBIOS
> +select VMPORT
> +select VMMOUSE
> +select FW_CFG_DMA
>
> config ISAPC
> bool
> select ISA_BUS
> +select PC
> +select IDE_ISA
> +select VGA_ISA
> +# FIXME: it is in the same file as i440fx, and does not compile
> +# if separated
> +depends on I440FX
>
> config Q35
> bool
> +select PC_PCI
> +select PC_ACPI
> +select PCI_EXPRESS_Q35
> +select LPC_ICH9
> +select AHCI
> +select DIMM
> +select SMBIOS
> +select VMPORT
> +select VMMOUSE
> +select FW_CFG_DMA
This lacks a DISPLAY dependency?
$ i386-softmmu/qemu-system-i386 -M q35
qemu-system-i386: Unknown device 'VGA' for bus 'PCIE'
Aborted (core dumped)
PCIDevice *pci_vga_init(PCIBus *bus)
{
switch (vg
[Qemu-devel] [PATCH 06/11] docs/conf.py: Don't include rST sources in HTML build
Sphinx defaults to including all the rST source files
in the HTML build and making each HTML page link to the
source file. Disable that.
Signed-off-by: Peter Maydell
---
docs/conf.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/docs/conf.py b/docs/conf.py
index 348e6358740..6ddaa549f28 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -112,6 +112,9 @@ html_sidebars = {
]
}
+# Don't copy the rST source files to the HTML output directory,
+# and don't put links to the sources into the output HTML.
+html_copy_source = False
# -- Options for HTMLHelp output --
--
2.20.1
Re: [Qemu-devel] [PATCH 00/11] Enable build and install of our rST docs
Patchew URL: https://patchew.org/QEMU/20190201145035.22739-1-peter.mayd...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [PATCH 00/11] Enable build and install of our rST docs Type: series Message-id: 20190201145035.22739-1-peter.mayd...@linaro.org === TEST SCRIPT BEGIN === #!/bin/bash git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20190201145035.22739-1-peter.mayd...@linaro.org -> patchew/20190201145035.22739-1-peter.mayd...@linaro.org Switched to a new branch 'test' fdcde82e94 docs/conf.py: Don't hard-code QEMU version 95ff495343 Makefile: Abstract out "identify the pkgversion" code ea4e36f66f Makefile, configure: Support building rST documentation 20761006f1 Separate conf.py for each manual we want ab8cacd266 docs/conf.py: Disable option warnings a58591c443 docs/conf.py: Don't include rST sources in HTML build a5e1fff319 docs/conf.py: Configure the 'alabaster' theme b9917d0a1b docs/conf.py: Disable unused _static directory fd769099b3 docs: Commit initial files from sphinx-quickstart 9888afe4ce docs: Convert memory.txt to rst format 7c59983957 docs/cpu-hotplug.rst: Fix rST markup issues === OUTPUT BEGIN === 1/11 Checking commit 7c59983957ec (docs/cpu-hotplug.rst: Fix rST markup issues) 2/11 Checking commit 9888afe4cedd (docs: Convert memory.txt to rst format) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #21: rename from docs/devel/memory.txt total: 0 errors, 1 warnings, 195 lines checked Patch 2/11 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/11 Checking commit fd769099b38b (docs: Commit initial files from sphinx-quickstart) ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 188 lines checked Patch 3/11 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/11 Checking commit b9917d0a1bce (docs/conf.py: Disable unused _static directory) 5/11 Checking commit a5e1fff319a6 (docs/conf.py: Configure the 'alabaster' theme) 6/11 Checking commit a58591c44317 (docs/conf.py: Don't include rST sources in HTML build) 7/11 Checking commit ab8cacd26625 (docs/conf.py: Disable option warnings) 8/11 Checking commit 20761006f1ff (Separate conf.py for each manual we want) ERROR: line over 90 characters #160: FILE: docs/interop/conf.py:15: +html_theme_options['description'] = u'System Emulation Management and Interoperability Guide' ERROR: Missing Signed-off-by: line(s) total: 2 errors, 0 warnings, 133 lines checked Patch 8/11 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 9/11 Checking commit ea4e36f66f9b (Makefile, configure: Support building rST documentation) 10/11 Checking commit 95ff495343ba (Makefile: Abstract out "identify the pkgversion" code) 11/11 Checking commit fdcde82e9438 (docs/conf.py: Don't hard-code QEMU version) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190201145035.22739-1-peter.mayd...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 00/11] Enable build and install of our rST docs
Patchew URL: https://patchew.org/QEMU/20190201145035.22739-1-peter.mayd...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [PATCH 00/11] Enable build and install of our rST docs Message-id: 20190201145035.22739-1-peter.mayd...@linaro.org Type: series === TEST SCRIPT BEGIN === #!/bin/bash git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20190201145035.22739-1-peter.mayd...@linaro.org -> patchew/20190201145035.22739-1-peter.mayd...@linaro.org Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone' Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc' Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers' Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF' Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe' Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios' Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware' Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode' Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa' Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios' Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot' Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot' Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex' Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3' Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3' Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into 'capstone'... Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf' Cloning into 'dtc'... Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536' Cloning into 'roms/QemuMacDrivers'... Submodule path 'roms/QemuMacDrivers': checked out 'd4e7d7ac663fcb55f1b93575445fcbca372f17a7' Cloning into 'roms/SLOF'... Submodule path 'roms/SLOF': checked out '9b7ab2fa020341dee8bf9df6c9cf40003e0136df' Cloning into 'roms/ipxe'... Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17' Cloning into 'roms/openbios'... Submodule path 'roms/openbios': checked out '441a84d3a642a10b948369c63f32367e8ff6395b' Cloning into 'roms/openhackware'... Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5' Cloning into 'roms/qemu-palcode'... Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097' Cloning into 'roms/seabios'... Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307' Cloning into 'roms/seabios-hppa'... Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0' Cloning into 'roms/sgabios'... Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a' Cloning into 'roms/skiboot'... Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc' Cloning into 'roms/u-boot'... Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943' Cloning into 'roms/u-boot-sam460ex'... Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588' Cloning into 'tests/fp/berkeley-softfloat-3'... Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037' Cloning into 'tests/fp/berkeley-testfloat-3'... Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3' Cloning into 'ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' Switched to a new branch 'test' fdcde82 docs/conf.py: Don't hard-code QEMU version 95ff495 Makefile: Abstract out "identify the pkgversion" code ea4e36f Makefile, configure: Support building rST documentation 2076100 Separate conf.py for each manual we want a
Re: [Qemu-devel] [PATCH 4/6] linux-user: Initialize aarch64 pac keys
On Fri, 25 Jan 2019 at 22:57, Richard Henderson
wrote:
>
> Initialize the keys to a non-zero value on process start.
>
> Signed-off-by: Richard Henderson
> +static uint64_t arm_rand64(void)
> +{
> +int shift = 64 - clz64(RAND_MAX);
> +int i, n = 64 / shift + (64 % shift != 0);
> +uint64_t ret = 0;
> +
> +for (i = 0; i < n; i++) {
> +ret = (ret << shift) | rand();
> +}
> +return ret;
> +}
I'm not a huge fan of the use of rand() here, but it's what
we're using to initialize AT_RANDOM in linux-user, so I guess
it's OK here. At some point we should investigate whether
there's something better we are guaranteed to have available,
I suppose. (Coverity gripes about use of rand().)
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 6/6] s390x/pci: Unplug remaining requested devices on pcihost reset
On 01.02.19 11:19, Cornelia Huck wrote:
> On Wed, 30 Jan 2019 16:57:33 +0100
> David Hildenbrand wrote:
>
>> When resetting the guest we should unplug and remove all devices that
>> are still pending.
>>
>> With this patch, the requested device will be unpluged on reboot
>> (S390_RESET_EXTERNAL and S390_RESET_REIPL, which reset the pcihost bridge
>> via qemu_devices_reset()).
>>
>> This approach is similar to what's done for acpi PCI hotplug in
>> acpi_pcihp_reset() -> acpi_pcihp_update() ->
>> acpi_pcihp_update_hotplug_bus() -> acpi_pcihp_eject_slot().
>>
>> s390_pci_generate_plug_event()'s will still be generated, I guess this
>> is not an issue. The same thing would happen right now when unplugging
>> a device just before starting the guest.
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/s390x/s390-pci-bus.c | 15 +++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>> index 867801ccf9..b9b0f44087 100644
>> --- a/hw/s390x/s390-pci-bus.c
>> +++ b/hw/s390x/s390-pci-bus.c
>> @@ -1092,6 +1092,21 @@ static void s390_pcihost_reset(DeviceState *dev)
>> {
>> S390pciState *s = S390_PCI_HOST_BRIDGE(dev);
>> PCIBus *bus = s->parent_obj.bus;
>> +S390PCIBusDevice *pbdev, *next;
>> +
>> +/* Process all pending unplug requests */
>> +QTAILQ_FOREACH_SAFE(pbdev, &s->zpci_devs, link, next) {
>> +if (pbdev->unplug_requested) {
>> +if (pbdev->summary_ind) {
>> +pci_dereg_irqs(pbdev);
>> +}
>> +if (pbdev->iommu->enabled) {
>> +pci_dereg_ioat(pbdev->iommu);
>> +}
>> +pbdev->state = ZPCI_FS_STANDBY;
>> +s390_pci_perform_unplug(pbdev);
>> +}
>> +}
>>
>> /*
>> * When resetting a PCI bridge, the assigned numbers are set to 0. So
>
> I'm afraid this one doesn't quite apply without the first patches in
> the series, and I'm not sure how to fix it up. I'll either take a
> rebase on a codebase without patches 1-3, or this patch together with
> patches 1-3 after they get acks (whatever comes first :)
>
I guess it's only the comment that get's added in patch #1, so no major
conflicts.
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH 5/6] linux-user: Implement PR_PAC_RESET_KEYS
On Fri, 25 Jan 2019 at 22:57, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/aarch64/target_syscall.h | 7 ++
> linux-user/syscall.c| 33 +
> 2 files changed, 40 insertions(+)
> @@ -9691,6 +9691,39 @@ static abi_long do_syscall1(void *cpu_env, int num,
> abi_long arg1,
> }
> }
> return ret;
> +case TARGET_PR_PAC_RESET_KEYS:
> +{
> +CPUARMState *env = cpu_env;
> +ARMCPU *cpu = arm_env_get_cpu(env);
The kernel implementation of this returns EINVAL if any
of arg3/arg4/arg5 are non-zero.
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [Qemu-devel] [PATCH RFCv2 7/9] hmp: Handle virtio-pmem when printing memory device infos
* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert" writes:
>
> > * Markus Armbruster (arm...@redhat.com) wrote:
> >> "Dr. David Alan Gilbert" writes:
> >>
> >> > * David Hildenbrand (da...@redhat.com) wrote:
> >> >> Print the memory device info just like for PCDIMM/NVDIMM.
> >> >>
> >> >> Signed-off-by: David Hildenbrand
> >> >> ---
> >> >> hmp.c | 27 +++
> >> >> 1 file changed, 15 insertions(+), 12 deletions(-)
> >> >>
> >> >> diff --git a/hmp.c b/hmp.c
> >> >> index 8da5fd8760..25c32e0810 100644
> >> >> --- a/hmp.c
> >> >> +++ b/hmp.c
> >> >> @@ -2553,6 +2553,7 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> >> QDict *qdict)
> >> >> Error *err = NULL;
> >> >> MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err);
> >> >> MemoryDeviceInfoList *info;
> >> >> +VirtioPMEMDeviceInfo *vpi;
> >> >> MemoryDeviceInfo *value;
> >> >> PCDIMMDeviceInfo *di;
> >> >>
> >> >> @@ -2562,19 +2563,9 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> >> QDict *qdict)
> >> >> if (value) {
> >> >> switch (value->type) {
> >> >> case MEMORY_DEVICE_INFO_KIND_DIMM:
> >> >> -di = value->u.dimm.data;
> >> >> -break;
> >> >> -
> >> >> case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> >> >> -di = value->u.nvdimm.data;
> >> >> -break;
> >> >> -
> >> >> -default:
> >> >> -di = NULL;
> >> >> -break;
> >> >> -}
> >> >> -
> >> >> -if (di) {
> >> >> +di = value->type == MEMORY_DEVICE_INFO_KIND_DIMM ?
> >> >> + value->u.dimm.data : value->u.nvdimm.data;
> >> >> monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
> >> >> MemoryDeviceInfoKind_str(value->type),
> >> >> di->id ? di->id : "");
> >> >> @@ -2587,6 +2578,18 @@ void hmp_info_memory_devices(Monitor *mon, const
> >> >> QDict *qdict)
> >> >> di->hotplugged ? "true" : "false");
> >> >> monitor_printf(mon, " hotpluggable: %s\n",
> >> >> di->hotpluggable ? "true" : "false");
> >> >> +break;
> >> >> +case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM:
> >> >> +vpi = value->u.virtio_pmem.data;
> >> >> +monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
> >> >> + MemoryDeviceInfoKind_str(value->type),
> >> >> + vpi->id ? vpi->id : "");
> >> >> +monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n",
> >> >> vpi->memaddr);
> >> >> +monitor_printf(mon, " size: %" PRIu64 "\n",
> >> >> vpi->size);
> >> >> +monitor_printf(mon, " memdev: %s\n", vpi->memdev);
> >> >> +break;
> >> >> +default:
> >> >> +g_assert_not_reached();
> >> >
> >> >
> >> > Although I'd prefer if that assert was replaced by a print
> >> > saying it was an unknown type.
> >>
> >> I would not. If we reach this, something must have scribbled over
> >> value->type and who knows what else. Continuing is unsafe. Looks like
> >> a textbook use of assertions to me.
> >
> > Or it could be that someone added a new type of memory device and forgot
> > to update the hmp code.
>
> Programming error -> assert. Nothing catches attention in testing like
> an assertion failure.
I don't want to get back through that; we've had this argument before;
but I'll still say in this case I'd prefer a clean error rather than
abort.
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/6] target/arm: Complete ARMv8.3-PAuth linux-user
On Fri, 25 Jan 2019 at 22:57, Richard Henderson wrote: > > (1) Fix a bug I introduced at the last moment in the last > patch set -- enable pac keys during reset, not before. > (2) Add the HWCAP bits. > (3) Add the new prctl > (4) Add a smoke test so that (1) doesn't happen again. > > > r~ > > > Richard Henderson (6): > target/arm: Always enable pac keys for user-only > aarch64-linux-user: Update HWCAP bits from linux 5.0-rc1 > aarch64-linux-user: Enable HWCAP bits for PAuth > linux-user: Initialize aarch64 pac keys > linux-user: Implement PR_PAC_RESET_KEYS > tests/tcg/aarch64: Add pauth smoke tests Applied patches 1-4 to target-arm.next (5 has a minor nit and 6 depends on 5). thanks -- PMM
Re: [Qemu-devel] [PATCH v11 2/3] qemu-img info lists bitmap directory entries
31.01.2019 16:46, Andrey Shinkevich wrote: > In the 'Format specific information' section of the 'qemu-img info' > command output, the supplemental information about existing QCOW2 > bitmaps will be shown, such as a bitmap name, flags and granularity: > > image: /vz/vmprivate/VM1/harddisk.hdd > file format: qcow2 > virtual size: 64G (68719476736 bytes) > disk size: 3.0M > cluster_size: 1048576 > Format specific information: > compat: 1.1 > lazy refcounts: true > bitmaps: > [0]: > flags: > [0]: in-use > [1]: auto > name: back-up1 > unknown flags: 4 > granularity: 65536 > [1]: > flags: > [0]: in-use > [1]: auto > name: back-up2 > unknown flags: 8 > granularity: 65536 > refcount bits: 16 > corrupt: false > > Signed-off-by: Andrey Shinkevich Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v2 2/5] virtio-blk: add "discard-wzeroes" boolean property
On Thu, Jan 31, 2019 at 04:19:11PM +0100, Stefano Garzarella wrote:
> In order to avoid migration issues, we enable DISCARD and
> WRITE ZEROES features only for machine type >= 4.0
>
> Suggested-by: Dr. David Alan Gilbert
> Signed-off-by: Stefano Garzarella
> ---
> hw/block/virtio-blk.c | 2 ++
> hw/core/machine.c | 1 +
> include/hw/virtio/virtio-blk.h | 1 +
> 3 files changed, 4 insertions(+)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 8a6754d9a2..542ec52536 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -1026,6 +1026,8 @@ static Property virtio_blk_properties[] = {
> DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> IOThread *),
> +DEFINE_PROP_BIT("discard-wzeroes", VirtIOBlock, conf.discard_wzeroes, 0,
> + true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
Thinking about it, are there security implications for discard?
Should we make it default false?
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 2629515363..ce98857af0 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -30,6 +30,7 @@ GlobalProperty hw_compat_3_1[] = {
> { "memory-backend-memfd", "x-use-canonical-path-for-ramblock-id", "true"
> },
> { "tpm-crb", "ppi", "false" },
> { "tpm-tis", "ppi", "false" },
> +{ "virtio-blk-device", "discard-wzeroes", "false" },
> };
> const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index 5117431d96..c336afb4cd 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -40,6 +40,7 @@ struct VirtIOBlkConf
> uint32_t request_merging;
> uint16_t num_queues;
> uint16_t queue_size;
> +uint32_t discard_wzeroes;
> };
>
> struct VirtIOBlockDataPlane;
> --
> 2.20.1
Re: [Qemu-devel] [Qemu-arm] [PATCH 0/3] target/arm: fix PAuth in user mode
On Fri, 25 Jan 2019 at 23:02, Rémi Denis-Courmont wrote:
>
> Hello,
>
> As things stand, ARMv8.3-PAuth does not work in user mode. These tiny patches
> attempt to fix that.
>
> Br,
>
>
> Remi Denis-Courmont (3):
> target/arm: fix AArch64 virtual address space size
> target/arm: actually enable PAuth in user mode
> target/arm: fix decoding of B{,L}RA{A,B}
Thanks for this patchset. I've applied patches 1 (v2) and 3
to my target-arm.next tree. (For patch 2 I prefer the approach
in Richard's patchset which deletes the cpu properties entirely.)
PS: for future patchsets, if you do a v2 of a patch it's better
to then resend the whole patchset as a fresh email thread (not a
followup. That makes our automated patch email handling tooling
happier as it can keep the v1 and v2 versions distinct.
thanks
-- PMM
